From 25441e7468cc5ee7431a6f1dce77018a41c5cbd3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20Vondru=C5=A1?= Date: Wed, 24 May 2023 19:46:47 +0200 Subject: [PATCH] *converter: support adding new option keys by prefixing them with +. Since it's adding new options, it doesn't warn if the key isn't found. --- .../Implementation/converterUtilities.h | 29 +++++++-- src/Magnum/SceneTools/sceneconverter.cpp | 6 +- src/Magnum/ShaderTools/shaderconverter.cpp | 31 +++++---- src/Magnum/Test/ConverterUtilitiesTest.cpp | 65 ++++++++++++++++++- src/Magnum/Trade/imageconverter.cpp | 6 +- 5 files changed, 112 insertions(+), 25 deletions(-) diff --git a/src/Magnum/Implementation/converterUtilities.h b/src/Magnum/Implementation/converterUtilities.h index f5d013c95..d08f23fce 100644 --- a/src/Magnum/Implementation/converterUtilities.h +++ b/src/Magnum/Implementation/converterUtilities.h @@ -45,6 +45,12 @@ void setOptions(const Containers::StringView pluginName, Utility::ConfigurationG keyValue[0] = keyValue[0].trimmed(); keyValue[2] = keyValue[2].trimmed(); + bool addValue = false; + if(keyValue[0].hasPrefix('+')) { + keyValue[0] = keyValue[0].exceptPrefix(1); + addValue = true; + } + const Containers::Array keyParts = keyValue[0].split('/'); CORRADE_INTERNAL_ASSERT(!keyParts.isEmpty()); Utility::ConfigurationGroup* group = &configuration; @@ -66,7 +72,6 @@ void setOptions(const Containers::StringView pluginName, Utility::ConfigurationG emptySubgroups.insert(subgroup); } group = subgroup; - } /* Provide a warning message in case the plugin doesn't define given @@ -81,7 +86,12 @@ void setOptions(const Containers::StringView pluginName, Utility::ConfigurationG if((groupNotRecognized || (!group->hasValue(keyParts.back()) && /* The warning isn't printed in case a value is added into an empty subgroup, see above */ - emptySubgroups.find(group) == emptySubgroups.end() + emptySubgroups.find(group) == emptySubgroups.end() && + /* The warning also isn't printed in case a new value is added + with `+` instead of modifying an existing one -- e.g. a + plugin can support 0 to n values of a certain key, which + means by default there won't be any */ + !addValue )) && pluginName != anyPluginName) { Warning{} << "Option" << keyValue[0] << "not recognized by" << pluginName; @@ -90,10 +100,17 @@ void setOptions(const Containers::StringView pluginName, Utility::ConfigurationG /* If the option doesn't have an =, treat it as a boolean flag that's set to true. While there's no similar way to do an inverse, it's still nicer than causing a fatal error with those. */ - if(keyValue[1]) - group->setValue(keyParts.back(), keyValue[2]); - else - group->setValue(keyParts.back(), true); + if(keyValue[1]) { + if(addValue) + group->addValue(keyParts.back(), keyValue[2]); + else + group->setValue(keyParts.back(), keyValue[2]); + } else { + if(addValue) + group->addValue(keyParts.back(), true); + else + group->setValue(keyParts.back(), true); + } } } diff --git a/src/Magnum/SceneTools/sceneconverter.cpp b/src/Magnum/SceneTools/sceneconverter.cpp index 4c41fae4f..4b8922fd6 100644 --- a/src/Magnum/SceneTools/sceneconverter.cpp +++ b/src/Magnum/SceneTools/sceneconverter.cpp @@ -284,7 +284,8 @@ with `--info-meshes` will print how many objects reference given mesh). The `-i`, `-c` and `-m` arguments accept a comma-separated list of key/value pairs to set in the importer / converter plugin configuration. If the `=` character is omitted, it's equivalent to saying `key=true`; configuration -subgroups are delimited with `/`. +subgroups are delimited with `/`. Prefix the key with `+` to add new options or +multiple options of the same name. It's possible to specify the `-C` option (and correspondingly also `-c`) multiple times in order to chain more converters together. All converters in @@ -487,7 +488,8 @@ will also list reference count (for example, --info-scenes together with The -i, -c and -m arguments accept a comma-separated list of key/value pairs to set in the importer / converter plugin configuration. If the = character is omitted, it's equivalent to saying key=true; configuration -subgroups are delimited with /. +subgroups are delimited with /. Prefix the key with + to add new options or +multiple options of the same name. It's possible to specify the -C option (and correspondingly also -c) multiple times in order to chain more scene converters together. All converters in the diff --git a/src/Magnum/ShaderTools/shaderconverter.cpp b/src/Magnum/ShaderTools/shaderconverter.cpp index 12b661137..4c303b7c9 100644 --- a/src/Magnum/ShaderTools/shaderconverter.cpp +++ b/src/Magnum/ShaderTools/shaderconverter.cpp @@ -153,12 +153,15 @@ save it to `output`. The `-c` / `--converter-options` argument accept a comma-separated list of key/value pairs to set in the converter plugin configuration. If the `=` character is omitted, it's equivalent to saying `key=true`; configuration -subgroups are delimited with `/`. It's possible to specify the `-C` / -`--converter` option (and correspondingly also `-c` / `--converter-options`, -`--input-format`, `--output-format`, `--input-version` and `--output-version`) -multiple times in order to chain more converters together. All converters in -the chain have to support the @ref ShaderTools::ConverterFeature::ConvertData -feature, if there's just one converter it's enough for it to support +subgroups are delimited with `/`. Prefix the key with `+` to add new options or +multiple options of the same name. + +It's possible to specify the `-C` / `--converter` option (and correspondingly +also `-c` / `--converter-options`, `--input-format`, `--output-format`, +`--input-version` and `--output-version`) multiple times in order to chain more +converters together. All converters in the chain have to support the +@ref ShaderTools::ConverterFeature::ConvertData feature, if there's just one +converter it's enough for it to support @ref ShaderTools::ConverterFeature::ConvertFile. If no `-C` / `--converter` is specified, @ref ShaderTools::AnyConverter "AnyShaderConverter" is used. @@ -284,13 +287,15 @@ specified, the utility will convert the input file using (one or more) passed The -c / --converter-options argument accept a comma-separated list of key/value pairs to set in the converter plugin configuration. If the = character is omitted, it's equivalent to saying key=true; configuration -subgroups are delimited with /. It's possible to specify the -C / --converter -option (and correspondingly also -c / --converter-options, --input-format, ---output-format, --input-version and --output-version) multiple times in order -to chain more converters together. All converters in the chain have to support -the ConvertData feature, if there's just one converter it's enough for it to -support ConvertFile. If no -C / --converter is specified, AnyShaderConverter is -used. +subgroups are delimited with /. Prefix the key with + to add new options or +multiple options of the same name. + +It's possible to specify the -C / --converter option (and correspondingly also +-c / --converter-options, --input-format, --output-format, --input-version and +--output-version) multiple times in order to chain more converters together. +All converters in the chain have to support the ConvertData feature, if there's +just one converter it's enough for it to support ConvertFile. If no -C / +--converter is specified, AnyShaderConverter is used. The -D / --define, -U / --undefine, -O / --optimize, -g / --debug-info, -E / --preprocess-only arguments apply only to the first converter. Split the diff --git a/src/Magnum/Test/ConverterUtilitiesTest.cpp b/src/Magnum/Test/ConverterUtilitiesTest.cpp index 633b2961a..8069b7325 100644 --- a/src/Magnum/Test/ConverterUtilitiesTest.cpp +++ b/src/Magnum/Test/ConverterUtilitiesTest.cpp @@ -25,6 +25,7 @@ #include #include +#include #include #include @@ -39,7 +40,7 @@ struct ConverterUtilitiesTest: TestSuite::Tester { }; const struct { - const char* name; + TestSuite::TestCaseDescriptionSourceLocation name; const char* config; const char* options; const char* anyPluginName; @@ -185,6 +186,64 @@ notFound=value )", /* The trailing space is there because the plugin name is empty */ "Option notFound not recognized by \n"}, + + /* Adding new unrecognized options to existing groups doesn't warn */ + {"add an unrecognized option that doesn't exist yet", R"([configuration] +option= +another= +)", + "+yetanother=value", "AnyPlugin", R"([configuration] +option= +another= +yetanother=value +)", nullptr}, + {"add an option that exists already", R"([configuration] +option= +another= +)", + "+option=value", "AnyPlugin", R"([configuration] +option= +another= +option=value +)", nullptr}, + {"two options, add second with implicit true", R"([configuration] +option= +another= +)", + "option=value,+yetanother", "AnyPlugin", R"([configuration] +option=value +another= +yetanother=true +)", nullptr}, + {"add an option to an existing subgroup", R"([configuration] +option= +[configuration/group] +option= +another= +)", + "+group/option=value", "AnyPlugin", R"([configuration] +option= +[configuration/group] +option= +another= +option=value +)", nullptr}, + {"add an option to an unrecognized subgroup", R"([configuration] +option= +[configuration/group] +option= +another= +)", + "+notFound/option=value", "AnyPlugin", R"([configuration] +option= +[configuration/group] +option= +another= +[configuration/notFound] +option=value +)", + /* The trailing space is there because the plugin name is empty */ + "Option notFound/option not recognized by \n"}, }; ConverterUtilitiesTest::ConverterUtilitiesTest() { @@ -219,7 +278,9 @@ void ConverterUtilitiesTest::setOptions() { CORRADE_COMPARE(conf.group("configuration")->configuration(), &conf); std::ostringstream out; conf.save(out); - CORRADE_COMPARE(out.str(), data.expectedConfig); + CORRADE_COMPARE_AS(out.str(), + data.expectedConfig, + TestSuite::Compare::String); } }}} diff --git a/src/Magnum/Trade/imageconverter.cpp b/src/Magnum/Trade/imageconverter.cpp index 408a616a0..6edde74f4 100644 --- a/src/Magnum/Trade/imageconverter.cpp +++ b/src/Magnum/Trade/imageconverter.cpp @@ -272,7 +272,8 @@ The `-i` / `--importer-options` and `-c` / `--converter-options` arguments accept a comma-separated list of key/value pairs to set in the importer / converter plugin configuration. If the `=` character is omitted, it's equivalent to saying `key=true`; configuration subgroups are delimited with -`/`. +`/`. Prefix the key with `+` to add new options or multiple options of the same +name. It's possible to specify the `-C` / `--converter` option (and correspondingly also `-c` / `--converter-options`) multiple times in order to chain more @@ -438,7 +439,8 @@ read but no conversion is done and output file doesn't need to be specified. The -i / --importer-options and -c / --converter-options arguments accept a comma-separated list of key/value pairs to set in the importer / converter plugin configuration. If the = character is omitted, it's equivalent to saying -key=true; configuration subgroups are delimited with /. +key=true; configuration subgroups are delimited with /. Prefix the key with + +to add new options or multiple options of the same name. It's possible to specify the -C / --converter option (and correspondingly also -c / --converter-options) multiple times in order to chain more converters