From fa5da7deaa1ae9df258bef7c9741525a752e4330 Mon Sep 17 00:00:00 2001
From: Henry Fredrick Schreiner <henry.fredrick.schreiner@cern.ch>
Date: Mon, 20 Nov 2017 16:03:19 -0500
Subject: [PATCH] Adding tests for inhert, a few fixes

---
 include/CLI/App.hpp    | 28 +++++++++----
 include/CLI/Option.hpp | 93 +++++++++++++++++++++++-------------------
 tests/CreationTest.cpp | 78 +++++++++++++++++++++++++++++++++++
 3 files changed, 149 insertions(+), 50 deletions(-)

diff --git a/include/CLI/App.hpp b/include/CLI/App.hpp
index 019e3110..4a9007d7 100644
--- a/include/CLI/App.hpp
+++ b/include/CLI/App.hpp
@@ -78,7 +78,7 @@ class App {
     ///@}
     /// @name Options
     ///@{
-    
+
     /// The default values for options, customizable and changeable INHERITABLE
     OptionDefaults option_defaults_;
 
@@ -143,21 +143,22 @@ class App {
     ///@}
 
     /// Special private constructor for subcommand
-    App(std::string description_, App* parent) : description_(std::move(description_)), parent_(parent) {
+    App(std::string description_, App *parent) : description_(std::move(description_)), parent_(parent) {
         // Inherit if not from a nullptr
         if(parent_ != nullptr) {
             if(parent_->help_ptr_ != nullptr)
                 set_help_flag(parent_->help_ptr_->get_name(), parent_->help_ptr_->get_description());
-            
+
             /// OptionDefaults
             option_defaults_ = parent_->option_defaults_;
-            
+
             // INHERITABLE
             allow_extras_ = parent_->allow_extras_;
             prefix_command_ = parent_->prefix_command_;
             ignore_case_ = parent_->ignore_case_;
             fallthrough_ = parent_->fallthrough_;
             group_ = parent_->group_;
+            footer_ = parent_->footer_;
         }
     }
 
@@ -176,6 +177,9 @@ class App {
         return this;
     }
 
+    /// Get footer.
+    std::string get_footer() const { return footer_; }
+
     /// Set a callback for the end of parsing.
     ///
     /// Due to a bug in c++11,
@@ -193,12 +197,17 @@ class App {
         return this;
     }
 
+    /// Get the status of allow extras
+    bool get_allow_extras() const { return allow_extras_; }
+
     /// Do not parse anything after the first unrecognised option and return
     App *prefix_command(bool allow = true) {
         prefix_command_ = allow;
         return this;
     }
 
+    bool get_prefix_command() const { return prefix_command_; }
+
     /// Ignore case. Subcommand inherit value.
     App *ignore_case(bool value = true) {
         ignore_case_ = value;
@@ -211,6 +220,8 @@ class App {
         return this;
     }
 
+    bool get_ignore_case() const { return ignore_case_; }
+
     /// Check to see if this subcommand was parsed, true only if received on command line.
     bool parsed() const { return parsed_; }
 
@@ -232,6 +243,9 @@ class App {
         return this;
     }
 
+    /// Check the status of fallthrough
+    bool get_fallthrough() const { return fallthrough_; }
+
     /// Changes the group membership
     App *group(std::string name) {
         group_ = name;
@@ -240,9 +254,9 @@ class App {
 
     /// Get the group of this subcommand
     const std::string &get_group() const { return group_; }
-    
+
     /// Get the OptionDefault object, to set option defaults
-    OptionDefaults* option_defaults() {return &option_defaults_;}
+    OptionDefaults *option_defaults() { return &option_defaults_; }
 
     ///@}
     /// @name Adding options
@@ -881,7 +895,7 @@ class App {
             std::set<std::string> subcmd_groups_seen;
             for(const App_p &com : subcommands_) {
                 const std::string &group_key = detail::to_lower(com->get_group());
-                if(group_key == "hidden" || subcmd_groups_seen.count(group_key) != 0)
+                if(group_key == "" || subcmd_groups_seen.count(group_key) != 0)
                     continue;
 
                 subcmd_groups_seen.insert(group_key);
diff --git a/include/CLI/Option.hpp b/include/CLI/Option.hpp
index db749316..b8f8b0d5 100644
--- a/include/CLI/Option.hpp
+++ b/include/CLI/Option.hpp
@@ -26,81 +26,82 @@ class App;
 
 using Option_p = std::unique_ptr<Option>;
 
-template<typename CRTP>
-class OptionBase {
-  friend App;
-    
+template <typename CRTP> class OptionBase {
+    friend App;
+
   protected:
-    
     /// The group membership
     std::string group_{"Options"};
 
     /// True if this is a required option
     bool required_{false};
-    
+
     /// Ignore the case when matching (option, not value)
     bool ignore_case_{false};
-    
+
     /// Only take the last argument (requires `expected_ == 1`)
     bool last_{false};
-    
-    template<typename T>
-    void copy_from(T& other) {
-        group_ = other.group_;
-        required_ = other.required_;
-        ignore_case_ = other.ignore_case_;
-        last_ = other.last_;
+
+    template <typename T> void copy_from(const T &other) {
+        group_ = other.get_group();
+        required_ = other.get_required();
+        ignore_case_ = other.get_ignore_case();
+        last_ = other.get_take_last();
     }
-    
+
   public:
-    
-    /// Set the option as required
-    CRTP *required(bool value = true) {
-        required_ = value;
-        return static_cast<CRTP*>(this);
-    }
-    
-    /// Support Plumbum term
-    CRTP *mandatory(bool value = true) { return required(value); }
-    
+    // setters
+
     /// Changes the group membership
     CRTP *group(std::string name) {
         group_ = name;
-        return static_cast<CRTP*>(this);;
+        return static_cast<CRTP *>(this);
+        ;
     }
-    
+
+    /// Set the option as required
+    CRTP *required(bool value = true) {
+        required_ = value;
+        return static_cast<CRTP *>(this);
+    }
+
+    /// Support Plumbum term
+    CRTP *mandatory(bool value = true) { return required(value); }
+
+    // Getters
+
+    /// Get the group of this option
+    const std::string &get_group() const { return group_; }
+
     /// True if this is a required option
     bool get_required() const { return required_; }
 
+    /// The status of ignore case
+    bool get_ignore_case() const { return ignore_case_; }
+
     /// The status of the take last flag
     bool get_take_last() const { return last_; }
-    
-    /// The status of ignore case
-    bool ignore_case() const {return ignore_case_;}
-    
-    /// Get the group of this option
-    const std::string &get_group() const { return group_; }
 };
-    
-class OptionDefaults : public OptionBase<Option> {
+
+class OptionDefaults : public OptionBase<OptionDefaults> {
   public:
     OptionDefaults() = default;
-    
+
     // Methods here need a different implementation if they are Option vs. OptionDefault
-    
+
     /// Take the last argument if given multiple times
     OptionDefaults *take_last(bool value = true) {
         last_ = value;
         return this;
     }
-    
+
     /// Ignore the case of the option name
     OptionDefaults *ignore_case(bool value = true) {
         ignore_case_ = value;
         return this;
     }
 };
-    
+
 class Option : public OptionBase<Option> {
     friend App;
 
@@ -146,7 +147,6 @@ class Option : public OptionBase<Option> {
     /// A private setting to allow args to not be able to accept incorrect expected values
     bool changeable_{false};
 
-
     /// A list of validators to run on each value parsed
     std::vector<std::function<bool(std::string)>> validators_;
 
@@ -226,7 +226,6 @@ class Option : public OptionBase<Option> {
         return this;
     }
 
-
     /// Sets required options
     Option *requires(Option *opt) {
         auto tup = requires_.insert(opt);
@@ -282,12 +281,18 @@ class Option : public OptionBase<Option> {
     /// You are never expected to add an argument to the template here.
     template <typename T = App> Option *ignore_case(bool value = true) {
         ignore_case_ = value;
-        for(const Option_p &opt : dynamic_cast<T *>(parent_)->options_)
+        T *parent = dynamic_cast<T *>(parent_);
+
+        if(parent == nullptr)
+            throw IncorrectConstruction("This should not happen, there is always a parent!");
+
+        for(const Option_p &opt : parent->options_)
             if(opt.get() != this && *opt == *this)
                 throw OptionAlreadyAdded(opt->get_name());
+
         return this;
     }
-    
+
     /// Take the last argument if given multiple times
     Option *take_last(bool value = true) {
         if(get_expected() != 0 && get_expected() != 1)
@@ -502,6 +507,8 @@ class Option : public OptionBase<Option> {
     void set_custom_option(std::string typeval, int expected = 1, bool changeable = false) {
         typeval_ = typeval;
         expected_ = expected;
+        if(expected == 0)
+            required_ = false;
         changeable_ = changeable;
     }
 
diff --git a/tests/CreationTest.cpp b/tests/CreationTest.cpp
index 839d66e8..4965ff3c 100644
--- a/tests/CreationTest.cpp
+++ b/tests/CreationTest.cpp
@@ -262,3 +262,81 @@ TEST_F(TApp, AllSpaces) {
     EXPECT_TRUE(myapp->check_sname("a"));
     EXPECT_TRUE(myapp->check_name("other"));
 }
+
+TEST_F(TApp, OptionFromDefaults) {
+    app.option_defaults()->required();
+
+    // Options should remember defaults
+    int x;
+    auto opt = app.add_option("--simple", x);
+    EXPECT_TRUE(opt->get_required());
+
+    // Flags cannot be required
+    auto flag = app.add_flag("--other");
+    EXPECT_FALSE(flag->get_required());
+
+    app.option_defaults()->required(false);
+    auto opt2 = app.add_option("--simple2", x);
+    EXPECT_FALSE(opt2->get_required());
+
+    app.option_defaults()->required()->ignore_case();
+
+    auto opt3 = app.add_option("--simple3", x);
+    EXPECT_TRUE(opt3->get_required());
+    EXPECT_TRUE(opt3->get_ignore_case());
+}
+
+TEST_F(TApp, OptionFromDefaultsSubcommands) {
+    // Initial defaults
+    EXPECT_FALSE(app.option_defaults()->get_required());
+    EXPECT_FALSE(app.option_defaults()->get_take_last());
+    EXPECT_FALSE(app.option_defaults()->get_ignore_case());
+    EXPECT_EQ(app.option_defaults()->get_group(), "Options");
+
+    app.option_defaults()->required()->take_last()->ignore_case()->group("Something");
+
+    auto app2 = app.add_subcommand("app2");
+
+    EXPECT_TRUE(app2->option_defaults()->get_required());
+    EXPECT_TRUE(app2->option_defaults()->get_take_last());
+    EXPECT_TRUE(app2->option_defaults()->get_ignore_case());
+    EXPECT_EQ(app2->option_defaults()->get_group(), "Something");
+}
+
+TEST_F(TApp, HelpFlagFromDefaultsSubcommands) {
+    app.set_help_flag("--that", "Wow");
+
+    auto app2 = app.add_subcommand("app2");
+
+    EXPECT_EQ(app2->get_help_ptr()->get_name(), "--that");
+    EXPECT_EQ(app2->get_help_ptr()->get_description(), "Wow");
+}
+
+TEST_F(TApp, SubcommandDefaults) {
+    // allow_extras, prefix_command, ignore_case, fallthrough, group
+
+    // Initial defaults
+    EXPECT_FALSE(app.get_allow_extras());
+    EXPECT_FALSE(app.get_prefix_command());
+    EXPECT_FALSE(app.get_ignore_case());
+    EXPECT_FALSE(app.get_fallthrough());
+    EXPECT_EQ(app.get_footer(), "");
+    EXPECT_EQ(app.get_group(), "Subcommands");
+
+    app.allow_extras();
+    app.prefix_command();
+    app.ignore_case();
+    app.fallthrough();
+    app.set_footer("footy");
+    app.group("Stuff");
+
+    auto app2 = app.add_subcommand("app2");
+
+    // Initial defaults
+    EXPECT_TRUE(app2->get_allow_extras());
+    EXPECT_TRUE(app2->get_prefix_command());
+    EXPECT_TRUE(app2->get_ignore_case());
+    EXPECT_TRUE(app2->get_fallthrough());
+    EXPECT_EQ(app2->get_footer(), "footy");
+    EXPECT_EQ(app2->get_group(), "Stuff");
+}