diff --git a/include/sharg/parser.hpp b/include/sharg/parser.hpp index 55ef49ed..dc5f5d1f 100644 --- a/include/sharg/parser.hpp +++ b/include/sharg/parser.hpp @@ -426,9 +426,6 @@ class parser // Determine the format and subcommand. determine_format_and_subcommand(); - // If a subcommand was provided, check that it is valid. - verify_subcommand(); - // Apply all defered operations to the parser, e.g., `add_option`, `add_flag`, `add_positional_option`. for (auto & operation : operations) operation(); @@ -824,7 +821,7 @@ class parser } else { - // Positional options are forbidden by design. Todo: Allow options. Forbidden in check_option_config. + // Positional options are forbidden by design. // Flags and options, which both start with '-', are allowed for the top-level parser. // Otherwise, this is a wrongly spelled subcommand. The error will be thrown in parse(). if (!arg.starts_with('-')) @@ -974,9 +971,6 @@ class parser template void verify_option_config(config const & config) { - if (!subcommands.empty()) - throw design_error{"You may only specify flags for the top-level parser."}; - verify_identifiers(config.short_id, config.long_id); if (config.required && !config.default_message.empty()) @@ -1005,7 +999,7 @@ class parser throw design_error{"Positional options are always required and therefore cannot be advanced nor hidden!"}; if (!subcommands.empty()) - throw design_error{"You may only specify flags for the top-level parser."}; + throw design_error{"You may only specify flags and options for the top-level parser."}; if (has_positional_list_option) throw design_error{"You added a positional option with a list value before so you cannot add " @@ -1075,27 +1069,6 @@ class parser } } - /*!\brief Verifies that the subcommand was correctly specified. - * \throws sharg::too_few_arguments if a subparser was configured at construction but a subcommand is missing. - */ - inline void verify_subcommand() - { - if (std::holds_alternative(format) && !subcommands.empty() && sub_parser == nullptr) - { - assert(!subcommands.empty()); - std::string subcommands_str{"["}; - for (std::string const & command : subcommands) - subcommands_str += command + ", "; - subcommands_str.replace(subcommands_str.size() - 2, 2, "]"); // replace last ", " by "]" - - throw too_few_arguments{"You misspelled the subcommand! Please specify which sub-program " - "you want to use: one of " - + subcommands_str - + ". Use -h/--help for more " - "information."}; - } - } - /*!\brief Parses the command line arguments according to the format. * \throws sharg::option_declared_multiple_times if an option that is not a list was declared multiple times. * \throws sharg::user_input_error if an incorrect argument is given as (positional) option value. @@ -1116,7 +1089,25 @@ class parser f.parse(info); }; - std::visit(std::move(format_parse_fn), format); + try + { + std::visit(std::move(format_parse_fn), format); + } + catch (sharg::too_many_arguments const & exception) + { + if (subcommands.empty() || sub_parser) + throw; + + std::string message = exception.what(); + message += "\nIf you intended to execute a subcommand, it is possible that you have misspelled it.\n" + "Available subcommands are: ["; + + for (std::string const & command : subcommands) + message += command + ", "; + message.replace(message.size() - 2, 2, "].\n"); // replace last ", " by "].\n" + + throw too_many_arguments{message}; + } } }; diff --git a/test/unit/detail/format_help_test.cpp b/test/unit/detail/format_help_test.cpp index 596934a4..ad85732e 100644 --- a/test/unit/detail/format_help_test.cpp +++ b/test/unit/detail/format_help_test.cpp @@ -540,10 +540,12 @@ TEST_F(format_help_test, copyright) TEST_F(format_help_test, subcommand_parser) { bool flag_value{false}; + int option_value{}; auto parser = get_subcommand_parser({"-h"}, {"sub1", "sub2"}); parser.info.description.push_back("description"); parser.add_flag(flag_value, sharg::config{.short_id = 'f', .long_id = "foo", .description = "A flag."}); + parser.add_option(option_value, sharg::config{.short_id = 'o', .long_id = "option", .description = "An option."}); std::string expected = "test_parser\n" "===========\n" @@ -561,10 +563,11 @@ TEST_F(format_help_test, subcommand_parser) " The following options belong to the top-level parser and need to be\n" " specified before the subcommand key word. Every argument after the\n" " subcommand key word is passed on to the corresponding sub-parser.\n" - "\n" - "OPTIONS\n" + "\nOPTIONS\n" " -f, --foo\n" " A flag.\n" + " -o, --option (signed 32 bit integer)\n" + " An option. Default: 0\n" "\n" + basic_options_str + "\n" + basic_version_str; EXPECT_EQ(get_parse_cout_on_exit(parser), expected); diff --git a/test/unit/parser/format_parse_test.cpp b/test/unit/parser/format_parse_test.cpp index fbf39670..fbcc0cc3 100644 --- a/test/unit/parser/format_parse_test.cpp +++ b/test/unit/parser/format_parse_test.cpp @@ -681,17 +681,22 @@ TEST_F(format_parse_test, subcommand_parser_success) TEST_F(format_parse_test, subcommand_parser_error) { + constexpr std::string_view expected_message = + "Too many arguments provided. Please see -h/--help for more information.\n" + "If you intended to execute a subcommand, it is possible that you have misspelled it.\n" + "Available subcommands are: [sub1].\n"; + // incorrect sub command regardless of following arguments, https://github.com/seqan/seqan3/issues/2172 auto parser = get_subcommand_parser({"subiddysub", "-f"}, {"sub1"}); - EXPECT_THROW(parser.parse(), sharg::parser_error); + EXPECT_THROW_MSG(parser.parse(), sharg::too_many_arguments, expected_message.data()); // incorrect sub command with no other arguments parser = get_subcommand_parser({"subiddysub"}, {"sub1"}); - EXPECT_THROW(parser.parse(), sharg::parser_error); + EXPECT_THROW_MSG(parser.parse(), sharg::too_many_arguments, expected_message.data()); // incorrect sub command with trailing special option, https://github.com/seqan/sharg-parser/issues/171 parser = get_subcommand_parser({"subiddysub", "-h"}, {"sub1"}); - EXPECT_THROW(parser.parse(), sharg::parser_error); + EXPECT_THROW_MSG(parser.parse(), sharg::too_many_arguments, expected_message.data()); } TEST_F(format_parse_test, issue1544) diff --git a/test/unit/parser/parser_design_error_test.cpp b/test/unit/parser/parser_design_error_test.cpp index d93e330e..895597c4 100644 --- a/test/unit/parser/parser_design_error_test.cpp +++ b/test/unit/parser/parser_design_error_test.cpp @@ -267,7 +267,7 @@ TEST_F(design_error_test, subcommand_parser_error) parser = get_subcommand_parser({"-f", "foo"}, {"foo"}); EXPECT_THROW(parser.add_positional_option(flag_value, sharg::config{}), sharg::design_error); - EXPECT_THROW(parser.add_option(flag_value, sharg::config{.short_id = 'o'}), sharg::design_error); + EXPECT_NO_THROW(parser.add_option(flag_value, sharg::config{.short_id = 'o'})); EXPECT_NO_THROW(parser.add_flag(flag_value, sharg::config{.short_id = 'f'})); EXPECT_THROW(parser.get_sub_parser(), sharg::design_error); EXPECT_EQ(flag_value, false); @@ -277,14 +277,14 @@ TEST_F(design_error_test, subcommand_parser_error) flag_value = false; - // no options are allowed + // options are allowed parser = get_subcommand_parser({"-o", "true"}, {"foo"}); EXPECT_THROW(parser.add_positional_option(flag_value, sharg::config{}), sharg::design_error); - EXPECT_THROW(parser.add_option(flag_value, sharg::config{.short_id = 'o'}), sharg::design_error); - EXPECT_EQ(flag_value, false); - EXPECT_THROW(parser.parse(), sharg::too_few_arguments); + EXPECT_NO_THROW(parser.add_option(flag_value, sharg::config{.short_id = 'o'})); EXPECT_EQ(flag_value, false); + EXPECT_NO_THROW(parser.parse()); + EXPECT_EQ(flag_value, true); } TEST_F(design_error_test, not_allowed_after_parse)