feat: counting flags (#709)

* add a counting flag to address and issue with optional<bool>  and make the flags more consistent

* move the add_flag to a single operation and add a Sum multi option policy

* style: pre-commit.ci fixes

* remove sum_flag_vector overloads

* style: pre-commit.ci fixes

* add limits include

* style: pre-commit.ci fixes

* fix some other warnings

* update docs describing the multi_option_policy

* Apply suggestions from code review

Co-authored-by: Henry Schreiner <HenrySchreinerIII@gmail.com>

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Henry Schreiner <HenrySchreinerIII@gmail.com>

README.md

@@ -242,7 +242,7 @@ While all options internally are the same type, there are several ways to add an
 app.add_option(option_name, help_str="")
 
 app.add_option(option_name,
-               variable_to_bind_to, // bool, char(see note), int, float, vector, enum, std::atomic, or string-like, or anything with a defined conversion from a string or that takes an int, double, or string in a constructor. Also allowed are tuples, std::array or std::pair. Also supported are complex numbers, wrapper types, and containers besides vectorof any other supported type.
+               variable_to_bind_to, // bool, char(see note), int, float, vector, enum, std::atomic, or string-like, or anything with a defined conversion from a string or that takes an int, double, or string in a constructor. Also allowed are tuples, std::array or std::pair. Also supported are complex numbers, wrapper types, and containers besides vectors of any other supported type.
                help_string="")
 
 app.add_option_function<type>(option_name,
@@ -363,7 +363,7 @@ Before parsing, you can set the following options:
 * `->allow_extra_args(true/false)`: If set to true the option will take an unlimited number of arguments like a vector, if false it will limit the number of arguments to the size of the type used in the option.  Default value depends on the nature of the type use, containers default to true, others default to false.
 * `->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()`,`->take_all()`, 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). `->join(delim)` can also be used to join with a specific delimiter. This equivalent to calling `->delimiter(delim)` and `->join()`
+* `->multi_option_policy(CLI::MultiOptionPolicy::Throw)`: Set the multi-option policy. Shortcuts available: `->take_last()`, `->take_first()`,`->take_all()`, 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). `->join(delim)` can also be used to join with a specific delimiter. This equivalent to calling `->delimiter(delim)` and `->join()`.  Valid values are `CLI::MultiOptionPolicy::Throw`, `CLI::MultiOptionPolicy::Throw`, `CLI::MultiOptionPolicy::TakeLast`, `CLI::MultiOptionPolicy::TakeFirst`, `CLI::MultiOptionPolicy::Join`, `CLI::MultiOptionPolicy::TakeAll`, and `CLI::MultiOptionPolicy::Sum` 🚧.
 * `->check(std::string(const std::string &), validator_name="",validator_description="")`: Define a check function.  The function should return a non empty string with the error message if the check fails
 * `->check(Validator)`: Use a Validator object to do the check see [Validators](#validators) for a description of available Validators and how to create new ones.
 * `->transform(std::string(std::string &), validator_name="",validator_description=")`: Converts the input string into the output string, in-place in the parsed options.

book/chapters/flags.md

@@ -15,7 +15,7 @@ This will bind the flag `-f` to the boolean `my_flag`. After the parsing step, `
 
 ## Integer flags
 
-If you want to allow multiple flags, simply use any integer-like instead of a bool:
+If you want to allow multiple flags and count their value, simply use any integral variables instead of a bool:
 
 ```cpp
 int my_flag{0};
@@ -24,6 +24,8 @@ app.add_flag("-f", my_flag, "Optional description");
 
 After the parsing step, `my_flag` will contain the number of times this flag was found on the command line, including 0 if not found.
 
+This behavior can also be controlled manually via `->multi_option_policy(CLI::MultiOptionPolicy::Sum)` as of version 2.2.
+
 ## Arbitrary type flags
 
 CLI11 allows the type of the variable to assign to in the `add_flag` function to be any supported type.   This is particularly useful in combination with specifying default values for flags.  The allowed types include  bool, int, float, vector, enum, or string-like.

book/chapters/options.md

@@ -171,7 +171,7 @@ When you call `add_option`, you get a pointer to the added option. You can use t
 | `->allow_extra_args()` | Allow extra argument values to be included when an option is passed. Enabled by default for vector options. |
 | `->disable_flag_override()` | specify that flag options cannot be overridden on the command line use `=<newval>` |
 | `->delimiter('<CH>')` | specify a character that can be used to separate elements in a command line argument, default is <none>, common values are ',', and ';' |
-| `->multi_option_policy( CLI::MultiOptionPolicy::Throw)` | Sets the policy for handling multiple arguments if the option was received on the command line several times. `Throw`ing an error is the default, but `TakeLast`, `TakeFirst`, `TakeAll`, and `Join` are also available. See the next four lines for shortcuts to set this more easily. |
+| `->multi_option_policy( CLI::MultiOptionPolicy::Throw)` | Sets the policy for handling multiple arguments if the option was received on the command line several times. `Throw`ing an error is the default, but `TakeLast`, `TakeFirst`, `TakeAll`, `Join`, and `Sum` are also available. See the next four lines for shortcuts to set this more easily. |
 | `->take_last()` | Only use the last option if passed several times. This is always true by default for bool options, regardless of the app default, but can be set to false explicitly with `->multi_option_policy()`. |
 | `->take_first()` | sets `->multi_option_policy(CLI::MultiOptionPolicy::TakeFirst)` |
 | `->take_all()` | sets `->multi_option_policy(CLI::MultiOptionPolicy::TakeAll)` |
@@ -211,7 +211,7 @@ One of CLI11's systems to allow customizability without high levels of verbosity
 
 * `group`: The group name starts as "Options"
 * `required`: If the option must be given. Defaults to `false`. Is ignored for flags.
-* `multi_option_policy`: What to do if several copies of an option are passed and one value is expected. Defaults to `CLI::MultiOptionPolicy::Throw`. This is also used for bool flags, but they always are created with the value `CLI::MultiOptionPolicy::TakeLast` regardless of the default, so that multiple bool flags does not cause an error. But you can override that flag by flag.
+* `multi_option_policy`: What to do if several copies of an option are passed and one value is expected. Defaults to `CLI::MultiOptionPolicy::Throw`. This is also used for bool flags, but they always are created with the value `CLI::MultiOptionPolicy::TakeLast` or `CLI::MultiOptionPolicy::Sum` regardless of the default, so that multiple bool flags does not cause an error. But you can override that setting by calling the `multi_option_policy` directly.
 * `ignore_case`: Allow any mixture of cases for the option or flag name
 * `ignore_underscore`: Allow any number of underscores in the option or flag name
 * `configurable`:  Specify whether an option can be configured through a config file

include/CLI/App.hpp

@@ -61,6 +61,22 @@ class App;
 
 using App_p = std::shared_ptr<App>;
 
+namespace detail {
+/// helper functions for adding in appropriate flag modifiers for add_flag
+
+template <typename T, enable_if_t<!std::is_integral<T>::value || (sizeof(T) <= 1U), detail::enabler> = detail::dummy>
+Option *default_flag_modifiers(Option *opt) {
+    return opt->always_capture_default();
+}
+
+/// summing modifiers
+template <typename T, enable_if_t<std::is_integral<T>::value && (sizeof(T) > 1U), detail::enabler> = detail::dummy>
+Option *default_flag_modifiers(Option *opt) {
+    return opt->multi_option_policy(MultiOptionPolicy::Sum)->default_str("0")->force_callback();
+}
+
+}  // namespace detail
+
 class Option_group;
 /// Creates a command line program, with very few defaults.
 /** To use, create a new `Program()` instance with `argc`, `argv`, and a help description. The templated
@@ -807,43 +823,21 @@ class App {
         return _add_flag_internal(flag_name, CLI::callback_t(), flag_description);
     }
 
-    /// Add option for flag with integer result - defaults to allowing multiple passings, but can be forced to one
-    /// if `multi_option_policy(CLI::MultiOptionPolicy::Throw)` is used.
-    template <
-        typename T,
-        enable_if_t<std::is_constructible<T, std::int64_t>::value && !std::is_const<T>::value && !is_bool<T>::value,
-                    detail::enabler> = detail::dummy>
-    Option *add_flag(std::string flag_name,
-                     T &flag_count,  ///< A variable holding the count
-                     std::string flag_description = "") {
-        flag_count = 0;
-        CLI::callback_t fun = [&flag_count](const CLI::results_t &res) {
-            try {
-                detail::sum_flag_vector(res, flag_count);
-            } catch(const std::invalid_argument &) {
-                return false;
-            }
-            return true;
-        };
-        return _add_flag_internal(flag_name, std::move(fun), std::move(flag_description))
-            ->multi_option_policy(MultiOptionPolicy::TakeAll);
-    }
-
     /// Other type version accepts all other types that are not vectors such as bool, enum, string or other classes
     /// that can be converted from a string
     template <typename T,
               enable_if_t<!detail::is_mutable_container<T>::value && !std::is_const<T>::value &&
-                              (!std::is_constructible<T, std::int64_t>::value || is_bool<T>::value) &&
                               !std::is_constructible<std::function<void(int)>, T>::value,
                           detail::enabler> = detail::dummy>
     Option *add_flag(std::string flag_name,
-                     T &flag_result,  ///< A variable holding true if passed
+                     T &flag_result,  ///< A variable holding the flag result
                      std::string flag_description = "") {
 
         CLI::callback_t fun = [&flag_result](const CLI::results_t &res) {
             return CLI::detail::lexical_cast(res[0], flag_result);
         };
-        return _add_flag_internal(flag_name, std::move(fun), std::move(flag_description))->run_callback_for_default();
+        auto *opt = _add_flag_internal(flag_name, std::move(fun), std::move(flag_description));
+        return detail::default_flag_modifiers<T>(opt);
     }
 
     /// Vector version to capture multiple flags.
@@ -888,13 +882,13 @@ class App {
                               std::string flag_description = "") {
 
         CLI::callback_t fun = [function](const CLI::results_t &res) {
-            std::int64_t flag_count = 0;
-            detail::sum_flag_vector(res, flag_count);
+            std::int64_t flag_count{0};
+            CLI::detail::lexical_cast(res[0], flag_count);
             function(flag_count);
             return true;
         };
         return _add_flag_internal(flag_name, std::move(fun), std::move(flag_description))
-            ->multi_option_policy(MultiOptionPolicy::TakeAll);
+            ->multi_option_policy(MultiOptionPolicy::Sum);
     }
 
 #ifdef CLI11_CPP14

include/CLI/Option.hpp

@@ -40,7 +40,8 @@ enum class MultiOptionPolicy : char {
     TakeLast,   //!< take only the last Expected number of arguments
     TakeFirst,  //!< take only the first Expected number of arguments
     Join,       //!< merge all the arguments together into a single string via the delimiter character default('\n')
-    TakeAll     //!< just get all the passed argument regardless
+    TakeAll,    //!< just get all the passed argument regardless
+    Sum         //!< sum all the arguments together if numerical or concatenate directly without delimiter
 };
 
 /// This is the CRTP base class for Option and OptionDefaults. It was designed this way
@@ -1266,6 +1267,9 @@ class Option : public OptionBase<Option> {
                 res.push_back(detail::join(original, std::string(1, (delimiter_ == '\0') ? '\n' : delimiter_)));
             }
             break;
+        case MultiOptionPolicy::Sum:
+            res.push_back(detail::sum_string_vector(original));
+            break;
         case MultiOptionPolicy::Throw:
         default: {
             auto num_min = static_cast<std::size_t>(get_items_expected_min());
@@ -1352,7 +1356,7 @@ class Option : public OptionBase<Option> {
         }
         return result_count;
     }
-};  // namespace CLI
+};
 
 // [CLI11:option_hpp:end]
 }  // namespace CLI

include/CLI/TypeTools.hpp

@@ -9,6 +9,7 @@
 // [CLI11:public_includes:set]
 #include <cstdint>
 #include <exception>
+#include <limits>
 #include <memory>
 #include <string>
 #include <type_traits>
@@ -799,7 +800,16 @@ bool integral_conversion(const std::string &input, T &output) noexcept {
     char *val = nullptr;
     std::uint64_t output_ll = std::strtoull(input.c_str(), &val, 0);
     output = static_cast<T>(output_ll);
-    return val == (input.c_str() + input.size()) && static_cast<std::uint64_t>(output) == output_ll;
+    if(val == (input.c_str() + input.size()) && static_cast<std::uint64_t>(output) == output_ll) {
+        return true;
+    }
+    val = nullptr;
+    std::int64_t output_sll = std::strtoll(input.c_str(), &val, 0);
+    if(val == (input.c_str() + input.size())) {
+        output = (output_sll < 0) ? static_cast<T>(0) : static_cast<T>(output_sll);
+        return (static_cast<std::int64_t>(output) == output_sll);
+    }
+    return false;
 }
 
 /// Convert to a signed integral
@@ -811,7 +821,15 @@ bool integral_conversion(const std::string &input, T &output) noexcept {
     char *val = nullptr;
     std::int64_t output_ll = std::strtoll(input.c_str(), &val, 0);
     output = static_cast<T>(output_ll);
-    return val == (input.c_str() + input.size()) && static_cast<std::int64_t>(output) == output_ll;
+    if(val == (input.c_str() + input.size()) && static_cast<std::int64_t>(output) == output_ll) {
+        return true;
+    }
+    if(input == "true") {
+        // this is to deal with a few oddities with flags and wrapper int types
+        output = static_cast<T>(1);
+        return true;
+    }
+    return false;
 }
 
 /// Convert a flag into an integer value  typically binary flags
@@ -1501,60 +1519,39 @@ bool lexical_conversion(const std::vector<std::string> &strings, AssignTo &outpu
     return false;
 }
 
-/// Sum a vector of flag representations
-/// The flag vector produces a series of strings in a vector,  simple true is represented by a "1",  simple false is
-/// by
-/// "-1" an if numbers are passed by some fashion they are captured as well so the function just checks for the most
-/// common true and false strings then uses stoll to convert the rest for summing
-template <typename T, enable_if_t<std::is_unsigned<T>::value, detail::enabler> = detail::dummy>
-void sum_flag_vector(const std::vector<std::string> &flags, T &output) {
-    std::int64_t count{0};
-    for(auto &flag : flags) {
-        count += detail::to_flag_value(flag);
+/// Sum a vector of strings
+inline std::string sum_string_vector(const std::vector<std::string> &values) {
+    double val{0.0};
+    bool fail{false};
+    std::string output;
+    for(const auto &arg : values) {
+        double tv{0.0};
+        auto comp = detail::lexical_cast<double>(arg, tv);
+        if(!comp) {
+            try {
+                tv = static_cast<double>(detail::to_flag_value(arg));
+            } catch(const std::exception &) {
+                fail = true;
+                break;
             }
-    output = (count > 0) ? static_cast<T>(count) : T{0};
         }
-
-/// Sum a vector of flag representations
-/// The flag vector produces a series of strings in a vector,  simple true is represented by a "1",  simple false is
-/// by
-/// "-1" an if numbers are passed by some fashion they are captured as well so the function just checks for the most
-/// common true and false strings then uses stoll to convert the rest for summing
-template <typename T, enable_if_t<std::is_signed<T>::value, detail::enabler> = detail::dummy>
-void sum_flag_vector(const std::vector<std::string> &flags, T &output) {
-    std::int64_t count{0};
-    for(auto &flag : flags) {
-        count += detail::to_flag_value(flag);
+        val += tv;
     }
-    output = static_cast<T>(count);
+    if(fail) {
+        for(const auto &arg : values) {
+            output.append(arg);
         }
-
-#ifdef _MSC_VER
-#pragma warning(push)
-#pragma warning(disable : 4800)
-#endif
-// with Atomic<XX> this could produce a warning due to the conversion but if atomic gets here it is an old style so will
-// most likely still work
-
-/// Sum a vector of flag representations
-/// The flag vector produces a series of strings in a vector,  simple true is represented by a "1",  simple false is
-/// by
-/// "-1" an if numbers are passed by some fashion they are captured as well so the function just checks for the most
-/// common true and false strings then uses stoll to convert the rest for summing
-template <typename T,
-          enable_if_t<!std::is_signed<T>::value && !std::is_unsigned<T>::value, detail::enabler> = detail::dummy>
-void sum_flag_vector(const std::vector<std::string> &flags, T &output) {
-    std::int64_t count{0};
-    for(auto &flag : flags) {
-        count += detail::to_flag_value(flag);
+    } else {
+        if(val <= static_cast<double>(std::numeric_limits<std::int64_t>::min()) ||
+           val >= static_cast<double>(std::numeric_limits<std::int64_t>::max()) ||
+           val == static_cast<std::int64_t>(val)) {
+            output = detail::value_string(static_cast<int64_t>(val));
+        } else {
+            output = detail::value_string(val);
         }
-    std::string out = detail::to_string(count);
-    lexical_cast(out, output);
     }
-
-#ifdef _MSC_VER
-#pragma warning(pop)
-#endif
+    return output;
+}
 
 }  // namespace detail
 // [CLI11:type_tools_hpp:end]

tests/AppTest.cpp

@@ -212,7 +212,7 @@ TEST_CASE_METHOD(TApp, "OneFlagRefValueFalse", "[app]") {
     run();
     CHECK(app.count("-c") == 1u);
     CHECK(app.count("--count") == 1u);
-    CHECK(ref == -1);
+    CHECK(ref == 0);
 
     args = {"--count=happy"};
     CHECK_THROWS_AS(run(), CLI::ConversionError);
@@ -774,6 +774,42 @@ TEST_CASE_METHOD(TApp, "JoinOpt", "[app]") {
     CHECK("one\ntwo" == str);
 }
 
+TEST_CASE_METHOD(TApp, "SumOpt", "[app]") {
+
+    int val;
+    app.add_option("--val", val)->multi_option_policy(CLI::MultiOptionPolicy::Sum);
+
+    args = {"--val=1", "--val=4"};
+
+    run();
+
+    CHECK(5 == val);
+}
+
+TEST_CASE_METHOD(TApp, "SumOptFloat", "[app]") {
+
+    double val;
+    app.add_option("--val", val)->multi_option_policy(CLI::MultiOptionPolicy::Sum);
+
+    args = {"--val=1.3", "--val=-0.7"};
+
+    run();
+
+    CHECK(0.6 == val);
+}
+
+TEST_CASE_METHOD(TApp, "SumOptString", "[app]") {
+
+    std::string val;
+    app.add_option("--val", val)->multi_option_policy(CLI::MultiOptionPolicy::Sum);
+
+    args = {"--val=i", "--val=2"};
+
+    run();
+
+    CHECK("i2" == val);
+}
+
 TEST_CASE_METHOD(TApp, "JoinOpt2", "[app]") {
 
     std::string str;

tests/ConfigFileTest.cpp

@@ -2254,7 +2254,7 @@ TEST_CASE_METHOD(TApp, "TomlOutputFlag", "[config]") {
     CHECK_THAT(str, Contains("simple=3"));
     CHECK_THAT(str, !Contains("nothing"));
     CHECK_THAT(str, Contains("onething=true"));
-    CHECK_THAT(str, Contains("something=[true, true]"));
+    CHECK_THAT(str, Contains("something=2"));
 
     str = app.config_to_str(true);
     CHECK_THAT(str, Contains("nothing"));
@@ -2685,7 +2685,7 @@ TEST_CASE_METHOD(TApp, "IniOutputFlag", "[config]") {
     CHECK_THAT(str, Contains("simple=3"));
     CHECK_THAT(str, !Contains("nothing"));
     CHECK_THAT(str, Contains("onething=true"));
-    CHECK_THAT(str, Contains("something=true true"));
+    CHECK_THAT(str, Contains("something=2"));
 
     str = app.config_to_str(true);
     CHECK_THAT(str, Contains("nothing"));

tests/OptionTypeTest.cpp

@@ -146,7 +146,7 @@ TEST_CASE_METHOD(TApp, "atomic_bool_flags", "[optiontype]") {
     std::atomic<int> iflag{0};
 
     app.add_flag("-b", bflag);
-    app.add_flag("-i,--int", iflag);
+    app.add_flag("-i,--int", iflag)->multi_option_policy(CLI::MultiOptionPolicy::Sum);
 
     args = {"-b", "-i"};
     run();

tests/OptionalTest.cpp

@@ -130,6 +130,27 @@ TEST_CASE_METHOD(TApp, "StdOptionalUint", "[optional]") {
                   CLI::detail::object_category::wrapper_value);
 }
 
+TEST_CASE_METHOD(TApp, "StdOptionalbool", "[optional]") {
+    std::optional<bool> opt{};
+    CHECK(!opt);
+    app.add_flag("--opt,!--no-opt", opt);
+    CHECK(!opt);
+    run();
+    CHECK(!opt);
+
+    args = {"--opt"};
+    run();
+    CHECK(opt);
+    CHECK(*opt);
+
+    args = {"--no-opt"};
+    run();
+    CHECK(opt);
+    CHECK_FALSE(*opt);
+    static_assert(CLI::detail::classify_object<std::optional<bool>>::value ==
+                  CLI::detail::object_category::wrapper_value);
+}
+
 #ifdef _MSC_VER
 #pragma warning(default : 4244)
 #endif