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

[derive] Argument test's selected action SetTrue contradicts takes_value #5421

Open
2 tasks done
hasezoey opened this issue Mar 24, 2024 · 5 comments
Open
2 tasks done
Labels
C-bug Category: Updating dependencies

Comments

@hasezoey
Copy link

hasezoey commented Mar 24, 2024

Please complete the following tasks

Rust Version

rustc 1.76.0 (07dca489a 2024-02-04)

Clap Version

4.5.3

Minimal reproducible code

use clap::Parser;

#[derive(Parser, Debug)]
#[clap(name = "testing")]
pub struct TestArg {
    #[arg(
        long = "test",
        default_missing_value = "true",
        num_args = 0..=1,
        require_equals = true,
    )]
    pub test: bool,
}

fn main() {
    let args = cli::TestArg::parse();

    println!("{:#?}", args);
}

Steps to reproduce the bug with the above code

cargo run -- --test=false

Actual Behaviour

Panic:

thread 'main' panicked at /home/hasezoey/.local/cargo/registry/src/index.crates.io-6f17d22bba15001f/clap_builder-4.5.2/src/builder/debug_asserts.rs:708:9:
Argument `test`'s selected action SetTrue contradicts `takes_value

Expected Behaviour

No panic, as per documentation default_missing_value.

Additional Context

i know that default_missing_value is currently not documented in derive attributes, but i thought it would work anyway.

in case it is not clear, i am trying to do the following without problems:

cargo run --  # test = false
cargo run --  --test # test = true
cargo run --  --test=true # test = true
cargo run --  --test=false # test = false

Debug Output

[clap_builder::builder::command]Command::_do_parse
[clap_builder::builder::command]Command::_build: name="testing"
[clap_builder::builder::command]Command::_propagate:testing
[clap_builder::builder::command]Command::_check_help_and_version:testing 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:testing
[clap_builder::builder::debug_asserts]Command::_debug_asserts
[clap_builder::builder::debug_asserts]Arg::_debug_asserts:test
thread 'main' panicked at /home/hasezoey/.local/cargo/registry/src/index.crates.io-6f17d22bba15001f/clap_builder-4.5.2/src/builder/debug_asserts.rs:708:9:
Argument `test`'s selected action SetTrue contradicts `takes_value`
@hasezoey hasezoey added the C-bug Category: Updating dependencies label Mar 24, 2024
@epage
Copy link
Member

epage commented Mar 25, 2024

There are attributes that are implied by the data type, see https://docs.rs/clap/latest/clap/_derive/index.html#arg-types and if you aren't using the data type in the built-in way, then you have to override those. In this case, you need to set action = ArgAction::Set.

@hasezoey
Copy link
Author

and if you aren't using the data type in the built-in way, then you have to override those. In this case, you need to set action = ArgAction::Set.

to my knowledge i am using data type bool, which is documented as .action(ArgAction::SetTrue), and ArgAction::SetTrue has documented

No value is allowed. To optionally accept a value, see Arg::default_missing_value

and default_missing_value writes:

NOTE: using this configuration option requires the use of the .num_args(0..N) and the .require_equals(true) configuration option. These are required in order to unambiguously determine what, if any, value was supplied for the argument.

and also a example:

fn cli() -> Command {
    Command::new("prog")
        .arg(Arg::new("create").long("create")
            .value_name("BOOL")
            .value_parser(value_parser!(bool))
            .num_args(0..=1)
            .require_equals(true)
            .default_missing_value("true")
        )
}

Note: i just discovered default_missing_value has a example for bools which has .value_parser(value_parser!(bool)) set, is this something i would also need to do in derive?


I have now tested using action = ArgAction::Set, and yes this resolves the error. Maybe some documentation updates would be good?

@epage
Copy link
Member

epage commented Mar 25, 2024

The discrepancy is Builder vs Derive APIs. The Builder API defaults to Set unconditionally while the Derive API tries to be a little smarter.

I've tried to avoid as much talk about the Derive API in the Builder API. Part of this was from a negative experience as a user when clap had the Builder, YAML, and Usage APIs and each entry point was documented with the schema for them which made the docs more bloated and harder to follow.

@hasezoey
Copy link
Author

I've tried to avoid as much talk about the Derive API in the Builder API.

i can understand that, so maybe add it as a note when default_missing_value gets mentioned in the derive API (as it currently is not listed there)?

@epage
Copy link
Member

epage commented Mar 25, 2024

default_missing_value is covered by a blanket statement on raw attributes. Its not great but neither are the alternatives. #4090 is for exploring ways of improving that.

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

No branches or pull requests

2 participants