add docs for remove_subcommand and add_subcommand in option_group

add some test of the remove_excludes functions

add test for Issue #256

add remove_subcommand fail test

add remove_subcommand function and add_subcommand to option_group and some tests associated with them.

README.md

@@ -479,6 +479,7 @@ There are several options that are supported on the main app and subcommands and
 -   `.require_subcommand(min, max)`: Explicitly set min and max allowed subcommands. Setting `max` to 0 is unlimited.
 -   `.add_subcommand(name="", description="")`: Add a subcommand, returns a pointer to the internally stored subcommand.
 -   `.add_subcommand(shared_ptr<App>)`: 🚧 Add a subcommand by shared_ptr, returns a pointer to the internally stored subcommand.
+-   `.remove_subcommand(App)`:🚧 Remove a subcommand from the app or subcommand.
 -   `.got_subcommand(App_or_name)`: Check to see if a subcommand was received on the command line.
 -   `.get_subcommands(filter)`: The list of subcommands that match a particular filter function.
 -   `.add_option_group(name="", description="")`: 🚧 Add an [option group](#option-groups) to an App,  an option group is specialized subcommand intended for containing groups of options or other groups for controlling how options interact.  
@@ -562,18 +563,23 @@ The subcommand method
 Will create an option Group, and return a pointer to it.  An option group allows creation of a collection of options, similar to the groups function on options, but with additional controls and requirements.  They allow specific sets of options to be composed and controlled as a collective.  For an example see [range test](./tests/ranges.cpp).  Option groups are a specialization of an App so all [functions](#subcommand-options) that work with an App also work on option groups.  Options can be created as part of an option group using the add functions just like a subcommand, or previously created options can be added through
 
 ```cpp
-ogroup->add_option(option_pointer)
+ogroup->add_option(option_pointer);
 ```
 
 ```cpp
-ogroup->add_options(option_pointer)
+ogroup->add_options(option_pointer);
 ```
 
 ```cpp
-ogroup->add_options(option1,option2,option3,...)
+ogroup->add_options(option1,option2,option3,...);
 ```
 
-The option pointers used in this function must be options defined in the parent application of the option group otherwise an error will be generated.  
+The option pointers used in this function must be options defined in the parent application of the option group otherwise an error will be generated.  Subcommands can also be added via
+```cpp
+ogroup->add_subcommand(subcom_pointer);
+```
+This results in the subcommand being moved from its parent into the option group.
+
 Options in an option group are searched for a command line match after any options in the main app, so any positionals in the main app would be matched first.  So care must be taken to make sure of the order when using positional arguments and option groups.
 Option groups work well with `excludes` and `require_options` methods, as an Application will treat an option group as a single option for the purpose of counting and requirements.  Option groups allow specifying requirements such as requiring 1 of 3 options in one group and 1 of 3 options in a different group. Option groups can contain other groups as well.   Disabling an option group will turn off all options within the group.
 

cmake/AddGoogletest.cmake

@@ -6,7 +6,10 @@
 #
 set(gtest_force_shared_crt ON CACHE BOOL "" FORCE)
 set(BUILD_SHARED_LIBS OFF)
-
+# older version of google tests doesn't support MSYS so needs this flag to compile
+if (MSYS)
+	set(gtest_disable_pthreads ON CACHE BOOL "" FORCE)
+endif()
 set(CMAKE_SUPPRESS_DEVELOPER_WARNINGS 1 CACHE BOOL "")
 add_subdirectory("${CLI11_SOURCE_DIR}/extern/googletest" "${CLI11_BINARY_DIR}/extern/googletest" EXCLUDE_FROM_ALL)
 

include/CLI/App.hpp

@@ -1074,6 +1074,21 @@ class App {
         return subcommands_.back().get();
     }
 
+    /// Removes a subcommand from the App. Takes a subcommand pointer. Returns true if found and removed.
+    bool remove_subcommand(App *subcom) {
+        // Make sure no links exist
+        for(App_p &sub : subcommands_) {
+            sub->remove_excludes(subcom);
+        }
+
+        auto iterator = std::find_if(
+            std::begin(subcommands_), std::end(subcommands_), [subcom](const App_p &v) { return v.get() == subcom; });
+        if(iterator != std::end(subcommands_)) {
+            subcommands_.erase(iterator);
+            return true;
+        }
+        return false;
+    }
     /// Check to see if a subcommand is part of this command (doesn't have to be in command line)
     /// returns the first subcommand if passed a nullptr
     App *get_subcommand(App *subcom) const {
@@ -1124,6 +1139,16 @@ class App {
         throw OptionNotFound(std::to_string(index));
     }
 
+    /// Check to see if an option group is part of this App
+    App *get_option_group(std::string group_name) const {
+        for(const App_p &app : subcommands_) {
+            if(app->name_.empty() && app->group_ == group_name) {
+                return app.get();
+            }
+        }
+        throw OptionNotFound(group_name);
+    }
+
     /// No argument version of count counts the number of times this subcommand was
     /// passed in. The main app will return 1. Unnamed subcommands will also return 1 unless
     /// otherwise modified in a callback
@@ -1437,7 +1462,9 @@ class App {
     bool remove_excludes(App *app) {
         auto iterator = std::find(std::begin(exclude_subcommands_), std::end(exclude_subcommands_), app);
         if(iterator != std::end(exclude_subcommands_)) {
+            auto other_app = *iterator;
             exclude_subcommands_.erase(iterator);
+            other_app->remove_excludes(this);
             return true;
         } else {
             return false;
@@ -2584,7 +2611,7 @@ class Option_group : public App {
         // option groups should have automatic fallthrough
     }
     using App::add_option;
-    /// add an existing option to the Option_group
+    /// Add an existing option to the Option_group
     Option *add_option(Option *opt) {
         if(get_parent() == nullptr) {
             throw OptionNotFound("Unable to locate the specified option");
@@ -2592,13 +2619,21 @@ class Option_group : public App {
         get_parent()->_move_option(opt, this);
         return opt;
     }
-    /// add an existing option to the Option_group
+    /// Add an existing option to the Option_group
     void add_options(Option *opt) { add_option(opt); }
-    /// add a bunch of options to the group
+    /// Add a bunch of options to the group
     template <typename... Args> void add_options(Option *opt, Args... args) {
         add_option(opt);
         add_options(args...);
     }
+    using App::add_subcommand;
+    /// Add an existing subcommand to be a member of an option_group
+    App *add_subcommand(App *subcom) {
+        App_p subc = subcom->get_parent()->get_subcommand_ptr(subcom);
+        subc->get_parent()->remove_subcommand(subcom);
+        add_subcommand(std::move(subc));
+        return subcom;
+    }
 };
 /// Helper function to enable one option group/subcommand when another is used
 inline void TriggerOn(App *trigger_app, App *app_to_enable) {

tests/OptionGroupTest.cpp

@@ -431,6 +431,11 @@ TEST_F(ManyGroups, ExcludesGroup) {
     args = {"--name1", "test", "--name2", "test2"};
 
     EXPECT_THROW(run(), CLI::ExcludesError);
+
+    EXPECT_TRUE(g1->remove_excludes(g2));
+    EXPECT_NO_THROW(run());
+    EXPECT_FALSE(g1->remove_excludes(g1));
+    EXPECT_FALSE(g1->remove_excludes(g2));
 }
 
 TEST_F(ManyGroups, SingleGroupError) {
@@ -605,6 +610,17 @@ TEST_F(ManyGroups, Inheritance) {
     EXPECT_EQ(t2->count(), 2u);
 }
 
+TEST_F(ManyGroups, Moving) {
+    remove_required();
+    auto mg = app.add_option_group("maing");
+    mg->add_subcommand(g1);
+    mg->add_subcommand(g2);
+
+    EXPECT_EQ(g1->get_parent(), mg);
+    EXPECT_EQ(g2->get_parent(), mg);
+    EXPECT_EQ(g3->get_parent(), main);
+}
+
 struct ManyGroupsPreTrigger : public ManyGroups {
     size_t triggerMain, trigger1{87u}, trigger2{34u}, trigger3{27u};
     ManyGroupsPreTrigger() {

tests/SubcommandTest.cpp

@@ -641,6 +641,27 @@ TEST_F(TApp, InheritHelpAllFlag) {
     EXPECT_EQ(help_opt_list.size(), 1u);
 }
 
+TEST_F(TApp, RequiredPosInSubcommand) {
+    app.require_subcommand();
+    std::string bar;
+
+    CLI::App *fooApp = app.add_subcommand("foo", "Foo a bar");
+    fooApp->add_option("bar", bar, "A bar to foo")->required();
+
+    CLI::App *bazApp = app.add_subcommand("baz", "Baz a bar");
+    bazApp->add_option("bar", bar, "A bar a baz")->required();
+
+    args = {"foo", "abc"};
+    run();
+    EXPECT_EQ(bar, "abc");
+    args = {"baz", "cba"};
+    run();
+    EXPECT_EQ(bar, "cba");
+
+    args = {};
+    EXPECT_THROW(run(), CLI::RequiredError);
+}
+
 struct SubcommandProgram : public TApp {
 
     CLI::App *start;
@@ -962,6 +983,22 @@ TEST_F(ManySubcommands, Required4Failure) {
     EXPECT_THROW(run(), CLI::RequiredError);
 }
 
+TEST_F(ManySubcommands, RemoveSub) {
+    run();
+    EXPECT_EQ(app.remaining_size(true), 0u);
+    app.remove_subcommand(sub1);
+    app.allow_extras();
+    run();
+    EXPECT_EQ(app.remaining_size(true), 1u);
+}
+
+TEST_F(ManySubcommands, RemoveSubFail) {
+    auto sub_sub = sub1->add_subcommand("subsub");
+    EXPECT_FALSE(app.remove_subcommand(sub_sub));
+    EXPECT_TRUE(sub1->remove_subcommand(sub_sub));
+    EXPECT_FALSE(app.remove_subcommand(nullptr));
+}
+
 TEST_F(ManySubcommands, manyIndexQuery) {
     auto s1 = app.get_subcommand(0);
     auto s2 = app.get_subcommand(1);
@@ -1100,13 +1137,15 @@ TEST_F(ManySubcommands, SubcommandOptionExclusion) {
     args = {"sub3", "sub4", "--exclude"};
     EXPECT_NO_THROW(run());
 
-    // the option comes later so doesn't exclude
     args = {"sub1", "sub3", "--exclude"};
     EXPECT_THROW(run(), CLI::ExcludesError);
+    EXPECT_TRUE(sub1->remove_excludes(excluder_flag));
+    EXPECT_NO_THROW(run());
+    EXPECT_FALSE(sub1->remove_excludes(excluder_flag));
 
     args = {"--exclude", "sub2", "sub4"};
     EXPECT_THROW(run(), CLI::ExcludesError);
-
+    EXPECT_EQ(sub1->excludes(excluder_flag), sub1);
     args = {"sub1", "--exclude", "sub2", "sub4"};
     try {
         run();