Commit a78f5bcdcf8d34f5191c01856445a256497b4a7f
Committed by
GitHub
1 parent
b683f4ed
Dropping links if option removed (#179)
Showing
3 changed files
with
62 additions
and
4 deletions
include/CLI/App.hpp
| ... | ... | @@ -781,6 +781,12 @@ class App { |
| 781 | 781 | |
| 782 | 782 | /// Removes an option from the App. Takes an option pointer. Returns true if found and removed. |
| 783 | 783 | bool remove_option(Option *opt) { |
| 784 | + // Make sure no links exist | |
| 785 | + for(Option_p &op : options_) { | |
| 786 | + op->remove_needs(opt); | |
| 787 | + op->remove_excludes(opt); | |
| 788 | + } | |
| 789 | + | |
| 784 | 790 | auto iterator = |
| 785 | 791 | std::find_if(std::begin(options_), std::end(options_), [opt](const Option_p &v) { return v.get() == opt; }); |
| 786 | 792 | if(iterator != std::end(options_)) { |
| ... | ... | @@ -1356,7 +1362,7 @@ class App { |
| 1356 | 1362 | throw RequiredError(opt->get_name()); |
| 1357 | 1363 | } |
| 1358 | 1364 | // Requires |
| 1359 | - for(const Option *opt_req : opt->requires_) | |
| 1365 | + for(const Option *opt_req : opt->needs_) | |
| 1360 | 1366 | if(opt->count() > 0 && opt_req->count() == 0) |
| 1361 | 1367 | throw RequiresError(opt->get_name(), opt_req->get_name()); |
| 1362 | 1368 | // Excludes | ... | ... |
include/CLI/Option.hpp
| ... | ... | @@ -200,7 +200,7 @@ class Option : public OptionBase<Option> { |
| 200 | 200 | std::vector<std::function<std::string(std::string &)>> validators_; |
| 201 | 201 | |
| 202 | 202 | /// A list of options that are required with this option |
| 203 | - std::set<Option *> requires_; | |
| 203 | + std::set<Option *> needs_; | |
| 204 | 204 | |
| 205 | 205 | /// A list of options that are excluded with this option |
| 206 | 206 | std::set<Option *> excludes_; |
| ... | ... | @@ -322,7 +322,7 @@ class Option : public OptionBase<Option> { |
| 322 | 322 | |
| 323 | 323 | /// Sets required options |
| 324 | 324 | Option *needs(Option *opt) { |
| 325 | - auto tup = requires_.insert(opt); | |
| 325 | + auto tup = needs_.insert(opt); | |
| 326 | 326 | if(!tup.second) |
| 327 | 327 | throw OptionAlreadyAdded::Requires(get_name(), opt->get_name()); |
| 328 | 328 | return this; |
| ... | ... | @@ -342,6 +342,18 @@ class Option : public OptionBase<Option> { |
| 342 | 342 | return needs(opt1, args...); |
| 343 | 343 | } |
| 344 | 344 | |
| 345 | + /// Remove needs link from an option. Returns true if the option really was in the needs list. | |
| 346 | + bool remove_needs(Option *opt) { | |
| 347 | + auto iterator = std::find(std::begin(needs_), std::end(needs_), opt); | |
| 348 | + | |
| 349 | + if(iterator != std::end(needs_)) { | |
| 350 | + needs_.erase(iterator); | |
| 351 | + return true; | |
| 352 | + } else { | |
| 353 | + return false; | |
| 354 | + } | |
| 355 | + } | |
| 356 | + | |
| 345 | 357 | /// Sets excluded options |
| 346 | 358 | Option *excludes(Option *opt) { |
| 347 | 359 | excludes_.insert(opt); |
| ... | ... | @@ -369,6 +381,18 @@ class Option : public OptionBase<Option> { |
| 369 | 381 | return excludes(opt1, args...); |
| 370 | 382 | } |
| 371 | 383 | |
| 384 | + /// Remove needs link from an option. Returns true if the option really was in the needs list. | |
| 385 | + bool remove_excludes(Option *opt) { | |
| 386 | + auto iterator = std::find(std::begin(excludes_), std::end(excludes_), opt); | |
| 387 | + | |
| 388 | + if(iterator != std::end(excludes_)) { | |
| 389 | + excludes_.erase(iterator); | |
| 390 | + return true; | |
| 391 | + } else { | |
| 392 | + return false; | |
| 393 | + } | |
| 394 | + } | |
| 395 | + | |
| 372 | 396 | /// Sets environment variable to read if no option given |
| 373 | 397 | Option *envname(std::string name) { |
| 374 | 398 | envname_ = name; |
| ... | ... | @@ -418,7 +442,7 @@ class Option : public OptionBase<Option> { |
| 418 | 442 | std::string get_envname() const { return envname_; } |
| 419 | 443 | |
| 420 | 444 | /// The set of options needed |
| 421 | - std::set<Option *> get_needs() const { return requires_; } | |
| 445 | + std::set<Option *> get_needs() const { return needs_; } | |
| 422 | 446 | |
| 423 | 447 | /// The set of options excluded |
| 424 | 448 | std::set<Option *> get_excludes() const { return excludes_; } | ... | ... |
tests/AppTest.cpp
| ... | ... | @@ -816,6 +816,34 @@ TEST_F(TApp, RemoveOption) { |
| 816 | 816 | EXPECT_THROW(run(), CLI::ExtrasError); |
| 817 | 817 | } |
| 818 | 818 | |
| 819 | +TEST_F(TApp, RemoveNeedsLinks) { | |
| 820 | + auto one = app.add_flag("--one"); | |
| 821 | + auto two = app.add_flag("--two"); | |
| 822 | + | |
| 823 | + two->needs(one); | |
| 824 | + one->needs(two); | |
| 825 | + | |
| 826 | + EXPECT_TRUE(app.remove_option(one)); | |
| 827 | + | |
| 828 | + args = {"--two"}; | |
| 829 | + | |
| 830 | + run(); | |
| 831 | +} | |
| 832 | + | |
| 833 | +TEST_F(TApp, RemoveExcludesLinks) { | |
| 834 | + auto one = app.add_flag("--one"); | |
| 835 | + auto two = app.add_flag("--two"); | |
| 836 | + | |
| 837 | + two->excludes(one); | |
| 838 | + one->excludes(two); | |
| 839 | + | |
| 840 | + EXPECT_TRUE(app.remove_option(one)); | |
| 841 | + | |
| 842 | + args = {"--two"}; | |
| 843 | + | |
| 844 | + run(); // Mostly hoping it does not crash | |
| 845 | +} | |
| 846 | + | |
| 819 | 847 | TEST_F(TApp, FileNotExists) { |
| 820 | 848 | std::string myfile{"TestNonFileNotUsed.txt"}; |
| 821 | 849 | ASSERT_NO_THROW(CLI::NonexistentPath(myfile)); | ... | ... |