Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix range of choices bug #377

Merged
merged 1 commit into from
Sep 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 31 additions & 21 deletions include/argparse/argparse.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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) {
Expand All @@ -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);
Expand Down Expand Up @@ -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()) {
Expand Down
48 changes: 48 additions & 0 deletions test/test_choices.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Loading