From 43072b8e8cef2f111343b526bc49409eee717ff6 Mon Sep 17 00:00:00 2001 From: Muhammad Hussein Date: Sat, 24 Aug 2024 11:23:04 +0300 Subject: [PATCH] Fix range of choices bug - Support range of choices. - Add testcases for range of choices scenario. fix #370 --- include/argparse/argparse.hpp | 52 +++++++++++++++++++++-------------- test/test_choices.cpp | 48 ++++++++++++++++++++++++++++++++ 2 files changed, 79 insertions(+), 21 deletions(-) diff --git a/include/argparse/argparse.hpp b/include/argparse/argparse.hpp index 72dd7883..a67f02e5 100644 --- a/include/argparse/argparse.hpp +++ b/include/argparse/argparse.hpp @@ -927,24 +927,26 @@ class Argument { } template - void find_value_in_choices_or_throw(Iterator it) const { + bool is_value_in_choices(Iterator option_it) const { const auto &choices = m_choices.value(); - if (std::find(choices.begin(), choices.end(), *it) == choices.end()) { - // provided arg not in list of allowed choices - // report error + return (std::find(choices.begin(), choices.end(), *option_it) != + choices.end()); + } - std::string choices_as_csv = - std::accumulate(choices.begin(), choices.end(), std::string(), - [](const std::string &a, const std::string &b) { - return a + (a.empty() ? "" : ", ") + b; - }); + template + void throw_invalid_arguments_error(Iterator option_it) const { + const auto &choices = m_choices.value(); + const std::string choices_as_csv = std::accumulate( + choices.begin(), choices.end(), std::string(), + [](const std::string &option_a, const std::string &option_b) { + return option_a + (option_a.empty() ? "" : ", ") + option_b; + }); - throw std::runtime_error(std::string{"Invalid argument "} + - details::repr(*it) + " - allowed options: {" + - choices_as_csv + "}"); - } + throw std::runtime_error(std::string{"Invalid argument "} + + details::repr(*option_it) + + " - allowed options: {" + choices_as_csv + "}"); } /* The dry_run parameter can be set to true to avoid running the actions, @@ -960,21 +962,30 @@ class Argument { } m_used_name = used_name; + std::size_t passed_options = 0; + if (m_choices.has_value()) { // Check each value in (start, end) and make sure // it is in the list of allowed choices/options - std::size_t i = 0; - auto max_number_of_args = m_num_args_range.get_max(); + const auto max_number_of_args = m_num_args_range.get_max(); + const auto min_number_of_args = m_num_args_range.get_min(); for (auto it = start; it != end; ++it) { - if (i == max_number_of_args) { + if (is_value_in_choices(it)) { + passed_options += 1; + continue; + } + + if ((passed_options >= min_number_of_args) && + (passed_options <= max_number_of_args)) { break; } - find_value_in_choices_or_throw(it); - i += 1; + + throw_invalid_arguments_error(it); } } - const auto num_args_max = m_num_args_range.get_max(); + const auto num_args_max = + (m_choices.has_value()) ? passed_options : m_num_args_range.get_max(); const auto num_args_min = m_num_args_range.get_min(); std::size_t dist = 0; if (num_args_max == 0) { @@ -1001,7 +1012,6 @@ class Argument { std::string(m_used_name) + "'."); } } - struct ActionApply { void operator()(valued_action &f) { std::transform(first, last, std::back_inserter(self.m_values), f); @@ -2244,7 +2254,7 @@ class ArgumentParser { preprocess_arguments(const std::vector &raw_arguments) const { std::vector arguments{}; for (const auto &arg : raw_arguments) { - + const auto argument_starts_with_prefix_chars = [this](const std::string &a) -> bool { if (!a.empty()) { diff --git a/test/test_choices.cpp b/test/test_choices.cpp index 4fc4383a..7e8917fa 100644 --- a/test/test_choices.cpp +++ b/test/test_choices.cpp @@ -155,3 +155,51 @@ TEST_CASE("Parse multiple arguments that are not in fixed number of allowed " "Invalid argument \"6\" - allowed options: {1, 2, 3, 4, 5}", std::runtime_error); } + +TEST_CASE("Parse multiple arguments that are in range of allowed " + "INTEGER choices (Min Range case)" * + test_suite("choices")) { + argparse::ArgumentParser program("test"); + program.add_argument("indices").nargs(1, 3).choices(1, 2, 3, 4, 5); + + REQUIRE_NOTHROW(program.parse_args({"test", "1"})); + REQUIRE(program.get>("indices") == + std::vector{"1"}); +} + +TEST_CASE("Parse multiple arguments that are in range of allowed choices (In " + "Range case)" * + test_suite("choices")) { + argparse::ArgumentParser program("test"); + program.add_argument("--foo"); + program.add_argument("--bar").nargs(1, 3).choices("a", "b", "c"); + + REQUIRE_NOTHROW( + program.parse_args({"test", "--bar", "a", "b", "--foo", "x"})); + REQUIRE(program.get>("--bar") == + std::vector{"a", "b"}); + REQUIRE(program.get("--foo") == "x"); +} + +TEST_CASE("Parse multiple arguments that are in range of allowed " + "INTEGER choices (Max Range case)" * + test_suite("choices")) { + argparse::ArgumentParser program("test"); + program.add_argument("indices").nargs(2, 3).choices(1, 2, 3, 4, 5); + + REQUIRE_NOTHROW(program.parse_args({"test", "3", "4", "5"})); + REQUIRE(program.get>("indices") == + std::vector{"3", "4", "5"}); +} + +TEST_CASE("Parse multiple arguments that are not in range of allowed choices" * + test_suite("choices")) { + argparse::ArgumentParser program("test"); + program.add_argument("--foo"); + program.add_argument("--bar").nargs(1, 3).choices("a", "b", "c"); + + REQUIRE_THROWS_WITH_AS( + program.parse_args({"test", "--bar", "d", "--foo", "x"}), + "Invalid argument \"d\" - allowed options: {a, b, c}", + std::runtime_error); +}