From 9acaeebd1ebf4692c5f80ad8eea88f67cbc10bd4 Mon Sep 17 00:00:00 2001
From: Henry Fredrick Schreiner <henry.fredrick.schreiner@cern.ch>
Date: Tue, 21 Nov 2017 13:10:40 -0500
Subject: [PATCH] Subcommand parse order corrected and preserved

---
 include/CLI/App.hpp      | 28 ++++++++++----
 tests/SubcommandTest.cpp | 82 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 102 insertions(+), 8 deletions(-)

diff --git a/include/CLI/App.hpp b/include/CLI/App.hpp
index 65a49960..246f8df3 100644
--- a/include/CLI/App.hpp
+++ b/include/CLI/App.hpp
@@ -102,6 +102,9 @@ class App {
     /// This is a list of pointers to options with the original parse order
     std::vector<Option *> parse_order_;
 
+    /// This is a list of the subcommands collected, in order
+    std::vector<App *> parsed_subcommands_;
+
     ///@}
     /// @name Subcommands
     ///@{
@@ -732,6 +735,7 @@ class App {
 
         parsed_ = false;
         missing_.clear();
+        parsed_subcommands_.clear();
 
         for(const Option_p &opt : options_) {
             opt->clear();
@@ -755,13 +759,18 @@ class App {
         throw OptionNotFound(name);
     }
 
-    /// Get a subcommand pointer list to the currently selected subcommands (after parsing)
-    std::vector<App *> get_subcommands() const {
-        std::vector<App *> subcomms;
-        for(const App_p &subcomptr : subcommands_)
-            if(subcomptr->parsed_)
-                subcomms.push_back(subcomptr.get());
-        return subcomms;
+    /// Get a subcommand pointer list to the currently selected subcommands (after parsing by default, in command line
+    /// order)
+    std::vector<App *> get_subcommands(bool parsed = true) const {
+        if(parsed) {
+            return parsed_subcommands_;
+        } else {
+            std::vector<App *> subcomms(subcommands_.size());
+            std::transform(std::begin(subcommands_), std::end(subcommands_), std::begin(subcomms), [](const App_p &v) {
+                return v.get();
+            });
+            return subcomms;
+        }
     }
 
     /// Check to see if given subcommand was selected
@@ -952,7 +961,7 @@ class App {
             miss_list.push_back(std::get<1>(miss));
         }
         if(recurse) {
-            for(const App_p &sub : subcommands_) {
+            for(const App *sub : parsed_subcommands_) {
                 std::vector<std::string> output = sub->remaining(recurse);
                 std::copy(std::begin(output), std::end(output), std::back_inserter(miss_list));
             }
@@ -1260,6 +1269,9 @@ class App {
         for(const App_p &com : subcommands_) {
             if(com->check_name(args.back())) {
                 args.pop_back();
+                if(std::find(std::begin(parsed_subcommands_), std::end(parsed_subcommands_), com.get()) ==
+                   std::end(parsed_subcommands_))
+                    parsed_subcommands_.push_back(com.get());
                 com->_parse(args);
                 return;
             }
diff --git a/tests/SubcommandTest.cpp b/tests/SubcommandTest.cpp
index 069e9595..1b024399 100644
--- a/tests/SubcommandTest.cpp
+++ b/tests/SubcommandTest.cpp
@@ -27,6 +27,7 @@ TEST_F(TApp, BasicSubcommands) {
 
     args = {"sub2"};
     run();
+    EXPECT_EQ((size_t)1, app.get_subcommands().size());
     EXPECT_EQ(sub2, app.get_subcommands().at(0));
 
     app.reset();
@@ -560,3 +561,84 @@ TEST_F(SubcommandProgram, Groups) {
     EXPECT_THAT(help, HasSubstr("More Commands:"));
     EXPECT_THAT(help, Not(HasSubstr("Subcommands:")));
 }
+
+TEST_F(SubcommandProgram, ExtrasErrors) {
+
+    args = {"one", "two", "start", "three", "four"};
+    EXPECT_THROW(run(), CLI::ExtrasError);
+    app.reset();
+
+    args = {"start", "three", "four"};
+    EXPECT_THROW(run(), CLI::ExtrasError);
+    app.reset();
+
+    args = {"one", "two"};
+    EXPECT_THROW(run(), CLI::ExtrasError);
+    app.reset();
+}
+
+TEST_F(SubcommandProgram, OrderedExtras) {
+
+    app.allow_extras();
+    args = {"one", "two", "start", "three", "four"};
+    EXPECT_THROW(run(), CLI::ExtrasError);
+    app.reset();
+
+    start->allow_extras();
+
+    run();
+
+    EXPECT_EQ(app.remaining(), std::vector<std::string>({"one", "two"}));
+    EXPECT_EQ(start->remaining(), std::vector<std::string>({"three", "four"}));
+    EXPECT_EQ(app.remaining(true), std::vector<std::string>({"one", "two", "three", "four"}));
+
+    app.reset();
+    args = {"one", "two", "start", "three", "--", "four"};
+
+    run();
+
+    EXPECT_EQ(app.remaining(), std::vector<std::string>({"one", "two"}));
+    EXPECT_EQ(start->remaining(), std::vector<std::string>({"three", "--", "four"}));
+    EXPECT_EQ(app.remaining(true), std::vector<std::string>({"one", "two", "three", "--", "four"}));
+}
+
+TEST_F(SubcommandProgram, MixedOrderExtras) {
+
+    app.allow_extras();
+    start->allow_extras();
+    stop->allow_extras();
+
+    args = {"one", "two", "start", "three", "four", "stop", "five", "six"};
+    run();
+
+    EXPECT_EQ(app.remaining(), std::vector<std::string>({"one", "two"}));
+    EXPECT_EQ(start->remaining(), std::vector<std::string>({"three", "four"}));
+    EXPECT_EQ(stop->remaining(), std::vector<std::string>({"five", "six"}));
+    EXPECT_EQ(app.remaining(true), std::vector<std::string>({"one", "two", "three", "four", "five", "six"}));
+
+    app.reset();
+    args = {"one", "two", "stop", "three", "four", "start", "five", "six"};
+    run();
+
+    EXPECT_EQ(app.remaining(), std::vector<std::string>({"one", "two"}));
+    EXPECT_EQ(stop->remaining(), std::vector<std::string>({"three", "four"}));
+    EXPECT_EQ(start->remaining(), std::vector<std::string>({"five", "six"}));
+    EXPECT_EQ(app.remaining(true), std::vector<std::string>({"one", "two", "three", "four", "five", "six"}));
+}
+
+TEST_F(SubcommandProgram, CallbackOrder) {
+    std::vector<int> callback_order;
+    start->set_callback([&callback_order]() { callback_order.push_back(1); });
+    stop->set_callback([&callback_order]() { callback_order.push_back(2); });
+
+    args = {"start", "stop"};
+    run();
+    EXPECT_EQ(callback_order, std::vector<int>({1, 2}));
+
+    app.reset();
+    callback_order.clear();
+
+    args = {"stop", "start"};
+    run();
+    EXPECT_EQ(callback_order, std::vector<int>({2, 1}));
+}