Option delimiter (#240)

* move delimiter controls to Option vs in the App callbacks

Option delimiter (#240)
* move delimiter controls to Option vs in the App callbacks
Philip Top · Henry Schreiner
1 parent 7254ea79
Showing 5 changed files with 151 additions and 104 deletions
README.md
include/CLI/App.hpp
include/CLI/Option.hpp
tests/AppTest.cpp
tests/NewParseTest.cpp
@@ -273,6 +273,7 @@ Before parsing, you can set the following options:
 -   `->ignore_case()`: Ignore the case on the command line (also works on subcommands, does not affect arguments).
 -   `->ignore_underscore()`: Ignore any underscores in the options names (also works on subcommands, does not affect arguments). For example "option_one" will match with "optionone".  This does not apply to short form options since they only have one character
 -   `->disable_flag_override()`:  from the command line long form flag option can be assigned a value on the command line using the `=` notation `--flag=value`. If this behavior is not desired, the `disable_flag_override()` disables it and will generate an exception if it is done on the command line.  The `=` does not work with short form flag options.
+-    `->delimiter(char)`: allows specification of a custom delimiter for separating single arguments into vector arguments, for example specifying `->delimiter(',')` on an option would result in `--opt=1,2,3` producing 3 elements of a vector and the equivalent of --opt 1 2 3 assuming opt is a vector value
 -   `->description(str)`: Set/change the description.
 -   `->multi_option_policy(CLI::MultiOptionPolicy::Throw)`: Set the multi-option policy. Shortcuts available: `->take_last()`, `->take_first()`, and `->join()`. This will only affect options expecting 1 argument or bool flags (which do not inherit their default but always start with a specific policy).
 -   `->check(CLI::IsMember(...))`: Require an option be a member of a given set. See below for options.
@@ -338,9 +339,7 @@ In most cases the fastest and easiest way is to return the results through a cal
  
 - `results()`: retrieves a vector of strings with all the results in the order they were given.
 - `results(variable_to_bind_to)`: gets the results according to the MultiOptionPolicy and converts them just like the `add_option_function` with a variable.
-- `results(vector_type_variable,delimiter)`: gets the results to a vector type and uses a delimiter to further split the values
-- `Value=as<type>()`: returns the result or default value directly as the specified type if possible.
-- `Vector_value=as<type>(delimiter): same the results function with the delimiter but returns the value directly.  
+- `Value=as<type>()`: returns the result or default value directly as the specified type if possible, can be vector to return all results, and a non-vector to get the result according to the MultiOptionPolicy in place.
  
 ### Subcommands
  
@@ -434,7 +433,7 @@ arguments, use `.config_to_str(default_also=false, prefix=&quot;&quot;, write_description=
  
 Many of the defaults for subcommands and even options are inherited from their creators. The inherited default values for subcommands are `allow_extras`, `prefix_command`, `ignore_case`, `ignore_underscore`, `fallthrough`, `group`, `footer`, and maximum number of required subcommands. The help flag existence, name, and description are inherited, as well.
  
-Options have defaults for `group`, `required`, `disable_flag_override`,`multi_option_policy`, `ignore_underscore`, and `ignore_case`. To set these defaults, you should set the `option_defaults()` object, for example:
+Options have defaults for `group`, `required`, `disable_flag_override`,`multi_option_policy`, `ignore_underscore`,`delimiter`, and `ignore_case`. To set these defaults, you should set the `option_defaults()` object, for example:
  
 ```cpp
 app.option_defaults()->required();
@@ -435,28 +435,20 @@ class App {
         return opt;
     }
  
-    /// Add option for vectors (no default)
+    /// Add option for vectors
     template <typename T>
     Option *add_option(std::string option_name,
                        std::vector<T> &variable, ///< The variable vector to set
-                       std::string option_description = "",
-                       char delimiter = '\0') {
+                       std::string option_description = "") {
  
-        CLI::callback_t fun = [&variable, delimiter](CLI::results_t res) {
+        CLI::callback_t fun = [&variable](CLI::results_t res) {
             bool retval = true;
             variable.clear();
+            variable.reserve(res.size());
             for(const auto &elem : res) {
-                if((delimiter != '\0') && (elem.find_first_of(delimiter) != std::string::npos)) {
-                    for(const auto &var : CLI::detail::split(elem, delimiter)) {
-                        if(!var.empty()) {
-                            variable.emplace_back();
-                            retval &= detail::lexical_cast(var, variable.back());
-                        }
-                    }
-                } else {
-                    variable.emplace_back();
-                    retval &= detail::lexical_cast(elem, variable.back());
-                }
+
+                variable.emplace_back();
+                retval &= detail::lexical_cast(elem, variable.back());
             }
             return (!variable.empty()) && retval;
         };
@@ -466,35 +458,28 @@ class App {
         return opt;
     }
  
-    /// Add option for vectors
+    /// Add option for vectors with defaulted argument
     template <typename T>
     Option *add_option(std::string option_name,
                        std::vector<T> &variable, ///< The variable vector to set
                        std::string option_description,
-                       bool defaulted,
-                       char delimiter = '\0') {
+                       bool defaulted) {
  
-        CLI::callback_t fun = [&variable, delimiter](CLI::results_t res) {
+        CLI::callback_t fun = [&variable](CLI::results_t res) {
             bool retval = true;
             variable.clear();
+            variable.reserve(res.size());
             for(const auto &elem : res) {
-                if((delimiter != '\0') && (elem.find_first_of(delimiter) != std::string::npos)) {
-                    for(const auto &var : CLI::detail::split(elem, delimiter)) {
-                        if(!var.empty()) {
-                            variable.emplace_back();
-                            retval &= detail::lexical_cast(var, variable.back());
-                        }
-                    }
-                } else {
-                    variable.emplace_back();
-                    retval &= detail::lexical_cast(elem, variable.back());
-                }
+
+                variable.emplace_back();
+                retval &= detail::lexical_cast(elem, variable.back());
             }
             return (!variable.empty()) && retval;
         };
  
         Option *opt = add_option(option_name, fun, option_description, defaulted);
         opt->type_name(detail::type_name<T>())->type_size(-1);
+
         if(defaulted)
             opt->default_str("[" + detail::join(variable) + "]");
         return opt;
@@ -504,25 +489,15 @@ class App {
     template <typename T, enable_if_t<is_vector<T>::value, detail::enabler> = detail::dummy>
     Option *add_option_function(std::string option_name,
                                 const std::function<bool(const T &)> &func, ///< the callback to execute
-                                std::string option_description = "",
-                                char delimiter = '\0') {
+                                std::string option_description = "") {
  
-        CLI::callback_t fun = [func, delimiter](CLI::results_t res) {
+        CLI::callback_t fun = [func](CLI::results_t res) {
             T values;
             bool retval = true;
             values.reserve(res.size());
             for(const auto &elem : res) {
-                if((delimiter != '\0') && (elem.find_first_of(delimiter) != std::string::npos)) {
-                    for(const auto &var : CLI::detail::split(elem, delimiter)) {
-                        if(!var.empty()) {
-                            values.emplace_back();
-                            retval &= detail::lexical_cast(var, values.back());
-                        }
-                    }
-                } else {
-                    values.emplace_back();
-                    retval &= detail::lexical_cast(elem, values.back());
-                }
+                values.emplace_back();
+                retval &= detail::lexical_cast(elem, values.back());
             }
             if(retval) {
                 return func(values);
@@ -2032,6 +2007,7 @@ class App {
         // Make sure we always eat the minimum for unlimited vectors
         int collected = 0;
         // deal with flag like things
+        int count = 0;
         if(num == 0) {
             auto res = op->get_flag_value(arg_name, value);
             op->add_result(res);
@@ -2039,20 +2015,22 @@ class App {
         }
         // --this=value
         else if(!value.empty()) {
+            op->add_result(value, count);
+            parse_order_.push_back(op.get());
+            collected += count;
             // If exact number expected
             if(num > 0)
-                num--;
-            op->add_result(value);
-            parse_order_.push_back(op.get());
-            collected += 1;
+                num = (num >= count) ? num - count : 0;
+
             // -Trest
         } else if(!rest.empty()) {
-            if(num > 0)
-                num--;
-            op->add_result(rest);
+            op->add_result(rest, count);
             parse_order_.push_back(op.get());
             rest = "";
-            collected += 1;
+            collected += count;
+            // If exact number expected
+            if(num > 0)
+                num = (num >= count) ? num - count : 0;
         }
  
         // Unlimited vector parser
@@ -2065,10 +2043,10 @@ class App {
                     if(_count_remaining_positionals() > 0)
                         break;
                 }
-                op->add_result(args.back());
+                op->add_result(args.back(), count);
                 parse_order_.push_back(op.get());
                 args.pop_back();
-                collected++;
+                collected += count;
             }
  
             // Allow -- to end an unlimited list and "eat" it
@@ -2077,11 +2055,11 @@ class App {
  
         } else {
             while(num > 0 && !args.empty()) {
-                num--;
                 std::string current_ = args.back();
                 args.pop_back();
-                op->add_result(current_);
+                op->add_result(current_, count);
                 parse_order_.push_back(op.get());
+                num -= count;
             }
  
             if(num > 0) {
@@ -52,7 +52,8 @@ template &lt;typename CRTP&gt; class OptionBase {
     bool configurable_{true};
     /// Disable overriding flag values with '=value'
     bool disable_flag_override_{false};
-
+    /// Specify a delimiter character for vector arguments
+    char delimiter_{'\0'};
     /// Policy for multiple arguments when `expected_ == 1`  (can be set on bool flags, too)
     MultiOptionPolicy multi_option_policy_{MultiOptionPolicy::Throw};
  
@@ -64,6 +65,7 @@ template &lt;typename CRTP&gt; class OptionBase {
         other->ignore_underscore(ignore_underscore_);
         other->configurable(configurable_);
         other->disable_flag_override(disable_flag_override_);
+        other->delimiter(delimiter_);
         other->multi_option_policy(multi_option_policy_);
     }
  
@@ -106,6 +108,7 @@ template &lt;typename CRTP&gt; class OptionBase {
     /// The status of configurable
     bool get_disable_flag_override() const { return disable_flag_override_; }
  
+    char get_delimiter() const { return delimiter_; }
     /// The status of the multi option policy
     MultiOptionPolicy get_multi_option_policy() const { return multi_option_policy_; }
  
@@ -137,6 +140,12 @@ template &lt;typename CRTP&gt; class OptionBase {
         configurable_ = value;
         return static_cast<CRTP *>(this);
     }
+
+    /// Allow in a configuration file
+    CRTP *delimiter(char value = '\0') {
+        delimiter_ = value;
+        return static_cast<CRTP *>(this);
+    }
 };
  
 /// This is a version of OptionBase that only supports setting values,
@@ -165,11 +174,17 @@ class OptionDefaults : public OptionBase&lt;OptionDefaults&gt; {
         return this;
     }
  
-    /// Ignore underscores in the option name
+    /// Disable overriding flag values with an '=<value>' segment
     OptionDefaults *disable_flag_override(bool value = true) {
         disable_flag_override_ = value;
         return this;
     }
+
+    /// set a delimiter character to split up single arguments to treat as multiple inputs
+    OptionDefaults *delimiter(char value = '\0') {
+        delimiter_ = value;
+        return this;
+    }
 };
  
 class Option : public OptionBase<Option> {
@@ -793,19 +808,23 @@ class Option : public OptionBase&lt;Option&gt; {
  
     /// Puts a result at the end
     Option *add_result(std::string s) {
-        results_.push_back(std::move(s));
+        _add_result(std::move(s));
+        callback_run_ = false;
+        return this;
+    }
+
+    /// Puts a result at the end and get a count of the number of arguments actually added
+    Option *add_result(std::string s, int &count) {
+        count = _add_result(std::move(s));
         callback_run_ = false;
         return this;
     }
  
     /// Puts a result at the end
     Option *add_result(std::vector<std::string> s) {
-        if(results_.empty()) {
-            results_ = std::move(s);
-        } else {
-            results_.insert(results_.end(), s.begin(), s.end());
+        for(auto &str : s) {
+            _add_result(std::move(str));
         }
-
         callback_run_ = false;
         return this;
     }
@@ -850,22 +869,13 @@ class Option : public OptionBase&lt;Option&gt; {
         }
     }
     /// get the results as a vector of a particular type
-    template <typename T> void results(std::vector<T> &output, char delim = '\0') const {
+    template <typename T> void results(std::vector<T> &output) const {
         output.clear();
         bool retval = true;
  
         for(const auto &elem : results_) {
-            if(delim != '\0') {
-                for(const auto &var : CLI::detail::split(elem, delim)) {
-                    if(!var.empty()) {
-                        output.emplace_back();
-                        retval &= detail::lexical_cast(var, output.back());
-                    }
-                }
-            } else {
-                output.emplace_back();
-                retval &= detail::lexical_cast(elem, output.back());
-            }
+            output.emplace_back();
+            retval &= detail::lexical_cast(elem, output.back());
         }
  
         if(!retval) {
@@ -874,20 +884,12 @@ class Option : public OptionBase&lt;Option&gt; {
     }
  
     /// return the results as a particular type
-    template <typename T, enable_if_t<!is_vector<T>::value, detail::enabler> = detail::dummy> T as() const {
+    template <typename T> T as() const {
         T output;
         results(output);
         return output;
     }
  
-    /// get the results as a vector of a particular type
-    template <typename T, enable_if_t<is_vector<T>::value, detail::enabler> = detail::dummy>
-    T as(char delim = '\0') const {
-        T output;
-        results(output, delim);
-        return output;
-    }
-
     /// See if the callback has been run already
     bool get_callback_run() const { return callback_run_; }
  
@@ -935,6 +937,28 @@ class Option : public OptionBase&lt;Option&gt; {
  
     /// Get the typename for this option
     std::string get_type_name() const { return type_name_(); }
+
+  private:
+    int _add_result(std::string &&result) {
+        int count = 0;
+        if(delimiter_ == '\0') {
+            results_.push_back(std::move(result));
+            ++count;
+        } else {
+            if((result.find_first_of(delimiter_) != std::string::npos)) {
+                for(const auto &var : CLI::detail::split(result, delimiter_)) {
+                    if(!var.empty()) {
+                        results_.push_back(var);
+                        ++count;
+                    }
+                }
+            } else {
+                results_.push_back(std::move(result));
+                ++count;
+            }
+        }
+        return count;
+    }
 };
  
 } // namespace CLI
@@ -1956,17 +1956,17 @@ TEST_F(TApp, CustomUserSepParse) {
  
     std::vector<int> vals = {1, 2, 3};
     args = {"--idx", "1,2,3"};
-    auto opt = app.add_option("--idx", vals, "", ',');
+    auto opt = app.add_option("--idx", vals)->delimiter(',');
     run();
     EXPECT_EQ(vals, std::vector<int>({1, 2, 3}));
     std::vector<int> vals2;
     // check that the results vector gets the results in the same way
-    opt->results(vals2, ',');
+    opt->results(vals2);
     EXPECT_EQ(vals2, vals);
  
     app.remove_option(opt);
  
-    app.add_option("--idx", vals, "", true, ',');
+    app.add_option("--idx", vals, "", true)->delimiter(',');
     run();
     EXPECT_EQ(vals, std::vector<int>({1, 2, 3}));
 }
@@ -1979,8 +1979,7 @@ TEST_F(TApp, DefaultUserSepParse) {
     auto opt = app.add_option("--idx", vals, "");
     run();
     EXPECT_EQ(vals, std::vector<std::string>({"1 2 3", "4 5 6"}));
-    app.remove_option(opt);
-    app.add_option("--idx", vals, "", true);
+    opt->delimiter(',');
     run();
     EXPECT_EQ(vals, std::vector<std::string>({"1 2 3", "4 5 6"}));
 }
@@ -1989,7 +1988,7 @@ TEST_F(TApp, DefaultUserSepParse) {
 TEST_F(TApp, BadUserSepParse) {
  
     std::vector<int> vals;
-    app.add_option("--idx", vals, "");
+    app.add_option("--idx", vals);
  
     args = {"--idx", "1,2,3"};
  
@@ -2001,13 +2000,13 @@ TEST_F(TApp, CustomUserSepParse2) {
  
     std::vector<int> vals = {1, 2, 3};
     args = {"--idx", "1,2,"};
-    auto opt = app.add_option("--idx", vals, "", ',');
+    auto opt = app.add_option("--idx", vals)->delimiter(',');
     run();
     EXPECT_EQ(vals, std::vector<int>({1, 2}));
  
     app.remove_option(opt);
  
-    app.add_option("--idx", vals, "", true, ',');
+    app.add_option("--idx", vals, "", true)->delimiter(',');
     run();
     EXPECT_EQ(vals, std::vector<int>({1, 2}));
 }
@@ -2020,13 +2019,28 @@ TEST_F(TApp, CustomUserSepParseFunction) {
                                               [&vals](std::vector<int> v) {
                                                   vals = std::move(v);
                                                   return true;
-                                              },
-                                              "",
-                                              ',');
+                                              })
+        ->delimiter(',');
     run();
     EXPECT_EQ(vals, std::vector<int>({1, 2, 3}));
 }
  
+// delimiter removal
+TEST_F(TApp, CustomUserSepParseToggle) {
+
+    std::vector<std::string> vals;
+    args = {"--idx", "1,2,3"};
+    auto opt = app.add_option("--idx", vals)->delimiter(',');
+    run();
+    EXPECT_EQ(vals, std::vector<std::string>({"1", "2", "3"}));
+    opt->delimiter('\0');
+    run();
+    EXPECT_EQ(vals, std::vector<std::string>({"1,2,3"}));
+    opt->delimiter(',');
+    run();
+    EXPECT_EQ(vals, std::vector<std::string>({"1", "2", "3"}));
+}
+
 // #209
 TEST_F(TApp, CustomUserSepParse3) {
  
@@ -2035,12 +2049,12 @@ TEST_F(TApp, CustomUserSepParse3) {
             "1",
             ","
             "2"};
-    auto opt = app.add_option("--idx", vals, "", ',');
+    auto opt = app.add_option("--idx", vals)->delimiter(',');
     run();
     EXPECT_EQ(vals, std::vector<int>({1, 2}));
     app.remove_option(opt);
  
-    app.add_option("--idx", vals, "", false, ',');
+    app.add_option("--idx", vals, "", false)->delimiter(',');
     run();
     EXPECT_EQ(vals, std::vector<int>({1, 2}));
 }
@@ -2050,13 +2064,13 @@ TEST_F(TApp, CustomUserSepParse4) {
  
     std::vector<int> vals;
     args = {"--idx", "1,    2"};
-    auto opt = app.add_option("--idx", vals, "", ',');
+    auto opt = app.add_option("--idx", vals)->delimiter(',');
     run();
     EXPECT_EQ(vals, std::vector<int>({1, 2}));
  
     app.remove_option(opt);
  
-    app.add_option("--idx", vals, "", true, ',');
+    app.add_option("--idx", vals, "", true)->delimiter(',');
     run();
     EXPECT_EQ(vals, std::vector<int>({1, 2}));
 }
@@ -77,6 +77,38 @@ TEST_F(TApp, BuiltinComplex) {
     EXPECT_DOUBLE_EQ(3, comp.imag());
 }
  
+TEST_F(TApp, BuiltinComplexWithDelimiter) {
+    cx comp{1, 2};
+    app.add_complex("-c,--complex", comp, "", true)->delimiter('+');
+
+    args = {"-c", "4+3i"};
+
+    std::string help = app.help();
+    EXPECT_THAT(help, HasSubstr("1"));
+    EXPECT_THAT(help, HasSubstr("2"));
+    EXPECT_THAT(help, HasSubstr("COMPLEX"));
+
+    EXPECT_DOUBLE_EQ(1, comp.real());
+    EXPECT_DOUBLE_EQ(2, comp.imag());
+
+    run();
+
+    EXPECT_DOUBLE_EQ(4, comp.real());
+    EXPECT_DOUBLE_EQ(3, comp.imag());
+
+    args = {"-c", "5+-3i"};
+    run();
+
+    EXPECT_DOUBLE_EQ(5, comp.real());
+    EXPECT_DOUBLE_EQ(-3, comp.imag());
+
+    args = {"-c", "6", "-4i"};
+    run();
+
+    EXPECT_DOUBLE_EQ(6, comp.real());
+    EXPECT_DOUBLE_EQ(-4, comp.imag());
+}
+
 TEST_F(TApp, BuiltinComplexIgnoreI) {
     cx comp{1, 2};
     app.add_complex("-c,--complex", comp);