diff --git a/include/seqan3/argument_parser/argument_parser.hpp b/include/seqan3/argument_parser/argument_parser.hpp index f90fda5b36..c40faf9149 100644 --- a/include/seqan3/argument_parser/argument_parser.hpp +++ b/include/seqan3/argument_parser/argument_parser.hpp @@ -287,6 +287,13 @@ class argument_parser std::string const & desc, validator_type validator = validator_type{}) // copy to bind rvalues { + if (has_positional_list_option) + throw parser_design_error{"You added a positional option with a list value before so you cannot add " + "any other positional options."}; + + if constexpr (SequenceContainer && !std::Same) + has_positional_list_option = true; // keep track of a list option because there must be only one! + // copy variables into the lambda because the calls are pushed to a stack // and the references would go out of scope. std::visit([=, &value] (auto & f) { f.add_positional_option(value, desc, validator); }, format); @@ -492,6 +499,9 @@ class argument_parser //!\brief Keeps track of whether the parse function has been called already. bool parse_was_called{false}; + //!\brief Keeps track of whether the user has added a positional list option to check if this was the very last. + bool has_positional_list_option{false}; + //!\brief Set on construction and indicates whether the developer deactivates the version check calls completely. bool version_check_dev_decision{}; diff --git a/include/seqan3/argument_parser/detail/format_base.hpp b/include/seqan3/argument_parser/detail/format_base.hpp index 20496c8589..ae28eb0ecb 100644 --- a/include/seqan3/argument_parser/detail/format_base.hpp +++ b/include/seqan3/argument_parser/detail/format_base.hpp @@ -247,7 +247,10 @@ class format_help_base : public format_base if (!(spec & option_spec::HIDDEN) && (!(spec & option_spec::ADVANCED) || show_advanced_options)) derived_t().print_list_item(prep_id_for_help(short_id, long_id) + " " + option_type_and_list_info(value), - desc + detail::to_string(" Default: ", value, ". ") + + desc + + ((spec & option_spec::REQUIRED) + ? std::string{" "} + : detail::to_string(" Default: ", value, ". ")) + validator.get_help_page_message()); }); } @@ -291,7 +294,11 @@ class format_help_base : public format_base ++positional_option_count; derived_t().print_list_item(detail::to_string("\\fBARGUMENT-", positional_option_count, "\\fP ", option_type_and_list_info(value)), - desc + detail::to_string(" Default: ", value, ". ") + + desc + + // a list at the end may be empty and thus have a default value + ((SequenceContainer && !std::Same) + ? detail::to_string(" Default: ", value, ". ") + : std::string{" "}) + validator.get_help_page_message()); }); } diff --git a/include/seqan3/argument_parser/detail/format_parse.hpp b/include/seqan3/argument_parser/detail/format_parse.hpp index dcaa7b0120..1be190c9df 100644 --- a/include/seqan3/argument_parser/detail/format_parse.hpp +++ b/include/seqan3/argument_parser/detail/format_parse.hpp @@ -683,8 +683,7 @@ class format_parse : public format_base if (SequenceContainer && !std::is_same_v) // vector/list will be filled with all remaining arguments { - if (positional_option_count != (positional_option_calls.size())) - throw parser_design_error("Lists are only allowed as the last positional option!"); + assert(positional_option_count == positional_option_calls.size()); // checked on set up. while (it != argv.end()) { diff --git a/include/seqan3/argument_parser/detail/version_check.hpp b/include/seqan3/argument_parser/detail/version_check.hpp index 08eb8e2d26..4c78a04d5f 100644 --- a/include/seqan3/argument_parser/detail/version_check.hpp +++ b/include/seqan3/argument_parser/detail/version_check.hpp @@ -215,7 +215,7 @@ class version_checker "_" + version + // !user input! escaped on construction of the version_checker #if defined(_WIN32) - "; exit [int] -not $?}\" > nul 2>&1" + "; exit [int] -not $?}\" > nul 2>&1"; #else " > /dev/null 2>&1"; #endif @@ -341,7 +341,7 @@ class version_checker // nor did the the cookie tell us what to do. We will now ask the user if possible or do the check by default. write_cookie("ASK"); // Ask again next time when we read the cookie, if this is not overwritten. - if (detail::is_terminal()) + if (detail::is_terminal()) // LCOV_EXCL_START { std::cerr << R"( ####################################################################### @@ -403,7 +403,7 @@ class version_checker )"; return true; // default: check version if you cannot ask the user } - } + } // LCOV_EXCL_STOP //!\brief The identification string that may appear in the version file if an app is unregistered. static constexpr std::string_view unregistered_app = "UNREGISTERED_APP"; diff --git a/test/unit/argument_parser/argument_parser_design_error_test.cpp b/test/unit/argument_parser/argument_parser_design_error_test.cpp index 6282d698a7..1f0076453e 100644 --- a/test/unit/argument_parser/argument_parser_design_error_test.cpp +++ b/test/unit/argument_parser/argument_parser_design_error_test.cpp @@ -70,8 +70,7 @@ TEST(parse_test, parser_design_error) std::vector vec; argument_parser parser7{"test_parser", 4, argv2}; parser7.add_positional_option(vec, "oh oh list not at the end."); - parser7.add_positional_option(option_value, "desc."); - EXPECT_THROW(parser7.parse(), parser_design_error); + EXPECT_THROW(parser7.add_positional_option(option_value, "desc."), parser_design_error); // using h, help, advanced-help, and export-help argument_parser parser8{"test_parser", 1, argv}; @@ -105,8 +104,8 @@ TEST(parse_test, parse_called_twice) { std::string option_value; - const char * argv[] = {"./argument_parser_test", "-s", "option_string"}; - argument_parser parser{"test_parser", 3, argv}; + const char * argv[] = {"./argument_parser_test", "--version-check", "0", "-s", "option_string"}; + argument_parser parser{"test_parser", 5, argv}; parser.add_option(option_value, 's', "string-option", "this is a string option."); testing::internal::CaptureStderr(); diff --git a/test/unit/argument_parser/detail/format_help_test.cpp b/test/unit/argument_parser/detail/format_help_test.cpp index ebd562cd90..578c851f16 100644 --- a/test/unit/argument_parser/detail/format_help_test.cpp +++ b/test/unit/argument_parser/detail/format_help_test.cpp @@ -197,6 +197,9 @@ TEST(help_page_printing, do_not_print_hidden_options) TEST(help_page_printing, full_information) { + int8_t required_option{}; + int8_t non_list_optional{1}; + // Add synopsis, description, short description, positional option, option, flag, and example. argument_parser parser6{"test_parser", 2, argv1}; parser6.info.synopsis.push_back("./some_binary_name synopsis"); @@ -205,10 +208,12 @@ TEST(help_page_printing, full_information) parser6.info.description.push_back("description2"); parser6.info.short_description = "so short"; parser6.add_option(option_value, 'i', "int", "this is a int option."); + parser6.add_option(required_option, 'r', "required-int", "this is another int option.", option_spec::REQUIRED); parser6.add_section("Flags"); parser6.add_subsection("SubFlags"); parser6.add_line("here come all the flags"); parser6.add_flag(flag_value, 'f', "flag", "this is a flag."); + parser6.add_positional_option(non_list_optional, "this is not a list."); parser6.add_positional_option(pos_opt_value, "this is a positional option."); parser6.info.examples.push_back("example"); parser6.info.examples.push_back("example2"); @@ -224,11 +229,15 @@ TEST(help_page_printing, full_information) "description\n" "description2\n" "POSITIONAL ARGUMENTS\n" - "ARGUMENT-1 (List of std::string's)\n" - "this is a positional option. Default: [].\n" + + "ARGUMENT-1 (signed 8 bit integer)\n" + "this is not a list.\n" + "ARGUMENT-2 (List of std::string's)\n" + "this is a positional option. Default: []. \n" + basic_options_str + "-i, --int (signed 32 bit integer)\n" "this is a int option. Default: 5.\n" + "-r, --required-int (signed 8 bit integer)\n" + "this is another int option.\n" "FLAGS\n" "SubFlags\n" "here come all the flags\n" diff --git a/test/unit/argument_parser/detail/format_html_test.cpp b/test/unit/argument_parser/detail/format_html_test.cpp index e9898c50d2..36dfe9ecaa 100644 --- a/test/unit/argument_parser/detail/format_html_test.cpp +++ b/test/unit/argument_parser/detail/format_html_test.cpp @@ -75,7 +75,8 @@ TEST(html_format, full_information_information) std::string expected; int option_value{5}; bool flag_value; - std::vector pos_opt_value{}; + int8_t non_list_pos_opt_value{1}; + std::vector list_pos_opt_value{}; // Full html help page. const char * argv0[] = {"./help_add_test --version-check 0", "--export-help", "html"}; @@ -90,11 +91,11 @@ TEST(html_format, full_information_information) parser1.info.long_copyright = "long_copyright"; parser1.info.citation = "citation"; parser1.add_option(option_value, 'i', "int", "this is a int option."); - parser1.add_option(option_value, 'j', "jint", "this is a int option."); + parser1.add_option(option_value, 'j', "jint", "this is a required int option.", option_spec::REQUIRED); parser1.add_flag(flag_value, 'f', "flag", "this is a flag."); parser1.add_flag(flag_value, 'k', "kflag", "this is a flag."); - parser1.add_positional_option(pos_opt_value, "this is a positional option."); - parser1.add_positional_option(pos_opt_value, "this is a positional option."); + parser1.add_positional_option(non_list_pos_opt_value, "this is a positional option."); + parser1.add_positional_option(list_pos_opt_value, "this is a positional option."); parser1.info.examples.push_back("example"); parser1.info.examples.push_back("example2"); testing::internal::CaptureStdout(); @@ -126,8 +127,8 @@ TEST(html_format, full_information_information) "

\n" "

Positional Arguments

\n" "
\n" - "
ARGUMENT-1 (List of std::string's)
\n" - "
this is a positional option. Default: [].
\n" + "
ARGUMENT-1 (signed 8 bit integer)
\n" + "
this is a positional option.
\n" "
ARGUMENT-2 (List of std::string's)
\n" "
this is a positional option. Default: [].
\n" "
\n" @@ -152,7 +153,7 @@ TEST(html_format, full_information_information) "
-i, --int (signed 32 bit integer)
\n" "
this is a int option. Default: 5.
\n" "
-j, --jint (signed 32 bit integer)
\n" - "
this is a int option. Default: 5.
\n" + "
this is a required int option.
\n" "
-f, --flag
\n" "
this is a flag.
\n" "
-k, --kflag
\n" diff --git a/test/unit/argument_parser/format_parse_validators_test.cpp b/test/unit/argument_parser/format_parse_validators_test.cpp index 093aba2c5a..4659e1d0d3 100644 --- a/test/unit/argument_parser/format_parse_validators_test.cpp +++ b/test/unit/argument_parser/format_parse_validators_test.cpp @@ -135,7 +135,7 @@ TEST(validator_test, input_file) "===========" "POSITIONAL ARGUMENTS" " ARGUMENT-1 (std::filesystem::path)" - " desc Default: \"\". Valid input file formats: fa, sam, fasta."} + + " desc Valid input file formats: fa, sam, fasta."} + basic_options_str + basic_version_str; EXPECT_TRUE(ranges::equal((my_stdout | std::view::filter(!is_space)), expected | std::view::filter(!is_space))); @@ -220,7 +220,7 @@ TEST(validator_test, output_file) "===========" "POSITIONAL ARGUMENTS" " ARGUMENT-1 (std::filesystem::path)" - " desc Default: \"\". Valid output file formats: fa, sam, fasta."} + + " desc Valid output file formats: fa, sam, fasta."} + basic_options_str + basic_version_str; EXPECT_TRUE(ranges::equal((my_stdout | std::view::filter(!is_space)), expected | std::view::filter(!is_space))); @@ -274,7 +274,7 @@ TEST(validator_test, input_directory) "===========" "POSITIONAL ARGUMENTS" " ARGUMENT-1 (std::filesystem::path)" - " desc Default: \"\". An existing, readable path for the input directory."} + + " desc An existing, readable path for the input directory."} + basic_options_str + basic_version_str; @@ -319,7 +319,7 @@ TEST(validator_test, output_directory) "===========" "POSITIONAL ARGUMENTS" " ARGUMENT-1 (std::filesystem::path)" - " desc Default: \"\". A valid path for the output directory."} + + " desc A valid path for the output directory."} + basic_options_str + basic_version_str;