From dcbcb4721dda5dab0a56d9faaaee50e6a30f7758 Mon Sep 17 00:00:00 2001
From: Philip Top <phlptp@gmail.com>
Date: Wed, 14 Sep 2022 07:45:22 -0700
Subject: [PATCH] fix: negated flag config files (#775)

* add some coverage tests, and fix the issue with negated flags and config files

* style: pre-commit.ci fixes

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
---
 include/CLI/impl/Config_inl.hpp |  3 +++
 tests/ConfigFileTest.cpp        | 23 +++++++++++++++++++++++
 tests/CreationTest.cpp          | 19 +++++++++++++++++++
 tests/SubcommandTest.cpp        | 16 ++++++++++++++++
 4 files changed, 61 insertions(+)

diff --git a/include/CLI/impl/Config_inl.hpp b/include/CLI/impl/Config_inl.hpp
index 0f9695f2..9fbb423f 100644
--- a/include/CLI/impl/Config_inl.hpp
+++ b/include/CLI/impl/Config_inl.hpp
@@ -344,6 +344,9 @@ ConfigBase::to_config(const App *app, bool default_also, bool write_description,
                 }
 
                 if(!value.empty()) {
+                    if(!opt->get_fnames().empty()) {
+                        value = opt->get_flag_value(name, value);
+                    }
                     if(write_description && opt->has_description()) {
                         out << '\n';
                         out << commentLead << detail::fix_newlines(commentLead, opt->get_description()) << '\n';
diff --git a/tests/ConfigFileTest.cpp b/tests/ConfigFileTest.cpp
index 0b11ffef..e6425f02 100644
--- a/tests/ConfigFileTest.cpp
+++ b/tests/ConfigFileTest.cpp
@@ -2497,6 +2497,29 @@ TEST_CASE_METHOD(TApp, "ConfigWriteReadWrite", "[config]") {
     CHECK(config2 == config1);
 }
 
+TEST_CASE_METHOD(TApp, "ConfigWriteReadNegated", "[config]") {
+
+    TempFile tmpini{"TestIniTmp.ini"};
+    bool flag{true};
+    app.add_flag("!--no-flag", flag);
+    args = {"--no-flag"};
+    run();
+
+    // Save config, with default values too
+    std::string config1 = app.config_to_str(false, false);
+    {
+        std::ofstream out{tmpini};
+        out << config1 << std::endl;
+    }
+    CHECK_FALSE(flag);
+    args.clear();
+    flag = true;
+    app.set_config("--config", tmpini, "Read an ini file", true);
+    run();
+
+    CHECK_FALSE(flag);
+}
+
 /////// INI output tests
 
 TEST_CASE_METHOD(TApp, "IniOutputSimple", "[config]") {
diff --git a/tests/CreationTest.cpp b/tests/CreationTest.cpp
index b58e0aab..9f55f6c8 100644
--- a/tests/CreationTest.cpp
+++ b/tests/CreationTest.cpp
@@ -549,6 +549,25 @@ TEST_CASE_METHOD(TApp, "GetOptionList", "[creation]") {
     }
 }
 
+TEST_CASE_METHOD(TApp, "GetOptionListFilter", "[creation]") {
+    int two{0};
+    auto *flag = app.add_flag("--one");
+    app.add_option("--two", two);
+
+    const CLI::App &const_app = app;  // const alias to force use of const-methods
+    std::vector<const CLI::Option *> opt_listc =
+        const_app.get_options([](const CLI::Option *opt) { return opt->get_name() == "--one"; });
+
+    REQUIRE(static_cast<std::size_t>(1) == opt_listc.size());
+    CHECK(flag == opt_listc.at(0));
+
+    std::vector<CLI::Option *> opt_list =
+        app.get_options([](const CLI::Option *opt) { return opt->get_name() == "--one"; });
+
+    REQUIRE(static_cast<std::size_t>(1) == opt_list.size());
+    CHECK(flag == opt_list.at(0));
+}
+
 TEST_CASE("ValidatorTests: TestValidatorCreation", "[creation]") {
     std::function<std::string(std::string &)> op1 = [](std::string &val) {
         return (val.size() >= 5) ? std::string{} : val;
diff --git a/tests/SubcommandTest.cpp b/tests/SubcommandTest.cpp
index a01b8863..84a62d82 100644
--- a/tests/SubcommandTest.cpp
+++ b/tests/SubcommandTest.cpp
@@ -916,6 +916,10 @@ TEST_CASE_METHOD(TApp, "SubcomInheritCaseCheck", "[subcom]") {
     CHECK(app.get_subcommands({}).size() == 2u);
     CHECK(app.get_subcommands([](const CLI::App *s) { return s->get_name() == "sub1"; }).size() == 1u);
 
+    // check the const version of get_subcommands
+    const auto &app_const = app;
+    CHECK(app_const.get_subcommands([](const CLI::App *s) { return s->get_name() == "sub1"; }).size() == 1u);
+
     args = {"SuB1"};
     run();
     CHECK(app.get_subcommands().at(0) == sub1);
@@ -1192,6 +1196,18 @@ TEST_CASE_METHOD(ManySubcommands, "manyIndexQueryPtr", "[subcom]") {
     CHECK_THROWS_AS(app.get_subcommand_ptr(4), CLI::OptionNotFound);
 }
 
+TEST_CASE_METHOD(ManySubcommands, "manyIndexQueryPtrByName", "[subcom]") {
+    auto s1 = app.get_subcommand_ptr("sub1");
+    auto s2 = app.get_subcommand_ptr("sub2");
+    auto s3 = app.get_subcommand_ptr("sub3");
+    auto s4 = app.get_subcommand_ptr("sub4");
+    CHECK(sub1 == s1.get());
+    CHECK(sub2 == s2.get());
+    CHECK(sub3 == s3.get());
+    CHECK(sub4 == s4.get());
+    CHECK_THROWS_AS(app.get_subcommand_ptr("sub5"), CLI::OptionNotFound);
+}
+
 TEST_CASE_METHOD(ManySubcommands, "Required1Fuzzy", "[subcom]") {
 
     app.require_subcommand(0, 1);