fix: several small fixes and added tests (#666)

* add a few tests related to github issues * change how the default is displayed in the help message prev was =XXXX, this was confusing in some cases particularly with flags or with multiple option names. Now is [default=XXXX] which makes it clearer what the value represents. * Try to fix RTTI issue * style: pre-commit.ci fixes * Fix subcommand callbacks being called multiple times if in an option group * style: pre-commit.ci fixes * remove extra group call * change [default=XXXXD] to just [XXXXX] for the default specification * update changelog Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

fix: several small fixes and added tests (#666)
* add a few tests related to github issues * change how the default is displayed in the help message prev was =XXXX, this was confusing in some cases particularly with flags or with multiple option names. Now is [default=XXXX] which makes it clearer what the value represents. * Try to fix RTTI issue * style: pre-commit.ci fixes * Fix subcommand callbacks being called multiple times if in an option group * style: pre-commit.ci fixes * remove extra group call * change [default=XXXXD] to just [XXXXX] for the default specification * update changelog Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Philip Top · GitHub
1 parent eba619fc
Showing 8 changed files with 94 additions and 16 deletions
CHANGELOG.md
azure-pipelines.yml
include/CLI/App.hpp
include/CLI/Formatter.hpp
include/CLI/Macros.hpp
tests/ConfigFileTest.cpp
tests/HelpTest.cpp
tests/SubcommandTest.cpp
@@ -45,6 +45,16 @@ is not passed, or every time the option is parsed.
 [#656]: https://github.com/CLIUtils/CLI11/pull/656
 [#657]: https://github.com/CLIUtils/CLI11/pull/657
+## Version 2.1.3: Bug Fixes and Tweaks
+
+* Change the way the default value is displayed in the included help text generation from `=XXXXX` to `[XXXXX]` to clean up some situations in which the text looked awkward and unclear [#666][]
+* Fix a bug where a subcommand callback could be executed multiple times if it was a member of an option group [#666][]
+* Fix an issue where the detection of RTTI being disabled on certain visual studio platforms did not disable the use of dynamic cast calls [#666][]
+* Add additional tests concerning the use of aliases for option groups in config files [#666][]
+* Resolve strict-overflow warning on some GCC compilers [#666][]
+
+[#666]: https://github.com/CLIUtils/CLI11/pull/666
+
 ## Version 2.0: Simplification
 This version focuses on cleaning up deprecated functionality, and some minor
@@ -102,6 +102,7 @@ jobs:
       gcc9:
         containerImage: gcc:9
         cli11.std: 17
+        cli11.options: -DCMAKE_CXX_FLAGS="-Wstrict-overflow=5"
       gcc11:
         containerImage: gcc:11
         cli11.std: 20
@@ -1213,8 +1213,8 @@ class App {
         }
         std::vector<std::string> args;
-        args.reserve(static_cast<std::size_t>(argc) - 1);
-        for(int i = argc - 1; i > 0; i--)
+        args.reserve(static_cast<std::size_t>(argc) - 1U);
+        for(auto i = static_cast<std::size_t>(argc) - 1U; i > 0U; --i)
             args.emplace_back(argv[i]);
         parse(std::move(args));
     }
@@ -1543,7 +1543,7 @@ class App {
     /// Access the config formatter as a configBase pointer
     std::shared_ptr<ConfigBase> get_config_formatter_base() const {
         // This is safer as a dynamic_cast if we have RTTI, as Config -> ConfigBase
-#if defined(__cpp_rtti) || (defined(__GXX_RTTI) && __GXX_RTTI) || (defined(_HAS_STATIC_RTTI) && (_HAS_STATIC_RTTI == 0))
+#if CLI11_USE_STATIC_RTTI == 0
         return std::dynamic_pointer_cast<ConfigBase>(config_formatter_);
 #else
         return std::static_pointer_cast<ConfigBase>(config_formatter_);
@@ -1945,7 +1945,9 @@ class App {
         }
         // run the callbacks for the received subcommands
         for(App *subc : get_subcommands()) {
-            subc->run_callback(true, suppress_final_callback);
+            if(subc->parent_ == this) {
+                subc->run_callback(true, suppress_final_callback);
+            }
         }
         // now run callbacks for option_groups
         for(auto &subc : subcommands_) {
@@ -249,7 +249,7 @@ inline std::string Formatter::make_option_opts(const Option *opt) const {
             if(!opt->get_type_name().empty())
                 out << " " << get_label(opt->get_type_name());
             if(!opt->get_default_str().empty())
-                out << "=" << opt->get_default_str();
+                out << " [" << opt->get_default_str() << "] ";
             if(opt->get_expected_max() == detail::expected_max_vector_size)
                 out << " ...";
             else if(opt->get_expected_min() > 1)
@@ -41,4 +41,20 @@
 #define CLI11_DEPRECATED(reason) __attribute__((deprecated(reason)))
 #endif
+/** detection of rtti */
+#ifndef CLI11_USE_STATIC_RTTI
+#if(defined(_HAS_STATIC_RTTI) && _HAS_STATIC_RTTI)
+#define CLI11_USE_STATIC_RTTI 1
+#elif defined(__cpp_rtti)
+#if(defined(_CPPRTTI) && _CPPRTTI == 0)
+#define CLI11_USE_STATIC_RTTI 1
+#else
+#define CLI11_USE_STATIC_RTTI 0
+#endif
+#elif(defined(__GCC_RTTI) && __GXX_RTTI)
+#define CLI11_USE_STATIC_RTTI 0
+#else
+#define CLI11_USE_STATIC_RTTI 1
+#endif
+#endif
 // [CLI11:macros_hpp:end]
@@ -1216,6 +1216,30 @@ TEST_CASE_METHOD(TApp, &quot;IniLayeredCustomSectionSeparator&quot;, &quot;[config]&quot;) {
     CHECK(!*subcom);
 }
+TEST_CASE_METHOD(TApp, "IniLayeredOptionGroupAlias", "[config]") {
+
+    TempFile tmpini{"TestIniTmp.ini"};
+
+    app.set_config("--config", tmpini);
+
+    {
+        std::ofstream out{tmpini};
+        out << "[default]" << std::endl;
+        out << "val=1" << std::endl;
+        out << "[ogroup]" << std::endl;
+        out << "val2=2" << std::endl;
+    }
+    int one{0}, two{0};
+    app.add_option("--val", one);
+    auto subcom = app.add_option_group("ogroup")->alias("ogroup");
+    subcom->add_option("--val2", two);
+
+    run();
+
+    CHECK(one == 1);
+    CHECK(two == 2);
+}
+
 TEST_CASE_METHOD(TApp, "IniSubcommandConfigurable", "[config]") {
     TempFile tmpini{"TestIniTmp.ini"};
@@ -289,7 +289,8 @@ TEST_CASE(&quot;THelp: VectorOpts&quot;, &quot;[help]&quot;) {
     std::string help = app.help();
-    CHECK_THAT(help, Contains("INT=[1,2] ..."));
+    CHECK_THAT(help, Contains("[1,2]"));
+    CHECK_THAT(help, Contains(" ..."));
 }
 TEST_CASE("THelp: MultiPosOpts", "[help]") {
@@ -394,18 +395,18 @@ TEST_CASE(&quot;THelp: ManualSetters&quot;, &quot;[help]&quot;) {
     std::string help = app.help();
-    CHECK_THAT(help, Contains("=12"));
+    CHECK_THAT(help, Contains("[12]"));
     CHECK_THAT(help, Contains("BIGGLES"));
     op1->default_val("14");
     CHECK(14 == x);
     help = app.help();
-    CHECK_THAT(help, Contains("=14"));
+    CHECK_THAT(help, Contains("[14]"));
     op1->default_val(12);
     CHECK(12 == x);
     help = app.help();
-    CHECK_THAT(help, Contains("=12"));
+    CHECK_THAT(help, Contains("[12]"));
     CHECK(op1->get_run_callback_for_default());
     op1->run_callback_for_default(false);
@@ -415,7 +416,7 @@ TEST_CASE(&quot;THelp: ManualSetters&quot;, &quot;[help]&quot;) {
     // x should not be modified in this case
     CHECK(12 == x);
     help = app.help();
-    CHECK_THAT(help, Contains("=18"));
+    CHECK_THAT(help, Contains("[18]"));
 }
 TEST_CASE("THelp: ManualSetterOverFunction", "[help]") {
@@ -432,7 +433,7 @@ TEST_CASE(&quot;THelp: ManualSetterOverFunction&quot;, &quot;[help]&quot;) {
     CHECK(1 == x);
     std::string help = app.help();
-    CHECK_THAT(help, Contains("=12"));
+    CHECK_THAT(help, Contains("[12]"));
     CHECK_THAT(help, Contains("BIGGLES"));
     CHECK_THAT(help, Contains("QUIGGLES"));
     CHECK_THAT(help, Contains("{1,2}"));
@@ -518,7 +519,7 @@ TEST_CASE(&quot;THelp: IntDefaults&quot;, &quot;[help]&quot;) {
     CHECK_THAT(help, Contains("--one"));
     CHECK_THAT(help, Contains("--set"));
     CHECK_THAT(help, Contains("1"));
-    CHECK_THAT(help, Contains("=2"));
+    CHECK_THAT(help, Contains("[2]"));
     CHECK_THAT(help, Contains("2,3,4"));
 }
@@ -532,7 +533,7 @@ TEST_CASE(&quot;THelp: SetLower&quot;, &quot;[help]&quot;) {
     std::string help = app.help();
     CHECK_THAT(help, Contains("--set"));
-    CHECK_THAT(help, Contains("=One"));
+    CHECK_THAT(help, Contains("[One]"));
     CHECK_THAT(help, Contains("oNe"));
     CHECK_THAT(help, Contains("twO"));
     CHECK_THAT(help, Contains("THREE"));
@@ -893,6 +894,14 @@ TEST_CASE(&quot;THelp: CheckEmptyTypeName&quot;, &quot;[help]&quot;) {
     CHECK(name.empty());
 }
+TEST_CASE("THelp: FlagDefaults", "[help]") {
+    CLI::App app;
+
+    app.add_flag("-t,--not{false}")->default_str("false");
+    auto str = app.help();
+    CHECK_THAT(str, Contains("--not{false}"));
+}
+
 TEST_CASE("THelp: AccessDescription", "[help]") {
     CLI::App app{"My description goes here"};
@@ -1162,7 +1171,9 @@ TEST_CASE(&quot;THelp: ChangingDefaults&quot;, &quot;[help]&quot;) {
     x = {5, 6};
     std::string help = app.help();
-    CHECK_THAT(help, Contains("INT=[3,4] ..."));
+    CHECK_THAT(help, Contains("[[3,4]]"));
+    CHECK_THAT(help, Contains("..."));
+    CHECK_THAT(help, Contains("INT"));
     CHECK(x[0] == 5);
 }
@@ -1179,7 +1190,8 @@ TEST_CASE(&quot;THelp: ChangingDefaultsWithAutoCapture&quot;, &quot;[help]&quot;) {
     std::string help = app.help();
-    CHECK_THAT(help, Contains("INT=[1,2] ..."));
+    CHECK_THAT(help, Contains("[[1,2]]"));
+    CHECK_THAT(help, Contains("..."));
 }
 TEST_CASE("THelp: FunctionDefaultString", "[help]") {
@@ -1194,7 +1206,7 @@ TEST_CASE(&quot;THelp: FunctionDefaultString&quot;, &quot;[help]&quot;) {
     std::string help = app.help();
-    CHECK_THAT(help, Contains("INT=Powerful"));
+    CHECK_THAT(help, Contains("[Powerful]"));
 }
 TEST_CASE("TVersion: simple_flag", "[help]") {
@@ -1955,3 +1955,16 @@ TEST_CASE_METHOD(TApp, &quot;MultiFinalCallbackCounts&quot;, &quot;[subcom]&quot;) {
         CHECK(subsub_final == 1);
     }
 }
+
+// From gitter issue
+TEST_CASE_METHOD(TApp, "SubcommandInOptionGroupCallbackCount", "[subcom]") {
+
+    int subcount{0};
+    auto group1 = app.add_option_group("FirstGroup");
+
+    group1->add_subcommand("g1c1")->callback([&subcount]() { ++subcount; });
+
+    args = {"g1c1"};
+    run();
+    CHECK(subcount == 1);
+}