reorder the processing of requirements and callbacks. (#1186)

Check the requirements first.
Previously the callbacks were done first, but with custom callbacks this
cause side effects unexpectedly.

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
This commit is contained in:
Philip Top
2025-07-25 06:53:17 -07:00
committed by GitHub
parent a9b7b96701
commit 2a59b281f0
3 changed files with 24 additions and 7 deletions

View File

@@ -16,6 +16,7 @@
// [CLI11:public_includes:set]
#include <algorithm>
#include <exception>
#include <iostream>
#include <memory>
#include <string>
@@ -1415,6 +1416,7 @@ 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();
std::exception_ptr config_exception;
try {
// the config file might generate a FileError but that should not be processed until later in the process
// to allow for help, version and other errors to generate first.
@@ -1423,15 +1425,17 @@ CLI11_INLINE void App::_process() {
// process env shouldn't throw but no reason to process it if config generated an error
_process_env();
} catch(const CLI::FileError &) {
// callbacks can generate exceptions which should take priority
// over the config file error if one exists.
_process_callbacks();
throw;
config_exception = std::current_exception();
}
// callbacks and requirements processing can generate exceptions which should take priority
// over the config file error if one exists.
_process_requirements();
_process_callbacks();
_process_requirements();
if(config_exception) {
std::rethrow_exception(config_exception);
}
}
CLI11_INLINE void App::_process_extras() {

View File

@@ -2057,6 +2057,19 @@ TEST_CASE_METHOD(TApp, "NeedsFlags", "[app]") {
CHECK_NOTHROW(opt->needs(opt));
}
TEST_CASE_METHOD(TApp, "needsOptionFunction", "[app]") {
std::string s1{"A"};
std::string s2{"B"};
app.add_flag("-s,--string");
app.add_option_function<std::string>("file", [&](std::string const &str) { s1 = str; }, "input file");
app.add_option_function<std::string>(
"--something", [&](std::string const &str) { s2 = str; }, "something")
->needs("file");
args = {"--something", "C"};
CHECK_THROWS_AS(run(), CLI::RequiresError);
CHECK(s2 == "B");
}
TEST_CASE_METHOD(TApp, "ExcludesFlags", "[app]") {
CLI::Option *opt = app.add_flag("-s,--string");
app.add_flag("--nostr")->excludes(opt);

View File

@@ -803,7 +803,7 @@ TEST_CASE_METHOD(TApp, "IniRequiredNoDefault", "[config]") {
int two{0};
app.add_option("--two", two);
REQUIRE_THROWS_AS(run(), CLI::FileError);
REQUIRE_THROWS_AS(run(), CLI::RequiredError);
// test to make sure help still gets called correctly
// GitHub issue #533 https://github.com/CLIUtils/CLI11/issues/553
args = {"--help"};
@@ -932,7 +932,7 @@ TEST_CASE_METHOD(TApp, "IniEnvironmentalFileName", "[config]") {
unset_env("CONFIG");
CHECK_THROWS_AS(run(), CLI::FileError);
CHECK_THROWS_AS(run(), CLI::RequiredError);
}
TEST_CASE_METHOD(TApp, "MultiConfig", "[config]") {