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

Multiple value doesn't work when the first argument attached to short flag #3903

Closed
2 tasks done
key262yek opened this issue Jul 9, 2022 · 7 comments
Closed
2 tasks done
Labels
A-parsing Area: Parser's logic and needs it changed somehow. C-bug Category: Updating dependencies S-wont-fix Status: Closed as there is no plan to fix this

Comments

@key262yek
Copy link

key262yek commented Jul 9, 2022

Please complete the following tasks

Rust Version

rustc 1.62.0 (a8314ef7d 2022-06-27)

Clap Version

3.2.8

Minimal reproducible code

use clap::Command;

fn main() {
    let sc = Command::new("test")
        .arg(
            Arg::new("m")
                .short('m')
                .takes_value(true)
                .multiple_values(true),
        )
        .get_matches_from(vec!["test", "-ma", "b", "c"]);

    assert_eq!(
        vec!["a", "b", "c"],
        sc.get_many::<String>("m")
            .unwrap()
            .into_iter()
            .map(|x| x.as_str())
            .collect::<Vec<&str>>()
    );
}

Steps to reproduce the bug with the above code

cargo run

Actual Behaviour

error: Found argument 'b' which wasn't expected, or isn't valid in this context

Expected Behaviour

program ends without error

Additional Context

In clap v2.34, following equivalent code works fine.

use clap::App;

fn main(){
let sc = App::new("test")
        .arg(
            Arg::with_name("m")
                .short("m")
                .takes_value(true)
                .multiple(true),
        )
        .get_matches_from(vec!["test", "-ma", "b", "c"]);

assert_eq!(
        vec!["a", "b", "c"],
        sc.values_of("m")
            .unwrap()
            .into_iter()
            .collect::<Vec<&str>>()
    );
}

I want to ask, this result is whether a bug or what you expected

Debug Output

error: Found argument 'b' which wasn't expected, or isn't valid in this context

@ciehanski
Copy link

ciehanski commented Jul 10, 2022

+1. Migrated from structopt 0.3.26. When attempting to parse a Vec<String> in derive mode, I experience the same errors as @key262yek in their builder mode example.

#[clap(arg_required_else_help = true)]
Add {
    #[clap(value_parser)]
    job: String,
    #[clap(short = 'f', long = "files", value_parser)]
    file_path: Vec<String>,
}
$ cargo run add -f "test" "test1" "test2"
error: Found argument 'test2' which wasn't expected, or isn't valid in this context

USAGE:
    kip add --files <FILE_PATH> <JOB>

For more information try --help

This functionality worked prior in structopt so I'm assuming this may be a side effect of the v3 upgrade.

Versions:

  • clap 3.2.8
  • rustc 1.62.0 (a8314ef7d 2022-06-27)

@epage
Copy link
Member

epage commented Jul 12, 2022

@ciehanski your case is slightly different. See #3213

@epage
Copy link
Member

epage commented Jul 12, 2022

I want to ask, this result is whether a bug or what you expected

While I don't know the original intent of the change between 2 and 3, I would say this is expected behavior.

My way of thinking of this is to look at the = case. -m=a b c doesn't make it look like b c are attached to -m. I think its the case also with -ma b c. The part I'm a little more unsure of is that we stop parsing when coming across delimited values, so -m a,b c means that c isn't attached to -m.

I'd be willing to hear other thoughts on this and we can dig into the history. I'll leave this open for now for that conversation but I'm assuming this will be closed as expected.

@epage epage added A-parsing Area: Parser's logic and needs it changed somehow. S-wont-fix Status: Closed as there is no plan to fix this labels Jul 12, 2022
@key262yek
Copy link
Author

key262yek commented Jul 12, 2022

I found -m quopri -d is working for both v2 and v3. (-d is a hyphen value for the argument "m") But -mquopri -d is working only for v2. If the ambiguity was problem, I thought the both cases would be changed. So this one-side change made me think this is more like accidental change rather than intentional change, because both cases looking like similar ambiguous to me.

@fanninpm
Copy link

@key262yek In CPython, -c and -m are "interface options", and once the interpreter encounters an interface option, it parses that argument, dumps the rest of the command into sys.argv[1:], and stops parsing altogether. So, in CPython…

python3 -m quopri -h

should have the same behavior as…

python3 -mquopri -h

(here, the -h flag is being dumped into sys.argv[1].)

I hope that helps.

@key262yek
Copy link
Author

@fanninpm I understand your two examples are equivalent.
The reason why I cannot change the way of passing arguments is because it is in unittests of rustpython.
Thus I need to find different way to parsing them without change argument form itself.

@fanninpm
Copy link

@key262yek Perhaps what is needed in this case is the allow_hyphen_values method.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-parsing Area: Parser's logic and needs it changed somehow. C-bug Category: Updating dependencies S-wont-fix Status: Closed as there is no plan to fix this
Projects
None yet
Development

No branches or pull requests

4 participants