From 8f863af31f814fb7ebc6a09e120050de159a71c2 Mon Sep 17 00:00:00 2001 From: Henry Schreiner Date: Thu, 11 Jun 2026 15:45:26 -0400 Subject: [PATCH] refactor: factor duplicated parse pipeline and remove dead code No behavior changes. Several verified simplifications and dead-code removals. Assisted-by: ClaudeCode:claude-opus-4-8 --- include/CLI/App.hpp | 10 +++ include/CLI/StringTools.hpp | 3 + include/CLI/Timer.hpp | 10 +-- include/CLI/impl/App_inl.hpp | 110 +++++++++++---------------- include/CLI/impl/Config_inl.hpp | 4 +- include/CLI/impl/Option_inl.hpp | 5 +- include/CLI/impl/StringTools_inl.hpp | 15 +--- 7 files changed, 64 insertions(+), 93 deletions(-) diff --git a/include/CLI/App.hpp b/include/CLI/App.hpp index 14a329231..d7c18f8db 100644 --- a/include/CLI/App.hpp +++ b/include/CLI/App.hpp @@ -1354,6 +1354,12 @@ class App { /// The flags allow recursive calls to remember if there was a help flag on a parent. void _process_help_flags(CallbackPriority priority, bool trigger_help = false, bool trigger_all_help = false) const; + /// Run the full priority-ordered callback/help-flag pipeline used when a parse-complete callback fires. + /// + /// When @p with_help_flags is true the help-flag passes and environment processing are included + /// (the option/dot-notation paths); when false they are omitted (the config-section-close path). + void _process_completion_callbacks(bool with_help_flags); + /// Verify required options and cross requirements. Subcommands too (only if selected). void _process_requirements(); @@ -1366,6 +1372,10 @@ class App { /// Internal function to recursively increment the parsed counter on the current app as well unnamed subcommands void increment_parsed(); + /// Shared prologue for the public parse() overloads: clears prior state, validates and configures, + /// and marks this app as the top-level app. + void _parse_setup(); + /// Internal parse function void _parse(std::vector &args); diff --git a/include/CLI/StringTools.hpp b/include/CLI/StringTools.hpp index 611def290..229469374 100644 --- a/include/CLI/StringTools.hpp +++ b/include/CLI/StringTools.hpp @@ -125,6 +125,9 @@ inline std::string trim_copy(const std::string &str) { return trim(s); } +/// remove a matching pair of outer characters (front and back) from a string if both equal `key` +CLI11_INLINE std::string &remove_outer(std::string &str, char key); + /// remove quotes at the front and back of a string either '"' or '\'' CLI11_INLINE std::string &remove_quotes(std::string &str); diff --git a/include/CLI/Timer.hpp b/include/CLI/Timer.hpp index 542234269..bf98d1f46 100644 --- a/include/CLI/Timer.hpp +++ b/include/CLI/Timer.hpp @@ -8,12 +8,6 @@ // IWYU pragma: private, include "CLI/CLI.hpp" -// On GCC < 4.8, the following define is often missing. Due to the -// fact that this library only uses sleep_for, this should be safe -#if defined(__GNUC__) && !defined(__clang__) && __GNUC__ < 5 && __GNUC_MINOR__ < 8 -#define _GLIBCXX_USE_NANOSLEEP -#endif - #include #include @@ -124,9 +118,7 @@ class Timer { /// This class prints out the time upon destruction class AutoTimer : public Timer { public: - /// Reimplementing the constructor is required in GCC 4.7 - explicit AutoTimer(std::string title = "Timer", time_print_t time_print = Simple) : Timer(title, time_print) {} - // GCC 4.7 does not support using inheriting constructors. + using Timer::Timer; /// This destructor prints the string ~AutoTimer() { std::cout << to_string() << '\n'; } diff --git a/include/CLI/impl/App_inl.hpp b/include/CLI/impl/App_inl.hpp index 2fb33d82a..bfcdf3fab 100644 --- a/include/CLI/impl/App_inl.hpp +++ b/include/CLI/impl/App_inl.hpp @@ -652,7 +652,7 @@ CLI11_INLINE void App::parse(std::wstring commandline, bool program_name_include parse(narrow(commandline), program_name_included); } -CLI11_INLINE void App::parse(std::vector &args) { +CLI11_INLINE void App::_parse_setup() { // Clear if parsed if(parsed_ > 0) clear(); @@ -666,26 +666,16 @@ CLI11_INLINE void App::parse(std::vector &args) { // set the parent as nullptr as this object should be the top now parent_ = nullptr; parsed_ = 0; +} +CLI11_INLINE void App::parse(std::vector &args) { + _parse_setup(); _parse(args); run_callback(); } CLI11_INLINE void App::parse(std::vector &&args) { - // Clear if parsed - if(parsed_ > 0) - clear(); - - // parsed_ is incremented in commands/subcommands, - // but placed here to make sure this is cleared when - // running parse after an error is thrown, even by _validate or _configure. - parsed_ = 1; - _validate(); - _configure(); - // set the parent as nullptr as this object should be the top now - parent_ = nullptr; - parsed_ = 0; - + _parse_setup(); _parse(std::move(args)); run_callback(); } @@ -1522,16 +1512,15 @@ CLI11_INLINE void App::_process() { CLI11_INLINE void App::_process_extras() { if(allow_extras_ == ExtrasMode::Error && prefix_command_ == PrefixCommandMode::Off) { - std::size_t num_left_over = remaining_size(); - if(num_left_over > 0) { + if(remaining_size() > 0) { throw ExtrasError(name_, remaining(false)); } } if(allow_extras_ == ExtrasMode::Error && prefix_command_ == PrefixCommandMode::SeparatorOnly) { - std::size_t num_left_over = remaining_size(); - if(num_left_over > 0) { - if(remaining(false).front() != "--") { - throw ExtrasError(name_, remaining(false)); + if(remaining_size() > 0) { + auto rem = remaining(false); + if(rem.front() != "--") { + throw ExtrasError(name_, std::move(rem)); } } } @@ -1549,6 +1538,34 @@ CLI11_INLINE void App::increment_parsed() { } } +CLI11_INLINE void App::_process_completion_callbacks(bool with_help_flags) { + _process_callbacks(CallbackPriority::FirstPreHelp); + if(with_help_flags) { + _process_help_flags(CallbackPriority::First); + } + _process_callbacks(CallbackPriority::First); + if(with_help_flags) { + _process_env(); + } + _process_callbacks(CallbackPriority::PreRequirementsCheckPreHelp); + if(with_help_flags) { + _process_help_flags(CallbackPriority::PreRequirementsCheck); + } + _process_callbacks(CallbackPriority::PreRequirementsCheck); + _process_requirements(); + _process_callbacks(CallbackPriority::NormalPreHelp); + if(with_help_flags) { + _process_help_flags(CallbackPriority::Normal); + } + _process_callbacks(CallbackPriority::Normal); + _process_callbacks(CallbackPriority::LastPreHelp); + if(with_help_flags) { + _process_help_flags(CallbackPriority::Last); + } + _process_callbacks(CallbackPriority::Last); + run_callback(false, with_help_flags); +} + CLI11_INLINE void App::_parse(std::vector &args) { increment_parsed(); _trigger_pre_parse(args.size()); @@ -1568,21 +1585,7 @@ CLI11_INLINE void App::_parse(std::vector &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(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); + _process_completion_callbacks(true); } } @@ -1594,7 +1597,9 @@ CLI11_INLINE void App::_parse(std::vector &&args) { bool positional_only = false; while(!args.empty()) { - _parse_single(args, positional_only); + if(!_parse_single(args, positional_only)) { + break; + } } _process(); @@ -1702,16 +1707,7 @@ 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(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(); + _process_completion_callbacks(false); } return true; } @@ -1968,9 +1964,7 @@ CLI11_INLINE bool App::_parse_positional(std::vector &args, bool ha } } if(positionals_at_end_) { - std::vector rargs; - rargs.resize(args.size()); - std::reverse_copy(args.begin(), args.end(), rargs.begin()); + std::vector rargs(args.rbegin(), args.rend()); throw CLI::ExtrasError(name_, rargs); } /// If this is an option group don't deal with it @@ -2177,21 +2171,7 @@ App::_parse_arg(std::vector &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(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); + sub->_process_completion_callbacks(true); } return true; } diff --git a/include/CLI/impl/Config_inl.hpp b/include/CLI/impl/Config_inl.hpp index 178fa7763..f555dc098 100644 --- a/include/CLI/impl/Config_inl.hpp +++ b/include/CLI/impl/Config_inl.hpp @@ -89,11 +89,11 @@ convert_arg_for_ini(const std::string &arg, char stringQuote, char literalQuote, } std::string return_string{multiline_literal_quote}; return_string.reserve(7 + arg.size()); - if(arg.front() == '\n' || arg.front() == '\r') { + if(arg.front() == '\n') { return_string.push_back('\n'); } return_string.append(arg); - if(arg.back() == '\n' || arg.back() == '\r') { + if(arg.back() == '\n') { return_string.push_back('\n'); } return_string.append(multiline_literal_quote, 3); diff --git a/include/CLI/impl/Option_inl.hpp b/include/CLI/impl/Option_inl.hpp index 0198c9792..d102f5534 100644 --- a/include/CLI/impl/Option_inl.hpp +++ b/include/CLI/impl/Option_inl.hpp @@ -732,15 +732,12 @@ CLI11_INLINE int Option::_add_result(std::string &&result, std::vector 1 && (str.front() == '"' || str.front() == '\'' || str.front() == '`')) { - if(str.front() == str.back()) { - str.pop_back(); - str.erase(str.begin(), str.begin() + 1); - } + if(!str.empty() && (str.front() == '"' || str.front() == '\'' || str.front() == '`')) { + remove_outer(str, str.front()); } return str; } @@ -202,14 +199,6 @@ CLI11_MODULE_INLINE const std::string &matchBracketChars() { return s; } -// CLI11_MODULE_INLINE constexpr char escapedChars[]="\b\t\n\f\r\"\\"; -// CLI11_MODULE_INLINE constexpr char escapedCharsCode[]="btnfr\"\\"; -/* -const std::string &escapedChars("\b\t\n\f\r\"\\"); - -CLI11_MODULE_INLINE const std::string &bracketChars("\"'`[(<{"); -CLI11_MODULE_INLINE const std::string &matchBracketChars("\"'`])>}"); -*/ CLI11_INLINE bool has_escapable_character(const std::string &str) { return (str.find_first_of(escapedChars()) != std::string::npos); }