From ca668272632b834ca5b28451d61860651d3bbabe Mon Sep 17 00:00:00 2001 From: Philip Top Date: Wed, 9 Oct 2024 04:46:45 -0700 Subject: [PATCH] Subcommand fallthrough (#1073) Add modifier for subcommands to restrict subcommands falling through to parent. This will resolve #1022 --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> --- .github/actions/quick_cmake/action.yml | 2 +- .github/workflows/tests.yml | 6 +++--- README.md | 11 +++++++--- book/chapters/subcommands.md | 16 +++++++++++++- include/CLI/App.hpp | 17 +++++++++++++-- include/CLI/impl/App_inl.hpp | 29 +++++++++++++++----------- tests/CMakeLists.txt | 2 +- tests/SubcommandTest.cpp | 15 ++++++++++++- 8 files changed, 74 insertions(+), 24 deletions(-) diff --git a/.github/actions/quick_cmake/action.yml b/.github/actions/quick_cmake/action.yml index d2b3825f..9889e24d 100644 --- a/.github/actions/quick_cmake/action.yml +++ b/.github/actions/quick_cmake/action.yml @@ -13,7 +13,7 @@ runs: using: composite steps: - name: CMake ${{ inputs.cmake-version }} - uses: jwlawson/actions-setup-cmake@v1.14 + uses: jwlawson/actions-setup-cmake@v2.0.2 with: cmake-version: "${{ inputs.cmake-version }}" - run: | diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 86e61e73..98b96355 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -292,10 +292,10 @@ jobs: cmake-version: "3.13" if: success() || failure() - - name: Check CMake 3.14 + - name: Check CMake 3.14.7 uses: ./.github/actions/quick_cmake with: - cmake-version: "3.14" + cmake-version: "3.14.7" args: -DCLI11_SANITIZERS=ON -DCLI11_BUILD_EXAMPLES_JSON=ON if: success() || failure() @@ -387,6 +387,6 @@ jobs: - name: Check CMake 3.28 (full) uses: ./.github/actions/quick_cmake with: - cmake-version: "3.28" + cmake-version: "3.28.X" args: -DCLI11_SANITIZERS=ON -DCLI11_BUILD_EXAMPLES_JSON=ON if: success() || failure() diff --git a/README.md b/README.md index 4a84885f..c3ae26fb 100644 --- a/README.md +++ b/README.md @@ -905,10 +905,15 @@ option_groups. These are: options are specified in the `add_option` calls or the ability to process options in the form of `-s --long --file=file_name.ext`. - `.fallthrough()`: Allow extra unmatched options and positionals to "fall - through" and be matched on a parent option. Subcommands always are allowed to - "fall through" as in they will first attempt to match on the current + through" and be matched on a parent option. Subcommands by default are allowed + to "fall through" as in they will first attempt to match on the current subcommand and if they fail will progressively check parents for matching - subcommands. + subcommands. This can be disabled through `subcommand_fallthrough(false)` 🚧. +- `.subcommand_fallthrough()`: 🚧 Allow subcommands to "fall through" and be + matched on a parent option. Disabling this prevents additional subcommands at + the same level from being matched. It can be useful in certain circumstances + where there might be ambiguity between subcommands and positionals. The + default is true. - `.configurable()`: Allow the subcommand to be triggered from a configuration file. By default subcommand options in a configuration file do not trigger a subcommand but will just update default values. diff --git a/book/chapters/subcommands.md b/book/chapters/subcommands.md index 7a0c41c7..0f621b86 100644 --- a/book/chapters/subcommands.md +++ b/book/chapters/subcommands.md @@ -132,7 +132,21 @@ gitbook:code $ ./my_program my_model_1 --model_flag --shared_flag ``` Here, `--shared_flag` was set on the main app, and on the command line it "falls -through" `my_model_1` to match on the main app. +through" `my_model_1` to match on the main app. This is set through +`->fallthrough()` on a subcommand. + +#### Subcommand fallthrough + +Subcommand fallthrough allows additional subcommands to be triggered after the +first subcommand. By default subcommand fallthrough is enabled, but it can be +turned off through `->subcommand_fallthrough(false)` on a subcommand. This will +prevent additional subcommands at the same inheritance level from triggering, +the strings would then be treated as positional values. As a technical note if +fallthrough is enabled but subcommand fallthrough disabled (this is not the +default in both cases), then subcommands on grandparents can still be triggered +from the grandchild subcommand, unless subcommand fallthrough is also disabled +on the parent. This is an unusual circumstance but may arise in some very +particular situations. ### Prefix command diff --git a/include/CLI/App.hpp b/include/CLI/App.hpp index f98463f9..dd8b3722 100644 --- a/include/CLI/App.hpp +++ b/include/CLI/App.hpp @@ -224,9 +224,13 @@ class App { /// If true, the program should ignore underscores INHERITABLE bool ignore_underscore_{false}; - /// Allow subcommand fallthrough, so that parent commands can collect commands after subcommand. INHERITABLE + /// Allow options or other arguments to fallthrough, so that parent commands can collect options after subcommand. + /// INHERITABLE bool fallthrough_{false}; + /// Allow subcommands to fallthrough, so that parent commands can trigger other subcommands after subcommand. + bool subcommand_fallthrough_{true}; + /// Allow '/' for options for Windows like options. Defaults to true on Windows, false otherwise. INHERITABLE bool allow_windows_style_options_{ #ifdef _WIN32 @@ -828,13 +832,19 @@ class App { return this; } - /// Stop subcommand fallthrough, so that parent commands cannot collect commands after subcommand. + /// Set fallthrough, set to true so that options will fallthrough to parent if not recognized in a subcommand /// Default from parent, usually set on parent. App *fallthrough(bool value = true) { fallthrough_ = value; return this; } + /// Set subcommand fallthrough, set to true so that subcommands on parents are recognized + App *subcommand_fallthrough(bool value = true) { + subcommand_fallthrough_ = value; + return this; + } + /// Check to see if this subcommand was parsed, true only if received on command line. /// This allows the subcommand to be directly checked. explicit operator bool() const { return parsed_ > 0; } @@ -1084,6 +1094,9 @@ class App { /// Check the status of fallthrough CLI11_NODISCARD bool get_fallthrough() const { return fallthrough_; } + /// Check the status of subcommand fallthrough + CLI11_NODISCARD bool get_subcommand_fallthrough() const { return subcommand_fallthrough_; } + /// Check the status of the allow windows style options CLI11_NODISCARD bool get_allow_windows_style_options() const { return allow_windows_style_options_; } diff --git a/include/CLI/impl/App_inl.hpp b/include/CLI/impl/App_inl.hpp index b66bab7c..f4325dce 100644 --- a/include/CLI/impl/App_inl.hpp +++ b/include/CLI/impl/App_inl.hpp @@ -1038,7 +1038,8 @@ CLI11_INLINE void App::run_callback(bool final_mode, bool suppress_final_callbac CLI11_NODISCARD CLI11_INLINE bool App::_valid_subcommand(const std::string ¤t, bool ignore_used) const { // Don't match if max has been reached - but still check parents - if(require_subcommand_max_ != 0 && parsed_subcommands_.size() >= require_subcommand_max_) { + if(require_subcommand_max_ != 0 && parsed_subcommands_.size() >= require_subcommand_max_ && + subcommand_fallthrough_) { return parent_ != nullptr && parent_->_valid_subcommand(current, ignore_used); } auto *com = _find_subcommand(current, true, ignore_used); @@ -1046,7 +1047,10 @@ CLI11_NODISCARD CLI11_INLINE bool App::_valid_subcommand(const std::string &curr return true; } // Check parent if exists, else return false - return parent_ != nullptr && parent_->_valid_subcommand(current, ignore_used); + if(subcommand_fallthrough_) { + return parent_ != nullptr && parent_->_valid_subcommand(current, ignore_used); + } + return false; } CLI11_NODISCARD CLI11_INLINE detail::Classifier App::_recognize(const std::string ¤t, @@ -1743,9 +1747,9 @@ CLI11_INLINE bool App::_parse_positional(std::vector &args, bool ha } } // let the parent deal with it if possible - if(parent_ != nullptr && fallthrough_) + if(parent_ != nullptr && fallthrough_) { return _get_fallthrough_parent()->_parse_positional(args, static_cast(parse_complete_callback_)); - + } /// Try to find a local subcommand that is repeated auto *com = _find_subcommand(args.back(), true, false); if(com != nullptr && (require_subcommand_max_ == 0 || require_subcommand_max_ > parsed_subcommands_.size())) { @@ -1756,15 +1760,16 @@ CLI11_INLINE bool App::_parse_positional(std::vector &args, bool ha com->_parse(args); return true; } - /// now try one last gasp at subcommands that have been executed before, go to root app and try to find a - /// subcommand in a broader way, if one exists let the parent deal with it - auto *parent_app = (parent_ != nullptr) ? _get_fallthrough_parent() : this; - com = parent_app->_find_subcommand(args.back(), true, false); - if(com != nullptr && (com->parent_->require_subcommand_max_ == 0 || - com->parent_->require_subcommand_max_ > com->parent_->parsed_subcommands_.size())) { - return false; + if(subcommand_fallthrough_) { + /// now try one last gasp at subcommands that have been executed before, go to root app and try to find a + /// subcommand in a broader way, if one exists let the parent deal with it + auto *parent_app = (parent_ != nullptr) ? _get_fallthrough_parent() : this; + com = parent_app->_find_subcommand(args.back(), true, false); + if(com != nullptr && (com->parent_->require_subcommand_max_ == 0 || + com->parent_->require_subcommand_max_ > com->parent_->parsed_subcommands_.size())) { + return false; + } } - if(positionals_at_end_) { throw CLI::ExtrasError(name_, args); } diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 6724fca0..53404bc3 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -4,7 +4,7 @@ if(CLI11_SANITIZERS AND ${CMAKE_VERSION} VERSION_GREATER "3.13.0") sanitizers GIT_REPOSITORY https://github.com/arsenm/sanitizers-cmake.git GIT_SHALLOW 1 - GIT_TAG 3f0542e) + GIT_TAG 0573e2e) FetchContent_GetProperties(sanitizers) diff --git a/tests/SubcommandTest.cpp b/tests/SubcommandTest.cpp index b7a8bb97..dc98a774 100644 --- a/tests/SubcommandTest.cpp +++ b/tests/SubcommandTest.cpp @@ -719,12 +719,25 @@ TEST_CASE_METHOD(TApp, "Required1SubCom", "[subcom]") { CHECK_THROWS_AS(run(), CLI::RequiredError); args = {"sub1"}; - run(); + CHECK_NOTHROW(run()); args = {"sub1", "sub2"}; CHECK_THROWS_AS(run(), CLI::ExtrasError); } +TEST_CASE_METHOD(TApp, "subcomNoSubComfallthrough", "[subcom]") { + auto *sub1 = app.add_subcommand("sub1"); + std::vector pos; + sub1->add_option("args", pos); + app.add_subcommand("sub2"); + app.add_subcommand("sub3"); + sub1->subcommand_fallthrough(false); + CHECK_FALSE(sub1->get_subcommand_fallthrough()); + args = {"sub1", "sub2", "sub3"}; + run(); + CHECK(pos.size() == 2); +} + TEST_CASE_METHOD(TApp, "BadSubcommandSearch", "[subcom]") { auto *one = app.add_subcommand("one");