Stop sharing argument values with parent parsers

See also: p-ranav/argparse#50

README.md

@@ -349,7 +349,7 @@ Here's what's happening:
 
 ### Parent Parsers
 
-Sometimes, several parsers share a common set of arguments. Rather than repeating the definitions of these arguments, a single parser with all the shared arguments can be added as a parent to another ArgumentParser instance. The ```.add_parents``` method takes a list of ArgumentParser objects, collects all the positional and optional actions from them, and adds these actions to the ArgumentParser object being constructed:
+Sometimes, several parsers share a common set of arguments. Rather than repeating the definitions of these arguments, a single parser with all the common arguments can be added as a parent to another ArgumentParser instance. The ```.add_parents``` method takes a list of ArgumentParser objects, collects all the positional and optional actions from them, and adds these actions to the ArgumentParser object being constructed:
 
 ```cpp
 argparse::ArgumentParser parent_parser("main");

include/argparse.hpp

@@ -35,7 +35,6 @@ SOFTWARE.
 #include <iterator>
 #include <list>
 #include <map>
-#include <memory>
 #include <numeric>
 #include <sstream>
 #include <stdexcept>
@@ -383,37 +382,30 @@ public:
   // Parameter packing
   // Call add_argument with variadic number of string arguments
   template <typename... Targs> Argument &add_argument(Targs... Fargs) {
-    std::shared_ptr<Argument> tArgument =
-        std::make_shared<Argument>(std::move(Fargs)...);
+    auto tArgument = mOptionalArguments.emplace(cend(mOptionalArguments),
+                                                std::move(Fargs)...);
 
-    if (tArgument->mIsOptional)
-      mOptionalArguments.emplace_back(tArgument);
-    else
-      mPositionalArguments.emplace_back(tArgument);
+    if (!tArgument->mIsOptional)
+      mPositionalArguments.splice(cend(mPositionalArguments),
+                                  mOptionalArguments, tArgument);
 
-    for (const auto &mName : tArgument->mNames) {
-      mArgumentMap.insert_or_assign(mName, tArgument);
-    }
+    index_argument(tArgument);
     return *tArgument;
   }
 
   // Parameter packed add_parents method
   // Accepts a variadic number of ArgumentParser objects
-  template <typename... Targs> void add_parents(Targs... Fargs) {
-    const auto tNewParentParsers = {Fargs...};
-    for (const auto &tParentParser : tNewParentParsers) {
-      const auto &tPositionalArguments = tParentParser.mPositionalArguments;
-      std::copy(std::begin(tPositionalArguments),
-                std::end(tPositionalArguments),
-                std::back_inserter(mPositionalArguments));
-
-      const auto &tOptionalArguments = tParentParser.mOptionalArguments;
-      std::copy(std::begin(tOptionalArguments), std::end(tOptionalArguments),
-                std::back_inserter(mOptionalArguments));
-
-      const auto &tArgumentMap = tParentParser.mArgumentMap;
-      for (const auto &[tKey, tValue] : tArgumentMap) {
-        mArgumentMap.insert_or_assign(tKey, tValue);
+  template <typename... Targs> void add_parents(const Targs &... Fargs) {
+    for (auto &tParentParser : {Fargs...}) {
+      for (auto &tArgument : tParentParser.mPositionalArguments) {
+        auto it =
+            mPositionalArguments.insert(cend(mPositionalArguments), tArgument);
+        index_argument(it);
+      }
+      for (auto &tArgument : tParentParser.mOptionalArguments) {
+        auto it =
+            mOptionalArguments.insert(cend(mOptionalArguments), tArgument);
+        index_argument(it);
       }
     }
   }
@@ -471,7 +463,7 @@ public:
       size_t tLongestArgumentLength = parser.get_length_of_longest_argument();
 
       for (const auto &argument : parser.mPositionalArguments) {
-        stream << argument->mNames.front() << " ";
+        stream << argument.mNames.front() << " ";
       }
       stream << "\n\n";
 
@@ -480,7 +472,7 @@ public:
 
       for (const auto &mPositionalArgument : parser.mPositionalArguments) {
         stream.width(tLongestArgumentLength);
-        stream << *mPositionalArgument;
+        stream << mPositionalArgument;
       }
 
       if (!parser.mOptionalArguments.empty())
@@ -489,7 +481,7 @@ public:
 
       for (const auto &mOptionalArgument : parser.mOptionalArguments) {
         stream.width(tLongestArgumentLength);
-        stream << *mOptionalArgument;
+        stream << mOptionalArgument;
       }
     }
 
@@ -533,7 +525,7 @@ private:
           throw std::runtime_error(
               "Maximum number of positional arguments exceeded");
         }
-        auto tArgument = *(positionalArgumentIt++);
+        auto tArgument = positionalArgumentIt++;
         it = tArgument->consume(it, end);
       } else if (auto tIterator = mArgumentMap.find(tCurrentArgument);
                  tIterator != mArgumentMap.end()) {
@@ -585,10 +577,17 @@ private:
                              std::end(argumentLengths));
   }
 
+  using list_iterator = std::list<Argument>::iterator;
+
+  void index_argument(list_iterator argIt) {
+    for (auto &mName : std::as_const(argIt->mNames))
+      mArgumentMap.insert_or_assign(mName, argIt);
+  }
+
   std::string mProgramName;
-  std::vector<std::shared_ptr<Argument>> mPositionalArguments;
-  std::vector<std::shared_ptr<Argument>> mOptionalArguments;
-  std::map<std::string, std::shared_ptr<Argument>> mArgumentMap;
+  std::list<Argument> mPositionalArguments;
+  std::list<Argument> mOptionalArguments;
+  std::map<std::string, list_iterator, std::less<>> mArgumentMap;
 };
 
 } // namespace argparse

test/test_parent_parsers.hpp

@@ -12,6 +12,7 @@ TEST_CASE("Add parent parsers", "[parent_parsers]") {
   child_parser.add_parents(parent_parser);
   child_parser.parse_args({ "./main", "--verbose"});
   REQUIRE(child_parser["--verbose"] == true);
+  REQUIRE(parent_parser["--verbose"] == false);
 }
 
 TEST_CASE("Add parent to multiple parent parsers", "[parent_parsers]") {
@@ -26,6 +27,7 @@ TEST_CASE("Add parent to multiple parent parsers", "[parent_parsers]") {
   foo_parser.parse_args({ "./main", "--parent", "2", "XXX" });
   REQUIRE(foo_parser["--parent"] == 2);
   REQUIRE(foo_parser["foo"] == std::string("XXX"));
+  REQUIRE(parent_parser["--parent"] == 0);
 
   argparse::ArgumentParser bar_parser("bar");
   bar_parser.add_argument("--bar");