From cb49a552220f6171262c0836d94d8449066d45c9 Mon Sep 17 00:00:00 2001
From: Muhammad Hussein <eng.mohamedhussien1996@gmail.com>
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         | 40 +++++++++++++++++++++++++++
 2 files changed, 71 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 <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 + "}");
-    }
+    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<std::string> &raw_arguments) const {
     std::vector<std::string> 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..ec2a207e 100644
--- a/test/test_choices.cpp
+++ b/test/test_choices.cpp
@@ -155,3 +155,43 @@ 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);
+
+  program.parse_args({"test", "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");
+
+  program.parse_args({"test", "--bar", "a", "b", "--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);
+
+  program.parse_args({"test", "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);
+}