Footer callback (#309)

fix incorrect parenthesis

update some clang-tidy fixes mainly else after return but a few conversions from into to bool

add extra newline before footer

add an extra field to the extra Error

add a footer callback for help operations

README.md

@@ -554,6 +554,7 @@ There are several options that are supported on the main app and subcommands and
 -   `.positionals_at_end()`: 🆕 Specify that positional arguments occur as the last arguments and throw an error if an unexpected positional is encountered.
 -   `.prefix_command()`: Like `allow_extras`, but stop immediately on the first unrecognized item. It is ideal for allowing your app or subcommand to be a "prefix" to calling another app.
 -   `.footer(message)`: Set text to appear at the bottom of the help string.
+-   `.footer(std::string())`: 🚧 Set a callback to generate a string that will appear at the end of the help string.
 -   `.set_help_flag(name, message)`: Set the help flag name and message, returns a pointer to the created option.
 -   `.set_help_all_flag(name, message)`: Set the help all flag name and message, returns a pointer to the created option. Expands subcommands.
 -   `.failure_message(func)`: Set the failure message function. Two provided: `CLI::FailureMessage::help` and `CLI::FailureMessage::simple` (the default).

include/CLI/App.hpp

@@ -118,6 +118,9 @@ class App {
     /// Footer to put after all options in the help output INHERITABLE
     std::string footer_;
 
+    /// This is a function that generates a footer to put after all other options in help output
+    std::function<std::string()> footer_callback_;
+
     /// A pointer to the help flag if there is one INHERITABLE
     Option *help_ptr_{nullptr};
 
@@ -461,9 +464,8 @@ class App {
                 option->capture_default_str();
 
             return option.get();
-
-        } else
-            throw OptionAlreadyAdded(myopt.get_name());
+        }
+        throw OptionAlreadyAdded(myopt.get_name());
     }
 
     /// Add option for non-vectors (duplicate copy needed without defaulted to avoid `iostream << value`)
@@ -950,6 +952,7 @@ class App {
             remove_option(config_ptr_);
             config_name_ = "";
             config_required_ = false; // Not really needed, but complete
+            config_ptr_ = nullptr;    // need to remove the config_ptr completely
         }
 
         // Only add config if option passed
@@ -1423,25 +1426,23 @@ class App {
     /// Removes an option from the excludes list of this subcommand
     bool remove_excludes(Option *opt) {
         auto iterator = std::find(std::begin(exclude_options_), std::end(exclude_options_), opt);
-        if(iterator != std::end(exclude_options_)) {
-            exclude_options_.erase(iterator);
-            return true;
-        } else {
+        if(iterator == std::end(exclude_options_)) {
             return false;
         }
+        exclude_options_.erase(iterator);
+        return true;
     }
 
     /// Removes a subcommand from this excludes list of this subcommand
     bool remove_excludes(App *app) {
         auto iterator = std::find(std::begin(exclude_subcommands_), std::end(exclude_subcommands_), app);
-        if(iterator != std::end(exclude_subcommands_)) {
-            auto other_app = *iterator;
-            exclude_subcommands_.erase(iterator);
-            other_app->remove_excludes(this);
-            return true;
-        } else {
+        if(iterator == std::end(exclude_subcommands_)) {
             return false;
         }
+        auto other_app = *iterator;
+        exclude_subcommands_.erase(iterator);
+        other_app->remove_excludes(this);
+        return true;
     }
 
     ///@}
@@ -1453,7 +1454,11 @@ class App {
         footer_ = std::move(footer_string);
         return this;
     }
-
+    /// Set footer.
+    App *footer(std::function<std::string()> footer_function) {
+        footer_callback_ = std::move(footer_function);
+        return this;
+    }
     /// Produce a string that could be read in as a config of the current values of the App. Set default_also to
     /// include default arguments. Prefix will add a string to the beginning of each option.
     std::string config_to_str(bool default_also = false, bool write_description = false) const {
@@ -1470,10 +1475,10 @@ class App {
 
         // Delegate to subcommand if needed
         auto selected_subcommands = get_subcommands();
-        if(!selected_subcommands.empty())
+        if(!selected_subcommands.empty()) {
             return selected_subcommands.at(0)->help(prev, mode);
-        else
-            return formatter_->make_help(this, prev, mode);
+        }
+        return formatter_->make_help(this, prev, mode);
     }
 
     ///@}
@@ -1592,8 +1597,8 @@ class App {
     /// Get the group of this subcommand
     const std::string &get_group() const { return group_; }
 
-    /// Get footer.
-    const std::string &get_footer() const { return footer_; }
+    /// Generate and return the footer.
+    std::string get_footer() const { return (footer_callback_) ? footer_callback_() + '\n' + footer_ : footer_; }
 
     /// Get the required min subcommand value
     size_t get_require_subcommand_min() const { return require_subcommand_min_; }
@@ -2090,7 +2095,7 @@ class App {
         if(!(allow_extras_ || prefix_command_)) {
             size_t num_left_over = remaining_size();
             if(num_left_over > 0) {
-                throw ExtrasError(remaining(false));
+                throw ExtrasError(name_, remaining(false));
             }
         }
 
@@ -2107,7 +2112,7 @@ class App {
             size_t num_left_over = remaining_size();
             if(num_left_over > 0) {
                 args = remaining(false);
-                throw ExtrasError(args);
+                throw ExtrasError(name_, args);
             }
         }
 
@@ -2372,7 +2377,7 @@ class App {
         }
 
         if(positionals_at_end_) {
-            throw CLI::ExtrasError(args);
+            throw CLI::ExtrasError(name_, args);
         }
         /// If this is an option group don't deal with it
         if(parent_ != nullptr && name_.empty()) {

include/CLI/ConfigFwd.hpp

@@ -93,7 +93,8 @@ class Config {
 /// This converter works with INI files
 class ConfigINI : public Config {
   public:
-    std::string to_config(const App *, bool default_also, bool write_description, std::string prefix) const override;
+    std::string
+    to_config(const App * /*app*/, bool default_also, bool write_description, std::string prefix) const override;
 
     std::vector<ConfigItem> from_config(std::istream &input) const override {
         std::string line;

include/CLI/Error.hpp

@@ -206,32 +206,32 @@ class RequiredError : public ParseError {
     CLI11_ERROR_DEF(ParseError, RequiredError)
     explicit RequiredError(std::string name) : RequiredError(name + " is required", ExitCodes::RequiredError) {}
     static RequiredError Subcommand(size_t min_subcom) {
-        if(min_subcom == 1)
+        if(min_subcom == 1) {
             return RequiredError("A subcommand");
-        else
-            return RequiredError("Requires at least " + std::to_string(min_subcom) + " subcommands",
-                                 ExitCodes::RequiredError);
+        }
+        return RequiredError("Requires at least " + std::to_string(min_subcom) + " subcommands",
+                             ExitCodes::RequiredError);
     }
     static RequiredError Option(size_t min_option, size_t max_option, size_t used, const std::string &option_list) {
         if((min_option == 1) && (max_option == 1) && (used == 0))
             return RequiredError("Exactly 1 option from [" + option_list + "]");
-        else if((min_option == 1) && (max_option == 1) && (used > 1))
+        if((min_option == 1) && (max_option == 1) && (used > 1))
             return RequiredError("Exactly 1 option from [" + option_list + "] is required and " + std::to_string(used) +
                                      " were given",
                                  ExitCodes::RequiredError);
-        else if((min_option == 1) && (used == 0))
+        if((min_option == 1) && (used == 0))
             return RequiredError("At least 1 option from [" + option_list + "]");
-        else if(used < min_option)
+        if(used < min_option)
             return RequiredError("Requires at least " + std::to_string(min_option) + " options used and only " +
                                      std::to_string(used) + "were given from [" + option_list + "]",
                                  ExitCodes::RequiredError);
-        else if(max_option == 1)
+        if(max_option == 1)
             return RequiredError("Requires at most 1 options be given from [" + option_list + "]",
                                  ExitCodes::RequiredError);
-        else
-            return RequiredError("Requires at most " + std::to_string(max_option) + " options be used and " +
-                                     std::to_string(used) + "were given from [" + option_list + "]",
-                                 ExitCodes::RequiredError);
+
+        return RequiredError("Requires at most " + std::to_string(max_option) + " options be used and " +
+                                 std::to_string(used) + "were given from [" + option_list + "]",
+                             ExitCodes::RequiredError);
     }
 };
 
@@ -279,6 +279,12 @@ class ExtrasError : public ParseError {
                                        : "The following argument was not expected: ") +
                           detail::rjoin(args, " "),
                       ExitCodes::ExtrasError) {}
+    ExtrasError(const std::string &name, std::vector<std::string> args)
+        : ExtrasError(name,
+                      (args.size() > 1 ? "The following arguments were not expected: "
+                                       : "The following argument was not expected: ") +
+                          detail::rjoin(args, " "),
+                      ExitCodes::ExtrasError) {}
 };
 
 /// Thrown when extra values are found in an INI file

include/CLI/Formatter.hpp

@@ -28,8 +28,8 @@ inline std::string Formatter::make_positionals(const App *app) const {
 
     if(opts.empty())
         return std::string();
-    else
-        return make_group(get_label("Positionals"), true, opts);
+
+    return make_group(get_label("Positionals"), true, opts);
 }
 
 inline std::string Formatter::make_groups(const App *app, AppFormatMode mode) const {
@@ -126,10 +126,10 @@ inline std::string Formatter::make_usage(const App *app, std::string name) const
 
 inline std::string Formatter::make_footer(const App *app) const {
     std::string footer = app->get_footer();
-    if(!footer.empty())
-        return footer + "\n";
-    else
-        return "";
+    if(footer.empty()) {
+        return std::string{};
+    }
+    return footer + "\n";
 }
 
 inline std::string Formatter::make_help(const App *app, std::string name, AppFormatMode mode) const {
@@ -151,7 +151,7 @@ inline std::string Formatter::make_help(const App *app, std::string name, AppFor
     out << make_positionals(app);
     out << make_groups(app, mode);
     out << make_subcommands(app, mode);
-    out << make_footer(app);
+    out << '\n' << make_footer(app);
 
     return out.str();
 }
@@ -222,8 +222,8 @@ inline std::string Formatter::make_expanded(const App *sub) const {
 inline std::string Formatter::make_option_name(const Option *opt, bool is_positional) const {
     if(is_positional)
         return opt->get_name(true, false);
-    else
-        return opt->get_name(false, true);
+
+    return opt->get_name(false, true);
 }
 
 inline std::string Formatter::make_option_opts(const Option *opt) const {

include/CLI/FormatterFwd.hpp

@@ -144,7 +144,7 @@ class Formatter : public FormatterBase {
     virtual std::string make_usage(const App *app, std::string name) const;
 
     /// This puts everything together
-    std::string make_help(const App *, std::string, AppFormatMode) const override;
+    std::string make_help(const App * /*app*/, std::string, AppFormatMode) const override;
 
     ///@}
     /// @name Options

include/CLI/Option.hpp

@@ -307,7 +307,7 @@ class Option : public OptionBase<Option> {
     size_t count() const { return results_.size(); }
 
     /// True if the option was not passed
-    size_t empty() const { return results_.empty(); }
+    bool empty() const { return results_.empty(); }
 
     /// This class is true if option is passed.
     operator bool() const { return !empty(); }
@@ -327,19 +327,19 @@ class Option : public OptionBase<Option> {
             throw IncorrectConstruction::SetFlag(get_name(true, true));
 
         // Setting 0 is not allowed
-        else if(value == 0)
+        if(value == 0)
             throw IncorrectConstruction::Set0Opt(get_name());
 
         // No change is okay, quit now
-        else if(expected_ == value)
+        if(expected_ == value)
             return this;
 
         // Type must be a vector
-        else if(type_size_ >= 0)
+        if(type_size_ >= 0)
             throw IncorrectConstruction::ChangeNotVector(get_name());
 
         // TODO: Can support multioption for non-1 values (except for join)
-        else if(value != 1 && multi_option_policy_ != MultiOptionPolicy::Throw)
+        if(value != 1 && multi_option_policy_ != MultiOptionPolicy::Throw)
             throw IncorrectConstruction::AfterMultiOpt(get_name());
 
         expected_ = value;
@@ -436,12 +436,11 @@ class Option : public OptionBase<Option> {
     bool remove_needs(Option *opt) {
         auto iterator = std::find(std::begin(needs_), std::end(needs_), opt);
 
-        if(iterator != std::end(needs_)) {
-            needs_.erase(iterator);
-            return true;
-        } else {
+        if(iterator == std::end(needs_)) {
             return false;
         }
+        needs_.erase(iterator);
+        return true;
     }
 
     /// Sets excluded options
@@ -475,12 +474,11 @@ class Option : public OptionBase<Option> {
     bool remove_excludes(Option *opt) {
         auto iterator = std::find(std::begin(excludes_), std::end(excludes_), opt);
 
-        if(iterator != std::end(excludes_)) {
-            excludes_.erase(iterator);
-            return true;
-        } else {
+        if(iterator == std::end(excludes_)) {
             return false;
         }
+        excludes_.erase(iterator);
+        return true;
     }
 
     /// Sets environment variable to read if no option given
@@ -626,8 +624,9 @@ class Option : public OptionBase<Option> {
             std::vector<std::string> name_list;
 
             /// The all list will never include a positional unless asked or that's the only name.
-            if((positional && pname_.length()) || (snames_.empty() && lnames_.empty()))
+            if((positional && (!pname_.empty())) || (snames_.empty() && lnames_.empty())) {
                 name_list.push_back(pname_);
+            }
             if((get_items_expected() == 0) && (!fnames_.empty())) {
                 for(const std::string &sname : snames_) {
                     name_list.push_back("-" + sname);
@@ -651,25 +650,22 @@ class Option : public OptionBase<Option> {
             }
 
             return detail::join(name_list);
-
-        } else {
-
-            // This returns the positional name no matter what
-            if(positional)
-                return pname_;
-
-            // Prefer long name
-            else if(!lnames_.empty())
-                return std::string("--") + lnames_[0];
-
-            // Or short name if no long name
-            else if(!snames_.empty())
-                return std::string("-") + snames_[0];
-
-            // If positional is the only name, it's okay to use that
-            else
-                return pname_;
         }
+
+        // This returns the positional name no matter what
+        if(positional)
+            return pname_;
+
+        // Prefer long name
+        if(!lnames_.empty())
+            return std::string("--") + lnames_[0];
+
+        // Or short name if no long name
+        if(!snames_.empty())
+            return std::string("-") + snames_[0];
+
+        // If positional is the only name, it's okay to use that
+        return pname_;
     }
 
     ///@}
@@ -758,20 +754,19 @@ class Option : public OptionBase<Option> {
 
         if(name.length() > 2 && name[0] == '-' && name[1] == '-')
             return check_lname(name.substr(2));
-        else if(name.length() > 1 && name.front() == '-')
+        if(name.length() > 1 && name.front() == '-')
             return check_sname(name.substr(1));
-        else {
-            std::string local_pname = pname_;
-            if(ignore_underscore_) {
-                local_pname = detail::remove_underscore(local_pname);
-                name = detail::remove_underscore(name);
-            }
-            if(ignore_case_) {
-                local_pname = detail::to_lower(local_pname);
-                name = detail::to_lower(name);
-            }
-            return name == local_pname;
+
+        std::string local_pname = pname_;
+        if(ignore_underscore_) {
+            local_pname = detail::remove_underscore(local_pname);
+            name = detail::remove_underscore(name);
         }
+        if(ignore_case_) {
+            local_pname = detail::to_lower(local_pname);
+            name = detail::to_lower(name);
+        }
+        return name == local_pname;
     }
 
     /// Requires "-" to be removed from string
@@ -1017,6 +1012,6 @@ class Option : public OptionBase<Option> {
         }
         return result_count;
     }
-};
+}; // namespace CLI
 
 } // namespace CLI

include/CLI/Split.hpp

@@ -19,8 +19,8 @@ inline bool split_short(const std::string &current, std::string &name, std::stri
         name = current.substr(1, 1);
         rest = current.substr(2);
         return true;
-    } else
-        return false;
+    }
+    return false;
 }
 
 // Returns false if not a long option. Otherwise, sets opt name and other side of = and returns true
@@ -35,8 +35,8 @@ inline bool split_long(const std::string &current, std::string &name, std::strin
             value = "";
         }
         return true;
-    } else
-        return false;
+    }
+    return false;
 }
 
 // Returns false if not a windows style option. Otherwise, sets opt name and value and returns true
@@ -51,8 +51,8 @@ inline bool split_windows_style(const std::string &current, std::string &name, s
             value = "";
         }
         return true;
-    } else
-        return false;
+    }
+    return false;
 }
 
 // Splits a string into multiple long and short names
@@ -103,9 +103,10 @@ get_names(const std::vector<std::string> &input) {
     std::string pos_name;
 
     for(std::string name : input) {
-        if(name.length() == 0)
+        if(name.length() == 0) {
             continue;
-        else if(name.length() > 1 && name[0] == '-' && name[1] != '-') {
+        }
+        if(name.length() > 1 && name[0] == '-' && name[1] != '-') {
             if(name.length() == 2 && valid_first_char(name[1]))
                 short_names.emplace_back(1, name[1]);
             else

include/CLI/Validators.hpp

@@ -160,8 +160,8 @@ class Validator {
             std::string s2 = f2(input);
             if(s1.empty() || s2.empty())
                 return std::string();
-            else
-                return std::string("(") + s1 + ") OR (" + s2 + ")";
+
+            return std::string("(") + s1 + ") OR (" + s2 + ")";
         };
         newval.active_ = (active_ & other.active_);
         return newval;
@@ -182,8 +182,8 @@ class Validator {
             std::string s1 = f1(test);
             if(s1.empty()) {
                 return std::string("check ") + dfunc1() + " succeeded improperly";
-            } else
-                return std::string{};
+            }
+            return std::string{};
         };
         newval.active_ = active_;
         return newval;
@@ -225,7 +225,8 @@ class ExistingFileValidator : public Validator {
             bool is_dir = (buffer.st_mode & S_IFDIR) != 0;
             if(!exist) {
                 return "File does not exist: " + filename;
-            } else if(is_dir) {
+            }
+            if(is_dir) {
                 return "File is actually a directory: " + filename;
             }
             return std::string();
@@ -243,7 +244,8 @@ class ExistingDirectoryValidator : public Validator {
             bool is_dir = (buffer.st_mode & S_IFDIR) != 0;
             if(!exist) {
                 return "Directory does not exist: " + filename;
-            } else if(!is_dir) {
+            }
+            if(!is_dir) {
                 return "Directory is actually a file: " + filename;
             }
             return std::string();

tests/FormatterTest.cpp

@@ -60,7 +60,7 @@ TEST(Formatter, OptCustomize) {
               "Usage: [OPTIONS]\n\n"
               "Options:\n"
               "  -h,--help              Print this help message and exit\n"
-              "  --opt INT (MUST HAVE)  Something\n");
+              "  --opt INT (MUST HAVE)  Something\n\n");
 }
 
 TEST(Formatter, OptCustomizeSimple) {
@@ -80,7 +80,7 @@ TEST(Formatter, OptCustomizeSimple) {
               "Usage: [OPTIONS]\n\n"
               "Options:\n"
               "  -h,--help              Print this help message and exit\n"
-              "  --opt INT (MUST HAVE)  Something\n");
+              "  --opt INT (MUST HAVE)  Something\n\n");
 }
 
 TEST(Formatter, FalseFlagExample) {
@@ -121,7 +121,7 @@ TEST(Formatter, AppCustomize) {
               "  -h,--help         Print this help message and exit\n\n"
               "Subcommands:\n"
               "  subcom1           This\n"
-              "  subcom2           This\n");
+              "  subcom2           This\n\n");
 }
 
 TEST(Formatter, AppCustomizeSimple) {
@@ -141,7 +141,7 @@ TEST(Formatter, AppCustomizeSimple) {
               "  -h,--help         Print this help message and exit\n\n"
               "Subcommands:\n"
               "  subcom1           This\n"
-              "  subcom2           This\n");
+              "  subcom2           This\n\n");
 }
 
 TEST(Formatter, AllSub) {

tests/HelpTest.cpp

@@ -35,6 +35,33 @@ TEST(THelp, Footer) {
     EXPECT_THAT(help, HasSubstr("Report bugs to bugs@example.com"));
 }
 
+TEST(THelp, FooterCallback) {
+    CLI::App app{"My prog"};
+    app.footer([]() { return "Report bugs to bugs@example.com"; });
+
+    std::string help = app.help();
+
+    EXPECT_THAT(help, HasSubstr("My prog"));
+    EXPECT_THAT(help, HasSubstr("-h,--help"));
+    EXPECT_THAT(help, HasSubstr("Options:"));
+    EXPECT_THAT(help, HasSubstr("Usage:"));
+    EXPECT_THAT(help, HasSubstr("Report bugs to bugs@example.com"));
+}
+
+TEST(THelp, FooterCallbackBoth) {
+    CLI::App app{"My prog"};
+    app.footer([]() { return "Report bugs to bugs@example.com"; });
+    app.footer(" foot!!!!");
+    std::string help = app.help();
+
+    EXPECT_THAT(help, HasSubstr("My prog"));
+    EXPECT_THAT(help, HasSubstr("-h,--help"));
+    EXPECT_THAT(help, HasSubstr("Options:"));
+    EXPECT_THAT(help, HasSubstr("Usage:"));
+    EXPECT_THAT(help, HasSubstr("Report bugs to bugs@example.com"));
+    EXPECT_THAT(help, HasSubstr("foot!!!!"));
+}
+
 TEST(THelp, OptionalPositional) {
     CLI::App app{"My prog", "program"};
 
@@ -602,7 +629,7 @@ TEST_F(CapturedHelp, CallForAllHelpOutput) {
               "  One description\n\n"
               "two\n"
               "  Options:\n"
-              "    --three                     \n\n");
+              "    --three                     \n\n\n");
 }
 TEST_F(CapturedHelp, NewFormattedHelp) {
     app.formatter_fn([](const CLI::App *, std::string, CLI::AppFormatMode) { return "New Help"; });

tests/IniTest.cpp

@@ -896,7 +896,8 @@ TEST_F(TApp, StopReadingConfigOnClear) {
     TempFile tmpini{"TestIniTmp.ini"};
 
     app.set_config("--config", tmpini);
-    app.set_config(); // Should *not* read config file
+    auto ptr = app.set_config(); // Should *not* read config file
+    EXPECT_EQ(ptr, nullptr);
 
     {
         std::ofstream out{tmpini};