feat: add a reverse multi option policy (#918)

use it for the default in `set_config` and simplify and add more
flexibility to the the config processing, and potentially in other
options as well.

The reverse policy returns a vector but in reversed order from normal.
This is what we want in the config processing

Inspired by #862, and updated with recent code changes.

---------

Co-authored-by: Volker Christian <[email protected]>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

Author: 3 people

book/chapters/config.md

@@ -8,7 +8,9 @@ config flag. The second item is the default file name. If that is specified, the
 config will try to read that file. The third item is the help string, with a
 reasonable default, and the final argument is a boolean (default: false) that
 indicates that the configuration file is required and an error will be thrown if
-the file is not found and this is set to true.
+the file is not found and this is set to true. The option pointer returned by
+`set_config` is the same type as returned by `add_option` and all modifiers
+including validators, and checks are valid.
 
 ### Adding a default path
 
@@ -98,6 +100,21 @@ If it is needed to get the configuration file name used this can be obtained via
 `app["--config"]->as<std::string>()` assuming `--config` was the configuration
 option name.
 
+### Order of precedence
+
+By default if multiple configuration files are given they are read in reverse
+order. With the last one given taking precedence over the earlier ones. This
+behavior can be changed through the `multi_option_policy`. For example:
+
+```cpp
+app.set_config("--config")
+    ->multi_option_policy(CLI::MultiOptionPolicy::TakeAll);
+```
+
+will read the files in the order given, which may be useful in some
+circumstances. Using `CLI::MultiOptionPolicy::TakeLast` would work similarly
+getting the last `N` files given.
+
 ## Configure file format
 
 Here is an example configuration file, in

book/chapters/options.md

@@ -222,7 +222,7 @@ that to add option modifiers. A full listing of the option modifiers:
 | `->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`, `Join`, and `Sum` 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`, `Reverse`, 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)`                                                                                                                                                                                                                                                                                                                                                                                             |
@@ -246,6 +246,28 @@ function of the form `bool function(std::string)` that runs on every value that
 the option receives, and returns a value that tells CLI11 whether the check
 passed or failed.
 
+### Multi Option policy
+
+The Multi option policy can be used to instruct CLI11 what to do when an option
+is called multiple times and how to return those values in a meaningful way.
+There are several options can be set through the
+`->multi_option_policy( CLI::MultiOptionPolicy::Throw)` option modifier.
+`Throw`ing an error is the default, but `TakeLast`, `TakeFirst`, `TakeAll`,
+`Join`, `Reverse`, and `Sum`
+
+| Value     | Description                                                                       |
+| --------- | --------------------------------------------------------------------------------- |
+| Throw     | Throws an error if more values are given then expected                            |
+| TakeLast  | Selects the last expected number of values given                                  |
+| TakeFirst | Selects the first expected number of of values given                              |
+| Join      | Joins the strings together using the `delimiter` given                            |
+| TakeAll   | Takes all the values                                                              |
+| Sum       | If the values are numeric, it sums them and returns the result                    |
+| Reverse   | Selects the last expected number of values given and return them in reverse order |
+
+NOTE: For reverse, the index used for an indexed validator is also applied in
+reverse order index 1 will be the last element and 2 second from last and so on.
+
 ## Using the `CLI::Option` pointer
 
 Each of the option creation mechanisms returns a pointer to the internally

include/CLI/App.hpp

@@ -1223,6 +1223,9 @@ class App {
     /// Read and process a configuration file (main app only)
     void _process_config_file();
 
+    /// Read and process a particular configuration file
+    void _process_config_file(const std::string &config_file, bool throw_error);
+
     /// Get envname options if not yet passed. Runs on *all* subcommands.
     void _process_env();
 

include/CLI/Option.hpp

@@ -41,7 +41,8 @@ enum class MultiOptionPolicy : char {
     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
-    Sum         //!< sum all the arguments together if numerical or concatenate directly without delimiter
+    Sum,        //!< sum all the arguments together if numerical or concatenate directly without delimiter
+    Reverse,    //!< take only the last Expected number of arguments in reverse order
 };
 
 /// This is the CRTP base class for Option and OptionDefaults. It was designed this way

include/CLI/impl/App_inl.hpp

@@ -318,8 +318,8 @@ CLI11_INLINE Option *App::set_config(std::string option_name,
             config_ptr_->force_callback_ = true;
         }
         config_ptr_->configurable(false);
-        // set the option to take the last value given by default
-        config_ptr_->take_last();
+        // set the option to take the last value and reverse given by default
+        config_ptr_->multi_option_policy(MultiOptionPolicy::Reverse);
     }
 
     return config_ptr_;
@@ -1013,6 +1013,21 @@ CLI11_NODISCARD CLI11_INLINE detail::Classifier App::_recognize(const std::strin
     return detail::Classifier::NONE;
 }
 
+CLI11_INLINE void App::_process_config_file(const std::string &config_file, bool throw_error) {
+    auto path_result = detail::check_path(config_file.c_str());
+    if(path_result == detail::path_type::file) {
+        try {
+            std::vector<ConfigItem> values = config_formatter_->from_file(config_file);
+            _parse_config(values);
+        } catch(const FileError &) {
+            if(throw_error)
+                throw;
+        }
+    } else if(throw_error) {
+        throw FileError::Missing(config_file);
+    }
+}
+
 CLI11_INLINE void App::_process_config_file() {
     if(config_ptr_ != nullptr) {
         bool config_required = config_ptr_->get_required();
@@ -1032,20 +1047,8 @@ CLI11_INLINE void App::_process_config_file() {
             }
             return;
         }
-        for(auto rit = config_files.rbegin(); rit != config_files.rend(); ++rit) {
-            const auto &config_file = *rit;
-            auto path_result = detail::check_path(config_file.c_str());
-            if(path_result == detail::path_type::file) {
-                try {
-                    std::vector<ConfigItem> values = config_formatter_->from_file(config_file);
-                    _parse_config(values);
-                } catch(const FileError &) {
-                    if(config_required || file_given)
-                        throw;
-                }
-            } else if(config_required || file_given) {
-                throw FileError::Missing(config_file);
-            }
+        for(const auto &config_file : config_files) {
+            _process_config_file(config_file, config_required || file_given);
         }
     }
 }

include/CLI/impl/Option_inl.hpp

@@ -500,7 +500,8 @@ CLI11_INLINE void Option::_validate_results(results_t &res) const {
         if(type_size_max_ > 1) {  // in this context index refers to the index in the type
             int index = 0;
             if(get_items_expected_max() < static_cast<int>(res.size()) &&
-               multi_option_policy_ == CLI::MultiOptionPolicy::TakeLast) {
+               (multi_option_policy_ == CLI::MultiOptionPolicy::TakeLast ||
+                multi_option_policy_ == CLI::MultiOptionPolicy::Reverse)) {
                 // create a negative index for the earliest ones
                 index = get_items_expected_max() - static_cast<int>(res.size());
             }
@@ -518,7 +519,8 @@ CLI11_INLINE void Option::_validate_results(results_t &res) const {
         } else {
             int index = 0;
             if(expected_max_ < static_cast<int>(res.size()) &&
-               multi_option_policy_ == CLI::MultiOptionPolicy::TakeLast) {
+               (multi_option_policy_ == CLI::MultiOptionPolicy::TakeLast ||
+                multi_option_policy_ == CLI::MultiOptionPolicy::Reverse)) {
                 // create a negative index for the earliest ones
                 index = expected_max_ - static_cast<int>(res.size());
             }
@@ -550,6 +552,15 @@ CLI11_INLINE void Option::_reduce_results(results_t &out, const results_t &origi
             out.assign(original.end() - static_cast<results_t::difference_type>(trim_size), original.end());
         }
     } break;
+    case MultiOptionPolicy::Reverse: {
+        // Allow multi-option sizes (including 0)
+        std::size_t trim_size = std::min<std::size_t>(
+            static_cast<std::size_t>(std::max<int>(get_items_expected_max(), 1)), original.size());
+        if(original.size() != trim_size || trim_size > 1) {
+            out.assign(original.end() - static_cast<results_t::difference_type>(trim_size), original.end());
+        }
+        std::reverse(out.begin(), out.end());
+    } break;
     case MultiOptionPolicy::TakeFirst: {
         std::size_t trim_size = std::min<std::size_t>(
             static_cast<std::size_t>(std::max<int>(get_items_expected_max(), 1)), original.size());

tests/AppTest.cpp

@@ -54,10 +54,21 @@ TEST_CASE_METHOD(TApp, "OneFlagShortValuesAs", "[app]") {
     auto vec = opt->as<std::vector<int>>();
     CHECK(1 == vec[0]);
     CHECK(2 == vec[1]);
+
+    flg->multi_option_policy(CLI::MultiOptionPolicy::Sum);
+    vec = opt->as<std::vector<int>>();
+    CHECK(3 == vec[0]);
+    CHECK(vec.size() == 1);
+
     flg->multi_option_policy(CLI::MultiOptionPolicy::Join);
     CHECK("1\n2" == opt->as<std::string>());
     flg->delimiter(',');
     CHECK("1,2" == opt->as<std::string>());
+    flg->multi_option_policy(CLI::MultiOptionPolicy::Reverse)->expected(1, 300);
+    vec = opt->as<std::vector<int>>();
+    REQUIRE(vec.size() == 2U);
+    CHECK(2 == vec[0]);
+    CHECK(1 == vec[1]);
 }
 
 TEST_CASE_METHOD(TApp, "OneFlagShortWindows", "[app]") {
@@ -866,6 +877,29 @@ TEST_CASE_METHOD(TApp, "SumOptString", "[app]") {
     CHECK("i2" == val);
 }
 
+TEST_CASE_METHOD(TApp, "ReverseOpt", "[app]") {
+
+    std::vector<std::string> val;
+    auto *opt1 = app.add_option("--val", val)->multi_option_policy(CLI::MultiOptionPolicy::Reverse);
+
+    args = {"--val=string1", "--val=string2", "--val", "string3", "string4"};
+
+    run();
+
+    CHECK(val.size() == 4U);
+
+    CHECK(val.front() == "string4");
+    CHECK(val.back() == "string1");
+
+    opt1->expected(1, 2);
+    run();
+    CHECK(val.size() == 2U);
+
+    CHECK(val.front() == "string4");
+    CHECK(val.back() == "string3");
+    CHECK(opt1->get_multi_option_policy() == CLI::MultiOptionPolicy::Reverse);
+}
+
 TEST_CASE_METHOD(TApp, "JoinOpt2", "[app]") {
 
     std::string str;

tests/ConfigFileTest.cpp

@@ -744,6 +744,90 @@ TEST_CASE_METHOD(TApp, "MultiConfig", "[config]") {
     CHECK(one == 55);
 }
 
+TEST_CASE_METHOD(TApp, "MultiConfig_takelast", "[config]") {
+
+    TempFile tmpini{"TestIniTmp.ini"};
+    TempFile tmpini2{"TestIniTmp2.ini"};
+
+    app.set_config("--config")->multi_option_policy(CLI::MultiOptionPolicy::TakeLast)->expected(1, 3);
+
+    {
+        std::ofstream out{tmpini};
+        out << "[default]" << std::endl;
+        out << "two=99" << std::endl;
+        out << "three=3" << std::endl;
+    }
+
+    {
+        std::ofstream out{tmpini2};
+        out << "[default]" << std::endl;
+        out << "one=55" << std::endl;
+        out << "three=4" << std::endl;
+    }
+
+    int one{0}, two{0}, three{0};
+    app.add_option("--one", one);
+    app.add_option("--two", two);
+    app.add_option("--three", three);
+
+    args = {"--config", tmpini, "--config", tmpini2};
+    run();
+
+    CHECK(two == 99);
+    CHECK(three == 3);
+    CHECK(one == 55);
+
+    two = 0;
+    args = {"--config", tmpini2, "--config", tmpini};
+    run();
+
+    CHECK(two == 99);
+    CHECK(three == 4);
+    CHECK(one == 55);
+}
+
+TEST_CASE_METHOD(TApp, "MultiConfig_takeAll", "[config]") {
+
+    TempFile tmpini{"TestIniTmp.ini"};
+    TempFile tmpini2{"TestIniTmp2.ini"};
+
+    app.set_config("--config")->multi_option_policy(CLI::MultiOptionPolicy::TakeAll);
+
+    {
+        std::ofstream out{tmpini};
+        out << "[default]" << std::endl;
+        out << "two=99" << std::endl;
+        out << "three=3" << std::endl;
+    }
+
+    {
+        std::ofstream out{tmpini2};
+        out << "[default]" << std::endl;
+        out << "one=55" << std::endl;
+        out << "three=4" << std::endl;
+    }
+
+    int one{0}, two{0}, three{0};
+    app.add_option("--one", one);
+    app.add_option("--two", two);
+    app.add_option("--three", three);
+
+    args = {"--config", tmpini, "--config", tmpini2};
+    run();
+
+    CHECK(two == 99);
+    CHECK(three == 3);
+    CHECK(one == 55);
+
+    two = 0;
+    args = {"--config", tmpini2, "--config", tmpini};
+    run();
+
+    CHECK(two == 99);
+    CHECK(three == 4);
+    CHECK(one == 55);
+}
+
 TEST_CASE_METHOD(TApp, "MultiConfig_single", "[config]") {
 
     TempFile tmpini{"TestIniTmp.ini"};