Skip to content

Commit

Permalink
allow disabled subcommand to bypass name matching check
Browse files Browse the repository at this point in the history
  • Loading branch information
phlptp authored and henryiii committed Oct 25, 2019
1 parent 18a58c4 commit a460029
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 6 deletions.
7 changes: 6 additions & 1 deletion include/CLI/App.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -2804,9 +2804,14 @@ class App {
/// Helper function to run through all possible comparisons of subcommand names to check there is no overlap
const std::string &_compare_subcommand_names(const App &subcom, const App &base) const {
static const std::string estring;

if(subcom.disabled_) {
return estring;
}
for(auto &subc : base.subcommands_) {
if(subc.get() != &subcom) {
if(subc->disabled_) {
continue;
}
if(!subcom.get_name().empty()) {
if(subc->check_name(subcom.get_name())) {
return subcom.get_name();
Expand Down
12 changes: 7 additions & 5 deletions tests/OptionGroupTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -473,7 +473,7 @@ TEST_F(TApp, ExistingSubcommandMatch) {
}
sshared->remove_subcommand(s1);

auto s3 = app.add_subcommand("sub3");
app.add_subcommand("sub3");
// now check that the subsubcommand overlaps
try {
app.add_subcommand(sshared);
Expand Down Expand Up @@ -569,18 +569,20 @@ TEST_F(ManyGroups, DisableFirst) {
TEST_F(ManyGroups, SameSubcommand) {
// only 1 group can be used if remove_required not used
remove_required();
auto sub1 = g1->add_subcommand("sub1");
auto sub2 = g2->add_subcommand("sub1");
auto sub1 = g1->add_subcommand("sub1")->disabled();
auto sub2 = g2->add_subcommand("sub1")->disabled();
auto sub3 = g3->add_subcommand("sub1");

// so when the subcommands are disabled they can have the same name
sub1->disabled(false);
sub2->disabled(false);
// if they are reenabled they are not checked for overlap on enabling so they can have the same name
args = {"sub1", "sub1", "sub1"};

run();

EXPECT_TRUE(*sub1);
EXPECT_TRUE(*sub2);
EXPECT_TRUE(*sub3);
/// This should be made to work at some point
auto subs = app.get_subcommands();
EXPECT_EQ(subs.size(), 3u);
EXPECT_EQ(subs[0], sub1);
Expand Down
4 changes: 4 additions & 0 deletions tests/SubcommandTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1661,6 +1661,10 @@ TEST_F(TApp, ExistingSubcommandMatch) {
} catch(const CLI::OptionAlreadyAdded &oaa) {
EXPECT_THAT(oaa.what(), HasSubstr("sub2"));
}
// now check that disabled subcommands can be added regardless of name
sshared->name("sub1");
sshared->disabled();
EXPECT_NO_THROW(app.add_subcommand(sshared));
}

TEST_F(TApp, AliasErrorsInOptionGroup) {
Expand Down

0 comments on commit a460029

Please sign in to comment.