From 4da8454a5ae72282abf222983d6194dd711e425b Mon Sep 17 00:00:00 2001 From: Stephan van Veen Date: Sat, 18 May 2019 11:44:21 +0200 Subject: [PATCH 01/10] Simplify is_optional check --- include/argparse.hpp | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/include/argparse.hpp b/include/argparse.hpp index f372dd3..58badbd 100644 --- a/include/argparse.hpp +++ b/include/argparse.hpp @@ -73,12 +73,6 @@ using enable_if_container = std::enable_if_t, T>; template using enable_if_not_container = std::enable_if_t, T>; - -// Check if string (haystack) starts with a substring (needle) -bool starts_with(const std::string& haystack, const std::string& needle) { - return needle.length() <= haystack.length() - && std::equal(needle.begin(), needle.end(), haystack.begin()); -} } class Argument { @@ -180,7 +174,7 @@ public: private: // If an argument starts with "-" or "--", then it's optional static bool is_optional(const std::string& aName) { - return (starts_with(aName, "--") || starts_with(aName, "-")); + return (!aName.empty() && aName[0] == '-'); } /* From 3c9a74049f8275e6fde823f6f743a3e3ba7cead3 Mon Sep 17 00:00:00 2001 From: Stephan van Veen Date: Wed, 15 May 2019 21:55:27 +0200 Subject: [PATCH 02/10] Implement parse_args_internal for optional parameters --- include/argparse.hpp | 52 +++++++++++++++++++++++++++++++++++++++----- 1 file changed, 46 insertions(+), 6 deletions(-) diff --git a/include/argparse.hpp b/include/argparse.hpp index 58badbd..aa2d70a 100644 --- a/include/argparse.hpp +++ b/include/argparse.hpp @@ -112,6 +112,31 @@ public: return *this; } + template + Iterator consume(Iterator start, Iterator end) { + if (mIsUsed) { + throw std::runtime_error("Duplicate argument"); + } + mIsUsed = true; + mUsedName = *start; + start = std::next(start); + if (mNumArgs == 0) { + mValues.emplace_back(mImplicitValue); + return start; + } + else if (mNumArgs <= std::distance(start, end)) { + end = std::next(start, mNumArgs); + std::transform(start, end, std::back_inserter(mValues), mAction); + return end; + } + else if (mDefaultValue.has_value()) { + return start; + } + else { + throw std::runtime_error("Too few arguments"); + } + } + /* * @throws std::runtime_error if argument values are not valid */ @@ -128,7 +153,7 @@ public: } } else { - if (mValues.size() != mNumArgs) { + if (mValues.size() != mNumArgs && !mDefaultValue.has_value()) { std::stringstream stream; stream << "error: " << mUsedName << ": expected " << mNumArgs << " argument(s). " << mValues.size() << " provided.\n" << std::endl; @@ -400,11 +425,26 @@ class ArgumentParser { * @throws std::runtime_error in case of any invalid argument */ void parse_args_internal(const std::vector& aArguments) { - std::vector argv; - for (const auto& arg : aArguments) - argv.emplace_back(const_cast(arg.data())); - argv.emplace_back(nullptr); - return parse_args_internal(int(argv.size()) - 1, argv.data()); + if (mProgramName.empty() && !aArguments.empty()) { + mProgramName = aArguments.front(); + } + auto end = std::end(aArguments); + for (auto it = std::next(std::begin(aArguments)); it != end;) { + const auto& tCurrentArgument = *it; + if (tCurrentArgument == Argument::mHelpOption || tCurrentArgument == Argument::mHelpOptionLong) { + throw std::runtime_error("help called"); + } + auto tIterator = mArgumentMap.find(tCurrentArgument); + if (tIterator != mArgumentMap.end()) { + // Start parsing optional argument + auto tArgument = tIterator->second; + it = tArgument->consume(it, end); + } + else { + // TODO: compound optional arguments + ++it; + } + } } /* From 44bef34e797b3586bee664d63c3a02879f5c778e Mon Sep 17 00:00:00 2001 From: Stephan van Veen Date: Wed, 15 May 2019 22:18:23 +0200 Subject: [PATCH 03/10] Implement parse_args_internal for positional parameters --- include/argparse.hpp | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/include/argparse.hpp b/include/argparse.hpp index aa2d70a..30e4c2e 100644 --- a/include/argparse.hpp +++ b/include/argparse.hpp @@ -113,19 +113,21 @@ public: } template - Iterator consume(Iterator start, Iterator end) { + Iterator consume(Iterator start, Iterator end, std::string usedName = {}) { if (mIsUsed) { throw std::runtime_error("Duplicate argument"); } mIsUsed = true; - mUsedName = *start; - start = std::next(start); + mUsedName = std::move(usedName); if (mNumArgs == 0) { mValues.emplace_back(mImplicitValue); return start; } else if (mNumArgs <= std::distance(start, end)) { end = std::next(start, mNumArgs); + if (std::any_of(start, end, Argument::is_optional)) { + throw std::runtime_error("optional argument in parameter sequence"); + } std::transform(start, end, std::back_inserter(mValues), mAction); return end; } @@ -436,12 +438,17 @@ class ArgumentParser { } auto tIterator = mArgumentMap.find(tCurrentArgument); if (tIterator != mArgumentMap.end()) { - // Start parsing optional argument auto tArgument = tIterator->second; + it = tArgument->consume(std::next(it), end, tCurrentArgument); + } + else if (!Argument::is_optional(tCurrentArgument)) { + if (mNextPositionalArgument >= mPositionalArguments.size()) { + throw std::runtime_error("Maximum number of positional arguments exceeded"); + } + auto tArgument = mPositionalArguments[mNextPositionalArgument++]; it = tArgument->consume(it, end); } - else { - // TODO: compound optional arguments + else { // TODO: compound optional arguments ++it; } } From d95f9d9f143eddd3cfb37dc1f65a2095c93577ca Mon Sep 17 00:00:00 2001 From: Stephan van Veen Date: Sat, 18 May 2019 11:54:06 +0200 Subject: [PATCH 04/10] First check for positional, then optional and compound --- include/argparse.hpp | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/include/argparse.hpp b/include/argparse.hpp index 30e4c2e..3a8cd5c 100644 --- a/include/argparse.hpp +++ b/include/argparse.hpp @@ -204,6 +204,10 @@ public: return (!aName.empty() && aName[0] == '-'); } + static bool is_positional(const std::string& aName) { + return !is_optional(aName); + } + /* * Getter for template non-container types * @throws std::logic_error in case of incompatible types @@ -436,18 +440,17 @@ class ArgumentParser { if (tCurrentArgument == Argument::mHelpOption || tCurrentArgument == Argument::mHelpOptionLong) { throw std::runtime_error("help called"); } - auto tIterator = mArgumentMap.find(tCurrentArgument); - if (tIterator != mArgumentMap.end()) { - auto tArgument = tIterator->second; - it = tArgument->consume(std::next(it), end, tCurrentArgument); - } - else if (!Argument::is_optional(tCurrentArgument)) { + if (Argument::is_positional(tCurrentArgument)) { if (mNextPositionalArgument >= mPositionalArguments.size()) { throw std::runtime_error("Maximum number of positional arguments exceeded"); } auto tArgument = mPositionalArguments[mNextPositionalArgument++]; it = tArgument->consume(it, end); } + else if (auto tIterator = mArgumentMap.find(tCurrentArgument); tIterator != mArgumentMap.end()) { + auto tArgument = tIterator->second; + it = tArgument->consume(std::next(it), end, tCurrentArgument); + } else { // TODO: compound optional arguments ++it; } From ecf8e4fd5b180dc6b097b188fc7a83fca14db9e4 Mon Sep 17 00:00:00 2001 From: Stephan van Veen Date: Sat, 18 May 2019 12:22:27 +0200 Subject: [PATCH 05/10] Implement parse_args_internal for compound parameters --- include/argparse.hpp | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/include/argparse.hpp b/include/argparse.hpp index 3a8cd5c..3fca9fb 100644 --- a/include/argparse.hpp +++ b/include/argparse.hpp @@ -451,7 +451,20 @@ class ArgumentParser { auto tArgument = tIterator->second; it = tArgument->consume(std::next(it), end, tCurrentArgument); } - else { // TODO: compound optional arguments + else if (const auto& tCompoundArgument = tCurrentArgument; + tCompoundArgument.size() > 1 && + tCompoundArgument[0] == '-' && + tCompoundArgument[1] != '-') { + ++it; + for (size_t j = 1; j < tCompoundArgument.size(); j++) { + auto tCurrentArgument = std::string{'-', tCompoundArgument[j]}; + if (auto tIterator = mArgumentMap.find(tCurrentArgument); tIterator != mArgumentMap.end()) { + auto tArgument = tIterator->second; + it = tArgument->consume(it, end, tCurrentArgument); + } + } + } + else { ++it; } } From 36bdfe4a55d2771eac0c29613b9508d2fe2e8178 Mon Sep 17 00:00:00 2001 From: Stephan van Veen Date: Sat, 18 May 2019 14:04:57 +0200 Subject: [PATCH 06/10] Change test cases --- test/test_compound_arguments.hpp | 25 +------------------------ test/test_positional_arguments.hpp | 8 +------- 2 files changed, 2 insertions(+), 31 deletions(-) diff --git a/test/test_compound_arguments.hpp b/test/test_compound_arguments.hpp index a096f02..b584246 100644 --- a/test/test_compound_arguments.hpp +++ b/test/test_compound_arguments.hpp @@ -77,30 +77,7 @@ TEST_CASE("Parse compound toggle arguments with implicit values and nargs and ot program.add_argument("--input_files") .nargs(3); - program.parse_args({ "./test.exe", "1", "-abc", "3.14", "2.718", "2", "--input_files", - "a.txt", "b.txt", "c.txt", "3" }); - - REQUIRE(program.get("-a") == true); - REQUIRE(program.get("-b") == true); - auto c = program.get>("-c"); - REQUIRE(c.size() == 2); - REQUIRE(c[0] == 3.14f); - REQUIRE(c[1] == 2.718f); - auto input_files = program.get>("--input_files"); - REQUIRE(input_files.size() == 3); - REQUIRE(input_files[0] == "a.txt"); - REQUIRE(input_files[1] == "b.txt"); - REQUIRE(input_files[2] == "c.txt"); - auto numbers = program.get>("numbers"); - REQUIRE(numbers.size() == 3); - REQUIRE(numbers[0] == 1); - REQUIRE(numbers[1] == 2); - REQUIRE(numbers[2] == 3); - auto numbers_list = program.get>("numbers"); - REQUIRE(numbers.size() == 3); - REQUIRE(testutility::get_from_list(numbers_list, 0) == 1); - REQUIRE(testutility::get_from_list(numbers_list, 1) == 2); - REQUIRE(testutility::get_from_list(numbers_list, 2) == 3); + REQUIRE_THROWS(program.parse_args({ "./test.exe", "1", "-abc", "3.14", "2.718", "2", "--input_files", "a.txt", "b.txt", "c.txt", "3" })); } TEST_CASE("Parse out-of-order compound arguments", "[compound_arguments]") { diff --git a/test/test_positional_arguments.hpp b/test/test_positional_arguments.hpp index e69d55f..6e605f1 100644 --- a/test/test_positional_arguments.hpp +++ b/test/test_positional_arguments.hpp @@ -44,13 +44,7 @@ TEST_CASE("Parse positional arguments with optional arguments in the middle", "[ program.add_argument("output").nargs(2); program.add_argument("--num_iterations") .action([](const std::string& value) { return std::stoi(value); }); - program.parse_args({ "test", "rocket.mesh", "thrust_profile.csv", "--num_iterations", "15", "output.mesh" }); - REQUIRE(program.get("--num_iterations") == 15); - REQUIRE(program.get("input") == "rocket.mesh"); - auto outputs = program.get>("output"); - REQUIRE(outputs.size() == 2); - REQUIRE(outputs[0] == "thrust_profile.csv"); - REQUIRE(outputs[1] == "output.mesh"); + REQUIRE_THROWS(program.parse_args({ "test", "rocket.mesh", "thrust_profile.csv", "--num_iterations", "15", "output.mesh" })); } TEST_CASE("Square a number", "[positional_arguments]") { From 9e7b80034efa7412f76739c747aa611ed0caca17 Mon Sep 17 00:00:00 2001 From: Stephan van Veen Date: Sat, 18 May 2019 14:22:10 +0200 Subject: [PATCH 07/10] Throw exception in case of unknown argument --- include/argparse.hpp | 5 ++++- test/test_invalid_arguments.hpp | 2 +- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/include/argparse.hpp b/include/argparse.hpp index 3fca9fb..69cc370 100644 --- a/include/argparse.hpp +++ b/include/argparse.hpp @@ -462,10 +462,13 @@ class ArgumentParser { auto tArgument = tIterator->second; it = tArgument->consume(it, end, tCurrentArgument); } + else { + throw std::runtime_error("Unknown argument"); + } } } else { - ++it; + throw std::runtime_error("Unknown argument"); } } } diff --git a/test/test_invalid_arguments.hpp b/test/test_invalid_arguments.hpp index f466be8..1806816 100644 --- a/test/test_invalid_arguments.hpp +++ b/test/test_invalid_arguments.hpp @@ -34,5 +34,5 @@ TEST_CASE("Parse unknown optional argument", "[compound_arguments]") { .action([](const std::string& val) { return std::stoull(val); }) .help("memory in MB to give the VMM when loading"); - bfm.parse_args({ "./test.exe", "-om" }); + REQUIRE_THROWS(bfm.parse_args({ "./test.exe", "-om" })); } From 62c2be634a70b6b286756eb21a7b8fdb57288b81 Mon Sep 17 00:00:00 2001 From: Stephan van Veen Date: Sat, 18 May 2019 14:27:32 +0200 Subject: [PATCH 08/10] Remove argc argv version of parse_args_internal --- include/argparse.hpp | 128 +------------------------------------------ 1 file changed, 3 insertions(+), 125 deletions(-) diff --git a/include/argparse.hpp b/include/argparse.hpp index 69cc370..324f4d0 100644 --- a/include/argparse.hpp +++ b/include/argparse.hpp @@ -327,8 +327,9 @@ class ArgumentParser { * @throws std::runtime_error in case of any invalid argument */ void parse_args(int argc, const char * const argv[]) { - parse_args_internal(argc, argv); - parse_args_validate(); + std::vector arguments; + std::copy(argv, argv + argc, std::back_inserter(arguments)); + parse_args(arguments); } /* Getter enabled for all template types other than std::vector and std::list @@ -473,129 +474,6 @@ class ArgumentParser { } } - /* - * @throws std::runtime_error in case of any invalid argument - */ - void parse_args_internal(int argc, const char * const argv[]) { - if (mProgramName.empty() && argc > 0) - mProgramName = argv[0]; - for (int i = 1; i < argc; i++) { - auto tCurrentArgument = std::string(argv[i]); - if (tCurrentArgument == Argument::mHelpOption || tCurrentArgument == Argument::mHelpOptionLong) { - throw std::runtime_error("help called"); - } - auto tIterator = mArgumentMap.find(argv[i]); - if (tIterator != mArgumentMap.end()) { - // Start parsing optional argument - auto tArgument = tIterator->second; - tArgument->mUsedName = tCurrentArgument; - tArgument->mIsUsed = true; - auto tCount = tArgument->mNumArgs; - - // Check to see if implicit value should be used - // Two cases to handle here: - // (1) User has explicitly programmed nargs to be 0 - // (2) User has provided an implicit value, which also sets nargs to 0 - if (tCount == 0) { - // Use implicit value for this optional argument - tArgument->mValues.emplace_back(tArgument->mImplicitValue); - tArgument->mRawValues.emplace_back(); - tCount = 0; - } - while (tCount > 0) { - i = i + 1; - if (i < argc) { - tArgument->mUsedName = tCurrentArgument; - tArgument->mRawValues.emplace_back(argv[i]); - if (tArgument->mAction != nullptr) - tArgument->mValues.emplace_back(tArgument->mAction(argv[i])); - else { - if (tArgument->mDefaultValue.has_value()) - tArgument->mValues.emplace_back(tArgument->mDefaultValue); - else - tArgument->mValues.emplace_back(std::string(argv[i])); - } - } - tCount -= 1; - } - } - else { - if (Argument::is_optional(argv[i])) { - // This is possibly a compound optional argument - // Example: We have three optional arguments -a, -u and -x - // The user provides ./main -aux ... - // Here -aux is a compound optional argument - std::string tCompoundArgument = std::string(argv[i]); - if (tCompoundArgument.size() > 1 && tCompoundArgument[0] == '-' && tCompoundArgument[1] != '-') { - for (size_t j = 1; j < tCompoundArgument.size(); j++) { - std::string tArgument(1, tCompoundArgument[j]); - size_t tNumArgs = 0; - tIterator = mArgumentMap.find("-" + tArgument); - if (tIterator != mArgumentMap.end()) { - auto tArgumentObject = tIterator->second; - tNumArgs = tArgumentObject->mNumArgs; - std::vector tArgumentsForRecursiveParsing = {"", "-" + tArgument}; - while (tNumArgs > 0 && i < argc) { - i += 1; - if (i < argc) { - tArgumentsForRecursiveParsing.emplace_back(argv[i]); - tNumArgs -= 1; - } - } - parse_args_internal(tArgumentsForRecursiveParsing); - } - else { - if (!tArgument.empty() && tArgument[0] == '-') - std::cout << "warning: unrecognized optional argument " << tArgument - << std::endl; - else - std::cout << "warning: unrecognized optional argument -" << tArgument - << std::endl; - } - } - } - else { - std::cout << "warning: unrecognized optional argument " << tCompoundArgument << std::endl; - } - } - else { - // This is a positional argument. - // Parse and save into mPositionalArguments vector - if (mNextPositionalArgument >= mPositionalArguments.size()) { - std::stringstream stream; - stream << "error: unexpected positional argument " << argv[i] << std::endl; - throw std::runtime_error(stream.str()); - } - auto tArgument = mPositionalArguments[mNextPositionalArgument]; - auto tCount = tArgument->mNumArgs - tArgument->mRawValues.size(); - while (tCount > 0) { - tIterator = mArgumentMap.find(argv[i]); - if (tIterator != mArgumentMap.end() || Argument::is_optional(argv[i])) { - i = i - 1; - break; - } - if (i < argc) { - tArgument->mUsedName = tCurrentArgument; - tArgument->mRawValues.emplace_back(argv[i]); - if (tArgument->mAction != nullptr) - tArgument->mValues.emplace_back(tArgument->mAction(argv[i])); - else { - if (tArgument->mDefaultValue.has_value()) - tArgument->mValues.emplace_back(tArgument->mDefaultValue); - else - tArgument->mValues.emplace_back(std::string(argv[i])); - } - } - tCount -= 1; - if (tCount > 0) i += 1; - } - if (tCount == 0) - mNextPositionalArgument += 1; - } - } - } - } - /* * @throws std::runtime_error in case of any invalid argument */ From f2e0bd0de1c5dcd4003d632d1530decd53933edf Mon Sep 17 00:00:00 2001 From: Stephan van Veen Date: Sat, 18 May 2019 14:35:37 +0200 Subject: [PATCH 09/10] Iterate over map instead of both lists --- include/argparse.hpp | 34 ++++++++++------------------------ 1 file changed, 10 insertions(+), 24 deletions(-) diff --git a/include/argparse.hpp b/include/argparse.hpp index 324f4d0..1d68436 100644 --- a/include/argparse.hpp +++ b/include/argparse.hpp @@ -478,26 +478,20 @@ class ArgumentParser { * @throws std::runtime_error in case of any invalid argument */ void parse_args_validate() { - try { - // Check if all positional arguments are parsed - std::for_each(std::begin(mPositionalArguments), - std::end(mPositionalArguments), - std::mem_fn(&Argument::validate)); - // Check if all user-provided optional argument values are parsed correctly - std::for_each(std::begin(mOptionalArguments), - std::end(mOptionalArguments), - std::mem_fn(&Argument::validate)); - } catch (const std::runtime_error& err) { - throw err; - } + // Check if all arguments are parsed + std::for_each(std::begin(mArgumentMap), std::end(mArgumentMap), [](const auto& argPair) { + const auto& [key, arg] = argPair; + arg->validate(); + }); } // Used by print_help. - size_t get_length_of_longest_argument(const std::vector>& aArguments) { - if (aArguments.empty()) + size_t get_length_of_longest_argument() { + if (mArgumentMap.empty()) return 0; - std::vector argumentLengths(aArguments.size()); - std::transform(std::begin(aArguments), std::end(aArguments), std::begin(argumentLengths), [](const auto& arg) { + std::vector argumentLengths(mArgumentMap.size()); + std::transform(std::begin(mArgumentMap), std::end(mArgumentMap), std::begin(argumentLengths), [](const auto& argPair) { + const auto& [key, arg] = argPair; const auto& names = arg->mNames; auto maxLength = std::accumulate(std::begin(names), std::end(names), std::string::size_type{0}, [](const auto& sum, const auto& s) { return sum + s.size() + 2; // +2 for ", " @@ -507,14 +501,6 @@ class ArgumentParser { return *std::max_element(std::begin(argumentLengths), std::end(argumentLengths)); } - // Used by print_help. - size_t get_length_of_longest_argument() { - const auto positionalArgMaxSize = get_length_of_longest_argument(mPositionalArguments); - const auto optionalArgMaxSize = get_length_of_longest_argument(mOptionalArguments); - - return std::max(positionalArgMaxSize, optionalArgMaxSize); - } - std::string mProgramName; std::vector mParentParsers; std::vector> mPositionalArguments; From d960a41e966bb339ad059e1cf0432c7b65d25a50 Mon Sep 17 00:00:00 2001 From: Stephan van Veen Date: Sat, 18 May 2019 14:41:00 +0200 Subject: [PATCH 10/10] Use local iterator instead of member counter --- include/argparse.hpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/include/argparse.hpp b/include/argparse.hpp index 1d68436..067d490 100644 --- a/include/argparse.hpp +++ b/include/argparse.hpp @@ -436,16 +436,17 @@ class ArgumentParser { mProgramName = aArguments.front(); } auto end = std::end(aArguments); + auto positionalArgumentIt = std::begin(mPositionalArguments); for (auto it = std::next(std::begin(aArguments)); it != end;) { const auto& tCurrentArgument = *it; if (tCurrentArgument == Argument::mHelpOption || tCurrentArgument == Argument::mHelpOptionLong) { throw std::runtime_error("help called"); } if (Argument::is_positional(tCurrentArgument)) { - if (mNextPositionalArgument >= mPositionalArguments.size()) { + if (positionalArgumentIt == std::end(mPositionalArguments)) { throw std::runtime_error("Maximum number of positional arguments exceeded"); } - auto tArgument = mPositionalArguments[mNextPositionalArgument++]; + auto tArgument = *(positionalArgumentIt++); it = tArgument->consume(it, end); } else if (auto tIterator = mArgumentMap.find(tCurrentArgument); tIterator != mArgumentMap.end()) { @@ -505,7 +506,6 @@ class ArgumentParser { std::vector mParentParsers; std::vector> mPositionalArguments; std::vector> mOptionalArguments; - size_t mNextPositionalArgument = 0; std::map> mArgumentMap; };