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

num_args = 2.. does not error with 0 args and error has incorrect wording with 1 arg #5526

Closed
2 tasks done
qtfkwk opened this issue Jun 10, 2024 · 2 comments · Fixed by #5527
Closed
2 tasks done

num_args = 2.. does not error with 0 args and error has incorrect wording with 1 arg #5526

qtfkwk opened this issue Jun 10, 2024 · 2 comments · Fixed by #5527
Labels
C-bug Category: Updating dependencies

Comments

@qtfkwk
Copy link

qtfkwk commented Jun 10, 2024

Please complete the following tasks

Rust Version

rustc 1.78.0 (9b00956e5 2024-04-29)

Clap Version

4.5.6

Minimal reproducible code

use clap::Parser;

#[derive(Parser)]
struct Cli {
    /// Args
    #[arg(value_name = "ARG", num_args = 2..)]
    args: Vec<String>,
}

fn main() {
    let cli = Cli::parse();
    println!("{:#?}", cli.args);
}

Steps to reproduce the bug with the above code

cargo new clap-num-args-issue
cd clap-num-args-issue
cargo add clap -F derive
cat <<EOF >src/main.rs
use clap::Parser;

#[derive(Parser)]
struct Cli {
    /// Args
    #[arg(value_name = "ARG", num_args = 2..)]
    args: Vec<String>,
}

fn main() {
    let cli = Cli::parse();
    println!("{:#?}", cli.args);
}
EOF
cargo run # false negative: prints "[]\n" instead of an error because len 0 < minimum of 2
cargo run -- asdf # error is not grammatically correct; see below
cargo run -- asdf blah # good: prints "[\n    \"asdf\",\n    \"blah\",\n]\n"
cargo run -- asdf blah spam # good: prints "[\n    \"asdf\",\n    \"blah\",\n    \"spam\",\n]\n"

Error when running cargo run -- asdf:

error: 2 more values required by '[ARG] [ARG]...'; only 1 was provided

Shouldn't it say either of the following?

  1. error: 1 more value required by '[ARG] [ARG]...'; only 1 was provided
  2. error: 2 values required by '[ARG] [ARG]...'; only 1 was provided (my personal preference is for this one)

Actual Behaviour

There are 2 issues:

  1. With num_args set to 2.. and user gives 0, it does not produce an error as it should because the minimum number of args is 2
  2. If the user gives 1 arg, it errors, but the message says it needs "2 more args" which is wrong; it needs a minimum of 2 args and 1 was given, so it needs 1 more arg not 2

Expected Behaviour

  1. With num_args set to 2.. and user gives 0, it should produce an error saying that it requires 2 args and 0 were given
  2. If the user gives 1 arg, it should say it requires 2 args and 1 was given
    • (or alternatively it could say it needs 1 more arg and 1 was given; but I feel this is less precise and requires calculating the diff)

Additional Context

  1. Unsure root cause on this one... a quick search did not find it... but I imagine that somewhere it tests the len against the num_args range (?)

  2. "{}{min_values}{} more values required by '{}{invalid_arg}{}'; only {}{actual_num_values}{}{were_provided}",
    :

    should be:

    "{}{min_values}{} values required by '{}{invalid_arg}{}'; only {}{actual_num_values}{}{were_provided}",

    remove the word "more"

Debug Output

$ cargo run -- asdf
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.02s
     Running `target/debug/clap-num-args-issue asdf`
[clap_builder::builder::command]Command::_do_parse
[clap_builder::builder::command]Command::_build: name="clap-num-args-issue"
[clap_builder::builder::command]Command::_propagate:clap-num-args-issue
[clap_builder::builder::command]Command::_check_help_and_version:clap-num-args-issue expand_help_tree=false
[clap_builder::builder::command]Command::long_help_exists
[clap_builder::builder::command]Command::_check_help_and_version: Building default --help
[clap_builder::builder::command]Command::_propagate_global_args:clap-num-args-issue
[clap_builder::builder::debug_asserts]Command::_debug_asserts
[clap_builder::builder::debug_asserts]Arg::_debug_asserts:args
[clap_builder::builder::debug_asserts]Arg::_debug_asserts:help
[clap_builder::builder::debug_asserts]Command::_verify_positionals
[clap_builder::parser::parser]Parser::get_matches_with
[clap_builder::parser::parser]Parser::get_matches_with: Begin parsing '"asdf"'
[clap_builder::parser::parser]Parser::possible_subcommand: arg=Ok("asdf")
[clap_builder::parser::parser]Parser::get_matches_with: sc=None
[clap_builder::parser::parser]Parser::get_matches_with: Positional counter...1
[clap_builder::parser::parser]Parser::get_matches_with: Low index multiples...false
[clap_builder::parser::parser]Parser::resolve_pending: id="args"
[clap_builder::parser::parser]Parser::react action=Append, identifier=Some(Index), source=CommandLine
[ clap_builder::output::usage]Usage::create_usage_with_title
[ clap_builder::output::usage]Usage::create_usage_no_title
[ clap_builder::output::usage]Usage::write_help_usage
[ clap_builder::output::usage]Usage::write_arg_usage; incl_reqs=true
[ clap_builder::output::usage]Usage::needs_options_tag
[ clap_builder::output::usage]Usage::needs_options_tag:iter: f=help
[ clap_builder::output::usage]Usage::needs_options_tag:iter Option is built-in
[ clap_builder::output::usage]Usage::needs_options_tag: [OPTIONS] not required
[ clap_builder::output::usage]Usage::write_args: incls=[]
[ clap_builder::output::usage]Usage::get_args: unrolled_reqs=[]
[ clap_builder::output::usage]Usage::write_subcommand_usage
[ clap_builder::output::usage]Usage::create_usage_with_title: usage=Usage: clap-num-args-issue [ARG] [ARG]...
[clap_builder::builder::command]Command::color: Color setting...
[clap_builder::builder::command]Auto
[clap_builder::builder::command]Command::color: Color setting...
[clap_builder::builder::command]Auto
error: 2 more values required by '[ARG] [ARG]...'; only 1 was provided

Usage: clap-num-args-issue [ARG] [ARG]...

For more information, try '--help'.
@qtfkwk qtfkwk added the C-bug Category: Updating dependencies label Jun 10, 2024
@epage
Copy link
Member

epage commented Jun 10, 2024

There are 2 issues:

In the future, please open an issue per concern.

With num_args set to 2.. and user gives 0, it does not produce an error as it should because the minimum number of args is 2

The behavior is correct. From the docs:

Specifies the number of arguments parsed per occurrence

This isn't as obvious for positionals but for options, the behavior is more clear.

use clap::Parser;

#[derive(Parser)]
struct Cli {
    /// Args
    #[arg(long, value_name = "ARG", num_args = 2..)]
    args: Vec<String>,
}

fn main() {
    let cli = Cli::parse();
    println!("{:#?}", cli.args);
}
$ # good: 2+ values
$ cmd --args asdf blah
$ # good: 2+ values
$ cmd --args asdf blah spam
$ # good: 2+ values per `--arg`
$ cmd --args asdf blah --arg spam pizza
$ # bad: too few values
$ cmd --args asdf
$ # bad: too few values for last `--arg`
$ cmd --args asdf blah --arg spam
$ # good:  no occurrences
$ cmd

What you are looking for is required = true. You also likely want to set action = ArgAction::Set because a Vec is automatically an Append, see https://docs.rs/clap/latest/clap/_derive/index.html#arg-types

use clap::Parser;
use clap::builder::ArgAction;

#[derive(Parser)]
struct Cli {
    /// Args
    #[arg(long, value_name = "ARG", num_args = 2.., required = true, action = ArgAction::Set)]
    args: Vec<String>,
}

fn main() {
    let cli = Cli::parse();
    println!("{:#?}", cli.args);
}

@qtfkwk
Copy link
Author

qtfkwk commented Jun 12, 2024

Thank you for the detailed response, @epage! Will try your suggested fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: Updating dependencies
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants