Use filter fn to simplify, sort includes

CLIUtils · May 6, 2018 · 101d847 · 101d847
1 parent 3ff6fca
commit 101d847
Show file tree

Hide file tree

Showing 18 changed files with 105 additions and 67 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -33,6 +33,8 @@ Other changes:
 * Testing (only) now uses submodules. [#111]
 * Removed `requires` in favor of `needs` (deprecated in last version) [#112]
 * Better CMake policy handling [#110]
+* Includes are properly sorted [#120]
+* `lname` and `sname` have getters, added `const get_parent` [#120] 
 
 [#109]: https://github.com/CLIUtils/CLI11/pull/109
 [#110]: https://github.com/CLIUtils/CLI11/pull/110
@@ -42,6 +44,7 @@ Other changes:
 [#116]: https://github.com/CLIUtils/CLI11/pull/116
 [#118]: https://github.com/CLIUtils/CLI11/pull/118
 [#118]: https://github.com/CLIUtils/CLI11/pull/119
+[#120]: https://github.com/CLIUtils/CLI11/pull/120
 
 ### Version 1.5.3: Compiler compatibility
 This version fixes older AppleClang compilers by removing the optimization for casting. The minimum version of Boost Optional supported has been clarified to be 1.58. CUDA 7.0 NVCC is now supported.

diff --git a/examples/enum.cpp b/examples/enum.cpp
@@ -1,5 +1,5 @@
-#include <sstream>
 #include <CLI/CLI.hpp>
+#include <sstream>
 
 enum class Level : int { High, Medium, Low };
 

diff --git a/examples/inter_argument_order.cpp b/examples/inter_argument_order.cpp
@@ -1,8 +1,8 @@
 #include <CLI/CLI.hpp>
+#include <algorithm>
 #include <iostream>
-#include <vector>
 #include <tuple>
-#include <algorithm>
+#include <vector>
 
 int main(int argc, char **argv) {
     CLI::App app{"An app to practice mixing unlimited arguments, but still recover the original order."};

diff --git a/include/CLI/App.hpp b/include/CLI/App.hpp
@@ -1013,8 +1013,8 @@ class App {
         for(const Option_p &opt : options_) {
 
             // Only process option with a long-name and configurable
-            if(!opt->lnames_.empty() && opt->get_configurable()) {
-                std::string name = prefix + opt->lnames_[0];
+            if(!opt->get_lnames().empty() && opt->get_configurable()) {
+                std::string name = prefix + opt->get_lnames()[0];
                 std::string value;
 
                 // Non-flags
@@ -1025,8 +1025,8 @@ class App {
                         value = detail::inijoin(opt->results());
 
                     // If the option has a default and is requested by optional argument
-                    else if(default_also && !opt->defaultval_.empty())
-                        value = opt->defaultval_;
+                    else if(default_also && !opt->get_defaultval().empty())
+                        value = opt->get_defaultval();
                     // Flag, one passed
                 } else if(opt->count() == 1) {
                     value = "true";
@@ -1148,6 +1148,9 @@ class App {
     /// Get the parent of this subcommand (or nullptr if master app)
     App *get_parent() { return parent_; }
 
+    /// Get the parent of this subcommand (or nullptr if master app) (const version)
+    const App *get_parent() const { return parent_; }
+
     /// Get a pointer to the config option. (const)
     const Option *get_config_ptr() const { return config_ptr_; }
 

diff --git a/include/CLI/Formatter.hpp b/include/CLI/Formatter.hpp
@@ -10,42 +10,44 @@
 
 namespace CLI {
 
-inline std::string
-Formatter::make_group(std::string group, std::vector<const Option *> opts, bool is_positional) const {
+inline std::string Formatter::make_group(const App *app,
+                                         std::string group,
+                                         bool is_positional,
+                                         std::function<bool(const Option *)> filter) const {
     std::stringstream out;
-    out << "\n" << group << ":\n";
-    for(const Option *opt : opts) {
-        out << make_option(opt, is_positional);
+    std::vector<const Option *> opts = app->get_options(filter);
+
+    if(!opts.empty()) {
+        out << "\n" << group << ":\n";
+        for(const Option *opt : opts) {
+            out << make_option(opt, is_positional);
+        }
     }
 
     return out.str();
 }
 
+inline std::string Formatter::make_positionals(const App *app) const {
+    return make_group(app, get_label("Positionals"), true, [](const Option *opt) {
+        return !opt->get_group().empty() && opt->get_positional();
+    });
+}
+
 inline std::string Formatter::make_groups(const App *app, AppFormatMode mode) const {
     std::stringstream out;
     std::vector<std::string> groups = app->get_groups();
-    std::vector<const Option *> positionals =
-        app->get_options([](const Option *opt) { return !opt->get_group().empty() && opt->get_positional(); });
-
-    if(!positionals.empty())
-        out << make_group(get_label("Positionals"), positionals, true);
 
     // Options
     for(const std::string &group : groups) {
-        std::vector<const Option *> grouped_items =
-            app->get_options([&group](const Option *opt) { return opt->nonpositional() && opt->get_group() == group; });
-
-        if(mode == AppFormatMode::Sub) {
-            grouped_items.erase(std::remove_if(grouped_items.begin(),
-                                               grouped_items.end(),
-                                               [app](const Option *opt) {
-                                                   return app->get_help_ptr() == opt || app->get_help_all_ptr() == opt;
-                                               }),
-                                grouped_items.end());
-        }
+        if(!group.empty()) {
+            out << make_group(app, group, false, [app, mode, &group](const Option *opt) {
+                return opt->get_group() == group                    // Must be in the right group
+                       && opt->nonpositional()                      // Must not be a positional
+                       && (mode != AppFormatMode::Sub               // If mode is Sub, then
+                           || (app->get_help_ptr() != opt           // Ignore help pointer
+                               && app->get_help_all_ptr() != opt)); // Ignore help all pointer
+            });
 
-        if(!group.empty() && !grouped_items.empty()) {
-            out << make_group(group, grouped_items, false);
             if(group != groups.back())
                 out << "\n";
         }
@@ -113,21 +115,19 @@ inline std::string Formatter::make_footer(const App *app) const {
 
 inline std::string Formatter::operator()(const App *app, std::string name, AppFormatMode mode) const {
 
+    // This immediatly forwards to the make_expanded method. This is done this way so that subcommands can
+    // have overridden formatters
+    if(mode == AppFormatMode::Sub)
+        return make_expanded(app);
+
     std::stringstream out;
-    if(mode == AppFormatMode::Normal) {
-        out << make_description(app);
-        out << make_usage(app, name);
-        out << make_groups(app, mode);
-        out << make_subcommands(app, mode);
-        out << make_footer(app);
-    } else if(mode == AppFormatMode::Sub) {
-        out << make_expanded(app);
-    } else if(mode == AppFormatMode::All) {
-        out << make_description(app);
-        out << make_usage(app, name);
-        out << make_groups(app, mode);
-        out << make_subcommands(app, mode);
-    }
+
+    out << make_description(app);
+    out << make_usage(app, name);
+    out << make_positionals(app);
+    out << make_groups(app, mode);
+    out << make_subcommands(app, mode);
+    out << make_footer(app);
 
     return out.str();
 }
@@ -151,8 +151,6 @@ inline std::string Formatter::make_subcommands(const App *app, AppFormatMode mod
     // For each group, filter out and print subcommands
     for(const std::string &group : subcmd_groups_seen) {
         out << "\n" << group << ":\n";
-        if(mode == AppFormatMode::All)
-            out << "\n";
         std::vector<const App *> subcommands_group = app->get_subcommands(
             [&group](const App *app) { return detail::to_lower(app->get_group()) == detail::to_lower(group); });
         for(const App *new_com : subcommands_group) {
@@ -177,7 +175,10 @@ inline std::string Formatter::make_subcommand(const App *sub) const {
 
 inline std::string Formatter::make_expanded(const App *sub) const {
     std::stringstream out;
-    out << sub->get_name() << "\n  " << sub->get_description();
+    if(sub->get_description().empty())
+        out << sub->get_name();
+    else
+        out << sub->get_name() << " -> " << sub->get_description();
     out << make_groups(sub, AppFormatMode::Sub);
     return out.str();
 }

diff --git a/include/CLI/FormatterFwd.hpp b/include/CLI/FormatterFwd.hpp
@@ -3,8 +3,8 @@
 // Distributed under the 3-Clause BSD License.  See accompanying
 // file LICENSE or https://github.com/CLIUtils/CLI11 for details.
 
-#include <string>
 #include <map>
+#include <string>
 
 #include "CLI/StringTools.hpp"
 
@@ -72,10 +72,16 @@ class Formatter {
     ///@{
 
     /// This prints out a group of options
-    virtual std::string make_group(std::string group, std::vector<const Option *> opts, bool is_positional) const;
+    ///
+    /// Use the filter to pick out the items you want in your group
+    virtual std::string
+    make_group(const App *app, std::string group, bool is_positional, std::function<bool(const Option *)> filter) const;
+
+    /// This prints out just the positionals "group"
+    virtual std::string make_positionals(const App *app) const;
 
     /// This prints out all the groups of options
-    virtual std::string make_groups(const App *app, AppFormatMode mode) const;
+    std::string make_groups(const App *app, AppFormatMode mode) const;
 
     /// This prints out all the subcommands
     virtual std::string make_subcommands(const App *app, AppFormatMode mode) const;
@@ -102,6 +108,7 @@ class Formatter {
     /// @name Options
     ///@{
 
+    /// This prints out an option help line, either positional or optional form
     virtual std::string make_option(const Option *opt, bool is_positional) const {
         std::stringstream out;
         detail::format_help(

diff --git a/include/CLI/Option.hpp b/include/CLI/Option.hpp
@@ -402,6 +402,12 @@ class Option : public OptionBase<Option> {
     /// The default value (for help printing)
     std::string get_defaultval() const { return defaultval_; }
 
+    /// Get the long names
+    const std::vector<std::string> get_lnames() const { return lnames_; }
+
+    /// Get the short names
+    const std::vector<std::string> get_snames() const { return snames_; }
+
     /// The number of times the option expects to be included
     int get_expected() const { return expected_; }
 

diff --git a/include/CLI/StringTools.hpp b/include/CLI/StringTools.hpp
@@ -8,8 +8,8 @@
 #include <locale>
 #include <sstream>
 #include <string>
-#include <vector>
 #include <type_traits>
+#include <vector>
 
 namespace CLI {
 namespace detail {

diff --git a/include/CLI/Validators.hpp b/include/CLI/Validators.hpp
@@ -28,7 +28,7 @@ namespace CLI {
 
 ///
 struct Validator {
-    /// This is the type name, if emtpy the type name will not be changed
+    /// This is the type name, if empty the type name will not be changed
     std::string tname;
     std::function<std::string(const std::string &filename)> func;
 
@@ -77,7 +77,7 @@ struct Validator {
     }
 };
 
-// The implemntation of the built in validators is using the Validator class;
+// The implementation of the built in validators is using the Validator class;
 // the user is only expected to use the const (static) versions (since there's no setup).
 // Therefore, this is in detail.
 namespace detail {

diff --git a/scripts/check_style.sh b/scripts/check_style.sh
@@ -3,7 +3,7 @@ set -evx
 
 clang-format --version
 
-git ls-files -- '*.cpp' '*.hpp' | xargs clang-format -i -style=file
+git ls-files -- '*.cpp' '*.hpp' | xargs clang-format -sort-includes -i -style=file
 
 git diff --exit-code --color
 

diff --git a/tests/AppTest.cpp b/tests/AppTest.cpp
@@ -1,6 +1,6 @@
 #include "app_helper.hpp"
-#include <cstdlib>
 #include <complex>
+#include <cstdlib>
 
 TEST_F(TApp, OneFlagShort) {
     app.add_flag("-c,--count");

diff --git a/tests/CreationTest.cpp b/tests/CreationTest.cpp
@@ -2,7 +2,10 @@
 #include <cstdlib>
 
 TEST_F(TApp, AddingExistingShort) {
-    app.add_flag("-c,--count");
+    CLI::Option *opt = app.add_flag("-c,--count");
+    EXPECT_EQ(opt->get_lnames(), std::vector<std::string>({"count"}));
+    EXPECT_EQ(opt->get_snames(), std::vector<std::string>({"c"}));
+
     EXPECT_THROW(app.add_flag("--cat,-c"), CLI::OptionAlreadyAdded);
 }
 

diff --git a/tests/FormatterTest.cpp b/tests/FormatterTest.cpp
@@ -4,8 +4,8 @@
 #include "CLI/CLI.hpp"
 #endif
 
-#include "gtest/gtest.h"
 #include "gmock/gmock.h"
+#include "gtest/gtest.h"
 #include <fstream>
 
 using ::testing::HasSubstr;

diff --git a/tests/HelpTest.cpp b/tests/HelpTest.cpp
@@ -4,8 +4,8 @@
 #include "CLI/CLI.hpp"
 #endif
 
-#include "gtest/gtest.h"
 #include "gmock/gmock.h"
+#include "gtest/gtest.h"
 #include <fstream>
 
 using ::testing::HasSubstr;
@@ -492,7 +492,8 @@ TEST_F(CapturedHelp, CallForAllHelp) {
     EXPECT_EQ(err.str(), "");
 }
 TEST_F(CapturedHelp, CallForAllHelpOutput) {
-    app.add_subcommand("one");
+    app.set_help_all_flag("--help-all", "Help all");
+    app.add_subcommand("one", "One description");
     CLI::App *sub = app.add_subcommand("two");
     sub->add_flag("--three");
 
@@ -502,6 +503,20 @@ TEST_F(CapturedHelp, CallForAllHelpOutput) {
     EXPECT_THAT(out.str(), HasSubstr("one"));
     EXPECT_THAT(out.str(), HasSubstr("two"));
     EXPECT_THAT(out.str(), HasSubstr("--three"));
+
+    EXPECT_EQ(out.str(),
+              "My Test Program\n"
+              "Usage: [OPTIONS] [SUBCOMMAND]\n"
+              "\n"
+              "Options:\n"
+              "  -h,--help                   Print this help message and exit\n"
+              "  --help-all                  Help all\n"
+              "\n"
+              "Subcommands:\n"
+              "one -> One description\n"
+              "two\n"
+              "Options:\n"
+              "  --three                     \n");
 }
 TEST_F(CapturedHelp, NewFormattedHelp) {
     app.formatter([](const CLI::App *, std::string, CLI::AppFormatMode) { return "New Help"; });

diff --git a/tests/HelpersTest.cpp b/tests/HelpersTest.cpp
@@ -1,10 +1,10 @@
 #include "app_helper.hpp"
 
+#include <complex>
+#include <cstdint>
 #include <cstdio>
 #include <fstream>
-#include <cstdint>
 #include <string>
-#include <complex>
 
 TEST(Split, SimpleByToken) {
     auto out = CLI::detail::split("one.two.three", '.');

diff --git a/tests/IniTest.cpp b/tests/IniTest.cpp
@@ -1,8 +1,8 @@
 #include "app_helper.hpp"
 
+#include "gmock/gmock.h"
 #include <cstdio>
 #include <sstream>
-#include "gmock/gmock.h"
 
 using ::testing::HasSubstr;
 using ::testing::Not;

diff --git a/tests/SubcommandTest.cpp b/tests/SubcommandTest.cpp
@@ -1,7 +1,7 @@
 #include "app_helper.hpp"
 
-#include "gtest/gtest.h"
 #include "gmock/gmock.h"
+#include "gtest/gtest.h"
 
 using ::testing::HasSubstr;
 using ::testing::Not;

diff --git a/tests/TimerTest.cpp b/tests/TimerTest.cpp
@@ -1,10 +1,10 @@
-#include "gtest/gtest.h"
-#include "gmock/gmock.h"
 #include "CLI/Timer.hpp"
-#include <string>
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
 #include <chrono>
-#include <thread>
 #include <sstream>
+#include <string>
+#include <thread>
 
 using ::testing::HasSubstr;