Dropping links if option removed (#179)

CLIUtils · Nov 13, 2018 · a78f5bc · a78f5bc
1 parent b683f4e
commit a78f5bc
Show file tree

Hide file tree

Showing 3 changed files with 62 additions and 4 deletions.
diff --git a/include/CLI/App.hpp b/include/CLI/App.hpp
@@ -781,6 +781,12 @@ class App {
 
     /// Removes an option from the App. Takes an option pointer. Returns true if found and removed.
     bool remove_option(Option *opt) {
+        // Make sure no links exist
+        for(Option_p &op : options_) {
+            op->remove_needs(opt);
+            op->remove_excludes(opt);
+        }
+
         auto iterator =
             std::find_if(std::begin(options_), std::end(options_), [opt](const Option_p &v) { return v.get() == opt; });
         if(iterator != std::end(options_)) {
@@ -1356,7 +1362,7 @@ class App {
                     throw RequiredError(opt->get_name());
             }
             // Requires
-            for(const Option *opt_req : opt->requires_)
+            for(const Option *opt_req : opt->needs_)
                 if(opt->count() > 0 && opt_req->count() == 0)
                     throw RequiresError(opt->get_name(), opt_req->get_name());
             // Excludes

diff --git a/include/CLI/Option.hpp b/include/CLI/Option.hpp
@@ -200,7 +200,7 @@ class Option : public OptionBase<Option> {
     std::vector<std::function<std::string(std::string &)>> validators_;
 
     /// A list of options that are required with this option
-    std::set<Option *> requires_;
+    std::set<Option *> needs_;
 
     /// A list of options that are excluded with this option
     std::set<Option *> excludes_;
@@ -322,7 +322,7 @@ class Option : public OptionBase<Option> {
 
     /// Sets required options
     Option *needs(Option *opt) {
-        auto tup = requires_.insert(opt);
+        auto tup = needs_.insert(opt);
         if(!tup.second)
             throw OptionAlreadyAdded::Requires(get_name(), opt->get_name());
         return this;
@@ -342,6 +342,18 @@ class Option : public OptionBase<Option> {
         return needs(opt1, args...);
     }
 
+    /// Remove needs link from an option. Returns true if the option really was in the needs list.
+    bool remove_needs(Option *opt) {
+        auto iterator = std::find(std::begin(needs_), std::end(needs_), opt);
+
+        if(iterator != std::end(needs_)) {
+            needs_.erase(iterator);
+            return true;
+        } else {
+            return false;
+        }
+    }
+
     /// Sets excluded options
     Option *excludes(Option *opt) {
         excludes_.insert(opt);
@@ -369,6 +381,18 @@ class Option : public OptionBase<Option> {
         return excludes(opt1, args...);
     }
 
+    /// Remove needs link from an option. Returns true if the option really was in the needs list.
+    bool remove_excludes(Option *opt) {
+        auto iterator = std::find(std::begin(excludes_), std::end(excludes_), opt);
+
+        if(iterator != std::end(excludes_)) {
+            excludes_.erase(iterator);
+            return true;
+        } else {
+            return false;
+        }
+    }
+
     /// Sets environment variable to read if no option given
     Option *envname(std::string name) {
         envname_ = name;
@@ -418,7 +442,7 @@ class Option : public OptionBase<Option> {
     std::string get_envname() const { return envname_; }
 
     /// The set of options needed
-    std::set<Option *> get_needs() const { return requires_; }
+    std::set<Option *> get_needs() const { return needs_; }
 
     /// The set of options excluded
     std::set<Option *> get_excludes() const { return excludes_; }

diff --git a/tests/AppTest.cpp b/tests/AppTest.cpp
@@ -816,6 +816,34 @@ TEST_F(TApp, RemoveOption) {
     EXPECT_THROW(run(), CLI::ExtrasError);
 }
 
+TEST_F(TApp, RemoveNeedsLinks) {
+    auto one = app.add_flag("--one");
+    auto two = app.add_flag("--two");
+
+    two->needs(one);
+    one->needs(two);
+
+    EXPECT_TRUE(app.remove_option(one));
+
+    args = {"--two"};
+
+    run();
+}
+
+TEST_F(TApp, RemoveExcludesLinks) {
+    auto one = app.add_flag("--one");
+    auto two = app.add_flag("--two");
+
+    two->excludes(one);
+    one->excludes(two);
+
+    EXPECT_TRUE(app.remove_option(one));
+
+    args = {"--two"};
+
+    run(); // Mostly hoping it does not crash
+}
+
 TEST_F(TApp, FileNotExists) {
     std::string myfile{"TestNonFileNotUsed.txt"};
     ASSERT_NO_THROW(CLI::NonexistentPath(myfile));