Option callback priority v2 (#1226)

Extension allowing all possible priority combinations.
Add a field callback_priority to OptionBase.

---------

Co-authored-by: Philip Top <phlptp@gmail.com>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

README.md

@@ -211,7 +211,10 @@ int main(int argc, char** argv) {
 }
 ```
 
-For more information about `ensure_utf8` the section on
+When adding options the names should not conflict with each other, if an option
+is added, or a modifier changed that would cause naming conflicts a run time
+error will be thrown in the add_option method. This includes default options for
+help `-h, --help`. For more information about `ensure_utf8` the section on
 [Unicode support](#unicode-support) below.
 
 <details><summary>Note: If you don't like macros, this is what that macro expands to: (click to expand)</summary><p>
@@ -500,6 +503,34 @@ Before parsing, you can set the following options:
   validation checks for the option to be executed when the option value is
   parsed vs. at the end of all parsing. This could cause the callback to be
   executed multiple times. Also works with positional options.
+- `->callback_priority(CallbackPriority priority)`: 🚧 changes the order in
+  which the option callback is executed. Four principal callback call-points
+  are available. `CallbackPriority::First` executes at the very beginning of
+  processing, before configuration files are read and environment variables are
+  interpreted. `CallbackPriority::PreRequirementsCheck` executes after
+  configuration and environment processing but before requirements checking.
+  `CallbackPriority::Normal` executes after the requirements check but before
+  any previously potentially raised exceptions are re-thrown.
+  `CallbackPriority::Last` executes after exception handling is completed.
+  For each position, both ordinary option callbacks and help callbacks are
+  invoked. The relative order between them can be controlled using the
+  corresponding `PreHelp` variants. `CallbackPriority::FirstPreHelp` executes
+  ordinary option callbacks before help callbacks at the very beginning of
+  processing. `CallbackPriority::PreRequirementsCheckPreHelp` executes ordinary
+  option callbacks before help callbacks after configuration and environment
+  processing but before requirements checking. `CallbackPriority::NormalPreHelp`
+  executes ordinary option callbacks before help callbacks after the
+  requirements check but before exception re-throwing.
+  `CallbackPriority::LastPreHelp` executes ordinary option callbacks before help
+  callbacks after exception handling has completed. When using the standard
+  priorities (`CallbackPriority::First`,
+  `CallbackPriority::PreRequirementsCheck`, `CallbackPriority::Normal`,
+  `CallbackPriority::Last`), help callbacks are executed before ordinary option
+  callbacks. By default, help callbacks use `CallbackPriority::First`, and
+  ordinary option callbacks use `CallbackPriority::Normal`. This mechanism
+  provides fine-grained control over when option values are set and when help or
+  requirement checks occur, enabling precise customization of the processing
+  sequence.
 
 These options return the `Option` pointer, so you can chain them together, and
 even skip storing the pointer entirely. The `each` function takes any function
@@ -639,7 +670,7 @@ setting `CLI11_ENABLE_EXTRA_VALIDATORS` to 1
   write permission. Requires C++17.
 - `CLI::ExecPermission`: Requires that the file given exist and have execution
   permission. Requires C++17.
--
+  -
 
 #### Validator Usage
 

include/CLI/App.hpp

@@ -1303,12 +1303,12 @@ class App {
     void _process_env();
 
     /// Process callbacks. Runs on *all* subcommands.
-    void _process_callbacks();
+    void _process_callbacks(CallbackPriority priority);
 
     /// Run help flag processing if any are found.
     ///
     /// The flags allow recursive calls to remember if there was a help flag on a parent.
-    void _process_help_flags(bool trigger_help = false, bool trigger_all_help = false) const;
+    void _process_help_flags(CallbackPriority priority, bool trigger_help = false, bool trigger_all_help = false) const;
 
     /// Verify required options and cross requirements. Subcommands too (only if selected).
     void _process_requirements();

include/CLI/Option.hpp

@@ -50,6 +50,18 @@ enum class MultiOptionPolicy : char {
     Reverse,    //!< take only the last Expected number of arguments in reverse order
 };
 
+/// @brief  enumeration for the callback priority
+enum class CallbackPriority : std::uint8_t {
+    FirstPreHelp = 0,
+    First = 1,
+    PreRequirementsCheckPreHelp = 2,
+    PreRequirementsCheck = 3,
+    NormalPreHelp = 4,
+    Normal = 5,
+    LastPreHelp = 6,
+    Last = 7
+};  // namespace CLI
+
 /// This is the CRTP base class for Option and OptionDefaults. It was designed this way
 /// to share parts of the class; an OptionDefaults can copy to an Option.
 template <typename CRTP> class OptionBase {
@@ -84,6 +96,9 @@ template <typename CRTP> class OptionBase {
     /// Policy for handling multiple arguments beyond the expected Max
     MultiOptionPolicy multi_option_policy_{MultiOptionPolicy::Throw};
 
+    /// Priority of callback
+    CallbackPriority callback_priority_{CallbackPriority::Normal};
+
     /// Copy the contents to another similar class (one based on OptionBase)
     template <typename T> void copy_to(T *other) const;
 
@@ -142,6 +157,9 @@ template <typename CRTP> class OptionBase {
     /// The status of the multi option policy
     CLI11_NODISCARD MultiOptionPolicy get_multi_option_policy() const { return multi_option_policy_; }
 
+    /// The priority of callback
+    CLI11_NODISCARD CallbackPriority get_callback_priority() const { return callback_priority_; }
+
     // Shortcuts for multi option policy
 
     /// Set the multi option policy to take last
@@ -201,6 +219,12 @@ class OptionDefaults : public OptionBase<OptionDefaults> {
 
     // Methods here need a different implementation if they are Option vs. OptionDefault
 
+    /// Set the callback priority
+    OptionDefaults *callback_priority(CallbackPriority value = CallbackPriority::Normal) {
+        callback_priority_ = value;
+        return this;
+    }
+
     /// Take the last argument if given multiple times
     OptionDefaults *multi_option_policy(MultiOptionPolicy value = MultiOptionPolicy::Throw) {
         multi_option_policy_ = value;
@@ -343,7 +367,6 @@ class Option : public OptionBase<Option> {
     bool trigger_on_result_{false};
     /// flag indicating that the option should force the callback regardless if any results present
     bool force_callback_{false};
-    ///@}
 
     /// Making an option by hand is not defined, it must be made by the App class
     Option(std::string option_name,
@@ -420,6 +443,13 @@ class Option : public OptionBase<Option> {
     /// Get the current value of run_callback_for_default
     CLI11_NODISCARD bool get_run_callback_for_default() const { return run_callback_for_default_; }
 
+    /// Set the value of callback priority which controls when the callback function should be called relative to other
+    /// parsing operations the default This is controlled automatically but could be manipulated by the user.
+    Option *callback_priority(CallbackPriority value = CallbackPriority::Normal) {
+        callback_priority_ = value;
+        return this;
+    }
+
     /// Adds a shared validator
     Option *check(Validator_p validator);
 

include/CLI/impl/App_inl.hpp

@@ -285,7 +285,7 @@ CLI11_INLINE Option *App::set_help_flag(std::string flag_name, const std::string
     // Empty name will simply remove the help flag
     if(!flag_name.empty()) {
         help_ptr_ = add_flag(flag_name, help_description);
-        help_ptr_->configurable(false);
+        help_ptr_->configurable(false)->callback_priority(CallbackPriority::First);
     }
 
     return help_ptr_;
@@ -301,7 +301,7 @@ CLI11_INLINE Option *App::set_help_all_flag(std::string help_name, const std::st
     // Empty name will simply remove the help all flag
     if(!help_name.empty()) {
         help_all_ptr_ = add_flag(help_name, help_description);
-        help_all_ptr_->configurable(false);
+        help_all_ptr_->configurable(false)->callback_priority(CallbackPriority::First);
     }
 
     return help_all_ptr_;
@@ -319,7 +319,7 @@ App::set_version_flag(std::string flag_name, const std::string &versionString, c
     if(!flag_name.empty()) {
         version_ptr_ = add_flag_callback(
             flag_name, [versionString]() { throw(CLI::CallForVersion(versionString, 0)); }, version_help);
-        version_ptr_->configurable(false);
+        version_ptr_->configurable(false)->callback_priority(CallbackPriority::First);
     }
 
     return version_ptr_;
@@ -336,7 +336,7 @@ App::set_version_flag(std::string flag_name, std::function<std::string()> vfunc,
     if(!flag_name.empty()) {
         version_ptr_ =
             add_flag_callback(flag_name, [vfunc]() { throw(CLI::CallForVersion(vfunc(), 0)); }, version_help);
-        version_ptr_->configurable(false);
+        version_ptr_->configurable(false)->callback_priority(CallbackPriority::First);
     }
 
     return version_ptr_;
@@ -1239,43 +1239,48 @@ CLI11_INLINE void App::_process_env() {
     }
 }
 
-CLI11_INLINE void App::_process_callbacks() {
+CLI11_INLINE void App::_process_callbacks(CallbackPriority priority) {
 
     for(App_p &sub : subcommands_) {
         // process the priority option_groups first
         if(sub->get_name().empty() && sub->parse_complete_callback_) {
             if(sub->count_all() > 0) {
-                sub->_process_callbacks();
-                sub->run_callback();
+                sub->_process_callbacks(priority);
+                if(priority == CallbackPriority::Normal) {
+                    // only run the subcommand callback at normal priority
+                    sub->run_callback();
+                }
             }
         }
     }
 
     for(const Option_p &opt : options_) {
-        if((*opt) && !opt->get_callback_run()) {
-            opt->run_callback();
+        if(opt->get_callback_priority() == priority) {
+            if((*opt) && !opt->get_callback_run()) {
+                opt->run_callback();
+            }
         }
     }
     for(App_p &sub : subcommands_) {
         if(!sub->parse_complete_callback_) {
-            sub->_process_callbacks();
+            sub->_process_callbacks(priority);
         }
     }
 }
 
-CLI11_INLINE void App::_process_help_flags(bool trigger_help, bool trigger_all_help) const {
+CLI11_INLINE void App::_process_help_flags(CallbackPriority priority, bool trigger_help, bool trigger_all_help) const {
     const Option *help_ptr = get_help_ptr();
     const Option *help_all_ptr = get_help_all_ptr();
 
-    if(help_ptr != nullptr && help_ptr->count() > 0)
+    if(help_ptr != nullptr && help_ptr->count() > 0 && help_ptr->get_callback_priority() == priority)
         trigger_help = true;
-    if(help_all_ptr != nullptr && help_all_ptr->count() > 0)
+    if(help_all_ptr != nullptr && help_all_ptr->count() > 0 && help_all_ptr->get_callback_priority() == priority)
         trigger_all_help = true;
 
     // If there were parsed subcommands, call those. First subcommand wins if there are multiple ones.
     if(!parsed_subcommands_.empty()) {
         for(const App *sub : parsed_subcommands_)
-            sub->_process_help_flags(trigger_help, trigger_all_help);
+            sub->_process_help_flags(priority, trigger_help, trigger_all_help);
 
         // Only the final subcommand should call for help. All help wins over help.
     } else if(trigger_all_help) {
@@ -1416,7 +1421,10 @@ CLI11_INLINE void App::_process_requirements() {
 CLI11_INLINE void App::_process() {
     // help takes precedence over other potential errors and config and environment shouldn't be processed if help
     // throws
-    _process_help_flags();
+    _process_callbacks(CallbackPriority::FirstPreHelp);
+    _process_help_flags(CallbackPriority::First);
+    _process_callbacks(CallbackPriority::First);
+
     std::exception_ptr config_exception;
     try {
         // the config file might generate a FileError but that should not be processed until later in the process
@@ -1430,13 +1438,23 @@ CLI11_INLINE void App::_process() {
     }
     // callbacks and requirements processing can generate exceptions which should take priority
     // over the config file error if one exists.
+    _process_callbacks(CallbackPriority::PreRequirementsCheckPreHelp);
+    _process_help_flags(CallbackPriority::PreRequirementsCheck);
+    _process_callbacks(CallbackPriority::PreRequirementsCheck);
+
     _process_requirements();
 
-    _process_callbacks();
+    _process_callbacks(CallbackPriority::NormalPreHelp);
+    _process_help_flags(CallbackPriority::Normal);
+    _process_callbacks(CallbackPriority::Normal);
 
     if(config_exception) {
         std::rethrow_exception(config_exception);
     }
+
+    _process_callbacks(CallbackPriority::LastPreHelp);
+    _process_help_flags(CallbackPriority::Last);
+    _process_callbacks(CallbackPriority::Last);
 }
 
 CLI11_INLINE void App::_process_extras() {
@@ -1496,10 +1514,20 @@ CLI11_INLINE void App::_parse(std::vector<std::string> &args) {
         // Convert missing (pairs) to extras (string only) ready for processing in another app
         args = remaining_for_passthrough(false);
     } else if(parse_complete_callback_) {
+        _process_callbacks(CallbackPriority::FirstPreHelp);
+        _process_help_flags(CallbackPriority::First);
+        _process_callbacks(CallbackPriority::First);
         _process_env();
-        _process_callbacks();
-        _process_help_flags();
+        _process_callbacks(CallbackPriority::PreRequirementsCheckPreHelp);
+        _process_help_flags(CallbackPriority::PreRequirementsCheck);
+        _process_callbacks(CallbackPriority::PreRequirementsCheck);
         _process_requirements();
+        _process_callbacks(CallbackPriority::NormalPreHelp);
+        _process_help_flags(CallbackPriority::Normal);
+        _process_callbacks(CallbackPriority::Normal);
+        _process_callbacks(CallbackPriority::LastPreHelp);
+        _process_help_flags(CallbackPriority::Last);
+        _process_callbacks(CallbackPriority::Last);
         run_callback(false, true);
     }
 }
@@ -1620,8 +1648,15 @@ CLI11_INLINE bool App::_parse_single_config(const ConfigItem &item, std::size_t
     // check for section close
     if(item.name == "--") {
         if(configurable_ && parse_complete_callback_) {
-            _process_callbacks();
+            _process_callbacks(CallbackPriority::FirstPreHelp);
+            _process_callbacks(CallbackPriority::First);
+            _process_callbacks(CallbackPriority::PreRequirementsCheckPreHelp);
+            _process_callbacks(CallbackPriority::PreRequirementsCheck);
             _process_requirements();
+            _process_callbacks(CallbackPriority::NormalPreHelp);
+            _process_callbacks(CallbackPriority::Normal);
+            _process_callbacks(CallbackPriority::LastPreHelp);
+            _process_callbacks(CallbackPriority::Last);
             run_callback();
         }
         return true;
@@ -2081,10 +2116,20 @@ App::_parse_arg(std::vector<std::string> &args, detail::Classifier current_type,
                     _trigger_pre_parse(args.size());
                     // run the parse complete callback since the subcommand processing is now complete
                     if(sub->parse_complete_callback_) {
+                        sub->_process_callbacks(CallbackPriority::FirstPreHelp);
+                        sub->_process_help_flags(CallbackPriority::First);
+                        sub->_process_callbacks(CallbackPriority::First);
                         sub->_process_env();
-                        sub->_process_callbacks();
-                        sub->_process_help_flags();
+                        sub->_process_callbacks(CallbackPriority::PreRequirementsCheckPreHelp);
+                        sub->_process_help_flags(CallbackPriority::PreRequirementsCheck);
+                        sub->_process_callbacks(CallbackPriority::PreRequirementsCheck);
                         sub->_process_requirements();
+                        sub->_process_callbacks(CallbackPriority::NormalPreHelp);
+                        sub->_process_help_flags(CallbackPriority::Normal);
+                        sub->_process_callbacks(CallbackPriority::Normal);
+                        sub->_process_callbacks(CallbackPriority::LastPreHelp);
+                        sub->_process_help_flags(CallbackPriority::Last);
+                        sub->_process_callbacks(CallbackPriority::Last);
                         sub->run_callback(false, true);
                     }
                     return true;

include/CLI/impl/Option_inl.hpp

@@ -32,6 +32,7 @@ template <typename CRTP> template <typename T> void OptionBase<CRTP>::copy_to(T
     other->delimiter(delimiter_);
     other->always_capture_default(always_capture_default_);
     other->multi_option_policy(multi_option_policy_);
+    other->callback_priority(callback_priority_);
 }
 
 CLI11_INLINE Option *Option::expected(int value) {

tests/AppTest.cpp

@@ -1025,6 +1025,51 @@ TEST_CASE_METHOD(TApp, "TakeFirstOptMulti", "[app]") {
     CHECK(std::vector<int>({1, 2}) == vals);
 }
 
+TEST_CASE_METHOD(TApp, "optionPriority", "[app]") {
+    std::vector<int> results;
+    auto *opt1 = app.add_flag_callback("-A", [&]() { results.push_back(1); });
+    auto *opt2 = app.add_flag_callback("-B", [&]() { results.push_back(2); });
+    auto *opt3 = app.add_flag_callback("-C", [&]() { results.push_back(3); });
+    auto *opt4 = app.add_flag_callback("-D", [&]() { results.push_back(4); });
+    auto *opt5 = app.add_flag_callback("-E", [&]() { results.push_back(5); });
+    args = {"-A", "-B", "-C", "-D", "-E"};
+    run();
+    CHECK(std::vector<int>({1, 2, 3, 4, 5}) == results);
+    results.clear();
+    opt2->callback_priority(CLI::CallbackPriority::FirstPreHelp);
+    run();
+    CHECK(std::vector<int>({2, 1, 3, 4, 5}) == results);
+    results.clear();
+    opt4->callback_priority(CLI::CallbackPriority::Last);
+    run();
+    CHECK(std::vector<int>({2, 1, 3, 5, 4}) == results);
+    results.clear();
+    opt5->callback_priority(CLI::CallbackPriority::PreRequirementsCheck);
+    run();
+    CHECK(std::vector<int>({2, 5, 1, 3, 4}) == results);
+    results.clear();
+
+    args = {"-A", "-B", "-C", "-D", "-E", "--help"};
+    CHECK_THROWS(run());
+    CHECK(std::vector<int>({2}) == results);
+    results.clear();
+
+    app.get_help_ptr()->callback_priority(CLI::CallbackPriority::Normal);
+    CHECK_THROWS(run());
+    CHECK(std::vector<int>({2, 5}) == results);
+    results.clear();
+
+    app.get_help_ptr()->callback_priority(CLI::CallbackPriority::Last);
+    CHECK_THROWS(run());
+    CHECK(std::vector<int>({2, 5, 1, 3}) == results);
+    results.clear();
+    opt3->excludes(opt1);
+    args = {"-A", "-B", "-C", "-D", "-E"};
+
+    CHECK_THROWS(run());
+    CHECK(std::vector<int>({2, 5}) == results);
+}
+
 TEST_CASE_METHOD(TApp, "ComplexOptMulti", "[app]") {
     std::complex<double> val;
     app.add_option("--long", val)->take_first()->allow_extra_args();

tests/HelpTest.cpp

@@ -1593,3 +1593,20 @@ TEST_CASE("TVersion: exit", "[help]") {
         CHECK(0 == ret);
     }
 }
+
+TEST_CASE("TVersion: exit_with_required", "[help]") {
+    // test that the version flag works even if there are required options
+    CLI::App app;
+
+    app.set_version_flag("--version", CLI11_VERSION);
+    app.add_option("--req")->required();
+
+    try {
+        app.parse("--version");
+    } catch(const CLI::CallForVersion &v) {
+        std::ostringstream out;
+        auto ret = app.exit(v, out);
+        CHECK_THAT(out.str(), Contains(CLI11_VERSION));
+        CHECK(0 == ret);
+    }
+}