Skip to content

Commit

Permalink
Config count anomaly (#1003)
Browse files Browse the repository at this point in the history
Correct an anomaly when using config file processing with a default. In
this case the count always shows 1 even if the default file were not
actually used. This caused some issues in some applications and was a
change from previous versions.

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
  • Loading branch information
phlptp and pre-commit-ci[bot] authored Feb 7, 2024
1 parent fd483ea commit cf6092b
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 5 deletions.
2 changes: 1 addition & 1 deletion include/CLI/App.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -1231,7 +1231,7 @@ class App {
void _process_config_file();

/// Read and process a particular configuration file
void _process_config_file(const std::string &config_file, bool throw_error);
bool _process_config_file(const std::string &config_file, bool throw_error);

/// Get envname options if not yet passed. Runs on *all* subcommands.
void _process_env();
Expand Down
22 changes: 19 additions & 3 deletions include/CLI/impl/App_inl.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -1065,18 +1065,23 @@ CLI11_NODISCARD CLI11_INLINE detail::Classifier App::_recognize(const std::strin
return detail::Classifier::NONE;
}

CLI11_INLINE void App::_process_config_file(const std::string &config_file, bool throw_error) {
CLI11_INLINE bool App::_process_config_file(const std::string &config_file, bool throw_error) {
auto path_result = detail::check_path(config_file.c_str());
if(path_result == detail::path_type::file) {
try {
std::vector<ConfigItem> values = config_formatter_->from_file(config_file);
_parse_config(values);
return true;
} catch(const FileError &) {
if(throw_error)
if(throw_error) {
throw;
}
return false;
}
} else if(throw_error) {
throw FileError::Missing(config_file);
} else {
return false;
}
}

Expand All @@ -1093,14 +1098,25 @@ CLI11_INLINE void App::_process_config_file() {
config_ptr_->run_callback();

auto config_files = config_ptr_->as<std::vector<std::string>>();
bool files_used{file_given};
if(config_files.empty() || config_files.front().empty()) {
if(config_required) {
throw FileError("config file is required but none was given");
}
return;
}
for(const auto &config_file : config_files) {
_process_config_file(config_file, config_required || file_given);
if(_process_config_file(config_file, config_required || file_given)) {
files_used = true;
}
}
if(!files_used) {
// this is done so the count shows as 0 if no callbacks were processed
config_ptr_->clear();
bool force = config_ptr_->force_callback_;
config_ptr_->force_callback_ = false;
config_ptr_->run_callback();
config_ptr_->force_callback_ = force;
}
}
}
Expand Down
13 changes: 12 additions & 1 deletion tests/ConfigFileTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1049,6 +1049,16 @@ TEST_CASE_METHOD(TApp, "IniRequiredNotFound", "[config]") {
CHECK_THROWS_AS(run(), CLI::FileError);
}

TEST_CASE_METHOD(TApp, "IniDefaultNotExist", "[config]") {

std::string noini = "TestIniNotExist.ini";
auto *cfg = app.set_config("--config", noini);

CHECK_NOTHROW(run());

CHECK(cfg->count() == 0);
}

TEST_CASE_METHOD(TApp, "IniNotRequiredPassedNotFound", "[config]") {

std::string noini = "TestIniNotExist.ini";
Expand Down Expand Up @@ -1084,7 +1094,7 @@ TEST_CASE_METHOD(TApp, "IniRequired", "[config]") {

TempFile tmpini{"TestIniTmp.ini"};

app.set_config("--config", tmpini, "", true);
auto *cfg = app.set_config("--config", tmpini, "", true);

{
std::ofstream out{tmpini};
Expand All @@ -1109,6 +1119,7 @@ TEST_CASE_METHOD(TApp, "IniRequired", "[config]") {
args = {"--one=1", "--two=2"};

CHECK_NOTHROW(run());
CHECK(cfg->count() == 1);
CHECK(1 == one);
CHECK(2 == two);
CHECK(3 == three);
Expand Down

0 comments on commit cf6092b

Please sign in to comment.