May Parser::update_from not deny the presence of flags (option of type bool) ?

[TD-Sky]

Please complete the following tasks

• I have searched the discussions
• I have searched the open and rejected issues

Clap Version

4.0.26

Describe your use case

I'm seeking a way to merge default options coming from environment variable and runtime options.

Even though the method Parser::update_from cannot distinguish the difference between None and default value, it is the right way. I used unwrap_or with custom default value to bypass this feature but found it very troublesome to deal with flags.

Here is the example code:

use clap::Parser;

#[derive(Parser)]
struct Cli {
    #[arg(long)]
    boolean: bool,
}

fn main() {
    let mut cli = Cli::parse();

    println!("flag `boolean`: {}", cli.boolean);

    cli.update_from(["issue"]);    // binary name

    println!("flag `boolean`: {}", cli.boolean);
}

Run with flag:

$ cargo run -- --boolean

Output:

flag `boolean`: true
flag `boolean`: false

The present flag is denied by update_from without --boolean.

Describe the solution you'd like

I wish update_from to not overwrite the presence of flags, true values should be kept during updating.

Alternatives, if applicable

No response

Additional Context

No response

[epage]

I wonder how well we can detect this case as users can overwrite defaults. We can check for various combinations of default_value and default_missing_value but those will be in textual form and we'd then need to also load up the value parser to check that.

[epage]

Oh, I guess the simpler thing is for update_from to ignore any value whose value_source is Default.

[epage]

The next question is if that would be disruptive enough for anyones workflow to be either a minor compatibility break (reserved for clap v4.X.0) or a major compatibility break (reserved for clap vX.0.0)

[TD-Sky]

Parser::update_from may not be an often used API and many projects are still using imperative style.

[TD-Sky]

So a minor compatibility break would be OK :)

[epage]

Minor or major is not a matter of how many people are impacted but by a characterization of how it affects applications that might or might not be relying on the behavior.

And honestly, I keep wondering if we should just remove update_from. I wish I could tell how many people were using it and how important it is to their workflow.

[TD-Sky]

The semantic of update_from is very appropriate. I think it not worth to highlight the importance by removing this API.

[Mehrbod2002]

@epage
Can I work on this ?

[epage]

The semantic of update_from is very appropriate. I think it not worth to highlight the importance by removing this API.

I'm not sold that update_from has strong enough use cases to justify its complexity and high bug ratio. There are also various semantic holes in it that I noticed when I polished it up for clap 3.0 though I do not remember what they are at this time.

@epage Can I work on this ?

The part that was missing was someone stepping through the analysis requested earlier

a characterization of how it affects applications that might or might not be relying on the behavior.

to determine if this is a major incompatibility, minor incompatibility, or just a bug fix.

[Mehrbod2002]

@epage
Understood!
Thanks for details.

[TD-Sky]

@epage Understood! Thanks for details.

Not to rush you, but I was wondering if you are still working on this?

[epage]

I triaged this but am not actively working on it.

The next step is

The part that was missing was someone stepping through the analysis requested earlier

[TD-Sky]

Is it a good idea to collect users' opinion about update_from by questionnaires?

[epage]

To be clear, the analysis I'm referring to is in this comment: #4501 (comment)

Removing update_from is separate from this

[TD-Sky]

To be clear, the analysis I'm referring to is in this comment: #4501 (comment)

Removing update_from is separate from this

Theoretically, this is a major compatibility break, either keep with internal changes or remove it.