chore: bump CMake limits and catch2 verison (#607)

* Fix excessive call to subcommand's final_callback

When parse_complete_callback_ is set there is an extra call to
run_callback() inside the App::_parse(std::vector<std::string>&) method.
This extra call also excessively calls a final_callback_ (when it is
also set) for the command and (since it is recursive) for it's
subcommands.

This commit adds extra boolean parameter for the run_callback() method
allowing to explicitly suppress calling to final_callback_. The value of
this parameter is also propagated to recursive calls to run_callback().

Fixes #572

* fix: main app should run final_callback, add tests

Co-authored-by: Henry Schreiner <henryschreineriii@gmail.com>

CMakeLists.txt

@@ -2,14 +2,14 @@ cmake_minimum_required(VERSION 3.4)
 # Note: this is a header only library. If you have an older CMake than 3.4,
 # just add the CLI11/include directory and that's all you need to do.
 
-# Make sure users don't get warnings on a tested (3.4 to 3.16) version
+# Make sure users don't get warnings on a tested (3.4 to 3.20) version
 # of CMake. For most of the policies, the new version is better (hence the change).
-# We don't use the 3.4...3.17 syntax because of a bug in an older MSVC's
+# We don't use the 3.4...3.20 syntax because of a bug in an older MSVC's
 # built-in and modified CMake 3.11
-if(${CMAKE_VERSION} VERSION_LESS 3.17)
+if(${CMAKE_VERSION} VERSION_LESS 3.20)
   cmake_policy(VERSION ${CMAKE_MAJOR_VERSION}.${CMAKE_MINOR_VERSION})
 else()
-  cmake_policy(VERSION 3.17)
+  cmake_policy(VERSION 3.20)
 endif()
 
 set(VERSION_REGEX "#define CLI11_VERSION[ \t]+\"(.+)\"")

include/CLI/App.hpp

@@ -113,6 +113,7 @@ class App {
 
     /// This is a function that runs when parsing has finished.
     std::function<void()> parse_complete_callback_{};
+
     /// This is a function that runs when all processing has completed
     std::function<void()> final_callback_{};
 
@@ -1934,8 +1935,9 @@ class App {
             app->_configure();
         }
     }
+
     /// Internal function to run (App) callback, bottom up
-    void run_callback(bool final_mode = false) {
+    void run_callback(bool final_mode = false, bool suppress_final_callback = false) {
         pre_callback();
         // in the main app if immediate_callback_ is set it runs the main callback before the used subcommands
         if(!final_mode && parse_complete_callback_) {
@@ -1943,18 +1945,18 @@ class App {
         }
         // run the callbacks for the received subcommands
         for(App *subc : get_subcommands()) {
-            subc->run_callback(true);
+            subc->run_callback(true, suppress_final_callback);
         }
         // now run callbacks for option_groups
         for(auto &subc : subcommands_) {
             if(subc->name_.empty() && subc->count_all() > 0) {
-                subc->run_callback(true);
+                subc->run_callback(true, suppress_final_callback);
             }
         }
 
         // finally run the main callback
-        if(final_callback_ && (parsed_ > 0)) {
-            if(!name_.empty() || count_all() > 0) {
+        if(final_callback_ && (parsed_ > 0) && (!suppress_final_callback)) {
+            if(!name_.empty() || count_all() > 0 || parent_ == nullptr) {
                 final_callback_();
             }
         }
@@ -2337,7 +2339,7 @@ class App {
             _process_callbacks();
             _process_help_flags();
             _process_requirements();
-            run_callback();
+            run_callback(false, true);
         }
     }
 

tests/CMakeLists.txt

@@ -70,11 +70,11 @@ target_include_directories(catch_main PUBLIC "${CMAKE_CURRENT_SOURCE_DIR}")
 # that would require changing the includes. FetchContent would be better, but
 # requires newer CMake.
 
-set(url https://github.com/philsquared/Catch/releases/download/v2.13.4/catch.hpp)
+set(url https://github.com/philsquared/Catch/releases/download/v2.13.6/catch.hpp)
 file(
   DOWNLOAD ${url} "${CMAKE_CURRENT_BINARY_DIR}/catch.hpp"
   STATUS status
-  EXPECTED_HASH SHA256=6e0fa3dd160891a01c1f3b34e8bcd6e0140abe08eca022e390027f27dec2050b)
+  EXPECTED_HASH SHA256=681e7505a50887c9085539e5135794fc8f66d8e5de28eadf13a30978627b0f47)
 list(GET status 0 error)
 if(error)
   message(FATAL_ERROR "Could not download ${url}")

tests/SubcommandTest.cpp

@@ -1874,3 +1874,63 @@ TEST_CASE_METHOD(ManySubcommands, "defaultEnabledSubcommand", "[subcom]") {
     CHECK(sub2->get_enabled_by_default());
     CHECK(!sub2->get_disabled());
 }
+
+// #572
+TEST_CASE_METHOD(TApp, "MultiFinalCallbackCounts", "[subcom]") {
+
+    int app_compl = 0;
+    int sub_compl = 0;
+    int subsub_compl = 0;
+    int app_final = 0;
+    int sub_final = 0;
+    int subsub_final = 0;
+
+    app.parse_complete_callback([&app_compl]() { app_compl++; });
+    app.final_callback([&app_final]() { app_final++; });
+
+    auto *sub = app.add_subcommand("sub");
+
+    sub->parse_complete_callback([&sub_compl]() { sub_compl++; });
+    sub->final_callback([&sub_final]() { sub_final++; });
+
+    auto *subsub = sub->add_subcommand("subsub");
+
+    subsub->parse_complete_callback([&subsub_compl]() { subsub_compl++; });
+    subsub->final_callback([&subsub_final]() { subsub_final++; });
+
+    SECTION("No specified subcommands") {
+        args = {};
+        run();
+
+        CHECK(app_compl == 1);
+        CHECK(app_final == 1);
+        CHECK(sub_compl == 0);
+        CHECK(sub_final == 0);
+        CHECK(subsub_compl == 0);
+        CHECK(subsub_final == 0);
+    }
+
+    SECTION("One layer of subcommands") {
+        args = {"sub"};
+        run();
+
+        CHECK(app_compl == 1);
+        CHECK(app_final == 1);
+        CHECK(sub_compl == 1);
+        CHECK(sub_final == 1);
+        CHECK(subsub_compl == 0);
+        CHECK(subsub_final == 0);
+    }
+
+    SECTION("Fully specified subcommands") {
+        args = {"sub", "subsub"};
+        run();
+
+        CHECK(app_compl == 1);
+        CHECK(app_final == 1);
+        CHECK(sub_compl == 1);
+        CHECK(sub_final == 1);
+        CHECK(subsub_compl == 1);
+        CHECK(subsub_final == 1);
+    }
+}