diff --git a/README.md b/README.md index 3fb84f7e..aa9e99dd 100644 --- a/README.md +++ b/README.md @@ -263,7 +263,7 @@ App* subcom = app.add_subcommand(name, description); Option_group *app.add_option_group(name,description); ``` -An option name must start with a alphabetic character, underscore, a number, '?', or '@'. For long options, after the first character '.', and '-' are also valid characters. For the `add_flag*` functions '{' has special meaning. Names are given as a comma separated string, with the dash or dashes. An option or flag can have as many names as you want, and afterward, using `count`, you can use any of the names, with dashes as needed, to count the options. One of the names is allowed to be given without proceeding dash(es); if present the option is a positional option, and that name will be used on the help line for its positional form. +An option name may start with any character except ('-', ' ', '\n', and '!') 🚧. For long options, after the first character all characters are allowed except ('=',':','{',' ', '\n')🚧. For the `add_flag*` functions '{' and '!' have special meaning which is why they are not allowed. Names are given as a comma separated string, with the dash or dashes. An option or flag can have as many names as you want, and afterward, using `count`, you can use any of the names, with dashes as needed, to count the options. One of the names is allowed to be given without proceeding dash(es); if present the option is a positional option, and that name will be used on the help line for its positional form. The `add_option_function(...` function will typically require the template parameter be given unless a `std::function` object with an exact match is passed. The type can be any type supported by the `add_option` function. The function should throw an error (`CLI::ConversionError` or `CLI::ValidationError` possibly) if the value is not valid. @@ -557,7 +557,7 @@ even exit the program through the callback. Multiple subcommands are allowed, to allow [`Click`][click] like series of commands (order is preserved). The same subcommand can be triggered multiple times but all positional arguments will take precedence over the second and future calls of the subcommand. `->count()` on the subcommand will return the number of times the subcommand was called. The subcommand callback will only be triggered once unless the `.immediate_callback()` flag is set or the callback is specified through the `parse_complete_callback()` function. The `final_callback()` is triggered only once. In which case the callback executes on completion of the subcommand arguments but after the arguments for that subcommand have been parsed, and can be triggered multiple times. Subcommands may also have an empty name either by calling `add_subcommand` with an empty string for the name or with no arguments. -Nameless subcommands function a similarly to groups in the main `App`. See [Option groups](#option-groups) to see how this might work. If an option is not defined in the main App, all nameless subcommands are checked as well. This allows for the options to be defined in a composable group. The `add_subcommand` function has an overload for adding a `shared_ptr` so the subcommand(s) could be defined in different components and merged into a main `App`, or possibly multiple `Apps`. Multiple nameless subcommands are allowed. Callbacks for nameless subcommands are only triggered if any options from the subcommand were parsed. +Nameless subcommands function a similarly to groups in the main `App`. See [Option groups](#option-groups) to see how this might work. If an option is not defined in the main App, all nameless subcommands are checked as well. This allows for the options to be defined in a composable group. The `add_subcommand` function has an overload for adding a `shared_ptr` so the subcommand(s) could be defined in different components and merged into a main `App`, or possibly multiple `Apps`. Multiple nameless subcommands are allowed. Callbacks for nameless subcommands are only triggered if any options from the subcommand were parsed. Subcommand names given through the `add_subcommand` method have the same restrictions as option names. #### Subcommand options @@ -668,7 +668,7 @@ The subcommand method .add_option_group(name,description) ``` -Will create an option group, and return a pointer to it. The argument for `description` is optional and can be omitted. An option group allows creation of a collection of options, similar to the groups function on options, but with additional controls and requirements. They allow specific sets of options to be composed and controlled as a collective. For an example see [range example](https://github.com/CLIUtils/CLI11/blob/master/examples/ranges.cpp). Option groups are a specialization of an App so all [functions](#subcommand-options) that work with an App or subcommand also work on option groups. Options can be created as part of an option group using the add functions just like a subcommand, or previously created options can be added through +Will create an option group, and return a pointer to it. The argument for `description` is optional and can be omitted. An option group allows creation of a collection of options, similar to the groups function on options, but with additional controls and requirements. They allow specific sets of options to be composed and controlled as a collective. For an example see [range example](https://github.com/CLIUtils/CLI11/blob/master/examples/ranges.cpp). Option groups are a specialization of an App so all [functions](#subcommand-options) that work with an App or subcommand also work on option groups. Options can be created as part of an option group using the add functions just like a subcommand, or previously created options can be added through. The name given in an option group must not contain newlines or null characters.🚧 ```cpp ogroup->add_option(option_pointer); diff --git a/book/chapters/flags.md b/book/chapters/flags.md index ba72352d..aa920fe8 100644 --- a/book/chapters/flags.md +++ b/book/chapters/flags.md @@ -11,7 +11,7 @@ bool my_flag{false}; app.add_flag("-f", my_flag, "Optional description"); ``` -This will bind the flag `-f` to the boolean `my_flag`. After the parsing step, `my_flag` will be `false` if the flag was not found on the command line, or `true` if it was. By default, it will be allowed any number of times, but if you explicitly[^1] request `->take_last(false)`, it will only be allowed once; passing something like `./my_app -f -f` or `./my_app -ff` will throw a `ParseError` with a nice help description. +This will bind the flag `-f` to the boolean `my_flag`. After the parsing step, `my_flag` will be `false` if the flag was not found on the command line, or `true` if it was. By default, it will be allowed any number of times, but if you explicitly\[^1\] request `->take_last(false)`, it will only be allowed once; passing something like `./my_app -f -f` or `./my_app -ff` will throw a `ParseError` with a nice help description. A flag name may start with any character except ('-', ' ', '\n', and '!'). For long flags, after the first character all characters are allowed except ('=',':','{',' ', '\n'). Names are given as a comma separated string, with the dash or dashes. An flag can have as many names as you want, and afterward, using `count`, you can use any of the names, with dashes as needed. ## Integer flags @@ -120,4 +120,4 @@ Flag int: 3 Flag plain: 1 ``` -[^1]: It will not inherit this from the parent defaults, since this is often useful even if you don't want all options to allow multiple passed options. +\[^1\]: It will not inherit this from the parent defaults, since this is often useful even if you don't want all options to allow multiple passed options. diff --git a/book/chapters/options.md b/book/chapters/options.md index 60a1a467..299f4a4e 100644 --- a/book/chapters/options.md +++ b/book/chapters/options.md @@ -26,12 +26,12 @@ You can use any C++ int-like type, not just `int`. CLI11 understands the followi | complex-number | std::complex or any type which has a real(), and imag() operations available, will allow 1 or 2 string definitions like "1+2j" or two arguments "1","2" | | enumeration | any enum or enum class type is supported through conversion from the underlying type(typically int, though it can be specified otherwise) | | container-like | a container(like vector) of any available types including other containers | -| wrapper | any other object with a `value_type` static definition where the type specified by `value_type` is one of type in this list | +| wrapper | any other object with a `value_type` static definition where the type specified by `value_type` is one of the type in this list, including `std::atomic<>` | | tuple | a tuple, pair, or array, or other type with a tuple size and tuple_type operations defined and the members being a type contained in this list | | function | A function that takes an array of strings and returns a string that describes the conversion failure or empty for success. May be the empty function. (`{}`) | | streamable | any other type with a `<<` operator will also work | -By default, CLI11 will assume that an option is optional, and one value is expected if you do not use a vector. You can change this on a specific option using option modifiers. +By default, CLI11 will assume that an option is optional, and one value is expected if you do not use a vector. You can change this on a specific option using option modifiers. An option name may start with any character except ('-', ' ', '\n', and '!'). For long options, after the first character all characters are allowed except ('=',':','{',' ', '\n'). Names are given as a comma separated string, with the dash or dashes. An option can have as many names as you want, and afterward, using `count`, you can use any of the names, with dashes as needed, to count the options. One of the names is allowed to be given without proceeding dash(es); if present the option is a positional option, and that name will be used on the help line for its positional form. ## Positional options and aliases @@ -282,4 +282,4 @@ There are some additional options that can be specified to modify an option for ## Unusual circumstances -There are a few cases where some things break down in the type system managing options and definitions. Using the `add_option` method defines a lambda function to extract a default value if required. In most cases this either straightforward or a failure is detected automatically and handled. But in a few cases a streaming template is available that several layers down may not actually be defined. The conditions in CLI11 cannot detect this circumstance automatically and will result in compile error. One specific known case is `boost::optional` if the boost optional_io header is included. This header defines a template for all boost optional values even if they do no actually have a streaming operator. For example `boost::optional` does not have a streaming operator but one is detected since it is part of a template. For these cases a secondary method `app->add_option_no_stream(...)` is provided that bypasses this operation completely and should compile in these cases. +There are a few cases where some things break down in the type system managing options and definitions. Using the `add_option` method defines a lambda function to extract a default value if required. In most cases this is either straightforward or a failure is detected automatically and handled. But in a few cases a streaming template is available that several layers down may not actually be defined. This results in CLI11 not being able to detect this circumstance automatically and will result in compile error. One specific known case is `boost::optional` if the boost optional_io header is included. This header defines a template for all boost optional values even if they do not actually have a streaming operator. For example `boost::optional` does not have a streaming operator but one is detected since it is part of a template. For these cases a secondary method `app->add_option_no_stream(...)` is provided that bypasses this operation completely and should compile in these cases. diff --git a/include/CLI/App.hpp b/include/CLI/App.hpp index b445e49d..259dfe22 100644 --- a/include/CLI/App.hpp +++ b/include/CLI/App.hpp @@ -368,23 +368,9 @@ class App { /// Set an alias for the app App *alias(std::string app_name) { - if(!detail::valid_name_string(app_name)) { - if(app_name.empty()) { - throw IncorrectConstruction("Empty aliases are not allowed"); - } - if(!detail::valid_first_char(app_name[0])) { - throw IncorrectConstruction( - "Alias starts with invalid character, allowed characters are [a-zA-z0-9]+'_','?','@' "); - } - for(auto c : app_name) { - if(!detail::valid_later_char(c)) { - throw IncorrectConstruction(std::string("Alias contains invalid character ('") + c + - "'), allowed characters are " - "[a-zA-z0-9]+'_','?','@','.','-' "); - } - } + if(app_name.empty() || !detail::valid_alias_name_string(app_name)) { + throw IncorrectConstruction("Aliases may not be empty or contain newlines or null characters"); } - if(parent_ != nullptr) { aliases_.push_back(app_name); auto &res = _compare_subcommand_names(*this, *_get_fallthrough_parent()); @@ -961,6 +947,9 @@ class App { /// creates an option group as part of the given app template T *add_option_group(std::string group_name, std::string group_description = "") { + if(!detail::valid_alias_name_string(group_name)) { + throw IncorrectConstruction("option group names may not contain newlines or null characters"); + } auto option_group = std::make_shared(std::move(group_description), group_name, this); auto ptr = option_group.get(); // move to App_p for overload resolution on older gcc versions @@ -978,13 +967,13 @@ class App { if(!subcommand_name.empty() && !detail::valid_name_string(subcommand_name)) { if(!detail::valid_first_char(subcommand_name[0])) { throw IncorrectConstruction( - "Subcommand name starts with invalid character, allowed characters are [a-zA-z0-9]+'_','?','@' "); + "Subcommand name starts with invalid character, '!' and '-' are not allowed"); } for(auto c : subcommand_name) { if(!detail::valid_later_char(c)) { throw IncorrectConstruction(std::string("Subcommand name contains invalid character ('") + c + - "'), allowed characters are " - "[a-zA-z0-9]+'_','?','@','.','-' "); + "'), all characters are allowed except" + "'=',':','{','}', and ' '"); } } } diff --git a/include/CLI/Option.hpp b/include/CLI/Option.hpp index 11ae2c05..5e0028aa 100644 --- a/include/CLI/Option.hpp +++ b/include/CLI/Option.hpp @@ -94,6 +94,9 @@ template class OptionBase { /// Changes the group membership CRTP *group(const std::string &name) { + if(!detail::valid_alias_name_string(name)) { + throw IncorrectConstruction("Group names may not contain newlines or null characters"); + } group_ = name; return static_cast(this); } diff --git a/include/CLI/StringTools.hpp b/include/CLI/StringTools.hpp index 84e21de8..4718aedd 100644 --- a/include/CLI/StringTools.hpp +++ b/include/CLI/StringTools.hpp @@ -157,6 +157,22 @@ inline std::string &remove_quotes(std::string &str) { return str; } +/// Add a leader to the beginning of all new lines (nothing is added +/// at the start of the first line). `"; "` would be for ini files +/// +/// Can't use Regex, or this would be a subs. +inline std::string fix_newlines(const std::string &leader, std::string input) { + std::string::size_type n = 0; + while(n != std::string::npos && n < input.size()) { + n = input.find('\n', n); + if(n != std::string::npos) { + input = input.substr(0, n + 1) + leader + input.substr(n + 1); + n += leader.size(); + } + } + return input; +} + /// Make a copy of the string and then trim it, any filter string can be used (any char in string is filtered) inline std::string trim_copy(const std::string &str, const std::string &filter) { std::string s = str; @@ -191,7 +207,7 @@ inline std::ostream &format_aliases(std::ostream &out, const std::vector bool valid_first_char(T c) { - return std::isalnum(c, std::locale()) || c == '_' || c == '?' || c == '@'; -} +/// - is a trigger character, ! has special meaning and new lines would just be annoying to deal with +template bool valid_first_char(T c) { return ((c != '-') && (c != '!') && (c != ' ') && c != '\n'); } /// Verify following characters of an option -template bool valid_later_char(T c) { return valid_first_char(c) || c == '.' || c == '-'; } +template bool valid_later_char(T c) { + // = and : are value separators, { has special meaning for option defaults, + // and \n would just be annoying to deal with in many places allowing space here has too much potential for + // inadvertent entry errors and bugs + return ((c != '=') && (c != ':') && (c != '{') && (c != ' ') && c != '\n'); +} -/// Verify an option name +/// Verify an option/subcommand name inline bool valid_name_string(const std::string &str) { - if(str.empty() || !valid_first_char(str[0])) + if(str.empty() || !valid_first_char(str[0])) { return false; - for(auto c : str.substr(1)) - if(!valid_later_char(c)) + } + auto e = str.end(); + for(auto c = str.begin() + 1; c != e; ++c) + if(!valid_later_char(*c)) return false; return true; } +/// Verify an app name +inline bool valid_alias_name_string(const std::string &str) { + static const std::string badChars(std::string("\n") + '\0'); + return (str.find_first_of(badChars) == std::string::npos); +} + /// check if a string is a container segment separator (empty or "%%") inline bool is_separator(const std::string &str) { static const std::string sep("%%"); @@ -260,7 +288,7 @@ inline bool has_default_flag_values(const std::string &flags) { } inline void remove_default_flag_values(std::string &flags) { - auto loc = flags.find_first_of('{'); + auto loc = flags.find_first_of('{', 2); while(loc != std::string::npos) { auto finish = flags.find_first_of("},", loc + 1); if((finish != std::string::npos) && (flags[finish] == '}')) { @@ -367,22 +395,6 @@ inline std::vector split_up(std::string str, char delimiter = '\0') return output; } -/// Add a leader to the beginning of all new lines (nothing is added -/// at the start of the first line). `"; "` would be for ini files -/// -/// Can't use Regex, or this would be a subs. -inline std::string fix_newlines(const std::string &leader, std::string input) { - std::string::size_type n = 0; - while(n != std::string::npos && n < input.size()) { - n = input.find('\n', n); - if(n != std::string::npos) { - input = input.substr(0, n + 1) + leader + input.substr(n + 1); - n += leader.size(); - } - } - return input; -} - /// This function detects an equal or colon followed by an escaped quote after an argument /// then modifies the string to replace the equality with a space. This is needed /// to allow the split up function to work properly and is intended to be used with the find_and_modify function diff --git a/tests/AppTest.cpp b/tests/AppTest.cpp index d779c6de..e810e115 100644 --- a/tests/AppTest.cpp +++ b/tests/AppTest.cpp @@ -127,6 +127,17 @@ TEST_CASE_METHOD(TApp, "DashedOptionsSingleString", "[app]") { CHECK(app.count("--that") == 2u); } +TEST_CASE_METHOD(TApp, "StrangeFlagNames", "[app]") { + app.add_flag("-="); + app.add_flag("--t\tt"); + app.add_flag("-{"); + CHECK_THROWS_AS(app.add_flag("--t t"), CLI::ConstructionError); + args = {"-=", "--t\tt"}; + run(); + CHECK(app.count("-=") == 1u); + CHECK(app.count("--t\tt") == 1u); +} + TEST_CASE_METHOD(TApp, "RequireOptionsError", "[app]") { using Catch::Matchers::Contains; @@ -582,6 +593,20 @@ TEST_CASE_METHOD(TApp, "SingleArgVector", "[app]") { CHECK("happy" == path); } +TEST_CASE_METHOD(TApp, "StrangeOptionNames", "[app]") { + app.add_option("-:"); + app.add_option("--t\tt"); + app.add_option("--{}"); + app.add_option("--:)"); + CHECK_THROWS_AS(app.add_option("--t t"), CLI::ConstructionError); + args = {"-:)", "--{}", "5"}; + run(); + CHECK(app.count("-:") == 1u); + CHECK(app.count("--{}") == 1u); + CHECK(app["-:"]->as() == ')'); + CHECK(app["--{}"]->as() == 5); +} + TEST_CASE_METHOD(TApp, "FlagLikeOption", "[app]") { bool val{false}; auto opt = app.add_option("--flag", val)->type_size(0)->default_str("true"); diff --git a/tests/HelpersTest.cpp b/tests/HelpersTest.cpp index 5d6c1712..7a497aa1 100644 --- a/tests/HelpersTest.cpp +++ b/tests/HelpersTest.cpp @@ -155,13 +155,14 @@ TEST_CASE("String: InvalidName", "[helpers]") { CHECK(CLI::detail::valid_name_string("valid")); CHECK_FALSE(CLI::detail::valid_name_string("-invalid")); CHECK(CLI::detail::valid_name_string("va-li-d")); - CHECK_FALSE(CLI::detail::valid_name_string("vali&d")); + CHECK_FALSE(CLI::detail::valid_name_string("valid{}")); CHECK(CLI::detail::valid_name_string("_valid")); - CHECK_FALSE(CLI::detail::valid_name_string("/valid")); + CHECK(CLI::detail::valid_name_string("/valid")); CHECK(CLI::detail::valid_name_string("vali?d")); CHECK(CLI::detail::valid_name_string("@@@@")); CHECK(CLI::detail::valid_name_string("b@d2?")); CHECK(CLI::detail::valid_name_string("2vali?d")); + CHECK_FALSE(CLI::detail::valid_name_string("!valid")); } TEST_CASE("StringTools: Modify", "[helpers]") { diff --git a/tests/OptionGroupTest.cpp b/tests/OptionGroupTest.cpp index 05ef2c19..ac6684f4 100644 --- a/tests/OptionGroupTest.cpp +++ b/tests/OptionGroupTest.cpp @@ -23,6 +23,16 @@ TEST_CASE_METHOD(TApp, "BasicOptionGroup", "[optiongroup]") { CHECK(1u == app.count_all()); } +TEST_CASE_METHOD(TApp, "OptionGroupInvalidNames", "[optiongroup]") { + CHECK_THROWS_AS(app.add_option_group("clusters\ncluster2", "description"), CLI::IncorrectConstruction); + + std::string groupName("group1"); + groupName += '\0'; + groupName.append("group2"); + + CHECK_THROWS_AS(app.add_option_group(groupName), CLI::IncorrectConstruction); +} + TEST_CASE_METHOD(TApp, "BasicOptionGroupExact", "[optiongroup]") { auto ogroup = app.add_option_group("clusters"); int res{0}; diff --git a/tests/OptionTypeTest.cpp b/tests/OptionTypeTest.cpp index 296334e3..48ac1664 100644 --- a/tests/OptionTypeTest.cpp +++ b/tests/OptionTypeTest.cpp @@ -269,6 +269,17 @@ TEST_CASE_METHOD(TApp, "vectorDefaults", "[optiontype]") { CHECK(std::vector({5}) == res); } +TEST_CASE_METHOD(TApp, "mapInput", "[optiontype]") { + std::map vals{}; + app.add_option("--long", vals); + + args = {"--long", "5", "test"}; + + run(); + + CHECK(vals.at(5) == "test"); +} + TEST_CASE_METHOD(TApp, "CallbackBoolFlags", "[optiontype]") { bool value{false}; diff --git a/tests/SubcommandTest.cpp b/tests/SubcommandTest.cpp index 108f9991..e1ffded6 100644 --- a/tests/SubcommandTest.cpp +++ b/tests/SubcommandTest.cpp @@ -815,10 +815,10 @@ TEST_CASE_METHOD(TApp, "invalidSubcommandName", "[subcom]") { bool gotError{false}; try { - app.add_subcommand("foo/foo", "Foo a bar"); + app.add_subcommand("!foo/foo", "Foo a bar"); } catch(const CLI::IncorrectConstruction &e) { gotError = true; - CHECK_THAT(e.what(), Contains("/")); + CHECK_THAT(e.what(), Contains("!")); } CHECK(gotError); } @@ -1645,6 +1645,28 @@ TEST_CASE_METHOD(TApp, "OptionGroupAlias", "[subcom]") { CHECK(-3 == val); } +TEST_CASE_METHOD(TApp, "OptionGroupAliasWithSpaces", "[subcom]") { + double val{0.0}; + auto sub = app.add_option_group("sub1"); + sub->alias("sub2 bb"); + sub->alias("sub3/b"); + sub->add_option("-v,--value", val); + args = {"sub1", "-v", "-3"}; + CHECK_THROWS_AS(run(), CLI::ExtrasError); + + args = {"sub2 bb", "--value", "-5"}; + run(); + CHECK(-5.0 == val); + + args = {"sub3/b", "-v", "7"}; + run(); + CHECK(7 == val); + + args = {"-v", "-3"}; + run(); + CHECK(-3 == val); +} + TEST_CASE_METHOD(TApp, "subcommand_help", "[subcom]") { auto sub1 = app.add_subcommand("help")->silent(); bool flag{false}; @@ -1666,9 +1688,8 @@ TEST_CASE_METHOD(TApp, "AliasErrors", "[subcom]") { auto sub1 = app.add_subcommand("sub1"); auto sub2 = app.add_subcommand("sub2"); - CHECK_THROWS_AS(sub2->alias("this is a not a valid alias"), CLI::IncorrectConstruction); - CHECK_THROWS_AS(sub2->alias("-alias"), CLI::IncorrectConstruction); - CHECK_THROWS_AS(sub2->alias("alia$"), CLI::IncorrectConstruction); + CHECK_THROWS_AS(sub2->alias("this is a not\n a valid alias"), CLI::IncorrectConstruction); + CHECK_NOTHROW(sub2->alias("-alias")); // this is allowed but would be unusable on command line parsers CHECK_THROWS_AS(app.add_subcommand("--bad_subcommand_name", "documenting the bad subcommand"), CLI::IncorrectConstruction);