From 8201a185683cb71d60daf3217085a6772350ac8b Mon Sep 17 00:00:00 2001
From: Zhihao Yuan <lichray@gmail.com>
Date: Sun, 17 Nov 2019 00:30:55 -0600
Subject: [PATCH] Fix various issues in Argument constructor

Before this change:

1. When the input is built-in string literal or cv-`char*`,
   `is_optional` constructs temporary `std::string` while
   `mNames` initializer is also constructing `std::string`
   due to the use of `std::initializer_list`.
2. When the input is `std::string_view`, doesn't compile.
3. When the input is `std::string`, `mNames` initializer
   moves `args`.  If argument name is longer than
   `std::string`'s SSO buffer, bad thing will happen because
   `is_optional` will be accessing `args` in moved-from
   states.

Because of the use of `strtol` which expects nul-terminated
input, `is_*` series functions must take `std::string`.  This
restriction may be removed after AppleClang adds `<charconv>`.
But for now, it complicates the patch.  My solution is to
create an array prvalue still, but use a array reference
rather than `std::initializer_list` to refer to it, so that
the code in delegated constructor can keep using fold
expressions after the necessary `std::string` objects being
created.
---
 include/argparse.hpp             | 23 ++++++++++++++++-------
 test/test_optional_arguments.hpp | 21 ++++++++++++++++++++-
 2 files changed, 36 insertions(+), 8 deletions(-)

diff --git a/include/argparse.hpp b/include/argparse.hpp
index c965f9a..a56ae6e 100644
--- a/include/argparse.hpp
+++ b/include/argparse.hpp
@@ -99,19 +99,28 @@ class Argument {
   friend auto operator<<(std::ostream &, ArgumentParser const &)
       -> std::ostream &;
 
-public:
-  Argument() = default;
-
-  template <typename... Args>
-  explicit Argument(Args... args)
-      : mNames({std::move(args)...}), mIsOptional((is_optional(args) || ...)),
-        mIsRequired(false), mIsUsed(false) {
+  template <size_t N, size_t... I>
+  explicit Argument(std::string(&&a)[N], std::index_sequence<I...>)
+      : mIsOptional((is_optional(a[I]) || ...)), mIsRequired(false),
+        mIsUsed(false) {
+    ((void)mNames.push_back(std::move(a[I])), ...);
     std::sort(
         mNames.begin(), mNames.end(), [](const auto &lhs, const auto &rhs) {
           return lhs.size() == rhs.size() ? lhs < rhs : lhs.size() < rhs.size();
         });
   }
 
+public:
+  Argument() = default;
+
+  template <typename... Args,
+            std::enable_if_t<
+                std::conjunction_v<std::is_constructible<std::string, Args>...>,
+                int> = 0>
+  explicit Argument(Args &&... args)
+      : Argument({std::string(std::forward<Args>(args))...},
+                 std::make_index_sequence<sizeof...(Args)>{}) {}
+
   Argument &help(std::string aHelp) {
     mHelp = std::move(aHelp);
     return *this;
diff --git a/test/test_optional_arguments.hpp b/test/test_optional_arguments.hpp
index 151f6ad..4f316c7 100644
--- a/test/test_optional_arguments.hpp
+++ b/test/test_optional_arguments.hpp
@@ -43,4 +43,23 @@ TEST_CASE("Parse multiple toggle arguments with implicit values", "[optional_arg
   REQUIRE(program.get<bool>("-a") == true);
   REQUIRE(program.get<bool>("-u") == false);
   REQUIRE(program.get<bool>("-x") == true);
-}
\ No newline at end of file
+}
+
+TEST_CASE("Parse arguments of different types", "[optional_arguments]") {
+  using namespace std::literals;
+
+  argparse::ArgumentParser program("test");
+  program.add_argument("--this-argument-is-longer-than-any-sso-buffer-that-"
+                       "makes-sense-unless-your-cache-line-is-this-long"s);
+
+  REQUIRE_NOTHROW(program.parse_args({"test"}));
+
+  program.add_argument("-string"s, "-string-view"sv, "-builtin")
+      .default_value(false)
+      .implicit_value(true);
+
+  program.parse_args({"test", "-string-view"});
+  REQUIRE(program["-string"sv] == true);
+  REQUIRE(program["-string-view"] == true);
+  REQUIRE(program["-builtin"s] == true);
+}