Allow specifying an option terminator ("--" on POSIX)

Add an option terminator to option styles. If not empty, this changes the parsing in the following ways:
 - Initially, arguments are not allowed to start with an option prefix;
 - unknown options will trigger a parse error;
 - after the specified terminator is encountered, all remaining tokens will be plain arguments;
 - the parser still tolerates "--" prefixes as option values. It has this in common with Python's
   argparse, but POSIX parsers tend to also error out with a "no option value provided" error.

Other change:
 - Unnamed arguments must be separated from a group of short options: the "abc" token in "-vabc" cannot be interpreted as an unnamed argument, unless -a, -b and -c options are defined this will now result in an error.

Having this terminator is not entirely backwards compatible, in particular it becomes cumbersome to pass in negative numbers as arguments. This is a known problem for POSIX style parsers.
This commit therefore defines a new style for POSIX with terminator.

Author: Roeland Schoukens

include/lyra/arg.hpp

@@ -80,6 +80,14 @@ class arg : public bound_parser<arg>
 
 		auto const & token = tokens.argument();
 
+		// argument not allowed at this point
+		if (token.type == detail::token_type::unknown)
+		{
+			// Nothing to match against.
+			return parse_result::ok(
+				detail::parse_state(parser_result_type::no_match, tokens));
+		}
+
 		auto valueRef = static_cast<detail::BoundValueRefBase *>(m_ref.get());
 
 		if (value_choices)

include/lyra/detail/tokens.hpp

@@ -124,8 +124,11 @@ class token_iterator
 		: style(opt_style)
 		, args_i(args.begin())
 		, args_e(args.end())
-		, args_i_sub(opt_style.short_option_size)
-	{}
+		, args_i_sub(0)
+		, had_terminator(false)
+	{
+		handle_next_arg_i();
+	}
 
 	explicit operator bool() const noexcept { return args_i != args_e; }
 
@@ -138,14 +141,14 @@ class token_iterator
 			if (++args_i_sub >= args_i->size())
 			{
 				++args_i;
-				args_i_sub = style.short_option_size;
+				handle_next_arg_i();
 			}
 		}
 		else
 		{
 			// Regular arg or long option, just advance to the next arg.
 			++args_i;
-			args_i_sub = style.short_option_size;
+			handle_next_arg_i();
 		}
 		return *this;
 	}
@@ -158,7 +161,7 @@ class token_iterator
 			args_i += 2;
 		else
 			++args_i;
-		args_i_sub = style.short_option_size;
+		handle_next_arg_i();
 		return *this;
 	}
 
@@ -195,6 +198,10 @@ class token_iterator
 	// Extract the current option token.
 	token option() const
 	{
+		if (had_terminator)
+		{
+			return token();
+		}
 		if (has_long_option_prefix())
 		{
 			if (has_value_delimiter())
@@ -249,7 +256,25 @@ class token_iterator
 		return token();
 	}
 
-	token argument() const { return token(token_type::argument, *args_i); }
+	token argument() const
+	{
+		if (args_i_sub != style.short_option_size)
+		{
+			// can't have value argument in the middle of a string of
+			// short options. Still return the argument string so it
+			// shows up in error messages.
+			return token(token_type::unknown, *args_i);
+		}
+		if (!style.option_terminator.empty()
+				&& has_option_prefix() && !had_terminator)
+		{
+			// can't have arguments starting with option prefix unless
+			// we had an explicit terminator for arguments. Still return
+			// the argument string so it shows up in error messages.
+			return token(token_type::unknown, *args_i);
+		}
+		return token(token_type::argument, *args_i);
+	}
 
 	static bool is_prefixed(
 		const std::string & prefix, std::size_t size, const std::string & s)
@@ -272,6 +297,7 @@ class token_iterator
 	std::vector<std::string>::const_iterator args_i;
 	std::vector<std::string>::const_iterator args_e;
 	std::string::size_type args_i_sub;
+	bool had_terminator;
 
 	inline bool is_opt_prefix(char c) const noexcept
 	{
@@ -295,6 +321,18 @@ class token_iterator
 		return std::string(
 			static_cast<typename std::string::size_type>(size), prefix[0]);
 	}
+
+	inline void handle_next_arg_i()
+	{
+		args_i_sub = style.short_option_size;
+		if (args_i != args_e
+				&& !style.option_terminator.empty()
+				&& *args_i == style.option_terminator)
+		{
+			had_terminator = true;
+			++args_i;
+		}
+	}
 };
 
 }} // namespace lyra::detail

include/lyra/option_style.hpp

@@ -59,6 +59,7 @@ struct option_style
 	std::size_t long_option_size = 0;
 	std::string short_option_prefix;
 	std::size_t short_option_size = 0;
+	std::string option_terminator = "";
 	opt_print_order options_print_order = opt_print_order::per_declaration;
 	std::size_t indent_size = 2;
 
@@ -70,12 +71,14 @@ struct option_style
 		std::string && short_option_prefix_chars = {},
 		std::size_t short_option_prefix_size = 0,
 		opt_print_order options_print_order_ = opt_print_order::per_declaration,
-		std::size_t indent_size_ = 2)
+		std::size_t indent_size_ = 2,
+		std::string && option_terminator_ = "")
 		: value_delimiters(std::move(value_delimiters_chars))
 		, long_option_prefix(std::move(long_option_prefix_chars))
 		, long_option_size(long_option_prefix_size)
 		, short_option_prefix(std::move(short_option_prefix_chars))
 		, short_option_size(short_option_prefix_size)
+		, option_terminator(std::move(option_terminator_))
 		, options_print_order(options_print_order_)
 		, indent_size(indent_size_)
 	{}
@@ -87,6 +90,7 @@ struct option_style
 
 	// Styles..
 
+	static const option_style & posix_with_terminator();
 	static const option_style & posix();
 	static const option_style & posix_brief();
 	static const option_style & windows();
@@ -189,6 +193,12 @@ These provide definitions for common syntax of option styles:
 
 end::reference[] */
 
+inline const option_style & option_style::posix_with_terminator()
+{
+	static const option_style style("= ", "-", 2, "-", 1, opt_print_order::per_declaration, 2, "--");
+	return style;
+}
+
 inline const option_style & option_style::posix()
 {
 	static const option_style style("= ", "-", 2, "-", 1);

tests/terminator_run_test.cpp

@@ -0,0 +1,80 @@
+#include "mini_test.hpp"
+#include <lyra/lyra.hpp>
+#include <iostream>
+
+using ArgList = std::initializer_list<std::string>;
+
+void test_terminator1(bfg::mini_test::scope& test, std::string test_value)
+{
+	using namespace lyra;
+	{
+		std::string default_value = "default1";
+		std::string arg_value_s = default_value;
+		std::string opt_value_s = default_value;
+
+		auto cli = lyra::cli().style(lyra::option_style::posix_with_terminator())
+			| arg(arg_value_s, "value")
+			| opt(opt_value_s, "OPTION").name("--option");
+		ArgList args = { "TestApp", "--", test_value.c_str() };
+		auto result = cli.parse(args);
+		if (!result) std::cerr << result.message() << '\n';
+		test(REQUIRE(result));
+		test(opt_value_s == default_value, "opt_value_s == " + default_value, CONTEXT);
+		test(arg_value_s == test_value, "arg_value_s == " + test_value, CONTEXT);
+	}
+}
+
+
+void test_terminator_no_terminator(bfg::mini_test::scope& test, std::string test_value)
+{
+	using namespace lyra;
+	{
+		std::string default_value = "default1";
+		std::string arg_value_s = default_value;
+		std::string opt_value_s = default_value;
+
+		auto cli = lyra::cli().style(lyra::option_style::posix_with_terminator())
+			| arg(arg_value_s, "value")
+			| opt(opt_value_s, "OPTION").name("--option");
+		ArgList args = { "TestApp", "--option", test_value.c_str() };
+		auto result = cli.parse(args);
+		if (!result) std::cerr << result.message() << '\n';
+		test(REQUIRE(result));
+		test(opt_value_s == test_value, "opt_value_s == " + test_value, CONTEXT);
+		test(arg_value_s == default_value, "arg_value_s == " + default_value, CONTEXT);
+	}
+}
+
+
+void test_terminator_unknown_option(bfg::mini_test::scope& test)
+{
+	using namespace lyra;
+	{
+		std::string default_value = "default1";
+		std::string arg_value_s = default_value;
+		std::string opt_value_s = default_value;
+
+		auto cli = lyra::cli().style(lyra::option_style::posix_with_terminator())
+			| arg(arg_value_s, "value")
+			| opt(opt_value_s, "OPTION").name("--option");
+		ArgList args = { "TestApp", "--unknown" };
+		auto result = cli.parse(args);
+		test(REQUIRE(!result));
+	}
+}
+
+
+int main()
+{
+	using namespace lyra;
+	bfg::mini_test::scope test;
+
+	test_terminator1(test, "test1");
+	test_terminator1(test, "--test1");
+	test_terminator1(test, "--option");
+	test_terminator_no_terminator(test, "test1");
+	test_terminator_no_terminator(test, "--test1"); // Q: should this also be an error?
+	test_terminator_unknown_option(test); // unknown option must raise an error
+
+	return test;
+}