Peter M. Groen / CLI11

Commit d0a2aa7908aee8d55babab21a22684879f97f97d

Authored by dryleev
Committed by GitHub
1 parent 75f2d774

fix: remove duplicated call to subcommand's final_callback (#584) 

* 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>
Inline Side-by-side
Showing 2 changed files with 68 additions and 6 deletions
include/CLI/App.hpp
  View file @d0a2aa7
@@ -113,6 +113,7 @@ class App { @@ -113,6 +113,7 @@ class App {
113 113
114 /// This is a function that runs when parsing has finished. 114 /// This is a function that runs when parsing has finished.
115 std::function<void()> parse_complete_callback_{}; 115 std::function<void()> parse_complete_callback_{};
  116 +
116 /// This is a function that runs when all processing has completed 117 /// This is a function that runs when all processing has completed
117 std::function<void()> final_callback_{}; 118 std::function<void()> final_callback_{};
118 119
@@ -1934,8 +1935,9 @@ class App { @@ -1934,8 +1935,9 @@ class App {
1934 app->_configure(); 1935 app->_configure();
1935 } 1936 }
1936 } 1937 }
  1938 +
1937 /// Internal function to run (App) callback, bottom up 1939 /// Internal function to run (App) callback, bottom up
1938 - void run_callback(bool final_mode = false) { 1940 + void run_callback(bool final_mode = false, bool suppress_final_callback = false) {
1939 pre_callback(); 1941 pre_callback();
1940 // in the main app if immediate_callback_ is set it runs the main callback before the used subcommands 1942 // in the main app if immediate_callback_ is set it runs the main callback before the used subcommands
1941 if(!final_mode && parse_complete_callback_) { 1943 if(!final_mode && parse_complete_callback_) {
@@ -1943,18 +1945,18 @@ class App { @@ -1943,18 +1945,18 @@ class App {
1943 } 1945 }
1944 // run the callbacks for the received subcommands 1946 // run the callbacks for the received subcommands
1945 for(App *subc : get_subcommands()) { 1947 for(App *subc : get_subcommands()) {
1946 - subc->run_callback(true); 1948 + subc->run_callback(true, suppress_final_callback);
1947 } 1949 }
1948 // now run callbacks for option_groups 1950 // now run callbacks for option_groups
1949 for(auto &subc : subcommands_) { 1951 for(auto &subc : subcommands_) {
1950 if(subc->name_.empty() && subc->count_all() > 0) { 1952 if(subc->name_.empty() && subc->count_all() > 0) {
1951 - subc->run_callback(true); 1953 + subc->run_callback(true, suppress_final_callback);
1952 } 1954 }
1953 } 1955 }
1954 1956
1955 // finally run the main callback 1957 // finally run the main callback
1956 - if(final_callback_ && (parsed_ > 0)) {  
1957 - if(!name_.empty() || count_all() > 0) { 1958 + if(final_callback_ && (parsed_ > 0) && (!suppress_final_callback)) {
  1959 + if(!name_.empty() || count_all() > 0 || parent_ == nullptr) {
1958 final_callback_(); 1960 final_callback_();
1959 } 1961 }
1960 } 1962 }
@@ -2337,7 +2339,7 @@ class App { @@ -2337,7 +2339,7 @@ class App {
2337 _process_callbacks(); 2339 _process_callbacks();
2338 _process_help_flags(); 2340 _process_help_flags();
2339 _process_requirements(); 2341 _process_requirements();
2340 - run_callback(); 2342 + run_callback(false, true);
2341 } 2343 }
2342 } 2344 }
2343 2345
tests/SubcommandTest.cpp
  View file @d0a2aa7
@@ -1874,3 +1874,63 @@ TEST_CASE_METHOD(ManySubcommands, &quot;defaultEnabledSubcommand&quot;, &quot;[subcom]&quot;) { @@ -1874,3 +1874,63 @@ TEST_CASE_METHOD(ManySubcommands, &quot;defaultEnabledSubcommand&quot;, &quot;[subcom]&quot;) {
1874 CHECK(sub2->get_enabled_by_default()); 1874 CHECK(sub2->get_enabled_by_default());
1875 CHECK(!sub2->get_disabled()); 1875 CHECK(!sub2->get_disabled());
1876 } 1876 }
  1877 +
  1878 +// #572
  1879 +TEST_CASE_METHOD(TApp, "MultiFinalCallbackCounts", "[subcom]") {
  1880 +
  1881 + int app_compl = 0;
  1882 + int sub_compl = 0;
  1883 + int subsub_compl = 0;
  1884 + int app_final = 0;
  1885 + int sub_final = 0;
  1886 + int subsub_final = 0;
  1887 +
  1888 + app.parse_complete_callback([&app_compl]() { app_compl++; });
  1889 + app.final_callback([&app_final]() { app_final++; });
  1890 +
  1891 + auto *sub = app.add_subcommand("sub");
  1892 +
  1893 + sub->parse_complete_callback([&sub_compl]() { sub_compl++; });
  1894 + sub->final_callback([&sub_final]() { sub_final++; });
  1895 +
  1896 + auto *subsub = sub->add_subcommand("subsub");
  1897 +
  1898 + subsub->parse_complete_callback([&subsub_compl]() { subsub_compl++; });
  1899 + subsub->final_callback([&subsub_final]() { subsub_final++; });
  1900 +
  1901 + SECTION("No specified subcommands") {
  1902 + args = {};
  1903 + run();
  1904 +
  1905 + CHECK(app_compl == 1);
  1906 + CHECK(app_final == 1);
  1907 + CHECK(sub_compl == 0);
  1908 + CHECK(sub_final == 0);
  1909 + CHECK(subsub_compl == 0);
  1910 + CHECK(subsub_final == 0);
  1911 + }
  1912 +
  1913 + SECTION("One layer of subcommands") {
  1914 + args = {"sub"};
  1915 + run();
  1916 +
  1917 + CHECK(app_compl == 1);
  1918 + CHECK(app_final == 1);
  1919 + CHECK(sub_compl == 1);
  1920 + CHECK(sub_final == 1);
  1921 + CHECK(subsub_compl == 0);
  1922 + CHECK(subsub_final == 0);
  1923 + }
  1924 +
  1925 + SECTION("Fully specified subcommands") {
  1926 + args = {"sub", "subsub"};
  1927 + run();
  1928 +
  1929 + CHECK(app_compl == 1);
  1930 + CHECK(app_final == 1);
  1931 + CHECK(sub_compl == 1);
  1932 + CHECK(sub_final == 1);
  1933 + CHECK(subsub_compl == 1);
  1934 + CHECK(subsub_final == 1);
  1935 + }
  1936 +}