Skip to content

Commit 2f50cc2

Browse files
rouaultrouault
authored
Merge pull request OSGeo#12230 from rouault/gdal_cli_co_fix
gdal cli: improve tokenization of creation options to avoid detecting FOO=BAR,BAZ as 2 options
2 parents 3c0d901 + 2e3b41f commit 2f50cc2

10 files changed

+145
-28
lines changed

‎apps/gdalalg_mdim_convert.cpp‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -75,9 +75,10 @@ GDALMdimConvertAlgorithm::GDALMdimConvertAlgorithm()
7575
_("Option passed to GDALGroup::GetMDArrayNames() to "
7676
"filter arrays."),
7777
&m_arrayOptions)
78-
.SetMetaVar("<KEY>=<VALUE>");
78+
.SetMetaVar("<KEY>=<VALUE>")
79+
.SetPackedValuesAllowed(false);
7980
arg.AddValidationAction([this, &arg]()
80-
{ return ValidateKeyValue(arg); });
81+
{ return ParseAndValidateKeyValue(arg); });
8182

8283
arg.SetAutoCompleteFunction(
8384
[this](const std::string &currentValue)

‎apps/gdalalg_mdim_info.cpp‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -75,9 +75,10 @@ GDALMdimInfoAlgorithm::GDALMdimInfoAlgorithm()
7575
_("Option passed to GDALGroup::GetMDArrayNames() to "
7676
"filter reported arrays."),
7777
&m_arrayOptions)
78-
.SetMetaVar("<KEY>=<VALUE>");
78+
.SetMetaVar("<KEY>=<VALUE>")
79+
.SetPackedValuesAllowed(false);
7980
arg.AddValidationAction([this, &arg]()
80-
{ return ValidateKeyValue(arg); });
81+
{ return ParseAndValidateKeyValue(arg); });
8182

8283
arg.SetAutoCompleteFunction(
8384
[this](const std::string &currentValue)

‎apps/gdalalg_raster_create.cpp‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -68,9 +68,10 @@ GDALRasterCreateAlgorithm::GDALRasterCreateAlgorithm()
6868

6969
{
7070
auto &arg = AddArg("metadata", 0, _("Add metadata item"), &m_metadata)
71-
.SetMetaVar("<KEY>=<VALUE>");
71+
.SetMetaVar("<KEY>=<VALUE>")
72+
.SetPackedValuesAllowed(false);
7273
arg.AddValidationAction([this, &arg]()
73-
{ return ValidateKeyValue(arg); });
74+
{ return ParseAndValidateKeyValue(arg); });
7475
arg.AddHiddenAlias("mo");
7576
}
7677
AddArg("copy-metadata", 0, _("Copy metadata from input dataset"),

‎apps/gdalalg_raster_edit.cpp‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -60,9 +60,10 @@ GDALRasterEditAlgorithm::GDALRasterEditAlgorithm(bool standaloneStep)
6060
{
6161
auto &arg = AddArg("metadata", 0, _("Add/update dataset metadata item"),
6262
&m_metadata)
63-
.SetMetaVar("<KEY>=<VALUE>");
63+
.SetMetaVar("<KEY>=<VALUE>")
64+
.SetPackedValuesAllowed(false);
6465
arg.AddValidationAction([this, &arg]()
65-
{ return ValidateKeyValue(arg); });
66+
{ return ParseAndValidateKeyValue(arg); });
6667
arg.AddHiddenAlias("mo");
6768
}
6869

‎apps/gdalalg_raster_index.cpp‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -98,9 +98,10 @@ void GDALRasterIndexAlgorithm::AddCommonOptions()
9898
{
9999
auto &arg =
100100
AddArg("metadata", 0, _("Add dataset metadata item"), &m_metadata)
101-
.SetMetaVar("<KEY>=<VALUE>");
101+
.SetMetaVar("<KEY>=<VALUE>")
102+
.SetPackedValuesAllowed(false);
102103
arg.AddValidationAction([this, &arg]()
103-
{ return ValidateKeyValue(arg); });
104+
{ return ParseAndValidateKeyValue(arg); });
104105
arg.AddHiddenAlias("mo");
105106
}
106107
}

‎apps/gdalalg_raster_reproject.cpp‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -97,9 +97,10 @@ GDALRasterReprojectAlgorithm::GDALRasterReprojectAlgorithm(bool standaloneStep)
9797
AddArg("warp-option", 0, _("Warping option(s)"), &m_warpOptions)
9898
.AddAlias("wo")
9999
.SetMetaVar("<NAME>=<VALUE>")
100-
.SetCategory(GAAC_ADVANCED);
100+
.SetCategory(GAAC_ADVANCED)
101+
.SetPackedValuesAllowed(false);
101102
arg.AddValidationAction([this, &arg]()
102-
{ return ValidateKeyValue(arg); });
103+
{ return ParseAndValidateKeyValue(arg); });
103104
arg.SetAutoCompleteFunction(
104105
[](const std::string &currentValue)
105106
{
@@ -114,9 +115,10 @@ GDALRasterReprojectAlgorithm::GDALRasterReprojectAlgorithm(bool standaloneStep)
114115
&m_transformOptions)
115116
.AddAlias("to")
116117
.SetMetaVar("<NAME>=<VALUE>")
117-
.SetCategory(GAAC_ADVANCED);
118+
.SetCategory(GAAC_ADVANCED)
119+
.SetPackedValuesAllowed(false);
118120
arg.AddValidationAction([this, &arg]()
119-
{ return ValidateKeyValue(arg); });
121+
{ return ParseAndValidateKeyValue(arg); });
120122
arg.SetAutoCompleteFunction(
121123
[](const std::string &currentValue)
122124
{

‎apps/gdalalg_vector_edit.cpp‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -61,9 +61,10 @@ GDALVectorEditAlgorithm::GDALVectorEditAlgorithm(bool standaloneStep)
6161
{
6262
auto &arg = AddArg("metadata", 0, _("Add/update dataset metadata item"),
6363
&m_metadata)
64-
.SetMetaVar("<KEY>=<VALUE>");
64+
.SetMetaVar("<KEY>=<VALUE>")
65+
.SetPackedValuesAllowed(false);
6566
arg.AddValidationAction([this, &arg]()
66-
{ return ValidateKeyValue(arg); });
67+
{ return ParseAndValidateKeyValue(arg); });
6768
arg.AddHiddenAlias("mo");
6869
}
6970

@@ -75,9 +76,10 @@ GDALVectorEditAlgorithm::GDALVectorEditAlgorithm(bool standaloneStep)
7576
auto &arg =
7677
AddArg("layer-metadata", 0, _("Add/update layer metadata item"),
7778
&m_layerMetadata)
78-
.SetMetaVar("<KEY>=<VALUE>");
79+
.SetMetaVar("<KEY>=<VALUE>")
80+
.SetPackedValuesAllowed(false);
7981
arg.AddValidationAction([this, &arg]()
80-
{ return ValidateKeyValue(arg); });
82+
{ return ParseAndValidateKeyValue(arg); });
8183
}
8284

8385
AddArg("unset-layer-metadata", 0, _("Remove layer metadata item"),

‎autotest/cpp/test_gdal_algorithm.cpp‎

Lines changed: 48 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3326,7 +3326,54 @@ TEST_F(test_gdal_algorithm, arg_co)
33263326

33273327
EXPECT_TRUE(alg.ParseCommandLineArguments(
33283328
{"--co", "foo=bar", "--co", "bar=baz"}));
3329-
EXPECT_EQ(alg.m_co.size(), 2U);
3329+
const std::vector<std::string> expected{"foo=bar", "bar=baz"};
3330+
EXPECT_EQ(alg.m_co, expected);
3331+
}
3332+
3333+
{
3334+
MyAlgorithm alg;
3335+
alg.GetUsageForCLI(false);
3336+
3337+
EXPECT_TRUE(alg.ParseCommandLineArguments({"--co", "foo=bar,bar=baz"}));
3338+
const std::vector<std::string> expected{"foo=bar", "bar=baz"};
3339+
EXPECT_EQ(alg.m_co, expected);
3340+
}
3341+
3342+
{
3343+
MyAlgorithm alg;
3344+
alg.GetUsageForCLI(false);
3345+
3346+
EXPECT_TRUE(alg.ParseCommandLineArguments({"--co", "foo=bar,baz"}));
3347+
const std::vector<std::string> expected{"foo=bar,baz"};
3348+
EXPECT_EQ(alg.m_co, expected);
3349+
}
3350+
3351+
{
3352+
MyAlgorithm alg;
3353+
alg.GetUsageForCLI(false);
3354+
3355+
EXPECT_TRUE(alg.ParseCommandLineArguments({"--co", "foo=bar=,a"}));
3356+
const std::vector<std::string> expected{"foo=bar=,a"};
3357+
EXPECT_EQ(alg.m_co, expected);
3358+
}
3359+
3360+
{
3361+
MyAlgorithm alg;
3362+
alg.GetUsageForCLI(false);
3363+
3364+
EXPECT_TRUE(alg.ParseCommandLineArguments({"--co", "foo=bar,,"}));
3365+
const std::vector<std::string> expected{"foo=bar,,"};
3366+
EXPECT_EQ(alg.m_co, expected);
3367+
}
3368+
3369+
{
3370+
MyAlgorithm alg;
3371+
alg.GetUsageForCLI(false);
3372+
3373+
EXPECT_TRUE(
3374+
alg.ParseCommandLineArguments({"--co", "foo=bar,\"foo=baz\""}));
3375+
const std::vector<std::string> expected{"foo=bar,\"foo=baz\""};
3376+
EXPECT_EQ(alg.m_co, expected);
33303377
}
33313378

33323379
{

‎gcore/gdalalgorithm.cpp‎

Lines changed: 69 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3178,9 +3178,13 @@ GDALAlgorithm::AddOpenOptionsArg(std::vector<std::string> *pValue,
31783178
auto &arg = AddArg(GDAL_ARG_NAME_OPEN_OPTION, 0,
31793179
MsgOrDefault(helpMessage, _("Open options")), pValue)
31803180
.AddAlias("oo")
3181-
.SetMetaVar("KEY=VALUE")
3181+
.SetMetaVar("<KEY>=<VALUE>")
3182+
.SetPackedValuesAllowed(false)
31823183
.SetCategory(GAAC_ADVANCED);
31833184

3185+
arg.AddValidationAction([this, &arg]()
3186+
{ return ParseAndValidateKeyValue(arg); });
3187+
31843188
arg.SetAutoCompleteFunction(
31853189
[this](const std::string &currentValue)
31863190
{
@@ -3692,10 +3696,10 @@ GDALAlgorithm::AddBandArg(std::vector<int> *pValue, const char *helpMessage)
36923696
}
36933697

36943698
/************************************************************************/
3695-
/* ValidateKeyValue() */
3699+
/* ParseAndValidateKeyValue() */
36963700
/************************************************************************/
36973701

3698-
bool GDALAlgorithm::ValidateKeyValue(const GDALAlgorithmArg &arg) const
3702+
bool GDALAlgorithm::ParseAndValidateKeyValue(GDALAlgorithmArg &arg)
36993703
{
37003704
const auto Validate = [this, &arg](const std::string &val)
37013705
{
@@ -3717,7 +3721,60 @@ bool GDALAlgorithm::ValidateKeyValue(const GDALAlgorithmArg &arg) const
37173721
}
37183722
else if (arg.GetType() == GAAT_STRING_LIST)
37193723
{
3720-
for (const auto &val : arg.Get<std::vector<std::string>>())
3724+
std::vector<std::string> &vals = arg.Get<std::vector<std::string>>();
3725+
if (vals.size() == 1)
3726+
{
3727+
// Try to split A=B,C=D into A=B and C=D if there is no ambiguity
3728+
std::vector<std::string> newVals;
3729+
std::string curToken;
3730+
bool canSplitOnComma = true;
3731+
char lastSep = 0;
3732+
bool inString = false;
3733+
bool equalFoundInLastToken = false;
3734+
for (char c : vals[0])
3735+
{
3736+
if (!inString && c == ',')
3737+
{
3738+
if (lastSep != '=' || !equalFoundInLastToken)
3739+
{
3740+
canSplitOnComma = false;
3741+
break;
3742+
}
3743+
lastSep = c;
3744+
newVals.push_back(curToken);
3745+
curToken.clear();
3746+
equalFoundInLastToken = false;
3747+
}
3748+
else if (!inString && c == '=')
3749+
{
3750+
if (lastSep == '=')
3751+
{
3752+
canSplitOnComma = false;
3753+
break;
3754+
}
3755+
equalFoundInLastToken = true;
3756+
lastSep = c;
3757+
curToken += c;
3758+
}
3759+
else if (c == '"')
3760+
{
3761+
inString = !inString;
3762+
curToken += c;
3763+
}
3764+
else
3765+
{
3766+
curToken += c;
3767+
}
3768+
}
3769+
if (canSplitOnComma && !inString && equalFoundInLastToken)
3770+
{
3771+
if (!curToken.empty())
3772+
newVals.emplace_back(std::move(curToken));
3773+
vals = std::move(newVals);
3774+
}
3775+
}
3776+
3777+
for (const auto &val : vals)
37213778
{
37223779
if (!Validate(val))
37233780
return false;
@@ -3845,8 +3902,10 @@ GDALAlgorithm::AddCreationOptionsArg(std::vector<std::string> *pValue,
38453902
auto &arg = AddArg("creation-option", 0,
38463903
MsgOrDefault(helpMessage, _("Creation option")), pValue)
38473904
.AddAlias("co")
3848-
.SetMetaVar("<KEY>=<VALUE>");
3849-
arg.AddValidationAction([this, &arg]() { return ValidateKeyValue(arg); });
3905+
.SetMetaVar("<KEY>=<VALUE>")
3906+
.SetPackedValuesAllowed(false);
3907+
arg.AddValidationAction([this, &arg]()
3908+
{ return ParseAndValidateKeyValue(arg); });
38503909

38513910
arg.SetAutoCompleteFunction(
38523911
[this](const std::string &currentValue)
@@ -3945,8 +4004,10 @@ GDALAlgorithm::AddLayerCreationOptionsArg(std::vector<std::string> *pValue,
39454004
AddArg("layer-creation-option", 0,
39464005
MsgOrDefault(helpMessage, _("Layer creation option")), pValue)
39474006
.AddAlias("lco")
3948-
.SetMetaVar("<KEY>=<VALUE>");
3949-
arg.AddValidationAction([this, &arg]() { return ValidateKeyValue(arg); });
4007+
.SetMetaVar("<KEY>=<VALUE>")
4008+
.SetPackedValuesAllowed(false);
4009+
arg.AddValidationAction([this, &arg]()
4010+
{ return ParseAndValidateKeyValue(arg); });
39504011

39514012
arg.SetAutoCompleteFunction(
39524013
[this](const std::string &currentValue)

‎gcore/gdalalgorithm.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2817,7 +2817,7 @@ class CPL_DLL GDALAlgorithmRegistry
28172817
std::vector<std::string> &oRet);
28182818

28192819
/** Validation function to use for key=value type of arguments. */
2820-
bool ValidateKeyValue(const GDALAlgorithmArg &arg) const;
2820+
bool ParseAndValidateKeyValue(GDALAlgorithmArg &arg);
28212821

28222822
/** Return whether output-format or output arguments express GDALG output */
28232823
bool IsGDALGOutput() const;

0 commit comments

Comments
 (0)