From d00a3a826b15eb440482ba53b2bc2baca6fa48d3 Mon Sep 17 00:00:00 2001 From: Alessandro Pasotti Date: Wed, 24 Apr 2024 11:40:41 +0200 Subject: [PATCH 1/5] [gdalcontour] Use argparser Uses the new argparser for the gdalcontour options. --- apps/gdal_contour.cpp | 520 +++++++++++++++++++----------------- apps/gdal_utils_priv.h | 2 + apps/gdalargumentparser.cpp | 30 ++- apps/gdalargumentparser.h | 16 +- 4 files changed, 295 insertions(+), 273 deletions(-) diff --git a/apps/gdal_contour.cpp b/apps/gdal_contour.cpp index 5f4fd9c9952f..ec40d3906495 100644 --- a/apps/gdal_contour.cpp +++ b/apps/gdal_contour.cpp @@ -33,45 +33,193 @@ #include "gdal_version.h" #include "gdal.h" #include "gdal_alg.h" +#include "gdalargumentparser.h" #include "ogr_api.h" #include "ogr_srs_api.h" #include "commonutils.h" /************************************************************************/ -/* ArgIsNumericContour() */ +/* GDALContourOptions */ /************************************************************************/ -static bool ArgIsNumericContour(const char *pszArg) +struct GDALContourOptions +{ + int nBand = 1; + double dfInterval = 0.0; + double dfNoData = 0.0; + double dfOffset = 0.0; + double dfExpBase = 0.0; + bool b3D = false; + bool bPolygonize = false; + bool bNoDataSet = false; + bool bIgnoreNoData = false; + std::string osNewLayerName = "contour"; + std::string osFormat; + std::string osElevAttrib; + std::string osElevAttribMin; + std::string osElevAttribMax; + std::vector adfFixedLevels; + CPLStringList aosOpenOptions; + CPLStringList aosCreationOptions; + bool bQuiet = false; + std::string aosDestFilename; + std::string aosSrcFilename; +}; + +/************************************************************************/ +/* GDALContourAppOptionsGetParser() */ +/************************************************************************/ +static std::unique_ptr +GDALContourAppOptionsGetParser(GDALContourOptions *psOptions) { - return CPLGetValueType(pszArg) != CPL_VALUE_STRING; + auto argParser = std::make_unique( + "gdal_contour", /* bForBinary */ true); + + argParser->add_description(_("Creates contour lines from a raster file.")); + argParser->add_epilog(_( + "For more details, consult the full documentation for the gdal_contour " + "utility: http://gdal.org/gdal_contour.html")); + + argParser->add_argument("-b") + .metavar("") + .default_value(1) + .nargs(1) + .scan<'i', int>() + .store_into(psOptions->nBand) + .help(_("Select an input band band containing the DEM data.")); + + argParser->add_argument("-a") + .metavar("") + .store_into(psOptions->osElevAttrib) + .help(_("Provides a name for the attribute in which to put the " + "elevation.")); + + argParser->add_argument("-amin") + .metavar("") + .store_into(psOptions->osElevAttribMin) + .help(_("Provides a name for the attribute in which to put the minimum " + "elevation.")); + + argParser->add_argument("-amax") + .metavar("") + .store_into(psOptions->osElevAttribMax) + .help(_("Provides a name for the attribute in which to put the maximum " + "elevation.")); + + argParser->add_argument("-3d") + .flag() + .store_into(psOptions->b3D) + .help(_("Force production of 3D vectors instead of 2D.")); + + argParser->add_argument("-inodata") + .flag() + .store_into(psOptions->bIgnoreNoData) + .help(_("Ignore any nodata value implied in the dataset - treat all " + "values as valid.")); + + argParser->add_argument("-snodata") + .metavar("") + .scan<'g', double>() + .action( + [psOptions](const auto &d) + { + psOptions->bNoDataSet = true; + psOptions->dfNoData = CPLAtofM(d.c_str()); + }) + .help(_("Input pixel value to treat as \"nodata\".")); + + argParser->add_argument("-f") + .metavar("") + .store_into(psOptions->osFormat) + .help(_("Select the output format. Normally the format is guessed from " + "the file extension.")); + + argParser->add_argument("-dsco") + .metavar("=") + .append() + .action([psOptions](const std::string &s) + { psOptions->aosOpenOptions.AddString(s.c_str()); }) + .help(_("Dataset creation option (format specific).")); + + argParser->add_argument("-lco") + .metavar("=") + .append() + .action([psOptions](const std::string &s) + { psOptions->aosCreationOptions.AddString(s.c_str()); }) + .help(_("Layer creation option (format specific).")); + + auto &group = argParser->add_mutually_exclusive_group(); + + group.add_argument("-i") + .metavar("") + .scan<'g', double>() + .store_into(psOptions->dfInterval) + .help(_("Elevation interval between contours.")); + + group.add_argument("-fl") + .metavar("") + .scan<'g', double>() + .append() + .action([psOptions](const std::string &s) + { psOptions->adfFixedLevels.push_back(CPLAtof(s.c_str())); }) + .help(_("Name one or more \"fixed levels\" to extract.")); + + group.add_argument("-e") + .metavar("") + .scan<'g', double>() + .store_into(psOptions->dfExpBase) + .help(_("Generate levels on an exponential scale: base ^ k, for k an " + "integer.")); + + argParser->add_argument("-off") + .metavar("") + .scan<'g', double>() + .store_into(psOptions->dfOffset) + .help(_("Offset from zero relative to which to interpret intervals.")); + + argParser->add_argument("-nln") + .metavar("") + .store_into(psOptions->osNewLayerName) + .help(_("Provide a name for the output vector layer. Defaults to " + "\"contour\".")); + + argParser->add_argument("-p") + .flag() + .store_into(psOptions->bPolygonize) + .help(_("Generate contour polygons instead of lines.")); + + argParser->add_quiet_argument(&psOptions->bQuiet); + + argParser->add_argument("src_filename") + .store_into(psOptions->aosSrcFilename) + .help("The source raster file."); + + argParser->add_argument("dst_filename") + .store_into(psOptions->aosDestFilename) + .help("The destination vector file."); + + return argParser; } /************************************************************************/ -/* Usage() */ +/* GDALInfoAppGetParserUsage() */ /************************************************************************/ -static void Usage(bool bIsError, const char *pszErrorMsg = nullptr) - +std::string GDALContourAppGetParserUsage() { - fprintf( - bIsError ? stderr : stdout, - "Usage: gdal_contour [--help] [--help-general]\n" - " [-b ] [-a ] [-amin " - "] [-amax ]\n" - " [-3d] [-inodata] [-snodata ] [-f ] " - "[-i ]\n" - " [-dsco =]... " - "[-lco =]...\n" - " [-off ] [-fl ...] [-e " - "]\n" - " [-nln ] [-q] [-p]\n" - " \n"); - - if (pszErrorMsg != nullptr) - fprintf(stderr, "\nFAILURE: %s\n", pszErrorMsg); - - exit(bIsError ? 1 : 0); + try + { + GDALContourOptions sOptions; + auto argParser = GDALContourAppOptionsGetParser(&sOptions); + return argParser->usage(); + } + catch (const std::exception &err) + { + CPLError(CE_Failure, CPLE_AppDefined, "Unexpected exception: %s", + err.what()); + return std::string(); + } } static void CreateElevAttrib(const char *pszElevAttrib, OGRLayerH hLayer) @@ -89,225 +237,87 @@ static void CreateElevAttrib(const char *pszElevAttrib, OGRLayerH hLayer) /* main() */ /************************************************************************/ -#define CHECK_HAS_ENOUGH_ADDITIONAL_ARGS(nExtraArg) \ - do \ - { \ - if (i + nExtraArg >= argc) \ - Usage(true, CPLSPrintf("%s option requires %d argument(s)", \ - argv[i], nExtraArg)); \ - } while (false) - MAIN_START(argc, argv) { - bool b3D = false; - int bNoDataSet = FALSE; - bool bIgnoreNoData = false; - int nBandIn = 1; - double dfInterval = 0.0; - double dfNoData = 0.0; - double dfOffset = 0.0; - double dfExpBase = 0.0; - const char *pszSrcFilename = nullptr; - const char *pszDstFilename = nullptr; - const char *pszElevAttrib = nullptr; - const char *pszElevAttribMin = nullptr; - const char *pszElevAttribMax = nullptr; - const char *pszFormat = nullptr; - char **papszDSCO = nullptr; - char **papszLCO = nullptr; - double adfFixedLevels[1000]; - int nFixedLevelCount = 0; - const char *pszNewLayerName = "contour"; - bool bQuiet = false; + GDALProgressFunc pfnProgress = nullptr; - bool bPolygonize = false; - // Check that we are running against at least GDAL 1.4. - // Note to developers: if we use newer API, please change the requirement. - if (atoi(GDALVersionInfo("VERSION_NUM")) < 1400) - { - fprintf(stderr, - "At least, GDAL >= 1.4.0 is required for this version of %s, " - "which was compiled against GDAL %s\n", - argv[0], GDAL_RELEASE_NAME); - exit(1); - } + EarlySetConfigOptions(argc, argv); + + /* -------------------------------------------------------------------- */ + /* Register standard GDAL drivers, and process generic GDAL */ + /* command options. */ + /* -------------------------------------------------------------------- */ GDALAllRegister(); OGRRegisterAll(); argc = GDALGeneralCmdLineProcessor(argc, &argv, 0); + if (argc < 1) + exit(-argc); /* -------------------------------------------------------------------- */ /* Parse arguments. */ /* -------------------------------------------------------------------- */ + + CPLStringList aosArgv; + for (int i = 1; i < argc; i++) { - if (EQUAL(argv[i], "--utility_version")) - { - printf("%s was compiled against GDAL %s and " - "is running against GDAL %s\n", - argv[0], GDAL_RELEASE_NAME, GDALVersionInfo("RELEASE_NAME")); - CSLDestroy(argv); - return 0; - } - else if (EQUAL(argv[i], "--help")) - Usage(false); - else if (EQUAL(argv[i], "-a")) - { - CHECK_HAS_ENOUGH_ADDITIONAL_ARGS(1); - // coverity[tainted_data] - pszElevAttrib = argv[++i]; - } - else if (EQUAL(argv[i], "-amin")) - { - CHECK_HAS_ENOUGH_ADDITIONAL_ARGS(1); - // coverity[tainted_data] - pszElevAttribMin = argv[++i]; - } - else if (EQUAL(argv[i], "-amax")) - { - CHECK_HAS_ENOUGH_ADDITIONAL_ARGS(1); - // coverity[tainted_data] - pszElevAttribMax = argv[++i]; - } - else if (EQUAL(argv[i], "-off")) - { - CHECK_HAS_ENOUGH_ADDITIONAL_ARGS(1); - // coverity[tainted_data] - dfOffset = CPLAtof(argv[++i]); - } - else if (EQUAL(argv[i], "-i")) - { - CHECK_HAS_ENOUGH_ADDITIONAL_ARGS(1); - // coverity[tainted_data] - dfInterval = CPLAtof(argv[++i]); - } - else if (EQUAL(argv[i], "-e")) - { - CHECK_HAS_ENOUGH_ADDITIONAL_ARGS(1); - // coverity[tainted_data] - dfExpBase = CPLAtof(argv[++i]); - } - else if (EQUAL(argv[i], "-p")) - { - bPolygonize = true; - } - else if (EQUAL(argv[i], "-fl")) - { - if (i >= argc - 1) - Usage(true, CPLSPrintf("%s option requires at least 1 argument", - argv[i])); - while (i < argc - 1 && - nFixedLevelCount < static_cast(sizeof(adfFixedLevels) / - sizeof(double)) && - ArgIsNumericContour(argv[i + 1])) - // coverity[tainted_data] - adfFixedLevels[nFixedLevelCount++] = CPLAtof(argv[++i]); - } - else if (EQUAL(argv[i], "-b")) - { - CHECK_HAS_ENOUGH_ADDITIONAL_ARGS(1); - // coverity[tainted_data] - nBandIn = atoi(argv[++i]); - } - else if (EQUAL(argv[i], "-f") || EQUAL(argv[i], "-of")) - { - CHECK_HAS_ENOUGH_ADDITIONAL_ARGS(1); - // coverity[tainted_data] - pszFormat = argv[++i]; - } - else if (EQUAL(argv[i], "-dsco")) - { - CHECK_HAS_ENOUGH_ADDITIONAL_ARGS(1); - // coverity[tainted_data] - papszDSCO = CSLAddString(papszDSCO, argv[++i]); - } - else if (EQUAL(argv[i], "-lco")) - { - CHECK_HAS_ENOUGH_ADDITIONAL_ARGS(1); - // coverity[tainted_data] - papszLCO = CSLAddString(papszLCO, argv[++i]); - } - else if (EQUAL(argv[i], "-3d")) - { - b3D = true; - } - else if (EQUAL(argv[i], "-snodata")) - { - CHECK_HAS_ENOUGH_ADDITIONAL_ARGS(1); - bNoDataSet = TRUE; - // coverity[tainted_data] - dfNoData = CPLAtof(argv[++i]); - } - else if (EQUAL(argv[i], "-nln")) - { - CHECK_HAS_ENOUGH_ADDITIONAL_ARGS(1); - // coverity[tainted_data] - pszNewLayerName = argv[++i]; - } - else if (EQUAL(argv[i], "-inodata")) - { - bIgnoreNoData = true; - } - else if (EQUAL(argv[i], "-q") || EQUAL(argv[i], "-quiet")) - { - bQuiet = TRUE; - } - else if (pszSrcFilename == nullptr) - { - pszSrcFilename = argv[i]; - } - else if (pszDstFilename == nullptr) - { - pszDstFilename = argv[i]; - } - else - Usage(true, "Too many command options."); + aosArgv.AddString(argv[i]); } - if (dfInterval == 0.0 && nFixedLevelCount == 0 && dfExpBase == 0.0) + if (aosArgv.size() < 1) { - Usage(true, "Neither -i nor -fl nor -e are specified."); + fprintf(stderr, "%s\n", GDALContourAppGetParserUsage().c_str()); + exit(1); } - if (pszSrcFilename == nullptr) + GDALContourOptions sOptions; + + try { - Usage(true, "Missing source filename."); + auto argParser = GDALContourAppOptionsGetParser(&sOptions); + argParser->parse_args_without_binary_name(argv + 1); } - - if (pszDstFilename == nullptr) + catch (const std::exception &error) { - Usage(true, "Missing destination filename."); + CPLError(CE_Failure, CPLE_AppDefined, "%s", error.what()); + exit(1); } - if (strcmp(pszDstFilename, "/vsistdout/") == 0 || - strcmp(pszDstFilename, "/dev/stdout") == 0) + if (sOptions.aosSrcFilename.find("/vsistdout/") != std::string::npos || + sOptions.aosDestFilename.find("/vsistdout/") != std::string::npos) { - bQuiet = true; + sOptions.bQuiet = true; } - if (!bQuiet) + if (!sOptions.bQuiet) pfnProgress = GDALTermProgress; /* -------------------------------------------------------------------- */ /* Open source raster file. */ /* -------------------------------------------------------------------- */ - GDALDatasetH hSrcDS = GDALOpen(pszSrcFilename, GA_ReadOnly); + GDALDatasetH hSrcDS = + GDALOpen(sOptions.aosSrcFilename.c_str(), GA_ReadOnly); if (hSrcDS == nullptr) exit(2); - GDALRasterBandH hBand = GDALGetRasterBand(hSrcDS, nBandIn); + GDALRasterBandH hBand = GDALGetRasterBand(hSrcDS, sOptions.nBand); if (hBand == nullptr) { CPLError(CE_Failure, CPLE_AppDefined, - "Band %d does not exist on dataset.", nBandIn); + "Band %d does not exist on dataset.", sOptions.nBand); exit(2); } - if (!bNoDataSet && !bIgnoreNoData) - dfNoData = GDALGetRasterNoDataValue(hBand, &bNoDataSet); + if (!sOptions.bNoDataSet && !sOptions.bIgnoreNoData) + { + int bNoDataSet; + sOptions.dfNoData = GDALGetRasterNoDataValue(hBand, &bNoDataSet); + sOptions.bNoDataSet = bNoDataSet; + } /* -------------------------------------------------------------------- */ /* Try to get a coordinate system from the raster. */ @@ -318,14 +328,14 @@ MAIN_START(argc, argv) /* Create the output file. */ /* -------------------------------------------------------------------- */ CPLString osFormat; - if (pszFormat == nullptr) + if (!sOptions.osFormat.empty()) { - const auto aoDrivers = - GetOutputDriversFor(pszDstFilename, GDAL_OF_VECTOR); + const auto aoDrivers = GetOutputDriversFor( + sOptions.aosDestFilename.c_str(), GDAL_OF_VECTOR); if (aoDrivers.empty()) { CPLError(CE_Failure, CPLE_AppDefined, "Cannot guess driver for %s", - pszDstFilename); + sOptions.aosDestFilename.c_str()); exit(10); } else @@ -334,14 +344,15 @@ MAIN_START(argc, argv) { CPLError(CE_Warning, CPLE_AppDefined, "Several drivers matching %s extension. Using %s", - CPLGetExtension(pszDstFilename), aoDrivers[0].c_str()); + CPLGetExtension(sOptions.aosDestFilename.c_str()), + aoDrivers[0].c_str()); } osFormat = aoDrivers[0]; } } else { - osFormat = pszFormat; + osFormat = sOptions.osFormat; } OGRSFDriverH hDriver = OGRGetDriverByName(osFormat.c_str()); @@ -353,16 +364,17 @@ MAIN_START(argc, argv) exit(10); } - OGRDataSourceH hDS = - OGR_Dr_CreateDataSource(hDriver, pszDstFilename, papszDSCO); + OGRDataSourceH hDS = OGR_Dr_CreateDataSource( + hDriver, sOptions.aosDestFilename.c_str(), sOptions.aosCreationOptions); if (hDS == nullptr) exit(1); OGRLayerH hLayer = OGR_DS_CreateLayer( - hDS, pszNewLayerName, hSRS, - bPolygonize ? (b3D ? wkbMultiPolygon25D : wkbMultiPolygon) - : (b3D ? wkbLineString25D : wkbLineString), - papszLCO); + hDS, sOptions.osNewLayerName.c_str(), hSRS, + sOptions.bPolygonize + ? (sOptions.b3D ? wkbMultiPolygon25D : wkbMultiPolygon) + : (sOptions.b3D ? wkbLineString25D : wkbLineString), + sOptions.aosCreationOptions); if (hLayer == nullptr) exit(1); @@ -371,11 +383,11 @@ MAIN_START(argc, argv) OGR_L_CreateField(hLayer, hFld, FALSE); OGR_Fld_Destroy(hFld); - if (bPolygonize) + if (sOptions.bPolygonize) { - if (pszElevAttrib) + if (!sOptions.osElevAttrib.empty()) { - pszElevAttrib = nullptr; + sOptions.osElevAttrib.clear(); CPLError(CE_Warning, CPLE_NotSupported, "-a is ignored in polygonal contouring mode. " "Use -amin and/or -amax instead"); @@ -383,88 +395,94 @@ MAIN_START(argc, argv) } else { - if (pszElevAttribMin != nullptr || pszElevAttribMax != nullptr) + if (!sOptions.osElevAttribMin.empty() || + !sOptions.osElevAttribMax.empty()) { - pszElevAttribMin = nullptr; - pszElevAttribMax = nullptr; + sOptions.osElevAttribMin.clear(); + sOptions.osElevAttribMax.clear(); CPLError(CE_Warning, CPLE_NotSupported, "-amin and/or -amax are ignored in line contouring mode. " "Use -a instead"); } } - if (pszElevAttrib) + if (!sOptions.osElevAttrib.empty()) { - CreateElevAttrib(pszElevAttrib, hLayer); + CreateElevAttrib(sOptions.osElevAttrib.c_str(), hLayer); } - if (pszElevAttribMin) + if (sOptions.osElevAttribMin.empty()) { - CreateElevAttrib(pszElevAttribMin, hLayer); + CreateElevAttrib(sOptions.osElevAttribMin.c_str(), hLayer); } - if (pszElevAttribMax) + if (sOptions.osElevAttribMax.empty()) { - CreateElevAttrib(pszElevAttribMax, hLayer); + CreateElevAttrib(sOptions.osElevAttribMax.c_str(), hLayer); } /* -------------------------------------------------------------------- */ /* Invoke. */ /* -------------------------------------------------------------------- */ int iIDField = OGR_FD_GetFieldIndex(OGR_L_GetLayerDefn(hLayer), "ID"); - int iElevField = - (pszElevAttrib == nullptr) - ? -1 - : OGR_FD_GetFieldIndex(OGR_L_GetLayerDefn(hLayer), pszElevAttrib); + int iElevField = (sOptions.osElevAttrib.empty()) + ? -1 + : OGR_FD_GetFieldIndex(OGR_L_GetLayerDefn(hLayer), + sOptions.osElevAttrib.c_str()); - int iElevFieldMin = (pszElevAttribMin == nullptr) - ? -1 - : OGR_FD_GetFieldIndex(OGR_L_GetLayerDefn(hLayer), - pszElevAttribMin); + int iElevFieldMin = + (sOptions.osElevAttribMin.empty()) + ? -1 + : OGR_FD_GetFieldIndex(OGR_L_GetLayerDefn(hLayer), + sOptions.osElevAttribMin.c_str()); - int iElevFieldMax = (pszElevAttribMax == nullptr) - ? -1 - : OGR_FD_GetFieldIndex(OGR_L_GetLayerDefn(hLayer), - pszElevAttribMax); + int iElevFieldMax = + (sOptions.osElevAttribMax.empty()) + ? -1 + : OGR_FD_GetFieldIndex(OGR_L_GetLayerDefn(hLayer), + sOptions.osElevAttribMax.c_str()); char **options = nullptr; - if (nFixedLevelCount > 0) + if (!sOptions.adfFixedLevels.empty()) { std::string values = "FIXED_LEVELS="; - for (int i = 0; i < nFixedLevelCount; i++) + for (size_t i = 0; i < sOptions.adfFixedLevels.size(); i++) { const int sz = 32; char *newValue = new char[sz + 1]; - if (i == nFixedLevelCount - 1) + if (i == sOptions.adfFixedLevels.size() - 1) { - CPLsnprintf(newValue, sz + 1, "%f", adfFixedLevels[i]); + CPLsnprintf(newValue, sz + 1, "%f", sOptions.adfFixedLevels[i]); } else { - CPLsnprintf(newValue, sz + 1, "%f,", adfFixedLevels[i]); + CPLsnprintf(newValue, sz + 1, "%f,", + sOptions.adfFixedLevels[i]); } values = values + std::string(newValue); delete[] newValue; } options = CSLAddString(options, values.c_str()); } - else if (dfExpBase != 0.0) + else if (sOptions.dfExpBase != 0.0) { - options = CSLAppendPrintf(options, "LEVEL_EXP_BASE=%f", dfExpBase); + options = + CSLAppendPrintf(options, "LEVEL_EXP_BASE=%f", sOptions.dfExpBase); } - else if (dfInterval != 0.0) + else if (sOptions.dfInterval != 0.0) { - options = CSLAppendPrintf(options, "LEVEL_INTERVAL=%f", dfInterval); + options = + CSLAppendPrintf(options, "LEVEL_INTERVAL=%f", sOptions.dfInterval); } - if (dfOffset != 0.0) + if (sOptions.dfOffset != 0.0) { - options = CSLAppendPrintf(options, "LEVEL_BASE=%f", dfOffset); + options = CSLAppendPrintf(options, "LEVEL_BASE=%f", sOptions.dfOffset); } - if (bNoDataSet) + if (sOptions.bNoDataSet) { - options = CSLAppendPrintf(options, "NODATA=%.19g", dfNoData); + options = CSLAppendPrintf(options, "NODATA=%.19g", sOptions.dfNoData); } if (iIDField != -1) { @@ -482,7 +500,7 @@ MAIN_START(argc, argv) { options = CSLAppendPrintf(options, "ELEV_FIELD_MAX=%d", iElevFieldMax); } - if (bPolygonize) + if (sOptions.bPolygonize) { options = CSLAppendPrintf(options, "POLYGONIZE=YES"); } @@ -496,8 +514,6 @@ MAIN_START(argc, argv) GDALClose(hSrcDS); CSLDestroy(argv); - CSLDestroy(papszDSCO); - CSLDestroy(papszLCO); GDALDestroyDriverManager(); OGRCleanupAll(); diff --git a/apps/gdal_utils_priv.h b/apps/gdal_utils_priv.h index 28784130975c..dce8c5b04ccc 100644 --- a/apps/gdal_utils_priv.h +++ b/apps/gdal_utils_priv.h @@ -243,6 +243,8 @@ std::string CPL_DLL GDALGridGetParserUsage(); std::string CPL_DLL GDALBuildVRTGetParserUsage(); +std::string CPL_DLL GDALContourGetParserUsage(); + #endif /* #ifndef DOXYGEN_SKIP */ #endif /* GDAL_UTILS_PRIV_H_INCLUDED */ diff --git a/apps/gdalargumentparser.cpp b/apps/gdalargumentparser.cpp index 8870b03805d5..daef1ce6dba1 100644 --- a/apps/gdalargumentparser.cpp +++ b/apps/gdalargumentparser.cpp @@ -107,7 +107,7 @@ void GDALArgumentParser::display_error_and_usage(const std::exception &err) /* add_quiet_argument() */ /************************************************************************/ -void GDALArgumentParser::add_quiet_argument(bool *pVar) +Argument &GDALArgumentParser::add_quiet_argument(bool *pVar) { auto &arg = this->add_argument("-q", "--quiet") @@ -117,15 +117,16 @@ void GDALArgumentParser::add_quiet_argument(bool *pVar) "output.")); if (pVar) arg.store_into(*pVar); + return arg; } /************************************************************************/ /* add_input_format_argument() */ /************************************************************************/ -void GDALArgumentParser::add_input_format_argument(CPLStringList *pvar) +Argument &GDALArgumentParser::add_input_format_argument(CPLStringList *pvar) { - add_argument("-if") + return add_argument("-if") .append() .metavar("") .action( @@ -149,22 +150,23 @@ void GDALArgumentParser::add_input_format_argument(CPLStringList *pvar) /* add_output_format_argument() */ /************************************************************************/ -void GDALArgumentParser::add_output_format_argument(std::string &var) +Argument &GDALArgumentParser::add_output_format_argument(std::string &var) { auto &arg = add_argument("-of") .metavar("") .store_into(var) .help(_("Output format.")); add_hidden_alias_for(arg, "-f"); + return arg; } /************************************************************************/ /* add_creation_options_argument() */ /************************************************************************/ -void GDALArgumentParser::add_creation_options_argument(CPLStringList &var) +Argument &GDALArgumentParser::add_creation_options_argument(CPLStringList &var) { - add_argument("-co") + return add_argument("-co") .metavar("=") .append() .action([&var](const std::string &s) { var.AddString(s.c_str()); }) @@ -175,9 +177,10 @@ void GDALArgumentParser::add_creation_options_argument(CPLStringList &var) /* add_metadata_item_options_argument() */ /************************************************************************/ -void GDALArgumentParser::add_metadata_item_options_argument(CPLStringList &var) +Argument & +GDALArgumentParser::add_metadata_item_options_argument(CPLStringList &var) { - add_argument("-mo") + return add_argument("-mo") .metavar("=") .append() .action([&var](const std::string &s) { var.AddString(s.c_str()); }) @@ -188,16 +191,16 @@ void GDALArgumentParser::add_metadata_item_options_argument(CPLStringList &var) /* add_open_options_argument() */ /************************************************************************/ -void GDALArgumentParser::add_open_options_argument(CPLStringList &var) +Argument &GDALArgumentParser::add_open_options_argument(CPLStringList &var) { - add_open_options_argument(&var); + return add_open_options_argument(&var); } /************************************************************************/ /* add_open_options_argument() */ /************************************************************************/ -void GDALArgumentParser::add_open_options_argument(CPLStringList *pvar) +Argument &GDALArgumentParser::add_open_options_argument(CPLStringList *pvar) { auto &arg = add_argument("-oo") .metavar("=") @@ -208,15 +211,16 @@ void GDALArgumentParser::add_open_options_argument(CPLStringList *pvar) arg.action([pvar](const std::string &s) { pvar->AddString(s.c_str()); }); } + return arg; } /************************************************************************/ /* add_output_type_argument() */ /************************************************************************/ -void GDALArgumentParser::add_output_type_argument(GDALDataType &eDT) +Argument &GDALArgumentParser::add_output_type_argument(GDALDataType &eDT) { - add_argument("-ot") + return add_argument("-ot") .metavar("Byte|Int8|[U]Int{16|32|64}|CInt{16|32}|[C]Float{32|64}") .action( [&eDT](const std::string &s) diff --git a/apps/gdalargumentparser.h b/apps/gdalargumentparser.h index c5b545748bf2..6a4929bd4e7b 100644 --- a/apps/gdalargumentparser.h +++ b/apps/gdalargumentparser.h @@ -64,28 +64,28 @@ class GDALArgumentParser : public ArgumentParser void display_error_and_usage(const std::exception &err); //! Add -q/--quiet argument, and store its value in *pVar (if pVar not null) - void add_quiet_argument(bool *pVar); + Argument &add_quiet_argument(bool *pVar); //! Add "-if format_name" argument for input format, and store its value into *pvar. - void add_input_format_argument(CPLStringList *pvar); + Argument &add_input_format_argument(CPLStringList *pvar); //! Add "-of format_name" argument for output format, and store its value into var. - void add_output_format_argument(std::string &var); + Argument &add_output_format_argument(std::string &var); //! Add "-co KEY=VALUE" argument for creation options, and store its value into var. - void add_creation_options_argument(CPLStringList &var); + Argument &add_creation_options_argument(CPLStringList &var); //! Add "-mo KEY=VALUE" argument for metadata item options, and store its value into var. - void add_metadata_item_options_argument(CPLStringList &var); + Argument &add_metadata_item_options_argument(CPLStringList &var); //! Add "-oo KEY=VALUE" argument for open options, and store its value into var. - void add_open_options_argument(CPLStringList &var); + Argument &add_open_options_argument(CPLStringList &var); //! Add "-oo KEY=VALUE" argument for open options, and store its value into *pvar. - void add_open_options_argument(CPLStringList *pvar); + Argument &add_open_options_argument(CPLStringList *pvar); //! Add "-ot data_type" argument for output type, and store its value into eDT. - void add_output_type_argument(GDALDataType &eDT); + Argument &add_output_type_argument(GDALDataType &eDT); //! Parse command line arguments, without the initial program name. void parse_args_without_binary_name(CSLConstList papszArgs); From 2cb0219c418585ee123d97d812db4d89dda7a4b8 Mon Sep 17 00:00:00 2001 From: Alessandro Pasotti Date: Wed, 24 Apr 2024 13:18:52 +0200 Subject: [PATCH 2/5] Fix after test run --- apps/gdal_contour.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/apps/gdal_contour.cpp b/apps/gdal_contour.cpp index ec40d3906495..55e1f5a57c03 100644 --- a/apps/gdal_contour.cpp +++ b/apps/gdal_contour.cpp @@ -159,8 +159,8 @@ GDALContourAppOptionsGetParser(GDALContourOptions *psOptions) group.add_argument("-fl") .metavar("") + .nargs(argparse::nargs_pattern::at_least_one) .scan<'g', double>() - .append() .action([psOptions](const std::string &s) { psOptions->adfFixedLevels.push_back(CPLAtof(s.c_str())); }) .help(_("Name one or more \"fixed levels\" to extract.")); @@ -328,7 +328,7 @@ MAIN_START(argc, argv) /* Create the output file. */ /* -------------------------------------------------------------------- */ CPLString osFormat; - if (!sOptions.osFormat.empty()) + if (sOptions.osFormat.empty()) { const auto aoDrivers = GetOutputDriversFor( sOptions.aosDestFilename.c_str(), GDAL_OF_VECTOR); @@ -411,12 +411,12 @@ MAIN_START(argc, argv) CreateElevAttrib(sOptions.osElevAttrib.c_str(), hLayer); } - if (sOptions.osElevAttribMin.empty()) + if (!sOptions.osElevAttribMin.empty()) { CreateElevAttrib(sOptions.osElevAttribMin.c_str(), hLayer); } - if (sOptions.osElevAttribMax.empty()) + if (!sOptions.osElevAttribMax.empty()) { CreateElevAttrib(sOptions.osElevAttribMax.c_str(), hLayer); } From 34a3670f0d679eb5809b9e4e49418e9df3377c2b Mon Sep 17 00:00:00 2001 From: Alessandro Pasotti Date: Wed, 24 Apr 2024 13:34:03 +0200 Subject: [PATCH 3/5] Fix typos --- apps/gdal_contour.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/apps/gdal_contour.cpp b/apps/gdal_contour.cpp index 55e1f5a57c03..3c30dced57af 100644 --- a/apps/gdal_contour.cpp +++ b/apps/gdal_contour.cpp @@ -203,10 +203,10 @@ GDALContourAppOptionsGetParser(GDALContourOptions *psOptions) } /************************************************************************/ -/* GDALInfoAppGetParserUsage() */ +/* GDALContourGetParserUsage */ /************************************************************************/ -std::string GDALContourAppGetParserUsage() +std::string GDALContourGetParserUsage() { try { @@ -270,7 +270,7 @@ MAIN_START(argc, argv) if (aosArgv.size() < 1) { - fprintf(stderr, "%s\n", GDALContourAppGetParserUsage().c_str()); + fprintf(stderr, "%s\n", GDALContourGetParserUsage().c_str()); exit(1); } From e6bde264fbe27b02404ff90006b1bd84b5120084 Mon Sep 17 00:00:00 2001 From: Alessandro Pasotti Date: Wed, 24 Apr 2024 14:13:47 +0200 Subject: [PATCH 4/5] Address PR comments --- apps/gdal_contour.cpp | 47 ++++++++++++------------------------------ apps/gdal_utils_priv.h | 2 -- 2 files changed, 13 insertions(+), 36 deletions(-) diff --git a/apps/gdal_contour.cpp b/apps/gdal_contour.cpp index 3c30dced57af..a86f28530671 100644 --- a/apps/gdal_contour.cpp +++ b/apps/gdal_contour.cpp @@ -129,11 +129,7 @@ GDALContourAppOptionsGetParser(GDALContourOptions *psOptions) }) .help(_("Input pixel value to treat as \"nodata\".")); - argParser->add_argument("-f") - .metavar("") - .store_into(psOptions->osFormat) - .help(_("Select the output format. Normally the format is guessed from " - "the file extension.")); + argParser->add_output_format_argument(psOptions->osFormat); argParser->add_argument("-dsco") .metavar("=") @@ -202,26 +198,6 @@ GDALContourAppOptionsGetParser(GDALContourOptions *psOptions) return argParser; } -/************************************************************************/ -/* GDALContourGetParserUsage */ -/************************************************************************/ - -std::string GDALContourGetParserUsage() -{ - try - { - GDALContourOptions sOptions; - auto argParser = GDALContourAppOptionsGetParser(&sOptions); - return argParser->usage(); - } - catch (const std::exception &err) - { - CPLError(CE_Failure, CPLE_AppDefined, "Unexpected exception: %s", - err.what()); - return std::string(); - } -} - static void CreateElevAttrib(const char *pszElevAttrib, OGRLayerH hLayer) { OGRFieldDefnH hFld = OGR_Fld_Create(pszElevAttrib, OFTReal); @@ -261,16 +237,19 @@ MAIN_START(argc, argv) /* Parse arguments. */ /* -------------------------------------------------------------------- */ - CPLStringList aosArgv; - - for (int i = 1; i < argc; i++) - { - aosArgv.AddString(argv[i]); - } - - if (aosArgv.size() < 1) + if (argc < 2) { - fprintf(stderr, "%s\n", GDALContourGetParserUsage().c_str()); + try + { + GDALContourOptions sOptions; + auto argParser = GDALContourAppOptionsGetParser(&sOptions); + fprintf(stderr, "%s\n", argParser->usage().c_str()); + } + catch (const std::exception &err) + { + CPLError(CE_Failure, CPLE_AppDefined, "Unexpected exception: %s", + err.what()); + } exit(1); } diff --git a/apps/gdal_utils_priv.h b/apps/gdal_utils_priv.h index dce8c5b04ccc..28784130975c 100644 --- a/apps/gdal_utils_priv.h +++ b/apps/gdal_utils_priv.h @@ -243,8 +243,6 @@ std::string CPL_DLL GDALGridGetParserUsage(); std::string CPL_DLL GDALBuildVRTGetParserUsage(); -std::string CPL_DLL GDALContourGetParserUsage(); - #endif /* #ifndef DOXYGEN_SKIP */ #endif /* GDAL_UTILS_PRIV_H_INCLUDED */ From 39e258ebabb0566626d57cca95f6fbc47e168226 Mon Sep 17 00:00:00 2001 From: Alessandro Pasotti Date: Wed, 24 Apr 2024 14:15:15 +0200 Subject: [PATCH 5/5] Revert argparser changes --- apps/gdalargumentparser.cpp | 30 +++++++++++++----------------- apps/gdalargumentparser.h | 16 ++++++++-------- 2 files changed, 21 insertions(+), 25 deletions(-) diff --git a/apps/gdalargumentparser.cpp b/apps/gdalargumentparser.cpp index daef1ce6dba1..8870b03805d5 100644 --- a/apps/gdalargumentparser.cpp +++ b/apps/gdalargumentparser.cpp @@ -107,7 +107,7 @@ void GDALArgumentParser::display_error_and_usage(const std::exception &err) /* add_quiet_argument() */ /************************************************************************/ -Argument &GDALArgumentParser::add_quiet_argument(bool *pVar) +void GDALArgumentParser::add_quiet_argument(bool *pVar) { auto &arg = this->add_argument("-q", "--quiet") @@ -117,16 +117,15 @@ Argument &GDALArgumentParser::add_quiet_argument(bool *pVar) "output.")); if (pVar) arg.store_into(*pVar); - return arg; } /************************************************************************/ /* add_input_format_argument() */ /************************************************************************/ -Argument &GDALArgumentParser::add_input_format_argument(CPLStringList *pvar) +void GDALArgumentParser::add_input_format_argument(CPLStringList *pvar) { - return add_argument("-if") + add_argument("-if") .append() .metavar("") .action( @@ -150,23 +149,22 @@ Argument &GDALArgumentParser::add_input_format_argument(CPLStringList *pvar) /* add_output_format_argument() */ /************************************************************************/ -Argument &GDALArgumentParser::add_output_format_argument(std::string &var) +void GDALArgumentParser::add_output_format_argument(std::string &var) { auto &arg = add_argument("-of") .metavar("") .store_into(var) .help(_("Output format.")); add_hidden_alias_for(arg, "-f"); - return arg; } /************************************************************************/ /* add_creation_options_argument() */ /************************************************************************/ -Argument &GDALArgumentParser::add_creation_options_argument(CPLStringList &var) +void GDALArgumentParser::add_creation_options_argument(CPLStringList &var) { - return add_argument("-co") + add_argument("-co") .metavar("=") .append() .action([&var](const std::string &s) { var.AddString(s.c_str()); }) @@ -177,10 +175,9 @@ Argument &GDALArgumentParser::add_creation_options_argument(CPLStringList &var) /* add_metadata_item_options_argument() */ /************************************************************************/ -Argument & -GDALArgumentParser::add_metadata_item_options_argument(CPLStringList &var) +void GDALArgumentParser::add_metadata_item_options_argument(CPLStringList &var) { - return add_argument("-mo") + add_argument("-mo") .metavar("=") .append() .action([&var](const std::string &s) { var.AddString(s.c_str()); }) @@ -191,16 +188,16 @@ GDALArgumentParser::add_metadata_item_options_argument(CPLStringList &var) /* add_open_options_argument() */ /************************************************************************/ -Argument &GDALArgumentParser::add_open_options_argument(CPLStringList &var) +void GDALArgumentParser::add_open_options_argument(CPLStringList &var) { - return add_open_options_argument(&var); + add_open_options_argument(&var); } /************************************************************************/ /* add_open_options_argument() */ /************************************************************************/ -Argument &GDALArgumentParser::add_open_options_argument(CPLStringList *pvar) +void GDALArgumentParser::add_open_options_argument(CPLStringList *pvar) { auto &arg = add_argument("-oo") .metavar("=") @@ -211,16 +208,15 @@ Argument &GDALArgumentParser::add_open_options_argument(CPLStringList *pvar) arg.action([pvar](const std::string &s) { pvar->AddString(s.c_str()); }); } - return arg; } /************************************************************************/ /* add_output_type_argument() */ /************************************************************************/ -Argument &GDALArgumentParser::add_output_type_argument(GDALDataType &eDT) +void GDALArgumentParser::add_output_type_argument(GDALDataType &eDT) { - return add_argument("-ot") + add_argument("-ot") .metavar("Byte|Int8|[U]Int{16|32|64}|CInt{16|32}|[C]Float{32|64}") .action( [&eDT](const std::string &s) diff --git a/apps/gdalargumentparser.h b/apps/gdalargumentparser.h index 6a4929bd4e7b..c5b545748bf2 100644 --- a/apps/gdalargumentparser.h +++ b/apps/gdalargumentparser.h @@ -64,28 +64,28 @@ class GDALArgumentParser : public ArgumentParser void display_error_and_usage(const std::exception &err); //! Add -q/--quiet argument, and store its value in *pVar (if pVar not null) - Argument &add_quiet_argument(bool *pVar); + void add_quiet_argument(bool *pVar); //! Add "-if format_name" argument for input format, and store its value into *pvar. - Argument &add_input_format_argument(CPLStringList *pvar); + void add_input_format_argument(CPLStringList *pvar); //! Add "-of format_name" argument for output format, and store its value into var. - Argument &add_output_format_argument(std::string &var); + void add_output_format_argument(std::string &var); //! Add "-co KEY=VALUE" argument for creation options, and store its value into var. - Argument &add_creation_options_argument(CPLStringList &var); + void add_creation_options_argument(CPLStringList &var); //! Add "-mo KEY=VALUE" argument for metadata item options, and store its value into var. - Argument &add_metadata_item_options_argument(CPLStringList &var); + void add_metadata_item_options_argument(CPLStringList &var); //! Add "-oo KEY=VALUE" argument for open options, and store its value into var. - Argument &add_open_options_argument(CPLStringList &var); + void add_open_options_argument(CPLStringList &var); //! Add "-oo KEY=VALUE" argument for open options, and store its value into *pvar. - Argument &add_open_options_argument(CPLStringList *pvar); + void add_open_options_argument(CPLStringList *pvar); //! Add "-ot data_type" argument for output type, and store its value into eDT. - Argument &add_output_type_argument(GDALDataType &eDT); + void add_output_type_argument(GDALDataType &eDT); //! Parse command line arguments, without the initial program name. void parse_args_without_binary_name(CSLConstList papszArgs);