From 15c6ee5f3db9bf83ebf738143f85fc8a7e79370d Mon Sep 17 00:00:00 2001
From: Henry Fredrick Schreiner <henry.fredrick.schreiner@cern.ch>
Date: Mon, 13 Feb 2017 16:04:45 -0500
Subject: [PATCH] Added Range and ValidationError, Refactor throwing errors to
 Option instead of App for Validation

---
 include/CLI/App.hpp        |  3 +--
 include/CLI/Error.hpp      |  5 ++++
 include/CLI/Option.hpp     |  7 ++---
 include/CLI/Validators.hpp | 18 ++++++++++++-
 tests/AppTest.cpp          | 55 +++++++++++++++++++++++++++++++++++---
 5 files changed, 79 insertions(+), 9 deletions(-)

diff --git a/include/CLI/App.hpp b/include/CLI/App.hpp
index 6eea7bd9..7c91e100 100644
--- a/include/CLI/App.hpp
+++ b/include/CLI/App.hpp
@@ -550,8 +550,7 @@ protected:
         // Process callbacks
         for(const Option_p& opt : options) {
             if (opt->count() > 0) {
-                if(!opt->run_callback())
-                    throw ConversionError(opt->get_name() + "=" + detail::join(opt->flatten_results()));
+                opt->run_callback();
             }
         }
 
diff --git a/include/CLI/Error.hpp b/include/CLI/Error.hpp
index 230f823f..3f87ca57 100644
--- a/include/CLI/Error.hpp
+++ b/include/CLI/Error.hpp
@@ -70,6 +70,11 @@ struct ConversionError : public ParseError {
     ConversionError(std::string name) : ParseError("ConversionError", name, 2) {}
 };
 
+/// Thrown when validation of results fails
+struct ValidationError : public ParseError {
+    ValidationError(std::string name) : ParseError("ValidationError", name, 2) {}
+};
+
 /// Thrown when a required option is missing
 struct RequiredError : public ParseError {
     RequiredError(std::string name) : ParseError("RequiredError", name, 5) {}
diff --git a/include/CLI/Option.hpp b/include/CLI/Option.hpp
index cc429470..fd74da7c 100644
--- a/include/CLI/Option.hpp
+++ b/include/CLI/Option.hpp
@@ -196,14 +196,15 @@ public:
 
 
     /// Process the callback
-    bool run_callback() const {
+    void run_callback() const {
+        if(!callback(results))
+            throw ConversionError(get_name() + "=" + detail::join(flatten_results()));
         if(_validators.size()>0) {
             for(const std::string & result : flatten_results())
                 for(const std::function<bool(std::string)> &vali : _validators)
                     if(!vali(result))
-                        return false;
+                        throw ValidationError(get_name() + "=" + result);
         }
-        return callback(results);
     }
 
     /// If options share any of the same names, they are equal (not counting positional)
diff --git a/include/CLI/Validators.hpp b/include/CLI/Validators.hpp
index 1067f409..c03f3651 100644
--- a/include/CLI/Validators.hpp
+++ b/include/CLI/Validators.hpp
@@ -5,7 +5,8 @@
 
 #include <string>
 #include <iostream>
-
+#include <functional>
+#include "CLI/TypeTools.hpp"
 
 // C standard library
 // Only needed for existence checking
@@ -61,5 +62,20 @@ bool NonexistentPath(std::string filename) {
     }
 }
 
+/// Produce a range validator function
+template<typename T>
+std::function<bool(std::string)> Range(T min, T max) {
+    return [min, max](std::string input){
+        T val;
+        detail::lexical_cast(input, val);
+        return val >= min && val <= max;
+    };
+}
+
+/// Range of one value is 0 to value
+template<typename T>
+std::function<bool(std::string)> Range(T max) {
+    return Range((T) 0, max);
+}
 
 }
diff --git a/tests/AppTest.cpp b/tests/AppTest.cpp
index 0772db11..66f85119 100644
--- a/tests/AppTest.cpp
+++ b/tests/AppTest.cpp
@@ -282,7 +282,7 @@ TEST_F(TApp, FileNotExists) {
     
     bool ok = static_cast<bool>(std::ofstream(myfile.c_str()).put('a')); // create file
     EXPECT_TRUE(ok);
-    EXPECT_THROW(run(), CLI::ConversionError);
+    EXPECT_THROW(run(), CLI::ValidationError);
 
     std::remove(myfile.c_str());
     EXPECT_FALSE(CLI::ExistingFile(myfile));
@@ -296,8 +296,7 @@ TEST_F(TApp, FileExists) {
     app.add_option("--file", filename)->check(CLI::ExistingFile);
     args = {"--file", myfile};
 
-    EXPECT_THROW(run(), CLI::ConversionError);
-    EXPECT_EQ("Failed", filename);
+    EXPECT_THROW(run(), CLI::ValidationError);
 
     app.reset();
 
@@ -509,3 +508,53 @@ TEST_F(TApp, Env) {
     EXPECT_THROW(run(), CLI::RequiredError);
 }
 
+TEST_F(TApp, RangeInt) {
+    int x=0;
+    app.add_option("--one", x)->check(CLI::Range(3,6));
+
+    args = {"--one=1"};
+    EXPECT_THROW(run(), CLI::ValidationError);
+
+    app.reset();
+    args = {"--one=7"};
+    EXPECT_THROW(run(), CLI::ValidationError);
+
+    app.reset();
+    args = {"--one=3"};
+    EXPECT_NO_THROW(run());
+
+    app.reset();
+    args = {"--one=5"};
+    EXPECT_NO_THROW(run());
+
+    app.reset();
+    args = {"--one=6"};
+    EXPECT_NO_THROW(run());
+}
+
+TEST_F(TApp, RangeDouble) {
+
+    double x=0;
+    /// Note that this must be a double in Range, too
+    app.add_option("--one", x)->check(CLI::Range(3.0,6.0));
+
+    args = {"--one=1"};
+    EXPECT_THROW(run(), CLI::ValidationError);
+
+    app.reset();
+    args = {"--one=7"};
+    EXPECT_THROW(run(), CLI::ValidationError);
+
+    app.reset();
+    args = {"--one=3"};
+    EXPECT_NO_THROW(run());
+
+    app.reset();
+    args = {"--one=5"};
+    EXPECT_NO_THROW(run());
+
+    app.reset();
+    args = {"--one=6"};
+    EXPECT_NO_THROW(run());
+}
+