Merge pull request #44 from pavel-belikov/better-value-parsing

Fix ValueReader for types assignable to std::string

2 files changed, 53 insertions, 25 deletions

diff --git a/args.hxx b/args.hxx
index f8caa8d..d31cdd6 100644
--- a/args.hxx
+++ b/args.hxx
@@ -2731,16 +2731,18 @@ namespace args
 
     /** A default Reader class for argument classes
      *
-     * Simply uses a std::istringstream to read into the destination type, and
+     * If destination type is assignable to std::string it uses an assignment to std::string.
+     * Otherwise ValueReader simply uses a std::istringstream to read into the destination type, and
      * raises a ParseError if there are any characters left.
      */
-    template <typename T>
     struct ValueReader
     {
-        bool operator ()(const std::string &name, const std::string &value, T &destination)
+        template <typename T>
+        typename std::enable_if<!std::is_assignable<T, std::string>::value, bool>::type
+        operator ()(const std::string &name, const std::string &value, T &destination)
         {
             std::istringstream ss(value);
-            ss >> destination;
+            ss >> destination >> std::ws;
 
             if (ss.rdbuf()->in_avail() > 0)
             {
@@ -2755,19 +2757,12 @@ namespace args
             }
             return true;
         }
-    };
 
-    /** std::string specialization for ValueReader
-     *
-     * By default, stream extraction into a string splits on white spaces, and
-     * it is more efficient to ust copy a string into the destination.
-     */
-    template <>
-    struct ValueReader<std::string>
-    {
-        bool operator()(const std::string &, const std::string &value, std::string &destination)
+        template <typename T>
+        typename std::enable_if<std::is_assignable<T, std::string>::value, bool>::type
+        operator()(const std::string &, const std::string &value, T &destination)
         {
-            destination.assign(value);
+            destination = value;
             return true;
         }
     };
@@ -2779,7 +2774,7 @@ namespace args
      */
     template <
         typename T,
-        typename Reader = ValueReader<T>>
+        typename Reader = ValueReader>
     class ValueFlag : public ValueFlagBase
     {
         protected:
@@ -2853,7 +2848,7 @@ namespace args
      */
     template <
         typename T,
-        typename Reader = ValueReader<T>>
+        typename Reader = ValueReader>
     class ImplicitValueFlag : public ValueFlag<T, Reader>
     {
         protected:
@@ -2904,7 +2899,7 @@ namespace args
     template <
         typename T,
         template <typename...> class List = std::vector,
-        typename Reader = ValueReader<T>>
+        typename Reader = ValueReader>
     class NargsValueFlag : public FlagBase
     {
         protected:
@@ -3006,7 +3001,7 @@ namespace args
     template <
         typename T,
         template <typename...> class List = std::vector,
-        typename Reader = ValueReader<T>>
+        typename Reader = ValueReader>
     class ValueFlagList : public ValueFlagBase
     {
         private:
@@ -3111,7 +3106,7 @@ namespace args
     template <
         typename K,
         typename T,
-        typename Reader = ValueReader<K>,
+        typename Reader = ValueReader,
         template <typename...> class Map = std::unordered_map>
     class MapFlag : public ValueFlagBase
     {
@@ -3211,7 +3206,7 @@ namespace args
         typename K,
         typename T,
         template <typename...> class List = std::vector,
-        typename Reader = ValueReader<K>,
+        typename Reader = ValueReader,
         template <typename...> class Map = std::unordered_map>
     class MapFlagList : public ValueFlagBase
     {
@@ -3327,7 +3322,7 @@ namespace args
      */
     template <
         typename T,
-        typename Reader = ValueReader<T>>
+        typename Reader = ValueReader>
     class Positional : public PositionalBase
     {
         private:
@@ -3376,7 +3371,7 @@ namespace args
     template <
         typename T,
         template <typename...> class List = std::vector,
-        typename Reader = ValueReader<T>>
+        typename Reader = ValueReader>
     class PositionalList : public PositionalBase
     {
         private:
@@ -3483,7 +3478,7 @@ namespace args
     template <
         typename K,
         typename T,
-        typename Reader = ValueReader<K>,
+        typename Reader = ValueReader,
         template <typename...> class Map = std::unordered_map>
     class MapPositional : public PositionalBase
     {
@@ -3550,7 +3545,7 @@ namespace args
         typename K,
         typename T,
         template <typename...> class List = std::vector,
-        typename Reader = ValueReader<K>,
+        typename Reader = ValueReader,
         template <typename...> class Map = std::unordered_map>
     class MapPositionalList : public PositionalBase
     {
diff --git a/test.cxx b/test.cxx
index a8d1676..897dcf3 100644
--- a/test.cxx
+++ b/test.cxx
@@ -1066,6 +1066,39 @@ TEST_CASE("HelpParams work as expected", "[args]")
 
 }
 
+struct StringAssignable
+{
+public:
+    StringAssignable() = default;
+    StringAssignable(const std::string &p) : path(p) {}
+    std::string path;
+
+    friend std::istream &operator >> (std::istream &s, StringAssignable &a)
+    { return s >> a.path; }
+};
+
+TEST_CASE("ValueParser works as expected", "[args]")
+{
+    static_assert(std::is_assignable<StringAssignable, std::string>::value, "StringAssignable must be assignable to std::string");
+
+    args::ArgumentParser p("parser");
+    args::ValueFlag<std::string> f(p, "name", "description", {'f'});
+    args::ValueFlag<StringAssignable> b(p, "name", "description", {'b'});
+    args::ValueFlag<int> i(p, "name", "description", {'i'});
+
+    REQUIRE_NOTHROW(p.ParseArgs(std::vector<std::string>{"-f", "a b"}));
+    REQUIRE(args::get(f) == "a b");
+
+    REQUIRE_NOTHROW(p.ParseArgs(std::vector<std::string>{"-b", "a b"}));
+    REQUIRE(args::get(b).path == "a b");
+
+    REQUIRE_NOTHROW(p.ParseArgs(std::vector<std::string>{"-i", "42 "}));
+    REQUIRE(args::get(i) == 42);
+
+    REQUIRE_NOTHROW(p.ParseArgs(std::vector<std::string>{"-i", " 12"}));
+    REQUIRE(args::get(i) == 12);
+}
+
 TEST_CASE("ActionFlag works as expected", "[args]")
 {
     args::ArgumentParser p("parser");