RFC104 implementation: take into account @abellgithub review

apps/gdal.cpp

@@ -43,11 +43,11 @@ MAIN_START(argc, argv)
     std::vector<std::string> args;
     for (int i = 1; i < argc; ++i)
         args.push_back(argv[i]);
+    CSLDestroy(argv);
 
     if (!alg->ParseCommandLineArguments(args))
     {
         fprintf(stderr, "%s", alg->GetUsageForCLI(true).c_str());
-        CSLDestroy(argv);
         return 1;
     }
 
@@ -77,8 +77,6 @@ MAIN_START(argc, argv)
         ret = 1;
     }
 
-    CSLDestroy(argv);
-
     return ret;
 }
 

apps/gdalalg_dispatcher.h

@@ -69,16 +69,24 @@ template <class RasterDispatcher, class VectorDispatcher>
 bool GDALDispatcherAlgorithm<RasterDispatcher, VectorDispatcher>::
     ParseCommandLineArguments(const std::vector<std::string> &args)
 {
+    // We first try to process with the raster specific algorithm (that has
+    // been instantiated in a special way to accept both raster and vector
+    // input datasets). If the raster specific algorithm can parse successfully
+    // the arguments *and* the dataset is a raster one, then continue processing
+    // with it. Otherwise try with the vector specific algorithm.
+
     bool ok;
+    if (args.size() > 1)
     {
-        std::unique_ptr<CPLErrorStateBackuper> oErrorHandler;
-        if (args.size() > 1)
-        {
-            oErrorHandler =
-                std::make_unique<CPLErrorStateBackuper>(CPLQuietErrorHandler);
-        }
+        // Silence errors as it might be rather for the vector algorithm
+        CPLErrorStateBackuper oErrorHandler(CPLQuietErrorHandler);
+        ok = m_rasterDispatcher->ParseCommandLineArguments(args);
+    }
+    else
+    {
+        // If there's just a single argument, we don't need to silence errors
+        // as this will trigger a legitimate error message about the subcommand.
         ok = m_rasterDispatcher->ParseCommandLineArguments(args);
-        CPL_IGNORE_RET_VAL(oErrorHandler);
     }
 
     if (m_rasterDispatcher->PropagateSpecialActionTo(this))

apps/gdalalg_main.cpp

@@ -45,6 +45,8 @@ GDALMainAlgorithm::GDALMainAlgorithm()
 bool GDALMainAlgorithm::ParseCommandLineArguments(
     const std::vector<std::string> &args)
 {
+    // Detect shortest form of pipeline:
+    // "gdal read in.tif ! .... ! write out.tif"
     if (args.size() >= 2 && args[0] == "read")
     {
         m_subAlg =
@@ -67,8 +69,18 @@ bool GDALMainAlgorithm::ParseCommandLineArguments(
                 return false;
             }
         }
+
+        return GDALAlgorithm::ParseCommandLineArguments(args);
     }
-    else if (!(args.size() >= 1 && InstantiateSubAlgorithm(args[0])))
+    // Generic case: "gdal {subcommand} arguments"
+    // where subcommand is a known subcommand
+    else if (args.size() >= 1 && InstantiateSubAlgorithm(args[0]))
+    {
+        return GDALAlgorithm::ParseCommandLineArguments(args);
+    }
+    // Otherwise check if that is the shortest form of "gdal read mydataset"
+    // where "read" is omitted: "gdal in.tif"
+    else
     {
         VSIStatBufL sStat;
         for (const auto &arg : args)
@@ -100,9 +112,9 @@ bool GDALMainAlgorithm::ParseCommandLineArguments(
                 }
             }
         }
-    }
 
-    return GDALAlgorithm::ParseCommandLineArguments(args);
+        return GDALAlgorithm::ParseCommandLineArguments(args);
+    }
 }
 
 /************************************************************************/

apps/gdalalg_vector_filter.cpp

@@ -79,9 +79,12 @@ bool GDALVectorFilterAlgorithm::RunStep(GDALProgressFunc, void *)
         }
     }
 
-    m_outputDataset.Set(m_inputDataset.GetDatasetRef());
+    if (ret)
+    {
+        m_outputDataset.Set(m_inputDataset.GetDatasetRef());
+    }
 
-    return true;
+    return ret;
 }
 
 //! @endcond

apps/gdalalg_vector_pipeline.cpp

@@ -21,6 +21,7 @@
 #include "cpl_string.h"
 
 #include <algorithm>
+#include <cassert>
 
 //! @cond Doxygen_Suppress
 
@@ -553,6 +554,7 @@ std::string GDALVectorPipelineAlgorithm::GetUsageForCLI(
     for (const std::string &name : m_stepRegistry.GetNames())
     {
         auto alg = GetStepAlg(name);
+        assert(alg);
         auto [options, maxOptLen] = alg->GetArgNamesForCLI();
         stepUsageOptions.maxOptLen =
             std::max(stepUsageOptions.maxOptLen, maxOptLen);
@@ -562,6 +564,7 @@ std::string GDALVectorPipelineAlgorithm::GetUsageForCLI(
         const auto name = GDALVectorReadAlgorithm::NAME;
         ret += '\n';
         auto alg = GetStepAlg(name);
+        assert(alg);
         alg->SetCallPath({name});
         ret += alg->GetUsageForCLI(shortUsage, stepUsageOptions);
     }
@@ -572,6 +575,7 @@ std::string GDALVectorPipelineAlgorithm::GetUsageForCLI(
         {
             ret += '\n';
             auto alg = GetStepAlg(name);
+            assert(alg);
             alg->SetCallPath({name});
             ret += alg->GetUsageForCLI(shortUsage, stepUsageOptions);
         }
@@ -580,6 +584,7 @@ std::string GDALVectorPipelineAlgorithm::GetUsageForCLI(
         const auto name = GDALVectorWriteAlgorithm::NAME;
         ret += '\n';
         auto alg = GetStepAlg(name);
+        assert(alg);
         alg->SetCallPath({name});
         ret += alg->GetUsageForCLI(shortUsage, stepUsageOptions);
     }

apps/gdalalg_vector_reproject.cpp

@@ -108,11 +108,11 @@ bool GDALVectorReprojectAlgorithm::RunStep(GDALProgressFunc, void *)
 
     const int nLayerCount = poSrcDS->GetLayerCount();
     bool ret = true;
-    for (int i = 0; i < nLayerCount; ++i)
+    for (int i = 0; ret && i < nLayerCount; ++i)
     {
         auto poSrcLayer = poSrcDS->GetLayer(i);
-        ret = ret && (poSrcLayer != nullptr);
-        if (poSrcLayer)
+        ret = (poSrcLayer != nullptr);
+        if (ret)
         {
             OGRSpatialReference *poSrcLayerCRS;
             if (poSrcCRS)
@@ -130,8 +130,8 @@ bool GDALVectorReprojectAlgorithm::RunStep(GDALProgressFunc, void *)
                 OGRCreateCoordinateTransformation(poSrcLayerCRS, &oDstCRS));
             auto poReversedCT = std::unique_ptr<OGRCoordinateTransformation>(
                 OGRCreateCoordinateTransformation(&oDstCRS, poSrcLayerCRS));
-            ret = ret && (poCT != nullptr) && (poReversedCT != nullptr);
-            if (poCT && poReversedCT)
+            ret = (poCT != nullptr) && (poReversedCT != nullptr);
+            if (ret)
             {
                 reprojectedDataset->AddLayer(std::make_unique<OGRWarpedLayer>(
                     poSrcLayer, /* iGeomField = */ 0,
@@ -141,7 +141,8 @@ bool GDALVectorReprojectAlgorithm::RunStep(GDALProgressFunc, void *)
         }
     }
 
-    m_outputDataset.Set(std::move(reprojectedDataset));
+    if (ret)
+        m_outputDataset.Set(std::move(reprojectedDataset));
 
     return ret;
 }

port/cpl_string.cpp

@@ -3186,6 +3186,8 @@ std::string CPLRemoveSQLComments(const std::string &osInput)
             {
                 if (pszLine[i] == chQuote)
                 {
+                    // Deal with escaped quote character which is repeated,
+                    // so 'foo''bar' or "foo""bar"
                     if (pszLine[i + 1] == chQuote)
                     {
                         i++;