From a3522c1ddf972722f6a91973de22e5ef95d8ba33 Mon Sep 17 00:00:00 2001
From: Henry Fredrick Schreiner <henry.fredrick.schreiner@cern.ch>
Date: Fri, 17 Mar 2017 12:04:22 -0400
Subject: [PATCH] Fix and add cov for subcom ini parser

---
 CHANGELOG.md           |  1 +
 include/CLI/App.hpp    | 14 +++++++++-----
 include/CLI/Option.hpp |  9 +++++++++
 tests/IniTest.cpp      | 31 +++++++++++++++++++++++++++++++
 4 files changed, 50 insertions(+), 5 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 4220d1f1..df3f902a 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -6,6 +6,7 @@
 * Added CodeCov code coverage reports
 * Lots of small bugfixes related to adding tests to increase coverage
 * Error handling now uses scoped enum in errors
+* Reparsing rules changed a little to accommodate Ini files. Callbacks are now called when parsing INI, and reset any time results are added.
 
 ## Version 0.6
 
diff --git a/include/CLI/App.hpp b/include/CLI/App.hpp
index 76ae579e..e1d4ef8e 100644
--- a/include/CLI/App.hpp
+++ b/include/CLI/App.hpp
@@ -758,7 +758,7 @@ protected:
                 std::vector<detail::ini_ret_t> values = detail::parse_ini(config_name_);
                 while(values.size() > 0) {
                     if(!_parse_ini(values)) {
-                        throw ExtrasError(values.back().name());
+                        throw ExtrasINIError(values.back().fullname);
                     }
                 }
             } catch (const FileError &) {
@@ -780,7 +780,7 @@ protected:
 
         // Process callbacks
         for(const Option_p& opt : options_) {
-            if (opt->count() > 0) {
+            if (opt->count() > 0 && !opt->get_callback_run()) {
                 opt->run_callback();
             }
         }
@@ -827,15 +827,17 @@ protected:
     /// Returns true if it managed to find the option, if false you'll need to remove the arg manully.
     bool _parse_ini(std::vector<detail::ini_ret_t> &args) {
         detail::ini_ret_t& current = args.back();
-        std::string parent = current.parent();
+        std::string parent = current.parent(); // respects curent.level
         std::string name = current.name();
+        std::cout << current.fullname << " " << parent << " " << name << std::endl;
+
+        // If a parent is listed, go to a subcommand
         if(parent != "") {
             current.level++;
             for(const App_p &com : subcommands_) 
                 if(com->check_name(parent))
                     return com->_parse_ini(args);
             return false;
-            //throw CLI::ExtraINIError(current.fullname);
         }
 
         auto op_ptr = std::find_if(std::begin(options_), std::end(options_),
@@ -868,8 +870,10 @@ protected:
                         }
                 } else
                     throw ConversionError(current.fullname + ": too many inputs for a flag");
-            } else
+            } else {
                 op->results_ = current.inputs;
+                op->run_callback();
+            }
         }
 
         args.pop_back();
diff --git a/include/CLI/Option.hpp b/include/CLI/Option.hpp
index 291dc421..6cf47ab3 100644
--- a/include/CLI/Option.hpp
+++ b/include/CLI/Option.hpp
@@ -106,6 +106,9 @@ protected:
     /// Results of parsing
     results_t results_;
 
+    /// Whether the callback has run (needed for INI parsing)
+    bool callback_run_ {false};
+
     ///@}
 
     /// Making an option by hand is not defined, it must be made by the App class
@@ -442,6 +445,7 @@ public:
     /// Puts a result at position r
     void add_result(std::string s) {
         results_.push_back(s);
+        callback_run_ = false;
     }
 
 
@@ -450,6 +454,11 @@ public:
         return results_;
     }
 
+    /// See if the callback has been run already
+    bool get_callback_run() const {
+        return callback_run_;
+    }
+
     ///@}
 
     protected:
diff --git a/tests/IniTest.cpp b/tests/IniTest.cpp
index af0b371a..d97ea725 100644
--- a/tests/IniTest.cpp
+++ b/tests/IniTest.cpp
@@ -294,6 +294,37 @@ TEST_F(TApp, IniVector) {
 
 }
 
+
+TEST_F(TApp, IniLayered) {
+
+    TempFile tmpini{"TestIniTmp.ini"};
+
+    app.add_config("--config", tmpini);
+
+    {
+        std::ofstream out{tmpini};
+        out << "[default]" << std::endl;
+        out << "val=1" << std::endl;
+        out << "[subcom]" << std::endl;
+        out << "val=2" << std::endl;
+        out << "subsubcom.val=3" << std::endl;
+    }
+
+    int one=0, two=0, three=0;
+    app.add_option("--val", one);
+    auto subcom = app.add_subcommand("subcom");
+    subcom->add_option("--val", two);
+    auto subsubcom = subcom->add_subcommand("subsubcom");
+    subsubcom->add_option("--val", three);
+
+    ASSERT_NO_THROW(run());
+
+    EXPECT_EQ(1, one);
+    EXPECT_EQ(2, two);
+    EXPECT_EQ(3, three);
+
+}
+
 TEST_F(TApp, IniFlags) {
     TempFile tmpini{"TestIniTmp.ini"};
     app.add_config("--config", tmpini);