From a8ab419fcee610891602eac72ffe051db656e943 Mon Sep 17 00:00:00 2001 From: "Taylor C. Richberger" Date: Mon, 9 May 2016 17:08:23 -0600 Subject: resolve this, and improve help generation --- README.md | 39 ++++++++++------- args.hxx | 140 ++++++++++++++++++++++++++++++++++++++++++++++++++------------ test.cxx | 37 +++++++++++++++++ 3 files changed, 174 insertions(+), 42 deletions(-) diff --git a/README.md b/README.md index 1f97f93..b3b3376 100644 --- a/README.md +++ b/README.md @@ -1,7 +1,7 @@ # args A simple, small, flexible, single-header C++11 argument parsing library, in -about 1.3K lines of code. +about 1K lines of code. This is designed to somewhat replicate the behavior of Python's argparse, but in C++, with static type checking, and hopefully a lot faster. @@ -42,16 +42,14 @@ It: * Lets you parse, by default, any type that has a stream extractor operator for it. If this doesn't work for your uses, you can supply a function and parse the string yourself if you like. +* Lets you decide not to allow separate-argument argument flags or joined ones + (like disallowing `--foo bar`, requiring `--foo=bar`, or the inverse, or the + same for short options). # What does it not do? There are tons of things this library does not do! -## It does not yet: - -* Let you decide not to allow separate-argument argument flags or joined ones - (like disallowing `--foo bar`, requiring `--foo=bar`, or the inverse, or the - same for short options). ## It will not ever: @@ -65,6 +63,12 @@ There are tons of things this library does not do! `--foo` in the same parser), though shortopt and longopt prefixes can be different. * Allow you to have argument flags only optionally accept arguments +* Allow you to make flag arguments sensitive to order (like gnu find), or make + them sensitive to relative ordering with positional flags. The only + orderings that are order-sensitive are: + * Positional options relative to one-another + * List positional options or flag arguments to each of their own respective + items * Allow you to use a positional argument list before any other positional arguments (the last argument list will slurp all subsequent positional arguments). The logic for allowing this would be a lot more code than I'd @@ -112,16 +116,19 @@ can be pulled from their value and values attributes, if applicable. # How fast is it? This should not really be a question you ask when you are looking for an -argument-parsing library, but I did run a simple benchmark against args, TCLAP, -and boost::program_options, which parses the command line `-i 7 -c a 2.7 --char -b 8.4 -c c 8.8 --char d` with a parser that parses -i as an int, -c as a list -of chars, and the positional parameters as a list of doubles (the command line -was originally much more complex, but TCLAP's limitations made me trim it down -so I could use a common command line across all libraries. I also have to copy -in the arguments list with every run, because TCLAP permutes its argument list -as it runs (and comparison would have been unfair without comparing all about -equally), but that surprisingly didn't affect much. Also tested is pulling the -arguments out, but that was fast compared to parsing, as would be expected. +argument-parsing library, but every test I've done shows args as being about +65% faster than TCLAP and 220% faster than boost::program_options. + +The simplest benchmark I threw together is the following one, which parses the +command line `-i 7 -c a 2.7 --char b 8.4 -c c 8.8 --char d` with a parser that +parses -i as an int, -c as a list of chars, and the positional parameters as a +list of doubles (the command line was originally much more complex, but TCLAP's +limitations made me trim it down so I could use a common command line across +all libraries. I also have to copy in the arguments list with every run, +because TCLAP permutes its argument list as it runs (and comparison would have +been unfair without comparing all about equally), but that surprisingly didn't +affect much. Also tested is pulling the arguments out, but that was fast +compared to parsing, as would be expected. ### The run: diff --git a/args.hxx b/args.hxx index 817af7c..6993016 100644 --- a/args.hxx +++ b/args.hxx @@ -151,7 +151,7 @@ namespace args } }; - /** A simple unified option type for unified initializer lists + /** A simple unified option type for unified initializer lists for the Matcher class. */ struct EitherOpt { @@ -195,9 +195,11 @@ namespace args - /** A class of "matchers", specifying short and long options that can possibly be matched + /** A class of "matchers", specifying short and long options that can + * possibly be matched. * - * This is supposed to be constructed and then passed in, not used directly from user code. + * This is supposed to be constructed and then passed in, not used directly + * from user code. */ class Matcher { @@ -227,7 +229,15 @@ namespace args /** Specify a mixed single initializer-list of both short and long opts * - * This is the fancy one: args::Matcher{'a'}, or args::Matcher{"foo"}, or args::Matcher{"foo", 'f', 'F', "FoO"} + * This is the fancy one. It takes a single initializer list of + * any number of any mixed kinds of options. Chars are + * automatically interpreted as short options, and strings are + * automatically interpreted as long options: + * + * args::Matcher{'a'} + * args::Matcher{"foo"} + * args::Matcher{'h', "help"} + * args::Matcher{"foo", 'f', 'F', "FoO"} */ Matcher(std::initializer_list in) : shortOpts(EitherOpt::GetShort(in)), longOpts(EitherOpt::GetLong(in)) {} @@ -270,18 +280,18 @@ namespace args /** (INTERNAL) Get all option strings as a vector, with the prefixes and names embedded */ - std::vector GetOptionStrings(const std::string &shortPrefix, const std::string &longPrefix, const std::string &name, const std::string longSeparator) const + std::vector GetOptionStrings(const std::string &shortPrefix, const std::string &longPrefix, const std::string &name, const std::string &shortSeparator, const std::string longSeparator) const { const std::string bracedname(std::string("[") + name + "]"); std::vector optStrings; optStrings.reserve(shortOpts.size() + longOpts.size()); for (const char opt: shortOpts) { - optStrings.emplace_back(shortPrefix + std::string(1, opt) + bracedname); + optStrings.emplace_back(shortPrefix + std::string(1, opt) + shortSeparator + bracedname); } for (const std::string &opt: longOpts) { - optStrings.emplace_back(longPrefix + opt + (longSeparator.empty() ? std::string(" ") : longSeparator) + bracedname); + optStrings.emplace_back(longPrefix + opt + longSeparator + bracedname); } return optStrings; } @@ -309,7 +319,7 @@ namespace args return Matched(); } - virtual std::tuple GetDescription(const std::string &shortPrefix, const std::string &longPrefix, const std::string &longSeparator) const + virtual std::tuple GetDescription(const std::string &shortPrefix, const std::string &longPrefix, const std::string &shortSeparator, const std::string &longSeparator) const { std::tuple description; std::get<1>(description) = help; @@ -333,7 +343,7 @@ namespace args NamedBase(const std::string &name, const std::string &help) : Base(help), name(name) {} virtual ~NamedBase() {} - virtual std::tuple GetDescription(const std::string &shortPrefix, const std::string &longPrefi, const std::string &longSeparator) const override + virtual std::tuple GetDescription(const std::string &shortPrefix, const std::string &longPrefi, const std::string &shortSeparator, const std::string &longSeparator) const override { std::tuple description; std::get<0>(description) = name; @@ -378,7 +388,7 @@ namespace args return nullptr; } - virtual std::tuple GetDescription(const std::string &shortPrefix, const std::string &longPrefix, const std::string &longSeparator) const override + virtual std::tuple GetDescription(const std::string &shortPrefix, const std::string &longPrefix, const std::string &shortSeparator, const std::string &longSeparator) const override { std::tuple description; const std::vector optStrings(matcher.GetOptionStrings(shortPrefix, longPrefix)); @@ -406,10 +416,10 @@ namespace args virtual ~ArgFlagBase() {} virtual void ParseArg(const std::string &value) = 0; - virtual std::tuple GetDescription(const std::string &shortPrefix, const std::string &longPrefix, const std::string &longSeparator) const override + virtual std::tuple GetDescription(const std::string &shortPrefix, const std::string &longPrefix, const std::string &shortSeparator, const std::string &longSeparator) const override { std::tuple description; - const std::vector optStrings(matcher.GetOptionStrings(shortPrefix, longPrefix, name, longSeparator)); + const std::vector optStrings(matcher.GetOptionStrings(shortPrefix, longPrefix, name, shortSeparator, longSeparator)); std::ostringstream flagstream; for (auto it = std::begin(optStrings); it != std::end(optStrings); ++it) { @@ -605,7 +615,7 @@ namespace args /** Get all the child descriptions for help generation */ - std::vector> GetChildDescriptions(const std::string &shortPrefix, const std::string &longPrefix, const std::string &longSeparator, unsigned int indent = 0) const + std::vector> GetChildDescriptions(const std::string &shortPrefix, const std::string &longPrefix, const std::string &shortSeparator, const std::string &longSeparator, unsigned int indent = 0) const { std::vector> descriptions; for (const auto &child: children) @@ -616,14 +626,14 @@ namespace args { // Push that group description on the back: descriptions.emplace_back(group->help, "", indent); - std::vector> groupDescriptions(group->GetChildDescriptions(shortPrefix, longPrefix, longSeparator, indent + 1)); + std::vector> groupDescriptions(group->GetChildDescriptions(shortPrefix, longPrefix, shortSeparator, longSeparator, indent + 1)); descriptions.insert( std::end(descriptions), std::make_move_iterator(std::begin(groupDescriptions)), std::make_move_iterator(std::end(groupDescriptions))); } else if (named) { - const std::tuple description(named->GetDescription(shortPrefix, longPrefix, longSeparator)); + const std::tuple description(named->GetDescription(shortPrefix, longPrefix, shortSeparator, longSeparator)); descriptions.emplace_back(std::get<0>(description), std::get<1>(description), indent); } } @@ -737,7 +747,14 @@ namespace args std::string terminator; + bool allowJoinedShortArgument; + bool allowJoinedLongArgument; + bool allowSeparateShortArgument; + bool allowSeparateLongArgument; + public: + /** A simple structure of parameters for easy user-modifyable help menus + */ struct HelpParams { /** The width of the help menu @@ -773,7 +790,11 @@ namespace args longprefix("--"), shortprefix("-"), longseparator("="), - terminator("--") {} + terminator("--"), + allowJoinedShortArgument(true), + allowJoinedLongArgument(true), + allowSeparateShortArgument(true), + allowSeparateLongArgument(true) {} /** The program name for help generation */ @@ -840,6 +861,41 @@ namespace args void Terminator(const std::string &terminator) { this->terminator = terminator; } + /** Get the current argument separation parameters. + * + * See SetArgumentSeparations for details on what each one means. + */ + void GetArgumentSeparations( + bool &allowJoinedShortArgument, + bool &allowJoinedLongArgument, + bool &allowSeparateShortArgument, + bool &allowSeparateLongArgument) const + { + allowJoinedShortArgument = this->allowJoinedShortArgument; + allowJoinedLongArgument = this->allowJoinedLongArgument; + allowSeparateShortArgument = this->allowSeparateShortArgument; + allowSeparateLongArgument = this->allowSeparateLongArgument; + } + + /** Change allowed option separation. + * + * \param allowJoinedShortArgument Allow a short flag that accepts an argument to be passed its argument immediately next to it (ie. in the same argv field) + * \param allowJoinedLongArgument Allow a long flag that accepts an argument to be passed its argument separated by the longseparator (ie. in the same argv field) + * \param allowSeparateShortArgument Allow a short flag that accepts an argument to be passed its argument separated by whitespace (ie. in the next argv field) + * \param allowSeparateLongArgument Allow a long flag that accepts an argument to be passed its argument separated by whitespace (ie. in the next argv field) + */ + void SetArgumentSeparations( + const bool allowJoinedShortArgument, + const bool allowJoinedLongArgument, + const bool allowSeparateShortArgument, + const bool allowSeparateLongArgument) + { + this->allowJoinedShortArgument = allowJoinedShortArgument; + this->allowJoinedLongArgument = allowJoinedLongArgument; + this->allowSeparateShortArgument = allowSeparateShortArgument; + this->allowSeparateLongArgument = allowSeparateLongArgument; + } + /** Pass the help menu into an ostream */ void Help(std::ostream &help) const @@ -881,7 +937,7 @@ namespace args } help << "\n"; help << std::string(helpParams.progindent, ' ') << "OPTIONS:\n\n"; - for (const auto &description: GetChildDescriptions(shortprefix, longprefix, longseparator)) + for (const auto &description: GetChildDescriptions(shortprefix, longprefix, allowJoinedShortArgument ? "" : " ", allowJoinedLongArgument ? longseparator : " ")) { const unsigned int groupindent = std::get<2>(description) * helpParams.eachgroupindent; const std::vector flags(Wrap(std::get<0>(description), helpParams.width - (helpParams.flagindent + helpParams.helpindent + helpParams.gutter))); @@ -978,18 +1034,34 @@ namespace args { if (separator != argchunk.npos) { - argbase->ParseArg(argchunk.substr(separator + longseparator.size())); + if (allowJoinedLongArgument) + { + argbase->ParseArg(argchunk.substr(separator + longseparator.size())); + } else + { + std::ostringstream problem; + problem << "Flag '" << arg << "' was passed a joined argument, but these are disallowed"; + throw ParseError(problem.str().c_str()); + } } else { ++it; if (it == end) { std::ostringstream problem; - problem << "Argument " << arg << " requires an argument but received none"; + problem << "Flag '" << arg << "' requires an argument but received none"; throw ParseError(problem.str().c_str()); } else { - argbase->ParseArg(*it); + if (allowSeparateLongArgument) + { + argbase->ParseArg(*it); + } else + { + std::ostringstream problem; + problem << "Flag '" << arg << "' was passed a separate argument, but these are disallowed"; + throw ParseError(problem.str().c_str()); + } } } } else if (separator != argchunk.npos) @@ -1001,7 +1073,7 @@ namespace args } else { std::ostringstream problem; - problem << "Argument could not be matched: " << arg; + problem << "Flag could not be matched: " << arg; throw ParseError(problem.str().c_str()); } // Check short args @@ -1021,18 +1093,34 @@ namespace args const std::string arg(++argit, std::end(argchunk)); if (!arg.empty()) { - argbase->ParseArg(arg); + if (allowJoinedShortArgument) + { + argbase->ParseArg(arg); + } else + { + std::ostringstream problem; + problem << "Flag '" << *argit << "' was passed a joined argument, but these are disallowed"; + throw ParseError(problem.str().c_str()); + } } else { ++it; if (it == end) { std::ostringstream problem; - problem << "Flag '" << arg << "' requires an argument but received none"; + problem << "Flag '" << *argit << "' requires an argument but received none"; throw ParseError(problem.str().c_str()); } else { - argbase->ParseArg(*it); + if (allowSeparateShortArgument) + { + argbase->ParseArg(*it); + } else + { + std::ostringstream problem; + problem << "Flag '" << *argit << "' was passed a separate argument, but these are disallowed"; + throw ParseError(problem.str().c_str()); + } } } // Because this argchunk is done regardless @@ -1041,7 +1129,7 @@ namespace args } else { std::ostringstream problem; - problem << "Argument could not be matched: '" << arg << "'"; + problem << "Flag could not be matched: '" << arg << "'"; throw ParseError(problem.str().c_str()); } } @@ -1054,7 +1142,7 @@ namespace args } else { std::ostringstream problem; - problem << "Passed in argument, but no positional arguments were ready to receive it" << chunk; + problem << "Passed in argument, but no positional arguments were ready to receive it: " << chunk; throw ParseError(problem.str().c_str()); } } diff --git a/test.cxx b/test.cxx index 916bf9f..4816691 100644 --- a/test.cxx +++ b/test.cxx @@ -68,6 +68,13 @@ TEST_CASE("Argument flags work as expected, with clustering", "[args]") REQUIRE_FALSE(bix); } +TEST_CASE("Passing an argument to a non-argument flag throws an error", "[args]") +{ + args::ArgumentParser parser("This is a test program.", "This goes after the options."); + args::Flag bar(parser, "BAR", "test flag", args::Matcher{'b', "bar"}); + REQUIRE_THROWS_AS(parser.ParseArgs(std::vector{"--bar=test"}), args::ParseError); +} + TEST_CASE("Unified argument lists for match work", "[args]") { args::ArgumentParser parser("This is a test program.", "This goes after the options."); @@ -278,3 +285,33 @@ TEST_CASE("Help menu can be grabbed as a string, passed into a stream, or by usi parser.Help(null); null << parser; } + +TEST_CASE("Required argument separation being violated throws an error", "[args]") +{ + args::ArgumentParser parser("This is a test program.", "This goes after the options."); + args::ArgFlag bar(parser, "BAR", "test flag", args::Matcher{'b', "bar"}); + REQUIRE_NOTHROW(parser.ParseArgs(std::vector{"-btest"})); + REQUIRE_NOTHROW(parser.ParseArgs(std::vector{"--bar=test"})); + REQUIRE_NOTHROW(parser.ParseArgs(std::vector{"-b", "test"})); + REQUIRE_NOTHROW(parser.ParseArgs(std::vector{"--bar", "test"})); + parser.SetArgumentSeparations(true, false, false, false); + REQUIRE_NOTHROW(parser.ParseArgs(std::vector{"-btest"})); + REQUIRE_THROWS_AS(parser.ParseArgs(std::vector{"--bar=test"}), args::ParseError); + REQUIRE_THROWS_AS(parser.ParseArgs(std::vector{"-b", "test"}), args::ParseError); + REQUIRE_THROWS_AS(parser.ParseArgs(std::vector{"--bar", "test"}), args::ParseError); + parser.SetArgumentSeparations(false, true, false, false); + REQUIRE_THROWS_AS(parser.ParseArgs(std::vector{"-btest"}), args::ParseError); + REQUIRE_NOTHROW(parser.ParseArgs(std::vector{"--bar=test"})); + REQUIRE_THROWS_AS(parser.ParseArgs(std::vector{"-b", "test"}), args::ParseError); + REQUIRE_THROWS_AS(parser.ParseArgs(std::vector{"--bar", "test"}), args::ParseError); + parser.SetArgumentSeparations(false, false, true, false); + REQUIRE_THROWS_AS(parser.ParseArgs(std::vector{"-btest"}), args::ParseError); + REQUIRE_THROWS_AS(parser.ParseArgs(std::vector{"--bar=test"}), args::ParseError); + REQUIRE_NOTHROW(parser.ParseArgs(std::vector{"-b", "test"})); + REQUIRE_THROWS_AS(parser.ParseArgs(std::vector{"--bar", "test"}), args::ParseError); + parser.SetArgumentSeparations(false, false, false, true); + REQUIRE_THROWS_AS(parser.ParseArgs(std::vector{"-btest"}), args::ParseError); + REQUIRE_THROWS_AS(parser.ParseArgs(std::vector{"--bar=test"}), args::ParseError); + REQUIRE_THROWS_AS(parser.ParseArgs(std::vector{"-b", "test"}), args::ParseError); + REQUIRE_NOTHROW(parser.ParseArgs(std::vector{"--bar", "test"})); +} -- cgit v1.2.1