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.

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,17 @@ 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 (!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;
+}