Allow boolean literals as values for flags

[emilk]

The behavior of --flag=false setting the flag to the value true has caused problems for me many times. The problem is that boolean arguments are treated differently than arguments of other types.

As an example, say you have a few arguments to your program being called from someplace like so:

my_app --name=${name} --age=${age} --alive=${alive}

The first two will work as expected, but the third will be a bug with no error or warning message. This special case for the boolean type is very annoying. I would advocate a more orthogonal design.

Here is my suggestion for how to parse boolean flags:

Argument	Flag value
empty	false
--flag	true
--flag=true	true
--flag=false	false
--flag=other	Helpful error message

The problem with this comes when considering how to treat other potential ways to express boolean, e.g. 0/1, no/yes, False/True. We could add these as well to the above table, or stick to the Rust convention (true/false). Either way would work for me.

In summary: the principle of least surprise suggests that if --int=42 should work the same as --bool=false.

[TeXitoi]

TeXitoi/structopt#212 (comment)

[CreepySkeleton]

@emilk Could you please provide an exact snippet of code you're having the problem with? Along with expected/actual behavior.

[pksunkara]

Possible solution is to have a setting which allows flags to also allow boolean literals as values.

[colemickens]

If there's a workaround someone could share, that would be great (I was wondering if maybe it's possible with the builder, or a custom parser? It seems like structopt has a way, but I can't find such functionality in clap at first glance).

To elaborate on justification for this bug... It is more than just surprising behavior. From what I can tell, it actively prevents certain designs. For example, I have a flag that really should be on by default (--create-users) and I don't want a --dont-create-users flag, I really want the user to have to say --create-users=false if they are expert enough to opt-out.

[CreepySkeleton]

You couldn't find this functionality because it doesn't exist.

The key is that flags do not take value in clap, period. Flags that take value are called options. Options can be either present (possibly with a number of values following) or not. There's no notion of "if this option has one value and the value equals false then pretend that the option wasn't present".

structopt can do it because it applies postprocessing - conversion - to the values, and what you observe is result of the conversion. It doesn't change the fact that the option was actually present. You can simulate it with

// create an option with optional value
Arg::new("pseudo-flag")
    .possible_values(&["true", "false"])
    .long("foo")
    .min_values(0)
    .max_values(1)
    
/// ...

let setting = app.value_of("pseudo-flag") != Some("false");

[colemickens]

Thank you for the detailed explanation @CreepySkeleton, I hadn't quite understood the important distinction between "options" and "flags". It turns out, this also works, since I prefer explicit boolean options over "flags" -- https://github.com/clap-rs/clap/blob/dd5e6b23131f16ef8b090a8482f1b97c6a693498/clap_derive/examples/true_or_false.rs

[dimo414]

#815 is related and might eliminate the need for boolean literals.

[ldm0]

I don't think clap needs to do something for this specific behaviour, you can get this behaviour by using default_missing_value and parsing later.

Here is the demo:
https://github.com/clap-rs/clap/pull/2649/files#diff-cf9f91bfd0a1de87f6e066071af5d05182179587e65ec540ed4cfba30ec1709bR113

[rami3l]

@pksunkara Sorry, I changed my PR #2619 description to close another issue other than this, but I didn't reword the commit. Would you mind reopening this one?

[epage]

Now that #2619 is merged, we have an example for how to use clap's existing API for accomplishing the request in this Issue (taken taken from a previous comment).

Is this sufficient for resolving the need here or is there still interest in there being some baked in way of getting this behavior (possibly controllable with a ArgSetting / AppSetting)?

[rami3l]

@epage In the end, no new examples has been added to the examples folder, so this remains to be a problem.

The thing that I'm considering now is that, for env vars it seems that we almost certainly have to include a string-literal-to-bool mechanism (which I'm currently working on) to replace the presence check, since it's quite a common practice.

Later on, if that's included, in my opinion it's better to do the same with flag literals for the sake of consistency. Of course, it would be better if this behavior is optional, and controlled by a new settings item.

Thus, the previously mentioned methods should only be considered as a workaround to satisfy some very specific needs.

[ldm0]

@epage In the end, no new examples has been added to the examples folder, so this remains to be a problem.

The thing that I'm considering now is that, for env vars it seems that we almost certainly have to include a string-literal-to-bool mechanism (which I'm currently working on) to replace the presence check, since it's quite a common practice.

Later on, if that's included, in my opinion it's better to do the same with flag literals for the sake of consistency. Of course, it would be better if this behavior is optional, and controlled by a new settings item.

Please check

clap/tests/default_missing_vals.rs

Lines 113 to 141 in 286fda0

	fn default_missing_value_flag_value() {
	let app = App::new("test").arg(
	Arg::new("flag")
	.long("flag")
	.takes_value(true)
	.default_missing_value("true"),
	);
	fn flag_value(m: ArgMatches) -> bool {
	match m.value_of("flag") {
	None => false,
	Some(x) => x.parse().expect("non boolean value"),
	}
	}
	assert_eq!(flag_value(app.clone().get_matches_from(&["test"])), false);
	assert_eq!(
	flag_value(app.clone().get_matches_from(&["test", "--flag"])),
	true
	);
	assert_eq!(
	flag_value(app.clone().get_matches_from(&["test", "--flag=true"])),
	true
	);
	assert_eq!(
	flag_value(app.clone().get_matches_from(&["test", "--flag=false"])),
	false
	);
	}

I don't think it's necessary to do this in clap.

[rami3l]

@ldm0 Thanks for your reference. Surely, I can see that this task can already be handled by the clap API with some handwritten validation functions.

I personally accept your solution, but the question that I really want to ask, is what level of API clap wants to provide for its users.

I wonder what a new user might guess about this library, and how we can guide them to get the solution that they want. That's all I care about. I chose a battery-included, opinionated approach, but you have the right to disagree, and as long as we keep being user-friendly, I'm satisfied with that outcome.

[epage]

Sorry, missed that and hadn't caught up with your comment in #2539.

Personally, I see args and envs as separate entities that have their own context from a combination of history and user expectations. We should be consistent within their context rather than forcing a consistency between things users generally have treated differently.

env vars have historically been C-like in using 0/1 or present/not-present. More recently I feel like I've seen true/false used. Hence the need for a str2bool.

For arguments, I've generally seen one of

• Present / not-present
• --present, --no-present
• --name <enumeration> with some being yes/no, true/false, always/never

Only in rare circumstances (--color) do I see any of these mixed. I've also not seen an argument parsing library that has baked in support (default or not) for mixing these without going custom (like the suggested workarounds)

As I've mentioned before, variable number of arguments has been brittle enough that clap warns about it. We can instead require an equals but I'm concerned about user confusion when some arguments need an = while others do not.

This issue and several others have been contributing a sense in me that we might want to re-think clap's API. Clap's development has been fairly generous in supporting everyone's use cases but the API is designed such that to do so, means it all has to be baked into clap, creating a large API surface and making it harder to get the behavior you need. After 3.0, I'm interested in exploring ways to allow people to customize the behavior of clap so that clap can be all things to all people without having to support all things out of the box.

[rami3l]

@ldm0 So at least something should be added to the examples, whether it's the demo of a new feature or appropriate use of existing features. Since this is a decision that must be made carefully, it's always nice to have a discussion.

@epage Yeah, I also have the impression that the clap codebase is lacking appropriate reorganization. In this sense, I agree that we should make less stuff baked-in. It's always nice to have some good defaults though, even as an external extension.

[nrdxp]

should this not be closed now that #2664 is merged?

[hasezoey]

should this not be closed now that #2264 is merged?

from what i can tell, #2264 is a issue, not a PR and does not look related - do you mean #2664?

[nrdxp]

yes, was a typo sorry about that 😅

[pksunkara]

No, that PR only deals with environment variables while this issue is also about converting option values into boolean. With our new direction regarding plugin based architecture post v3, this is a prime candidate for being made a plugin.

[bernardoaraujor]

the solution I found for this issue was to start using flag: Option<bool> instead of just flag: bool
that way, --flag=false gets the expected outcome and avoids confusion on the user's mind

downside is that I can't derive default_value (error: default_value is meaningless for Option ), so this workaround is also necessary:

let flag = match args.flag {
    Some(f) => f,
    None => false, // default
};

[epage]

With clap 3.2, you can now override the bool behavior with #[clap(action = ArgAction::Set)].

For example, from our test suite:

#[test]
fn override_implicit_action() {
    #[derive(Parser, PartialEq, Debug)]
    struct Opt {
        #[clap(long, action = clap::ArgAction::Set)]
        arg: bool,
    }

    assert_eq!(
        Opt { arg: false },
        Opt::try_parse_from(&["test", "--arg", "false"]).unwrap()
    );

    assert_eq!(
        Opt { arg: true },
        Opt::try_parse_from(&["test", "--arg", "true"]).unwrap()
    );
}

If you had concerns with the rough upgrade to clap 3.2, the deprecations are now off by default, see #3830 for details. When you do start working through the deprecations as part of upgrading to 4.0, we've also improved the warnings for derive users. See #3832 for examples.

[jbtobar]

action = clap::ArgAction::Set

this example is incredibly useful. Should probably be better documented. I just spent too long digging through issues, documentation, debugging, etc to achieve the pretty basic --something=false functionality on the Parser macro.

Thank you!

[gibfahn]

I'm a bit confused by the resolution to this issue. I'm looking for something like the original post:

Argument	Flag value
empty	false
--flag	true
--flag=true	true
--flag=false	false
--flag=other	Helpful error message

Where the user can provide nothing (defaulting to true), or --flag=true or --flag=false, or not provide the flag at all (defaulting to either true or false), and the output is a bool. Anything else should error.

I can get this to work with Option<bool>:

    #[clap(
        long,
        default_missing_value = "true",
        require_equals = true,
        num_args(0..=1),
    )]
    pub(crate) flag: Option<bool>,

but I can't seem to work out how to get this to work with a plain bool, e.g.:

    #[clap(
        long,
        default_missing_value("true"),
        default_value("false"),
        num_args(0..=1),
        require_equals(true),
    )]
    pub(crate) flag: bool,

This fails with:

assertion `left == right` failed: Argument augment: mismatch between `num_args` (0..=1) and `takes_value`
  left: true
 right: false

Seems like there isn't a way to say "require equals or assume no argument passed".

---

EDIT: actually if you do this:

    #[clap(
        long,
        default_missing_value("true"),
        default_value("true"),
        num_args(0..=1),
        require_equals(true),
    )]
    pub(crate) flag: Option<bool>,

then you get the default behaviour I'm looking for, so the flag value will never be None, but rather Some(true) if not set. It's a bit odd to have a non-optional option, but it works well enough.

[epage]

Try

    #[clap(
        long,
        default_missing_value("true"),
        default_value("true"),
        num_args(0..=1),
        require_equals(true),
        action = ArgAction::Set,
    )]
    pub(crate) flag: bool,

[gibfahn]

Ahh that works, thanks!

[JasonkayZK]

Try

    #[clap(
        long,
        default_missing_value("true"),
        default_value("true"),
        num_args(0..=1),
        require_equals(true),
        action = ArgAction::Set,
    )]
    pub(crate) flag: bool,

Thanks, this works for me. 👍

And if you want default to be false, try this:

    #[clap(
        long,
        default_missing_value("true"),
        default_value("false"),
        num_args(0..=1),
        require_equals(true),
        action = ArgAction::Set,
    )]
    pub(crate) flag: bool,