From a364f3f1e7abeb9c7ebd8492d7f2ca2d97274179 Mon Sep 17 00:00:00 2001 From: Stephan van Veen Date: Sat, 11 May 2019 11:35:32 +0200 Subject: [PATCH 01/10] Remove code duplication --- include/argparse.hpp | 26 +++++++++++--------------- 1 file changed, 11 insertions(+), 15 deletions(-) diff --git a/include/argparse.hpp b/include/argparse.hpp index 466e7c0..421d8ea 100644 --- a/include/argparse.hpp +++ b/include/argparse.hpp @@ -634,23 +634,11 @@ class ArgumentParser { } // Used by print_help. - size_t get_length_of_longest_argument() { + size_t get_length_of_longest_argument(const std::vector>& aArguments) { size_t tResult = 0; - for (const auto& mPositionalArgument : mPositionalArguments) { + for (const auto& argument : aArguments) { size_t tCurrentArgumentLength = 0; - auto tNames = mPositionalArgument->mNames; - for (size_t j = 0; j < tNames.size() - 1; j++) { - auto tNameLength = tNames[j].length(); - tCurrentArgumentLength += tNameLength + 2; // +2 for ", " - } - tCurrentArgumentLength += tNames[tNames.size() - 1].length(); - if (tCurrentArgumentLength > tResult) - tResult = tCurrentArgumentLength; - } - - for (const auto& mOptionalArgument : mOptionalArguments) { - size_t tCurrentArgumentLength = 0; - auto tNames = mOptionalArgument->mNames; + auto tNames = argument->mNames; for (size_t j = 0; j < tNames.size() - 1; j++) { auto tNameLength = tNames[j].length(); tCurrentArgumentLength += tNameLength + 2; // +2 for ", " @@ -662,6 +650,14 @@ class ArgumentParser { return tResult; } + // 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 7c938b1f2b085a20c83c9347fffe72d4fa2ac158 Mon Sep 17 00:00:00 2001 From: Stephan van Veen Date: Sat, 11 May 2019 12:24:01 +0200 Subject: [PATCH 02/10] Use stl algorithms instead of nested for loops --- include/argparse.hpp | 27 +++++++++++++-------------- 1 file changed, 13 insertions(+), 14 deletions(-) diff --git a/include/argparse.hpp b/include/argparse.hpp index 421d8ea..b8a240c 100644 --- a/include/argparse.hpp +++ b/include/argparse.hpp @@ -40,6 +40,7 @@ SOFTWARE. #include #include #include +#include namespace argparse { @@ -635,19 +636,17 @@ class ArgumentParser { // Used by print_help. size_t get_length_of_longest_argument(const std::vector>& aArguments) { - size_t tResult = 0; - for (const auto& argument : aArguments) { - size_t tCurrentArgumentLength = 0; - auto tNames = argument->mNames; - for (size_t j = 0; j < tNames.size() - 1; j++) { - auto tNameLength = tNames[j].length(); - tCurrentArgumentLength += tNameLength + 2; // +2 for ", " - } - tCurrentArgumentLength += tNames[tNames.size() - 1].length(); - if (tCurrentArgumentLength > tResult) - tResult = tCurrentArgumentLength; - } - return tResult; + if (aArguments.empty()) + return 0; + std::vector argumentLengths(aArguments.size()); + std::transform(std::begin(aArguments), std::end(aArguments), std::begin(argumentLengths), [](const auto& arg) { + const auto& names = arg->mNames; + auto maxLength = std::accumulate(std::begin(names), std::end(names), 0, [](const auto& sum, const auto& s) { + return sum + s.size() + 2; // +2 for ", " + }); + return maxLength - 2; // -2 since the last one doesn't need ", " + }); + return *std::max_element(std::begin(argumentLengths), std::end(argumentLengths)); } // Used by print_help. @@ -670,7 +669,7 @@ class ArgumentParser { try { \ parser.parse_args(argc, argv); \ } catch (const std::runtime_error& err) { \ - std::cerr << err.what() << std::endl; \ + std::cout << err.what() << std::endl; \ parser.print_help(); \ exit(0); \ } From 603e87ae69382af72ef4904d8bf4a79e3b5ce094 Mon Sep 17 00:00:00 2001 From: Stephan van Veen Date: Sat, 11 May 2019 12:36:32 +0200 Subject: [PATCH 03/10] Move validation logic into Argument class itself --- include/argparse.hpp | 70 ++++++++++++++++++++++++++------------------ 1 file changed, 41 insertions(+), 29 deletions(-) diff --git a/include/argparse.hpp b/include/argparse.hpp index b8a240c..f55d141 100644 --- a/include/argparse.hpp +++ b/include/argparse.hpp @@ -105,6 +105,36 @@ public: return *this; } + /* + * @throws std::runtime_error if argument values are not valid + */ + void validate() const { + if (mIsOptional) { + if (mIsUsed && mNumArgs > 0) { + if (mValues.size() != mNumArgs) { + // All cool if there's a default value to return + // If no default value, then there's a problem + if (!mDefaultValue.has_value()) { + std::cout << "error: " << mUsedName << ": expected " << mNumArgs << " argument(s). " + << mValues.size() << " provided.\n" << std::endl; + throw std::runtime_error("wrong number of arguments"); + } + } + } + else { + // TODO: check if an implicit value was programmed for this argument + } + } + else { + if (mValues.size() != mNumArgs) { + std::cout << "error: " << mUsedName << ": expected " << mNumArgs << " argument(s). " + << mValues.size() << " provided.\n" << std::endl; + throw std::runtime_error("wrong number of arguments"); + } + } + } + + template bool operator!=(const T& aRhs) const { return !(*this == aRhs); @@ -602,35 +632,17 @@ class ArgumentParser { * @throws std::runtime_error in case of any invalid argument */ void parse_args_validate() { - // Check if all positional arguments are parsed - for (const auto& tArgument : mPositionalArguments) { - if (tArgument->mValues.size() != tArgument->mNumArgs) { - std::stringstream stream; - stream << "error: " << tArgument->mUsedName << ": expected " - << tArgument->mNumArgs << (tArgument->mNumArgs == 1 ? " argument. " : " arguments. ") - << tArgument->mValues.size() << " provided.\n" << std::endl; - throw std::runtime_error(stream.str()); - } - } - - // Check if all user-provided optional argument values are parsed correctly - for (const auto& tArgument : mOptionalArguments) { - if (tArgument->mIsUsed && tArgument->mNumArgs > 0) { - if (tArgument->mValues.size() != tArgument->mNumArgs) { - // All cool if there's a default value to return - // If no default value, then there's a problem - if (!tArgument->mDefaultValue.has_value()) { - std::stringstream stream; - stream << "error: " << tArgument->mUsedName << ": expected " - << tArgument->mNumArgs << (tArgument->mNumArgs == 1 ? " argument. " : " arguments. ") - << tArgument->mValues.size() << " provided.\n" << std::endl; - throw std::runtime_error(stream.str()); - } - } - } - else { - // TODO: check if an implicit value was programmed for this argument - } + 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; } } From 8dd508d4b629d5757d02c8f332c1d902ec23d849 Mon Sep 17 00:00:00 2001 From: Stephan van Veen Date: Sat, 11 May 2019 12:41:57 +0200 Subject: [PATCH 04/10] Put error message into exception instead of std::cerr --- include/argparse.hpp | 22 +++++++++------------- 1 file changed, 9 insertions(+), 13 deletions(-) diff --git a/include/argparse.hpp b/include/argparse.hpp index f55d141..c8dfdc6 100644 --- a/include/argparse.hpp +++ b/include/argparse.hpp @@ -110,16 +110,11 @@ public: */ void validate() const { if (mIsOptional) { - if (mIsUsed && mNumArgs > 0) { - if (mValues.size() != mNumArgs) { - // All cool if there's a default value to return - // If no default value, then there's a problem - if (!mDefaultValue.has_value()) { - std::cout << "error: " << mUsedName << ": expected " << mNumArgs << " argument(s). " - << mValues.size() << " provided.\n" << std::endl; - throw std::runtime_error("wrong number of arguments"); - } - } + if (mIsUsed && mValues.size() != mNumArgs && !mDefaultValue.has_value()) { + std::stringstream stream; + stream << "error: " << mUsedName << ": expected " << mNumArgs << " argument(s). " + << mValues.size() << " provided.\n" << std::endl; + throw std::runtime_error(stream.str()); } else { // TODO: check if an implicit value was programmed for this argument @@ -127,9 +122,10 @@ public: } else { if (mValues.size() != mNumArgs) { - std::cout << "error: " << mUsedName << ": expected " << mNumArgs << " argument(s). " - << mValues.size() << " provided.\n" << std::endl; - throw std::runtime_error("wrong number of arguments"); + std::stringstream stream; + stream << "error: " << mUsedName << ": expected " << mNumArgs << " argument(s). " + << mValues.size() << " provided.\n" << std::endl; + throw std::runtime_error(stream.str()); } } } From 95746ae159e40da67ada2feae2a5aa90549b0a63 Mon Sep 17 00:00:00 2001 From: Stephan van Veen Date: Sat, 11 May 2019 13:08:21 +0200 Subject: [PATCH 05/10] Put helper methods into anonymous namespace --- include/argparse.hpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/include/argparse.hpp b/include/argparse.hpp index c8dfdc6..0b6c1eb 100644 --- a/include/argparse.hpp +++ b/include/argparse.hpp @@ -44,6 +44,7 @@ SOFTWARE. namespace argparse { +namespace { // anonymous namespace for helper methods - not visible outside this header file // Some utility structs to check template specialization template class Ref> struct is_specialization : std::false_type {}; @@ -67,6 +68,7 @@ T get_from_list(const std::list& aList, size_t aIndex) { } return T(); } +} class Argument { friend class ArgumentParser; From e923cf2ac3fcd7da598932523d69b8c84506bd9a Mon Sep 17 00:00:00 2001 From: Stephan van Veen Date: Sat, 11 May 2019 13:18:37 +0200 Subject: [PATCH 06/10] Invert if-condition to improve readability --- include/argparse.hpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/include/argparse.hpp b/include/argparse.hpp index 0b6c1eb..f17c5b5 100644 --- a/include/argparse.hpp +++ b/include/argparse.hpp @@ -314,10 +314,10 @@ class ArgumentParser { Argument& add_argument(Targs... Fargs) { std::shared_ptr tArgument = std::make_shared(std::move(Fargs)...); - if (!tArgument->mIsOptional) - mPositionalArguments.emplace_back(tArgument); - else + if (tArgument->mIsOptional) mOptionalArguments.emplace_back(tArgument); + else + mPositionalArguments.emplace_back(tArgument); for (auto& mName : tArgument->mNames) { mArgumentMap.insert_or_assign(mName, tArgument); From 3bd7b342f0f20fa9fdd3dcf5a98462b098a73111 Mon Sep 17 00:00:00 2001 From: Stephan van Veen Date: Sat, 11 May 2019 13:19:17 +0200 Subject: [PATCH 07/10] Add const --- include/argparse.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/argparse.hpp b/include/argparse.hpp index f17c5b5..0c0f07b 100644 --- a/include/argparse.hpp +++ b/include/argparse.hpp @@ -319,7 +319,7 @@ class ArgumentParser { else mPositionalArguments.emplace_back(tArgument); - for (auto& mName : tArgument->mNames) { + for (const auto& mName : tArgument->mNames) { mArgumentMap.insert_or_assign(mName, tArgument); } return *tArgument; From ecf8286c9eda365eb4ca1620a896ceb374fed701 Mon Sep 17 00:00:00 2001 From: Stephan van Veen Date: Sat, 11 May 2019 13:33:08 +0200 Subject: [PATCH 08/10] Resolve template recursion in add_parents method --- include/argparse.hpp | 36 +++++++++++++++--------------------- 1 file changed, 15 insertions(+), 21 deletions(-) diff --git a/include/argparse.hpp b/include/argparse.hpp index 0c0f07b..533ee82 100644 --- a/include/argparse.hpp +++ b/include/argparse.hpp @@ -325,30 +325,24 @@ class ArgumentParser { return *tArgument; } - // Base case for add_parents parameter packing - void add_parents() { - for (const auto& tParentParser : mParentParsers) { - auto tPositionalArguments = tParentParser.mPositionalArguments; - for (auto& tArgument : tPositionalArguments) { - mPositionalArguments.emplace_back(tArgument); - } - auto tOptionalArguments = tParentParser.mOptionalArguments; - for (auto& tArgument : tOptionalArguments) { - mOptionalArguments.emplace_back(tArgument); - } - auto tArgumentMap = tParentParser.mArgumentMap; - for (auto&[tKey, tValue] : tArgumentMap) { + // Parameter packed add_parents method + // Accepts a variadic number of ArgumentParser objects + template + void add_parents(Targs... Fargs) { + const auto tNewParentParsers = {Fargs...}; + for (const auto& tParentParser : tNewParentParsers) { + const auto& tPositionalArguments = tParentParser.mPositionalArguments; + std::copy(std::begin(tPositionalArguments), std::end(tPositionalArguments), std::back_inserter(mPositionalArguments)); + + const auto& tOptionalArguments = tParentParser.mOptionalArguments; + std::copy(std::begin(tOptionalArguments), std::end(tOptionalArguments), std::back_inserter(mOptionalArguments)); + + const auto& tArgumentMap = tParentParser.mArgumentMap; + for (const auto&[tKey, tValue] : tArgumentMap) { mArgumentMap.insert_or_assign(tKey, tValue); } } - } - - // Parameter packed add_parents method - // Accepts a variadic number of ArgumentParser objects - template - void add_parents(T aArgumentParser, Targs... Fargs) { - mParentParsers.emplace_back(aArgumentParser); - add_parents(Fargs...); + std::move(std::begin(tNewParentParsers), std::end(tNewParentParsers), std::back_inserter(mParentParsers)); } /* Call parse_args_internal - which does all the work From 84adf5b2dfa43e516809931c2d438b01336d6c65 Mon Sep 17 00:00:00 2001 From: Stephan van Veen Date: Sun, 12 May 2019 14:46:24 +0200 Subject: [PATCH 09/10] Simplify creating of help option --- include/argparse.hpp | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/include/argparse.hpp b/include/argparse.hpp index 533ee82..c5e8863 100644 --- a/include/argparse.hpp +++ b/include/argparse.hpp @@ -291,6 +291,10 @@ public: size_t mNumArgs = 1; bool mIsOptional = false; bool mIsUsed = false; // relevant for optional arguments. True if used by user + + public: + static constexpr auto mHelpOption = "-h"; + static constexpr auto mHelpOptionLong = "--help"; }; class ArgumentParser { @@ -298,14 +302,11 @@ class ArgumentParser { explicit ArgumentParser(std::string aProgramName = {}) : mProgramName(std::move(aProgramName)) { - std::shared_ptr tArgument = std::make_shared("-h", "--help"); - tArgument->mHelp = "show this help message and exit"; - tArgument->mNumArgs = 0; - tArgument->mDefaultValue = false; - tArgument->mImplicitValue = true; - mOptionalArguments.emplace_back(tArgument); - mArgumentMap.insert_or_assign(std::string("-h"), tArgument); - mArgumentMap.insert_or_assign(std::string("--help"), tArgument); + add_argument(Argument::mHelpOption, Argument::mHelpOptionLong) + .help("show this help message and exit") + .nargs(0) + .default_value(false) + .implicit_value(true); } // Parameter packing @@ -505,7 +506,7 @@ class ArgumentParser { mProgramName = argv[0]; for (int i = 1; i < argc; i++) { auto tCurrentArgument = std::string(argv[i]); - if (tCurrentArgument == "-h" || tCurrentArgument == "--help") { + if (tCurrentArgument == Argument::mHelpOption || tCurrentArgument == Argument::mHelpOptionLong) { throw std::runtime_error("help called"); } auto tIterator = mArgumentMap.find(argv[i]); From 15f45c20548dd4771c4754a1fe9dcc8f8234a201 Mon Sep 17 00:00:00 2001 From: Stephan van Veen Date: Sun, 12 May 2019 15:21:11 +0200 Subject: [PATCH 10/10] Simplify get method --- include/argparse.hpp | 40 ++++++++++------------------------------ 1 file changed, 10 insertions(+), 30 deletions(-) diff --git a/include/argparse.hpp b/include/argparse.hpp index c5e8863..4910c9a 100644 --- a/include/argparse.hpp +++ b/include/argparse.hpp @@ -150,7 +150,7 @@ public: template typename std::enable_if::value, bool>::type operator==(const T& aRhs) const { - T tLhs = get_vector(); + T tLhs = get(); if (tLhs.size() != aRhs.size()) return false; else { @@ -167,7 +167,7 @@ public: template typename std::enable_if::value, bool>::type operator==(const T& aRhs) const { - T tLhs = get_list(); + T tLhs = get(); if (tLhs.size() != aRhs.size()) return false; else { @@ -188,7 +188,9 @@ public: // Getter for template types other than std::vector and std::list template - T get() const { + typename std::enable_if::value && + !is_specialization::value, T>::type + get() const { if (mValues.empty()) { if (mDefaultValue.has_value()) { return std::any_cast(mDefaultValue); @@ -210,7 +212,8 @@ public: // Getter for std::vector. Here T = std::vector<...> template - T get_vector() const { + typename std::enable_if::value, T>::type + get() const { T tResult; if (mValues.empty()) { if (mDefaultValue.has_value()) { @@ -246,7 +249,8 @@ public: // Getter for std::list. Here T = std::list<...> template - T get_list() const { + typename std::enable_if::value, T>::type + get() const { T tResult; if (mValues.empty()) { if (mDefaultValue.has_value()) { @@ -366,9 +370,7 @@ class ArgumentParser { // Getter enabled for all template types other than std::vector and std::list template - typename std::enable_if::value && - !is_specialization::value, T>::type - get(const char * aArgumentName) { + T get(const std::string& aArgumentName) { auto tIterator = mArgumentMap.find(aArgumentName); if (tIterator != mArgumentMap.end()) { return tIterator->second->get(); @@ -376,28 +378,6 @@ class ArgumentParser { return T(); } - // Getter enabled for std::vector - template - typename std::enable_if::value, T>::type - get(const char * aArgumentName) { - auto tIterator = mArgumentMap.find(aArgumentName); - if (tIterator != mArgumentMap.end()) { - return tIterator->second->get_vector(); - } - return T(); - } - - // Getter enabled for std::list - template - typename std::enable_if::value, T>::type - get(const char * aArgumentName) { - auto tIterator = mArgumentMap.find(aArgumentName); - if (tIterator != mArgumentMap.end()) { - return tIterator->second->get_list(); - } - return T(); - } - // Indexing operator. Return a reference to an Argument object // Used in conjuction with Argument.operator== e.g., parser["foo"] == true Argument& operator[](const std::string& aArgumentName) {