Skip to content

Commit

Permalink
[MISC] Allow options for subcommands
Browse files Browse the repository at this point in the history
  • Loading branch information
eseiler committed Mar 1, 2024
1 parent 01f848e commit b0acb3e
Show file tree
Hide file tree
Showing 4 changed files with 39 additions and 40 deletions.
51 changes: 21 additions & 30 deletions include/sharg/parser.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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('-'))
Expand Down Expand Up @@ -974,9 +971,6 @@ class parser
template <typename validator_t>
void verify_option_config(config<validator_t> 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())
Expand Down Expand Up @@ -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 "
Expand Down Expand Up @@ -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<detail::format_parse>(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.
Expand All @@ -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};
}
}
};

Expand Down
7 changes: 5 additions & 2 deletions test/unit/detail/format_help_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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);
Expand Down
11 changes: 8 additions & 3 deletions test/unit/parser/format_parse_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
10 changes: 5 additions & 5 deletions test/unit/parser/parser_design_error_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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)
Expand Down

0 comments on commit b0acb3e

Please sign in to comment.