fix: Update positional api (#493)

* fix: Overwrite m_positional_set in parse_positional()

* feat: Allow append in positionals and fix discrepancy between m_positional and m_positional_set during replace

README.md

@@ -152,6 +152,12 @@ options.parse_positional({"script", "server", "filenames"});
 options.parse(argc, argv);
 ```
 
+Note : `parse_positional` defaults to replacing the positionals with the arguments provided in the function call. To append to the existing list of positionals, please use `cxxopts::PositionalMode::Append`.
+
+```cpp
+options.parse_positional("another_option", cxxopts::PositionalMode::Append);
+```
+
 For example, parsing the following arguments:
 ~~~
 my_script.py my_server.com file1.txt file2.txt file3.txt

include/cxxopts.hpp

@@ -369,6 +369,11 @@ enum class ImplicitArgPolicy {
   Enabled
 };
 
+enum class PositionalMode {
+  Replace,
+  Append
+};
+
 // some older versions of GCC warn under this warning
 CXXOPTS_IGNORE_WARNING("-Weffc++")
 class Value : public std::enable_shared_from_this<Value>
@@ -2109,18 +2114,18 @@ class Options
 
   //parse positional arguments into the given option
   void
-  parse_positional(std::string option);
+  parse_positional(std::string option, PositionalMode mode = PositionalMode::Replace);
 
   void
-  parse_positional(std::vector<std::string> options);
+  parse_positional(std::vector<std::string> options, PositionalMode mode = PositionalMode::Replace);
 
   void
-  parse_positional(std::initializer_list<std::string> options);
+  parse_positional(std::initializer_list<std::string> options, PositionalMode mode = PositionalMode::Replace);
 
   template <typename Iterator>
   void
-  parse_positional(Iterator begin, Iterator end) {
-    parse_positional(std::vector<std::string>{begin, end});
+  parse_positional(Iterator begin, Iterator end, PositionalMode mode = PositionalMode::Replace) {
+    parse_positional(std::vector<std::string>{begin, end}, mode);
   }
 
   std::string
@@ -2559,25 +2564,32 @@ OptionParser::consume_positional(const std::string& a, PositionalListIterator& n
 
 inline
 void
-Options::parse_positional(std::string option)
+Options::parse_positional(std::string option, PositionalMode mode)
 {
-  parse_positional(std::vector<std::string>{std::move(option)});
+  parse_positional(std::vector<std::string>{std::move(option)}, mode);
 }
 
 inline
 void
-Options::parse_positional(std::vector<std::string> options)
+Options::parse_positional(std::vector<std::string> options, PositionalMode mode)
 {
-  m_positional = std::move(options);
-
-  m_positional_set.insert(m_positional.begin(), m_positional.end());
+  switch(mode){
+    case PositionalMode::Replace:
+      m_positional = std::move(options);
+      m_positional_set = std::unordered_set<std::string>(m_positional.begin(), m_positional.end());
+      break;
+    case PositionalMode::Append:
+      m_positional.insert(m_positional.end(), options.begin(), options.end());
+      m_positional_set.insert(options.begin(), options.end());
+      break;
+  }
 }
 
 inline
 void
-Options::parse_positional(std::initializer_list<std::string> options)
+Options::parse_positional(std::initializer_list<std::string> options, PositionalMode mode)
 {
-  parse_positional(std::vector<std::string>(options));
+  parse_positional(std::vector<std::string>(options), mode);
 }
 
 inline

test/options.cpp

@@ -559,6 +559,49 @@ TEST_CASE("Positional with list delimiter", "[positional]") {
   CHECK(positional[1] == "e");
 }
 
+TEST_CASE("Multiple calls to parse_positionals", "[positional]") {
+  const char prog_name[] = "multiple_parse_positionals";
+
+  Argv av{prog_name, "1", "2", "3", "last"};
+
+  struct testcase{
+    std::string name;
+    bool use_append_for_c;
+    std::vector<std::string> unmatched;
+    std::vector<std::pair<std::string, int>> exp_values;
+  } tests_eq[] = {
+    {"Append c", true, {"last"}, {{"a", 1}, {"b", 2}, {"c", 3}}},
+    {"Replace c", false, {"2", "3", "last"}, {{"c", 1}}},
+  };
+
+  for(const auto& tc : tests_eq) {
+    SECTION(tc.name){
+      cxxopts::Options options(prog_name, "Multiple calls to parse_positionals");
+      options.add_options()
+        ("a", "First value", cxxopts::value<int>())
+        ("b", "First value", cxxopts::value<int>())
+        ("c", "First value", cxxopts::value<int>());
+      options.parse_positional({"a", "b"});
+      if(tc.use_append_for_c) {
+        options.parse_positional({"c"}, cxxopts::PositionalMode::Append);
+      } else {
+        options.parse_positional({"c"});
+      }
+
+      auto res = options.parse(av.argc(), av.argv());
+      auto um = res.unmatched();
+      CHECK(um.size() == tc.unmatched.size());
+      for(size_t i=0; i<um.size(); ++i) {
+        CHECK(um[i] == tc.unmatched[i]);
+      }
+
+      for (const auto& p : tc.exp_values) {
+        CHECK(res[p.first].as<int>() == p.second);
+      }
+    }
+  }
+}
+
 TEST_CASE("Empty with implicit value", "[implicit]")
 {
   cxxopts::Options options("empty_implicit", "doesn't handle empty");