Comparison of AI vs Developer solution to issue #1315

[phlptp]

Comparison of AI to developer solution

This is in regard to issue #1315

AI generated solution

include/CLI/App.hpp          |  7 +++++++
include/CLI/impl/App_inl.hpp | 46 ++++++++++++++++++++++++++++++++------------
tests/SubcommandTest.cpp     | 29 ++++++++++++++++++++++++++++
3 files changed, 70 insertions(+), 12 deletions(-)

diff --git a/include/CLI/App.hpp b/include/CLI/App.hpp
index 73e348c..e714e6f 100644
--- a/include/CLI/App.hpp
+++ b/include/CLI/App.hpp
@@ -1438,6 +1438,13 @@ class App {
    /// Get the appropriate parent to fallthrough to which is the first one that has a name or the main app
    CLI11_NODISCARD const App *_get_fallthrough_parent() const noexcept;

+    std::vector<const Option *> _get_options(const std::function<bool(const Option *)> &filter,
+                                             const App *exclude_subcommand) const;
+    std::vector<Option *> _get_options(const std::function<bool(Option *)> &filter, const App *exclude_subcommand);
+    CLI11_NODISCARD Option *_get_option_no_throw(std::string option_name, const App *exclude_subcommand) noexcept;
+    CLI11_NODISCARD const Option *_get_option_no_throw(std::string option_name,
+                                                       const App *exclude_subcommand) const noexcept;
+
    /// Helper function to run through all possible comparisons of subcommand names to check there is no overlap
    CLI11_NODISCARD const std::string &_compare_subcommand_names(const App &subcom, const App &base) const;

diff --git a/include/CLI/impl/App_inl.hpp b/include/CLI/impl/App_inl.hpp
index 94c6c3e..af7e416 100644
--- a/include/CLI/impl/App_inl.hpp
+++ b/include/CLI/impl/App_inl.hpp
@@ -830,6 +830,11 @@ CLI11_NODISCARD CLI11_INLINE std::string App::version() const {
}

CLI11_INLINE std::vector<const Option *> App::get_options(const std::function<bool(const Option *)> filter) const {
+    return _get_options(filter, nullptr);
+}
+
+CLI11_INLINE std::vector<const Option *>
+App::_get_options(const std::function<bool(const Option *)> &filter, const App *exclude_subcommand) const {
    std::vector<const Option *> options(options_.size());
    std::transform(
        std::begin(options_), std::end(options_), std::begin(options), [](const Option_p &val) { return val.get(); });
@@ -843,14 +848,15 @@ CLI11_INLINE std::vector<const Option *> App::get_options(const std::function<bo
    for(const auto &subcp : subcommands_) {
        // also check down into nameless subcommands
        const App *subc = subcp.get();
-        if(subc->get_name().empty() && !subc->get_group().empty() && subc->get_group().front() == '+') {
-            std::vector<const Option *> subcopts = subc->get_options(filter);
+        if(subc != exclude_subcommand && subc->get_name().empty() && !subc->get_group().empty() &&
+           subc->get_group().front() == '+') {
+            std::vector<const Option *> subcopts = subc->_get_options(filter, nullptr);
            options.insert(options.end(), subcopts.begin(), subcopts.end());
        }
    }
    if(fallthrough_ && parent_ != nullptr) {
        const auto *fallthrough_parent = _get_fallthrough_parent();
-        std::vector<const Option *> subcopts = fallthrough_parent->get_options(filter);
+        std::vector<const Option *> subcopts = fallthrough_parent->_get_options(filter, this);
        for(const auto *opt : subcopts) {
            if(std::find_if(options.begin(), options.end(), [opt](const Option *opt2) {
                   return opt->check_name(opt2->get_name());
@@ -863,6 +869,11 @@ CLI11_INLINE std::vector<const Option *> App::get_options(const std::function<bo
}

CLI11_INLINE std::vector<Option *> App::get_options(const std::function<bool(Option *)> filter) {
+    return _get_options(filter, nullptr);
+}
+
+CLI11_INLINE std::vector<Option *>
+App::_get_options(const std::function<bool(Option *)> &filter, const App *exclude_subcommand) {
    std::vector<Option *> options(options_.size());
    std::transform(
        std::begin(options_), std::end(options_), std::begin(options), [](const Option_p &val) { return val.get(); });
@@ -874,14 +885,15 @@ CLI11_INLINE std::vector<Option *> App::get_options(const std::function<bool(Opt
    }
    for(auto &subc : subcommands_) {
        // also check down into nameless subcommands and specific groups
-        if(subc->get_name().empty() || (!subc->get_group().empty() && subc->get_group().front() == '+')) {
-            auto subcopts = subc->get_options(filter);
+        if(subc.get() != exclude_subcommand &&
+           (subc->get_name().empty() || (!subc->get_group().empty() && subc->get_group().front() == '+'))) {
+            auto subcopts = subc->_get_options(filter, nullptr);
            options.insert(options.end(), subcopts.begin(), subcopts.end());
        }
    }
    if(fallthrough_ && parent_ != nullptr) {
        auto *fallthrough_parent = _get_fallthrough_parent();
-        std::vector<Option *> subcopts = fallthrough_parent->get_options(filter);
+        std::vector<Option *> subcopts = fallthrough_parent->_get_options(filter, this);
        for(auto *opt : subcopts) {
            if(std::find_if(options.begin(), options.end(), [opt](Option *opt2) {
                   return opt->check_name(opt2->get_name());
@@ -894,6 +906,11 @@ CLI11_INLINE std::vector<Option *> App::get_options(const std::function<bool(Opt
}

CLI11_NODISCARD CLI11_INLINE Option *App::get_option_no_throw(std::string option_name) noexcept {
+    return _get_option_no_throw(std::move(option_name), nullptr);
+}
+
+CLI11_NODISCARD CLI11_INLINE Option *App::_get_option_no_throw(std::string option_name,
+                                                               const App *exclude_subcommand) noexcept {
    for(Option_p &opt : options_) {
        if(opt->check_name(option_name)) {
            return opt.get();
@@ -901,20 +918,25 @@ CLI11_NODISCARD CLI11_INLINE Option *App::get_option_no_throw(std::string option
    }
    for(auto &subc : subcommands_) {
        // also check down into nameless subcommands
-        if(subc->get_name().empty()) {
-            auto *opt = subc->get_option_no_throw(option_name);
+        if(subc.get() != exclude_subcommand && subc->get_name().empty()) {
+            auto *opt = subc->_get_option_no_throw(option_name, nullptr);
            if(opt != nullptr) {
                return opt;
            }
        }
    }
    if(fallthrough_ && parent_ != nullptr) {
-        return _get_fallthrough_parent()->get_option_no_throw(option_name);
+        return _get_fallthrough_parent()->_get_option_no_throw(std::move(option_name), this);
    }
    return nullptr;
}

CLI11_NODISCARD CLI11_INLINE const Option *App::get_option_no_throw(std::string option_name) const noexcept {
+    return _get_option_no_throw(std::move(option_name), nullptr);
+}
+
+CLI11_NODISCARD CLI11_INLINE const Option *App::_get_option_no_throw(std::string option_name,
+                                                                     const App *exclude_subcommand) const noexcept {
    for(const Option_p &opt : options_) {
        if(opt->check_name(option_name)) {
            return opt.get();
@@ -922,15 +944,15 @@ CLI11_NODISCARD CLI11_INLINE const Option *App::get_option_no_throw(std::string
    }
    for(const auto &subc : subcommands_) {
        // also check down into nameless subcommands
-        if(subc->get_name().empty()) {
-            auto *opt = subc->get_option_no_throw(option_name);
+        if(subc.get() != exclude_subcommand && subc->get_name().empty()) {
+            auto *opt = subc->_get_option_no_throw(option_name, nullptr);
            if(opt != nullptr) {
                return opt;
            }
        }
    }
    if(fallthrough_ && parent_ != nullptr) {
-        return _get_fallthrough_parent()->get_option_no_throw(option_name);
+        return _get_fallthrough_parent()->_get_option_no_throw(std::move(option_name), this);
    }
    return nullptr;
}
diff --git a/tests/SubcommandTest.cpp b/tests/SubcommandTest.cpp
index 8f55fe3..93641e7 100644
--- a/tests/SubcommandTest.cpp
+++ b/tests/SubcommandTest.cpp
@@ -1726,6 +1726,35 @@ TEST_CASE_METHOD(TApp, "FallthroughFind2level", "[subcom]") {
    CHECK(opts.size() == 3);
}

+TEST_CASE_METHOD(TApp, "FallthroughOptionGroupInSubcommand", "[subcom]") {
+    bool flag{false};
+    std::string path;
+
+    app.add_flag("--flag,!--no-flag", flag)->default_val(false);
+    app.fallthrough();
+
+    auto *sub = app.add_subcommand("sub");
+    auto *group = sub->add_option_group("GROUP");
+    REQUIRE_NOTHROW(group->add_option("-p,--path", path)->check(CLI::ExistingFile));
+    sub->fallthrough();
+
+    auto *opt = sub->get_option("--path");
+    REQUIRE(opt != nullptr);
+
+    TempFile tempfile{"FallthroughOptionGroupInSubcommand.txt"};
+    {
+        std::ofstream out{tempfile.c_str()};
+        REQUIRE(out.good());
+        out << "test";
+    }
+
+    args = {"sub", "-p", static_cast<const std::string &>(tempfile), "--flag"};
+    run();
+
+    CHECK(flag);
+    CHECK(static_cast<const std::string &>(tempfile) == path);
+}
+
TEST_CASE_METHOD(TApp, "UnnamedSubMix", "[subcom]") {
    double val{0.0}, val2{0.0}, val3{0.0};
    app.add_option("-t", val2);

##Developer solution no AI

diff --git a/README.md b/README.md
index ca5a2d1c7..fe14e0a81 100644
--- a/README.md
+++ b/README.md
@@ -1077,11 +1077,13 @@ option_groups. These are:
   along with the parent for subcommands with fallthrough.
 - `.get_option_no_throw(name)`: Get an option pointer by option name. This
   function will return a `nullptr` instead of throwing if the option is not
-  available.
+  available. This method will not search parents of option_options or nameless
+  subcommands regardless of fallthrough status 🚧, this behavior is slightly
+  different from `get_option`.
 - `.get_options(filter)`: Get the list of all defined option pointers (useful
   for processing the app for custom output formats). If used on a subcommand
   will also get options that are in the parent app if the subcommand has
-  fallthrough.
+  fallthrough (and is not nameless 🚧).
 - `.parse_order()`: Get the list of option pointers in the order they were
   parsed (including duplicates).
 - `.formatter(std::shared_ptr<formatterBase> fmt)`: Set a custom formatter for
diff --git a/include/CLI/App.hpp b/include/CLI/App.hpp
index 73e348cc8..14a329231 100644
--- a/include/CLI/App.hpp
+++ b/include/CLI/App.hpp
@@ -776,6 +776,7 @@ class App {
             throw IncorrectConstruction("option group names may not contain newlines or null characters");
         }
         auto option_group = std::make_shared<T>(std::move(group_description), group_name, this);
+        option_group->fallthrough(false);
         auto *ptr = option_group.get();
         // move to App_p for overload resolution on older gcc versions
         App_p app_ptr = std::static_pointer_cast<App>(option_group);
@@ -1139,22 +1140,10 @@ class App {
     CLI11_NODISCARD const Option *get_option_no_throw(std::string option_name) const noexcept;
 
     /// Get an option by name
-    CLI11_NODISCARD const Option *get_option(std::string option_name) const {
-        const auto *opt = get_option_no_throw(option_name);
-        if(opt == nullptr) {
-            throw OptionNotFound(option_name);
-        }
-        return opt;
-    }
+    CLI11_NODISCARD const Option *get_option(std::string option_name) const;
 
     /// Get an option by name (non-const version)
-    CLI11_NODISCARD Option *get_option(std::string option_name) {
-        auto *opt = get_option_no_throw(option_name);
-        if(opt == nullptr) {
-            throw OptionNotFound(option_name);
-        }
-        return opt;
-    }
+    CLI11_NODISCARD Option *get_option(std::string option_name);
 
     /// Shortcut bracket operator for getting a pointer to an option
     const Option *operator[](const std::string &option_name) const { return get_option(option_name); }
diff --git a/include/CLI/impl/App_inl.hpp b/include/CLI/impl/App_inl.hpp
index 94c6c3edf..2fb33d82a 100644
--- a/include/CLI/impl/App_inl.hpp
+++ b/include/CLI/impl/App_inl.hpp
@@ -848,7 +848,7 @@ CLI11_INLINE std::vector<const Option *> App::get_options(const std::function<bo
             options.insert(options.end(), subcopts.begin(), subcopts.end());
         }
     }
-    if(fallthrough_ && parent_ != nullptr) {
+    if(fallthrough_ && parent_ != nullptr && !name_.empty()) {
         const auto *fallthrough_parent = _get_fallthrough_parent();
         std::vector<const Option *> subcopts = fallthrough_parent->get_options(filter);
         for(const auto *opt : subcopts) {
@@ -879,7 +879,7 @@ CLI11_INLINE std::vector<Option *> App::get_options(const std::function<bool(Opt
             options.insert(options.end(), subcopts.begin(), subcopts.end());
         }
     }
-    if(fallthrough_ && parent_ != nullptr) {
+    if(fallthrough_ && parent_ != nullptr && !name_.empty()) {
         auto *fallthrough_parent = _get_fallthrough_parent();
         std::vector<Option *> subcopts = fallthrough_parent->get_options(filter);
         for(auto *opt : subcopts) {
@@ -893,6 +893,36 @@ CLI11_INLINE std::vector<Option *> App::get_options(const std::function<bool(Opt
     return options;
 }
 
+/// Get an option by name
+CLI11_NODISCARD CLI11_INLINE const Option *App::get_option(std::string option_name) const {
+    const auto *opt = get_option_no_throw(option_name);
+    if(opt == nullptr) {
+        if(fallthrough_ && parent_ != nullptr && name_.empty()) {
+            // as a special case option groups with fallthrough enabled can also check the parent for options if the
+            // option is not found in the group this will not recurse as the internal call is to the no_throw version
+            // which will not check the parent again for option groups even with fallthrough enabled
+            return _get_fallthrough_parent()->get_option(option_name);
+        }
+        throw OptionNotFound(option_name);
+    }
+    return opt;
+}
+
+/// Get an option by name (non-const version)
+CLI11_NODISCARD CLI11_INLINE Option *App::get_option(std::string option_name) {
+    auto *opt = get_option_no_throw(option_name);
+    if(opt == nullptr) {
+        if(fallthrough_ && parent_ != nullptr && name_.empty()) {
+            // as a special case option groups with fallthrough enabled can also check the parent for options if the
+            // option is not found in the group this will not recurse as the internal call is to the no_throw version
+            // which will not check the parent again for option groups even with fallthrough enabled
+            return _get_fallthrough_parent()->get_option(option_name);
+        }
+        throw OptionNotFound(option_name);
+    }
+    return opt;
+}
+
 CLI11_NODISCARD CLI11_INLINE Option *App::get_option_no_throw(std::string option_name) noexcept {
     for(Option_p &opt : options_) {
         if(opt->check_name(option_name)) {
@@ -908,7 +938,9 @@ CLI11_NODISCARD CLI11_INLINE Option *App::get_option_no_throw(std::string option
             }
         }
     }
-    if(fallthrough_ && parent_ != nullptr) {
+    if(fallthrough_ && parent_ != nullptr && !name_.empty()) {
+        // if there is fallthrough and a parent and this is not an option_group then also check the parent for the
+        // option
         return _get_fallthrough_parent()->get_option_no_throw(option_name);
     }
     return nullptr;
@@ -929,7 +961,7 @@ CLI11_NODISCARD CLI11_INLINE const Option *App::get_option_no_throw(std::string
             }
         }
     }
-    if(fallthrough_ && parent_ != nullptr) {
+    if(fallthrough_ && parent_ != nullptr && !name_.empty()) {
         return _get_fallthrough_parent()->get_option_no_throw(option_name);
     }
     return nullptr;
diff --git a/tests/OptionGroupTest.cpp b/tests/OptionGroupTest.cpp
index 9a1ec355b..596333711 100644
--- a/tests/OptionGroupTest.cpp
+++ b/tests/OptionGroupTest.cpp
@@ -843,7 +843,27 @@ TEST_CASE_METHOD(ManyGroups, "OptionFind", "[optiongroup]") {
     g1->fallthrough();
     auto *opt_name = g1->get_option("--base");
     CHECK(opt_name == opt_main);
+    CHECK_THROWS_AS(g1->get_option("--notfound"), CLI::OptionNotFound);
     auto const *g1_const = g1;
     const auto *opt_name_const = g1_const->get_option("--base");
     CHECK(opt_name_const == opt_main);
+    CHECK_THROWS_AS(g1_const->get_option("--notfound"), CLI::OptionNotFound);
+}
+
+// from https://github.com/CLIUtils/CLI11/issues/1315
+TEST_CASE_METHOD(TApp, "SubcommandOptionGroupWithFallthrough", "[optiongroup]") {
+    // code from https://github.com/The0Dev
+    bool flag{false};
+    std::string str;
+    app.add_flag("--flag,!--no-flag", flag, "Enable a flag");
+
+    // Disabling this prevents the issue:
+    app.fallthrough(true);
+
+    auto *sub = app.add_subcommand("sub", "Execute a subcommand");
+
+    auto *group = sub->add_option_group("GROUP");
+
+    // Fails:
+    group->add_option("-p,--path", str, "An option");
 }