From ddc695ebacf7bccbb5f6cd99368e23e6db7ca437 Mon Sep 17 00:00:00 2001
From: Alexander Galanin <al-github@galanin.nnov.ru>
Date: Thu, 15 Jun 2023 01:34:26 +0400
Subject: [PATCH] Don't split by list delimiter in positional lists (#398)

---
 include/cxxopts.hpp | 52 ++++++++++++++++++++++++++++++++++++++++-----
 test/options.cpp    | 31 +++++++++++++++++++++++++++
 2 files changed, 78 insertions(+), 5 deletions(-)

diff --git a/include/cxxopts.hpp b/include/cxxopts.hpp
index 062c17c..cbbef80 100644
--- a/include/cxxopts.hpp
+++ b/include/cxxopts.hpp
@@ -368,6 +368,9 @@ class Value : public std::enable_shared_from_this<Value>
   std::shared_ptr<Value>
   clone() const = 0;
 
+  virtual void
+  add(const std::string& text) const = 0;
+
   virtual void
   parse(const std::string& text) const = 0;
 
@@ -1064,6 +1067,22 @@ parse_value(const std::string& text, std::vector<T>& value)
   }
 }
 
+template <typename T>
+void
+add_value(const std::string& text, T& value)
+{
+  parse_value(text, value);
+}
+
+template <typename T>
+void
+add_value(const std::string& text, std::vector<T>& value)
+{
+  T v;
+  add_value(text, v);
+  value.emplace_back(std::move(v));
+}
+
 #ifdef CXXOPTS_HAS_OPTIONAL
 template <typename T>
 void
@@ -1137,6 +1156,12 @@ class abstract_value : public Value
     m_implicit_value = rhs.m_implicit_value;
   }
 
+  void
+  add(const std::string& text) const override
+  {
+    add_value(text, *m_store);
+  }
+
   void
   parse(const std::string& text) const override
   {
@@ -1422,6 +1447,19 @@ struct HelpGroupDetails
 class OptionValue
 {
   public:
+  void
+  add
+  (
+    const std::shared_ptr<const OptionDetails>& details,
+    const std::string& text
+  )
+  {
+    ensure_value(details);
+    ++m_count;
+    m_value->add(text);
+    m_long_names = &details->long_names();
+  }
+
   void
   parse
   (
@@ -1792,7 +1830,7 @@ class OptionParser
   );
 
   void
-  add_to_option(OptionMap::const_iterator iter, const std::string& option, const std::string& arg);
+  add_to_option(const std::shared_ptr<OptionDetails>& value, const std::string& arg);
 
   void
   parse_option
@@ -2339,9 +2377,13 @@ OptionParser::checked_parse_arg
 
 inline
 void
-OptionParser::add_to_option(OptionMap::const_iterator iter, const std::string& option, const std::string& arg)
+OptionParser::add_to_option(const std::shared_ptr<OptionDetails>& value, const std::string& arg)
 {
-  parse_option(iter->second, option, arg);
+  auto hash = value->hash();
+  auto& result = m_parsed[hash];
+  result.add(value, arg);
+
+  m_sequential.emplace_back(value->essential_name(), arg);
 }
 
 inline
@@ -2358,14 +2400,14 @@ OptionParser::consume_positional(const std::string& a, PositionalListIterator& n
         auto& result = m_parsed[iter->second->hash()];
         if (result.count() == 0)
         {
-          add_to_option(iter, *next, a);
+          add_to_option(iter->second, a);
           ++next;
           return true;
         }
         ++next;
         continue;
       }
-      add_to_option(iter, *next, a);
+      add_to_option(iter->second, a);
       return true;
     }
     throw_or_mimic<exceptions::no_such_option>(*next);
diff --git a/test/options.cpp b/test/options.cpp
index 9b4ae0a..b6f1ba4 100644
--- a/test/options.cpp
+++ b/test/options.cpp
@@ -288,6 +288,37 @@ TEST_CASE("Positional with empty arguments", "[positional]") {
   REQUIRE(actual == expected);
 }
 
+TEST_CASE("Positional with list delimiter", "[positional]") {
+  std::string single;
+  std::vector<std::string> positional;
+
+  cxxopts::Options options("test_all_positional_list_delimiter", " - test all positional with list delimiters");
+  options.add_options()
+    ("single", "Single positional param",
+      cxxopts::value<std::string>(single))
+    ("positional", "Positional parameters vector",
+      cxxopts::value<std::vector<std::string>>(positional))
+  ;
+
+  Argv av({"tester", "a,b", "c,d", "e"});
+
+  auto argc = av.argc();
+  auto argv = av.argv();
+
+  std::vector<std::string> pos_names = {"single", "positional"};
+
+  options.parse_positional(pos_names.begin(), pos_names.end());
+
+  auto result = options.parse(argc, argv);
+
+  CHECK(result.unmatched().size() == 0);
+  REQUIRE(positional.size() == 2);
+
+  CHECK(single == "a,b");
+  CHECK(positional[0] == "c,d");
+  CHECK(positional[1] == "e");
+}
+
 TEST_CASE("Empty with implicit value", "[implicit]")
 {
   cxxopts::Options options("empty_implicit", "doesn't handle empty");