Merge pull request #377 from Eng-MohamedHussien/fix/370_range_of_choices

Fix range of choices bug

include/argparse/argparse.hpp

@@ -929,24 +929,26 @@ public:
   }
 
   template <typename Iterator>
-  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 <typename Iterator>
+  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 + "}");
-    }
+                             details::repr(*option_it) +
+                             " - allowed options: {" + choices_as_csv + "}");
   }
 
   /* The dry_run parameter can be set to true to avoid running the actions,
@@ -962,21 +964,30 @@ public:
     }
     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) {
@@ -1003,7 +1014,6 @@ public:
                                    std::string(m_used_name) + "'.");
         }
       }
-
       struct ActionApply {
         void operator()(valued_action &f) {
           std::transform(first, last, std::back_inserter(self.m_values), f);

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<std::vector<std::string>>("indices") ==
+          std::vector<std::string>{"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<std::vector<std::string>>("--bar") ==
+          std::vector<std::string>{"a", "b"});
+  REQUIRE(program.get<std::string>("--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<std::vector<std::string>>("indices") ==
+          std::vector<std::string>{"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);
+}