From 50c91a0bcc5e26d45eeac52dfe397949dcfa9920 Mon Sep 17 00:00:00 2001 From: Zhihao Yuan Date: Sat, 16 Nov 2019 21:17:53 -0600 Subject: [PATCH 1/7] Clang-format the code --- include/argparse.hpp | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/include/argparse.hpp b/include/argparse.hpp index 724fa58..3a5ff34 100644 --- a/include/argparse.hpp +++ b/include/argparse.hpp @@ -201,8 +201,8 @@ public: if (mIsOptional) { if (mIsUsed && mValues.size() != mNumArgs && !mDefaultValue.has_value()) { std::stringstream stream; - stream << mUsedName << ": expected " << mNumArgs - << " argument(s). " << mValues.size() << " provided."; + stream << mUsedName << ": expected " << mNumArgs << " argument(s). " + << mValues.size() << " provided."; throw std::runtime_error(stream.str()); } else { // TODO: check if an implicit value was programmed for this argument @@ -220,8 +220,8 @@ public: } else { if (mValues.size() != mNumArgs && !mDefaultValue.has_value()) { std::stringstream stream; - stream << mUsedName << ": expected " << mNumArgs - << " argument(s). " << mValues.size() << " provided."; + stream << mUsedName << ": expected " << mNumArgs << " argument(s). " + << mValues.size() << " provided."; throw std::runtime_error(stream.str()); } } @@ -256,7 +256,8 @@ public: * @throws std::logic_error in case of incompatible types */ template - std::enable_if_t, bool> operator==(const T &aRhs) const { + std::enable_if_t, bool> + operator==(const T &aRhs) const { return get() == aRhs; } @@ -265,7 +266,8 @@ public: * @throws std::logic_error in case of incompatible types */ template - std::enable_if_t, bool> operator==(const T &aRhs) const { + std::enable_if_t, bool> + operator==(const T &aRhs) const { using ValueType = typename T::value_type; auto tLhs = get(); if (tLhs.size() != aRhs.size()) @@ -326,7 +328,8 @@ private: * Getter for container types * @throws std::logic_error in case of incompatible types */ - template details::enable_if_container get() const { + template + details::enable_if_container get() const { using ValueType = typename CONTAINER::value_type; CONTAINER tResult; if (!mValues.empty()) { From 955e1e1e6c86d5ce9daaeedec7585a315d099046 Mon Sep 17 00:00:00 2001 From: Zhihao Yuan Date: Sat, 16 Nov 2019 21:26:49 -0600 Subject: [PATCH 2/7] Simplify code with four-legged std::equal --- include/argparse.hpp | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/include/argparse.hpp b/include/argparse.hpp index 3a5ff34..d6a744a 100644 --- a/include/argparse.hpp +++ b/include/argparse.hpp @@ -270,14 +270,10 @@ public: operator==(const T &aRhs) const { using ValueType = typename T::value_type; auto tLhs = get(); - if (tLhs.size() != aRhs.size()) - return false; - else { - return std::equal(std::begin(tLhs), std::end(tLhs), std::begin(aRhs), - [](const auto &lhs, const auto &rhs) { - return std::any_cast(lhs) == rhs; - }); - } + return std::equal(std::begin(tLhs), std::end(tLhs), std::begin(aRhs), + std::end(aRhs), [](const auto &lhs, const auto &rhs) { + return std::any_cast(lhs) == rhs; + }); } private: From daeca099e2ec13f94a5b1f63ae82b2f8da6ce8bf Mon Sep 17 00:00:00 2001 From: Zhihao Yuan Date: Sat, 16 Nov 2019 21:45:18 -0600 Subject: [PATCH 3/7] Remove undesired access control --- include/argparse.hpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/include/argparse.hpp b/include/argparse.hpp index d6a744a..8d11ec2 100644 --- a/include/argparse.hpp +++ b/include/argparse.hpp @@ -64,7 +64,7 @@ struct is_container< decltype(std::declval().begin()), decltype(std::declval().end()), decltype(std::declval().size())>, - void>> : public std::true_type {}; + void>> : std::true_type {}; template static constexpr bool is_container_v = is_container::value; @@ -363,7 +363,6 @@ private: bool mIsRequired = false; bool mIsUsed = false; // relevant for optional arguments. True if used by user -public: static constexpr auto mHelpOption = "-h"; static constexpr auto mHelpOptionLong = "--help"; }; From f7fe9cf4395ae3cb0a3a4f2c8e051d6a57517095 Mon Sep 17 00:00:00 2001 From: Zhihao Yuan Date: Sat, 16 Nov 2019 21:51:01 -0600 Subject: [PATCH 4/7] Remove unused space in Argument structure --- include/argparse.hpp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/include/argparse.hpp b/include/argparse.hpp index 8d11ec2..dd937b7 100644 --- a/include/argparse.hpp +++ b/include/argparse.hpp @@ -104,7 +104,8 @@ public: template explicit Argument(Args... args) - : mNames({std::move(args)...}), mIsOptional((is_optional(args) || ...)) { + : mNames({std::move(args)...}), mIsOptional((is_optional(args) || ...)), + mIsRequired(false), mIsUsed(false) { std::sort( mNames.begin(), mNames.end(), [](const auto &lhs, const auto &rhs) { return lhs.size() == rhs.size() ? lhs < rhs : lhs.size() < rhs.size(); @@ -357,11 +358,10 @@ private: std::in_place_type, [](const std::string &aValue) { return aValue; }}; std::vector mValues; - std::vector mRawValues; size_t mNumArgs = 1; - bool mIsOptional = false; - bool mIsRequired = false; - bool mIsUsed = false; // relevant for optional arguments. True if used by user + bool mIsOptional : 1; + bool mIsRequired : 1; + bool mIsUsed : 1; // True if the optional argument is used by user static constexpr auto mHelpOption = "-h"; static constexpr auto mHelpOptionLong = "--help"; From 56c041707a698f7d47118c0ee8fde44595b1fc52 Mon Sep 17 00:00:00 2001 From: Zhihao Yuan Date: Sat, 16 Nov 2019 21:58:49 -0600 Subject: [PATCH 5/7] Use string_view in getter interface --- include/argparse.hpp | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/include/argparse.hpp b/include/argparse.hpp index dd937b7..253cea9 100644 --- a/include/argparse.hpp +++ b/include/argparse.hpp @@ -454,19 +454,15 @@ public: * @throws std::logic_error in case of an invalid argument name * @throws std::logic_error in case of incompatible types */ - template T get(const std::string &aArgumentName) { - auto tIterator = mArgumentMap.find(aArgumentName); - if (tIterator != mArgumentMap.end()) { - return tIterator->second->get(); - } - throw std::logic_error("No such argument"); + template T get(std::string_view aArgumentName) { + return (*this)[aArgumentName].get(); } /* Indexing operator. Return a reference to an Argument object * Used in conjuction with Argument.operator== e.g., parser["foo"] == true * @throws std::logic_error in case of an invalid argument name */ - Argument &operator[](const std::string &aArgumentName) { + Argument &operator[](std::string_view aArgumentName) { auto tIterator = mArgumentMap.find(aArgumentName); if (tIterator != mArgumentMap.end()) { return *(tIterator->second); From 7dd6655a9ecb997a2a4f216154229de2814212ec Mon Sep 17 00:00:00 2001 From: Zhihao Yuan Date: Sat, 16 Nov 2019 22:14:14 -0600 Subject: [PATCH 6/7] Avoid extra copy made by initializer_list --- include/argparse.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/argparse.hpp b/include/argparse.hpp index 253cea9..c965f9a 100644 --- a/include/argparse.hpp +++ b/include/argparse.hpp @@ -416,7 +416,7 @@ public: // Parameter packed add_parents method // Accepts a variadic number of ArgumentParser objects template void add_parents(const Targs &... Fargs) { - for (auto &tParentParser : {Fargs...}) { + for (const ArgumentParser &tParentParser : {std::ref(Fargs)...}) { for (auto &tArgument : tParentParser.mPositionalArguments) { auto it = mPositionalArguments.insert(cend(mPositionalArguments), tArgument); From 8201a185683cb71d60daf3217085a6772350ac8b Mon Sep 17 00:00:00 2001 From: Zhihao Yuan Date: Sun, 17 Nov 2019 00:30:55 -0600 Subject: [PATCH 7/7] 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 ``. 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 - explicit Argument(Args... args) - : mNames({std::move(args)...}), mIsOptional((is_optional(args) || ...)), - mIsRequired(false), mIsUsed(false) { + template + explicit Argument(std::string(&&a)[N], std::index_sequence) + : 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 ...>, + int> = 0> + explicit Argument(Args &&... args) + : Argument({std::string(std::forward(args))...}, + std::make_index_sequence{}) {} + 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("-a") == true); REQUIRE(program.get("-u") == false); REQUIRE(program.get("-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); +}