Vector input to config file (#1069)

From #1067, there is a bit of ambiguity in the handling of config file
with vector input and multiple consecutive parameters. For example

```toml
option1=[3,4,5]
option1=[4,5,6]
```

Currently this is handled as if it were 
```toml
option1=[3,4,5,4,5,6]
```
But this could be confusing in the case where the input was referring to
a vector of vectors.
This PR adds a separator in the sequence to separate the vector so they
are two vectors of 3 elements each.
Will need to verify if this change has other side effects. It is a
pretty unusual situation.

---------

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

book/chapters/config.md

@@ -222,6 +222,43 @@ characters. Characters not in this form will be translated as given. If argument
 values with unprintable characters are used to generate a config file this
 binary form will be used in the output string.
 
+### vector of vector inputs
+
+It is possible to specify vector of vector inputs in config file. This can be
+done in a couple different ways
+
+```toml
+# Examples of vector of vector inputs in config
+
+# this example is how config_to_str writes it out
+vector1 = [1,2,3,"",4,5,6]
+
+# alternative with vector separator sequence
+vector2 = [1,2,3,"%%",4,5,6]
+
+# multiline format
+vector3 = [1,2,3]
+vector3 = [4,5,6]
+
+```
+
+The `%%` is ignored in multiline format if the inject_separator modifier on the
+option is set to false, thus for vector 3 if the option is storing to a single
+vector all the elements will be in that vector.
+
+For config file multiple sequential duplicate variable names are treated as if
+they are a vector input, with possible separator insertion in the case of
+multiple input vectors.
+
+The config parser has a modifier
+
+```C++
+ app.get_config_formatter_base()->allowDuplicateFields();
+```
+
+This modification will insert the separator between each line even if not
+sequential. This allows an input option to be configured with multiple lines.
+
 ## Multiple configuration files
 
 If it is desired that multiple configuration be allowed. Use

include/CLI/ConfigFwd.hpp

@@ -34,12 +34,14 @@ struct ConfigItem {
     std::string name{};
     /// Listing of inputs
     std::vector<std::string> inputs{};
-
+    /// @brief indicator if a multiline vector separator was inserted
+    bool multiline{false};
     /// The list of parents and name joined by "."
     CLI11_NODISCARD std::string fullname() const {
         std::vector<std::string> tmp = parents;
         tmp.emplace_back(name);
         return detail::join(tmp, ".");
+        (void)multiline;  // suppression for cppcheck false positive
     }
 };
 
@@ -102,6 +104,8 @@ class ConfigBase : public Config {
     char parentSeparatorChar{'.'};
     /// Specify the configuration index to use for arrayed sections
     int16_t configIndex{-1};
+    /// specify the config reader should collapse repeated field names to a single vector
+    bool allowMultipleDuplicateFields{false};
     /// Specify the configuration section that should be used
     std::string configSection{};
 
@@ -166,6 +170,11 @@ class ConfigBase : public Config {
         configIndex = sectionIndex;
         return this;
     }
+    /// specify that multiple duplicate arguments should be merged even if not sequential
+    ConfigBase *allowDuplicateFields(bool value = true) {
+        allowMultipleDuplicateFields = value;
+        return this;
+    }
 };
 
 /// the default Config is the TOML file format

include/CLI/impl/App_inl.hpp

@@ -1529,9 +1529,17 @@ CLI11_INLINE bool App::_parse_single_config(const ConfigItem &item, std::size_t
         }
         throw ConfigError::NotConfigurable(item.fullname());
     }
-
     if(op->empty()) {
-
+        std::vector<std::string> buffer;  // a buffer to use for copying an modifying inputs in a few cases
+        bool useBuffer{false};
+        if(item.multiline) {
+            if(!op->get_inject_separator()) {
+                buffer = item.inputs;
+                buffer.erase(std::remove(buffer.begin(), buffer.end(), "%%"), buffer.end());
+                useBuffer = true;
+            }
+        }
+        const std::vector<std::string> &inputs = (useBuffer) ? buffer : item.inputs;
         if(op->get_expected_min() == 0) {
             if(item.inputs.size() <= 1) {
                 // Flag parsing
@@ -1555,18 +1563,18 @@ CLI11_INLINE bool App::_parse_single_config(const ConfigItem &item, std::size_t
                 op->add_result(res);
                 return true;
             }
-            if(static_cast<int>(item.inputs.size()) > op->get_items_expected_max() &&
+            if(static_cast<int>(inputs.size()) > op->get_items_expected_max() &&
                op->get_multi_option_policy() != MultiOptionPolicy::TakeAll) {
                 if(op->get_items_expected_max() > 1) {
-                    throw ArgumentMismatch::AtMost(item.fullname(), op->get_items_expected_max(), item.inputs.size());
+                    throw ArgumentMismatch::AtMost(item.fullname(), op->get_items_expected_max(), inputs.size());
                 }
 
                 if(!op->get_disable_flag_override()) {
                     throw ConversionError::TooManyInputsFlag(item.fullname());
                 }
                 // if the disable flag override is set then we must have the flag values match a known flag value
                 // this is true regardless of the output value, so an array input is possible and must be accounted for
-                for(const auto &res : item.inputs) {
+                for(const auto &res : inputs) {
                     bool valid_value{false};
                     if(op->default_flag_values_.empty()) {
                         if(res == "true" || res == "false" || res == "1" || res == "0") {
@@ -1590,7 +1598,7 @@ CLI11_INLINE bool App::_parse_single_config(const ConfigItem &item, std::size_t
                 return true;
             }
         }
-        op->add_result(item.inputs);
+        op->add_result(inputs);
         op->run_callback();
     }
 

include/CLI/impl/Config_inl.hpp

@@ -201,6 +201,27 @@ CLI11_INLINE bool hasMLString(std::string const &fullString, char check) {
     auto it = fullString.rbegin();
     return (*it == check) && (*(it + 1) == check) && (*(it + 2) == check);
 }
+
+/// @brief  find a matching configItem in a list
+inline auto find_matching_config(std::vector<ConfigItem> &items,
+                                 const std::vector<std::string> &parents,
+                                 const std::string &name,
+                                 bool fullSearch) -> decltype(items.begin()) {
+    if(items.empty()) {
+        return items.end();
+    }
+    auto search = items.end() - 1;
+    do {
+        if(search->parents == parents && search->name == name) {
+            return search;
+        }
+        if(search == items.begin()) {
+            break;
+        }
+        --search;
+    } while(fullSearch);
+    return items.end();
+}
 }  // namespace detail
 
 inline std::vector<ConfigItem> ConfigBase::from_config(std::istream &input) const {
@@ -426,8 +447,17 @@ inline std::vector<ConfigItem> ConfigBase::from_config(std::istream &input) cons
             parents.erase(parents.begin());
             inSection = true;
         }
-        if(!output.empty() && name == output.back().name && parents == output.back().parents) {
-            output.back().inputs.insert(output.back().inputs.end(), items_buffer.begin(), items_buffer.end());
+        auto match = detail::find_matching_config(output, parents, name, allowMultipleDuplicateFields);
+        if(match != output.end()) {
+            if((match->inputs.size() > 1 && items_buffer.size() > 1) || allowMultipleDuplicateFields) {
+                // insert a separator if one is not already present
+                if(!(match->inputs.back().empty() || items_buffer.front().empty() || match->inputs.back() == "%%" ||
+                     items_buffer.front() == "%%")) {
+                    match->inputs.emplace_back("%%");
+                    match->multiline = true;
+                }
+            }
+            match->inputs.insert(match->inputs.end(), items_buffer.begin(), items_buffer.end());
         } else {
             output.emplace_back();
             output.back().parents = std::move(parents);

tests/ConfigFileTest.cpp

@@ -1555,6 +1555,104 @@ TEST_CASE_METHOD(TApp, "TOMLVectordirect", "[config]") {
     CHECK(three == std::vector<int>({1, 2, 3}));
 }
 
+TEST_CASE_METHOD(TApp, "TOMLVectorVector", "[config]") {
+
+    TempFile tmpini{"TestIniTmp.ini"};
+
+    app.set_config("--config", tmpini);
+
+    app.config_formatter(std::make_shared<CLI::ConfigTOML>());
+
+    {
+        std::ofstream out{tmpini};
+        out << "#this is a comment line\n";
+        out << "[default]\n";
+        out << "two=1,2,3\n";
+        out << "two= 4, 5, 6\n";
+        out << "three=1,2,3\n";
+        out << "three= 4, 5, 6\n";
+        out << "four=1,2\n";
+        out << "four= 3,4\n";
+        out << "four=5,6\n";
+        out << "four= 7,8\n";
+    }
+
+    std::vector<std::vector<int>> two;
+    std::vector<int> three, four;
+    app.add_option("--two", two)->delimiter(',');
+    app.add_option("--three", three)->delimiter(',');
+    app.add_option("--four", four)->delimiter(',');
+
+    run();
+
+    auto str = app.config_to_str();
+    CHECK(two == std::vector<std::vector<int>>({{1, 2, 3}, {4, 5, 6}}));
+    CHECK(three == std::vector<int>({1, 2, 3, 4, 5, 6}));
+    CHECK(four == std::vector<int>({1, 2, 3, 4, 5, 6, 7, 8}));
+}
+
+TEST_CASE_METHOD(TApp, "TOMLVectorVectorSeparated", "[config]") {
+
+    TempFile tmpini{"TestIniTmp.ini"};
+
+    app.set_config("--config", tmpini);
+
+    app.config_formatter(std::make_shared<CLI::ConfigTOML>());
+    app.get_config_formatter_base()->allowDuplicateFields();
+    {
+        std::ofstream out{tmpini};
+        out << "#this is a comment line\n";
+        out << "[default]\n";
+        out << "two=1,2,3\n";
+        out << "three=1,2,3\n";
+        out << "three= 4, 5, 6\n";
+        out << "two= 4, 5, 6\n";
+    }
+
+    std::vector<std::vector<int>> two;
+    std::vector<int> three;
+    app.add_option("--two", two)->delimiter(',');
+    app.add_option("--three", three)->delimiter(',');
+
+    run();
+
+    auto str = app.config_to_str();
+    CHECK(two == std::vector<std::vector<int>>({{1, 2, 3}, {4, 5, 6}}));
+    CHECK(three == std::vector<int>({1, 2, 3, 4, 5, 6}));
+}
+
+TEST_CASE_METHOD(TApp, "TOMLVectorVectorSeparatedSingleElement", "[config]") {
+
+    TempFile tmpini{"TestIniTmp.ini"};
+
+    app.set_config("--config", tmpini);
+
+    app.config_formatter(std::make_shared<CLI::ConfigTOML>());
+    app.get_config_formatter_base()->allowDuplicateFields();
+    {
+        std::ofstream out{tmpini};
+        out << "#this is a comment line\n";
+        out << "[default]\n";
+        out << "two=1\n";
+        out << "three=1\n";
+        out << "three= 4\n";
+        out << "three= 5\n";
+        out << "two= 2\n";
+        out << "two=3\n";
+    }
+
+    std::vector<std::vector<int>> two;
+    std::vector<int> three;
+    app.add_option("--two", two)->delimiter(',');
+    app.add_option("--three", three)->delimiter(',');
+
+    run();
+
+    auto str = app.config_to_str();
+    CHECK(two == std::vector<std::vector<int>>({{1}, {2}, {3}}));
+    CHECK(three == std::vector<int>({1, 4, 5}));
+}
+
 TEST_CASE_METHOD(TApp, "TOMLStringVector", "[config]") {
 
     TempFile tmptoml{"TestTomlTmp.toml"};