clap-rs / clap

A full featured, fast Command Line Argument Parser for Rust
docs.rs/clap
Apache License 2.0
14.26k stars 1.04k forks source link

`UnknownArgumentValueParser` can only be used with string value argument #5081

Open weihanglo opened 1 year ago

weihanglo commented 1 year ago

Please complete the following tasks

Rust Version

rustc 1.72.0-beta.11 (7a3a43a3b 2023-08-18)

Clap Version

4.3.23

Minimal reproducible code

#!/usr/bin/env -S cargo +nightly -Zscript

//! ```cargo
//! [dependencies]
//! clap = "=4.3.23"
//! ```

fn main() {
    let args = clap::Command::new("test")
        .args([clap::Arg::new("libtest-ignore")
            .long("ignored")
            .action(clap::ArgAction::SetTrue)
            .value_parser(
                clap::builder::UnknownArgumentValueParser::suggest_arg("-- --ignored")
                    .and_suggest("not much else to say"),
            )
            .hide(true)])
        .get_matches();

    assert!(matches!(
        args.try_get_one::<bool>("libtest-ignore"),
        Err(clap::parser::MatchesError::Downcast { .. })
    ));

    args.try_get_one::<bool>("libtest-ignore").unwrap();
    // thread 'main' panicked at run.rs:25:48:
    // called `Result::unwrap()` on an `Err` value: Downcast { actual: alloc::string::String, expected: bool }
    // note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
}

Steps to reproduce the bug with the above code

Run the script above.

Actual Behaviour

I am not sure if this is expected. It turns out that the unknown argument must be defined as --flag <string>. Might be the result of StringValueParser in use.

Here is the current cargo integration that failed https://github.com/rust-lang/cargo/pull/12529.

keep-going is always evaluated when constructing BuildConfig even it is not a valid argument. Cargo provides a default value for unknown argument, which contradicts the definition of bool --keep-going.

Expected Behaviour

UnknownArgumentValueParser can use with any type of argument.

Additional Context

cc #5080

Debug Output

No response

epage commented 1 year ago

I had considered making the it parametrized but thought that might be too complex.

Maybe with a defaulted generic argument it won't be too bad and likely won't be a breaking change.

Alternatively, cargo could ignore the "invalid type" error since its pattern of reuse is a bit weird in of itself.

epage commented 1 year ago

Even though in some circles, a new defaulted generic parameter is not a breaking change, for how UnknownArgumentValueParser is used, it is a breaking change so we can't resolve this until clap v5.