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

ArgAction::Count starts from 0 rather than default_value #5483

Open
2 tasks done
eirnym opened this issue May 3, 2024 · 6 comments
Open
2 tasks done

ArgAction::Count starts from 0 rather than default_value #5483

eirnym opened this issue May 3, 2024 · 6 comments
Labels
A-parsing Area: Parser's logic and needs it changed somehow. C-bug Category: Updating dependencies M-breaking-change Meta: Implementing or merging this will introduce a breaking change. S-waiting-on-decision Status: Waiting on a go/no-go before implementing

Comments

@eirnym
Copy link

eirnym commented May 3, 2024

Please complete the following tasks

Rust Version

rustc 1.78.0 (9b00956e5 2024-04-29)

Clap Version

4.5.0

Minimal reproducible code

use clap::{ArgAction, Parser};

#[derive(Debug, Clone, Parser)]
pub struct CliArgs {
    #[clap(short = 'v', action = ArgAction::Count, default_value_t=5)]
    pub verbose: u8,
}
fn main() {
    let args = CliArgs::parse();
    println!("verbose count: {}", args.verbose);
}

Steps to reproduce the bug with the above code

$ cargo run -- -vvv

Actual Behaviour

default_value_t is not taken into consideration for the starting point of Count

output:

$ cargo run 
verbose count: 5
$ cargo run -- -vvv
verbose count: 3

Expected Behaviour

default_value_t is taken into consideration for the starting point of Count

output:

$ cargo run 
verbose count: 5
$ cargo run -- -vvv
verbose count: 8

Additional Context

Debug Output

No response

@eirnym eirnym added the C-bug Category: Updating dependencies label May 3, 2024
@eirnym eirnym changed the title ArgAction::Count doesn't ArgAction::Count doesn't recognize default_value_t May 3, 2024
@eirnym eirnym changed the title ArgAction::Count doesn't recognize default_value_t ArgAction::Count doesn't recognize default_value_t May 3, 2024
@epage
Copy link
Member

epage commented May 3, 2024

I am seeing the same behavior for both default_value and default_value_t and the behavior is correct.

Could you give more details, like copies of the output that is expected and unexpected?

#!/usr/bin/env nargo
---
[dependencies]
clap = { path = "../clap", features = ["derive"] }
---

use clap::{ArgAction, Parser};

#[derive(Debug, Clone, Parser)]
pub struct CliArgs {
    #[arg(short = 'v', action = ArgAction::Count, default_value_t=5)]
    pub verbose: u8,
}
fn main() {
    let args = CliArgs::parse();
    println!("verbose count: {}", args.verbose);
}
$ ./clap-5483.rs
verbose count: 5
$ ./clap-5483.rs -vvv
verbose count: 3

Running

#!/usr/bin/env nargo
---
[dependencies]
clap = { path = "../clap", features = ["derive"] }
---

use clap::{ArgAction, Parser};

#[derive(Debug, Clone, Parser)]
pub struct CliArgs {
    #[arg(short = 'v', action = ArgAction::Count, default_value="5")]
    pub verbose: u8,
}
fn main() {
    let args = CliArgs::parse();
    println!("verbose count: {}", args.verbose);
}
$ ./clap-5483.rs
verbose count: 5
$ ./clap-5483.rs -vvv
verbose count: 3

@eirnym
Copy link
Author

eirnym commented May 3, 2024

@epage Thank you for your reply. I updated my comment above. TL;DR I expect, that ArgAction::Count will start count starting at given value. From documentation this action sets default_value to 0 if not provided, so I assumed, that it will start counting at default_value

@epage
Copy link
Member

epage commented May 3, 2024

In all ways within clap, default_value is assigned if nothing else is used. iirc Python's argparse has the same behavior and I've run into the same frustration :).

This means that your application needs to take this into account. https://github.com/clap-rs/clap-verbosity-flag is a crate that encodes all of this for that one specific use case.

I'm hesitant to change it and inclined to close this but I have just enough doubt that I think I'll leave this open to see what additional insights it might generate.

@epage epage changed the title ArgAction::Count doesn't recognize default_value_t ArgAction::Count starts from 0 rather than default_value May 3, 2024
@epage
Copy link
Member

epage commented May 3, 2024

I've updated the Issue to reflect the latest clarification. To be clear, there is no difference in behavior between default_value and default_value_t. If you are seeing some, we'd need a reproduction case to better understand it.

@epage epage added S-waiting-on-decision Status: Waiting on a go/no-go before implementing A-parsing Area: Parser's logic and needs it changed somehow. M-breaking-change Meta: Implementing or merging this will introduce a breaking change. labels May 3, 2024
@eirnym
Copy link
Author

eirnym commented May 3, 2024

It could be a different action if there's no way to change this one. At least clarification for this action is required.

Example could be found in my recently published tool I'd like to start my verbose level with 2 (Info). -q is an opposite of -v and then I calculate actual verbosity level. (beginning of main.rs and args.rs)

@epage
Copy link
Member

epage commented May 3, 2024

Added a clarification in 11ff6cc

There is a cost for everything we add to the API. I'd be hesitant to add a variant that changes the default behavior especially when this can be worked around. I'd be more inclined towards the custom Actions that I've been considering.

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 M-breaking-change Meta: Implementing or merging this will introduce a breaking change. S-waiting-on-decision Status: Waiting on a go/no-go before implementing
Projects
None yet
Development

No branches or pull requests

2 participants