Structopt: "default_value is meaningless for bool" should be a warning

[polarathene]

In my struct I have a field like this:

    // parameter value, not used in CLI arg/flags, so hidden
    #[structopt(raw(hide = "true"), default_value = "true")]
    centerWindow: bool,

Which produces a compile error of:

message: default_value is meaningless for bool

It's not used as a CLI flag, but related to the struct as I use it for settings/params. I think this should be more of a warning rather than preventing me from compiling?

That said there are some occurrences where I have bool fields that would be nice to have true as default with the actual flag flipping it to false instead(behaviour of a C project I'm porting where these reverse bool flags use short names with an upper case letter).

[TeXitoi]

Error handling is quite binary in custom derive, warning is not yet feasible in stable.

For your case you can use this workaround:

    // parameter value, not used in CLI arg/flags, so hidden
    #[structopt(raw(hide = "true"), parse(try_from_str), default_value = "true")]
    centerWindow: bool,

Maybe using something for skipping some fields can be interesting:

// skip the field with the method
#[structopt(skip("my::method"))
bool to_skip,
// as skip("Default::default")
#[structopt(skip)
bool to_skip_with_default,

For inverted bool, you can workaround like that:

fn toggle_bool(i: u64) -> bool { i == 0 }
struct Opt {
    #[structopt(raw(multiple = "false"), parse(from_occurrences = "toggle_bool"))]
    flag: bool,
}

Maybe adding a parse(from_presence = "toggle_bool") can be an interested feature.

[TeXitoi]

Does it fix your case? Any feedback on the propositions?

[polarathene]

Error handling is quite binary in custom derive, warning is not yet feasible in stable.

Nightly is able to support? Is the requirements for it likely to arrive in stable this year?

I don't mind using Nightly if error handling is better :) A cfg/feature flag might be able to enable that. Though since StructOpt is meant to be integrated into clap at some point, I'm not sure if it's worth doing.

Maybe using something for skipping some fields can be interesting:

Not assigning an attribute would skip/ignore the field from StructOpt wouldn't? I did not get the my::method example, is it meant to be an alternative to assigning the value like Default::default()? I think the user can derive Default on the struct? Would the default_value attribute not override that? Or could cause other concerns, so a per field default/skip attribute is more appropriate?

For inverted bool, you can workaround like that:

That is interesting to know, but for default true I did like the other original workaround. I get that it would not work properly if not hiding and allowing CLI as the flag would still set the bool to true wouldn't?

I'm not sure how important/useful this behaviour/default is to others, so it might not justify having it's own attribute as structopt(inverted_bool) or similar for being a simple way to indicate the bool is true by default and the flag will switch it to false instead.

Clap has matches.is_present("field_name") which is true if the flag was used in the command args, or matches.value_of("field_name") which for bool seems to always return None, not sure if that's important to StructOpt or not.

Does it fix your case? Any feedback on the propositions?

Yes, that seems to work thanks :) Since it's not used as a CLI arg, I'm not sure if I should be using StructOpt attributes for the field at all(or that it should belong in the same struct). In case anyone does come across this issue, slight correction on the raw attribute, it's hidden not hide so:

#[structopt(raw(hidden = "true"), parse(try_from_str), default_value = "false")]

I misunderstood how the flatten attribute worked(I originally thought the fields were merged into the parent struct). A structopt attribute could indicate a field(or all the fields in a nested struct) aren't for CLI?

I'd like to later support env vars and config files, where CLI args might cover only some fields. Since some of the fields would overlap with CLI args, I'm not sure how the rest should be handled. Maybe:

let mut config:  Opt = Default::default();
// some fields get changed before CLI args processed
// structopt attribute fields from CLI args override their related fields.
let final_config = Opt::merge_with_args(config);

I tried out clap today and realized that with clap it doesn't make much sense to have flags that do the inverse(bools with true as default that toggle to false) since it'd probably be best to treat it like the C project with a state struct that I check for the flag and change the state bool to false if the flag existed.

With StructOpt it's a bit different in that it can skip that approach with a single struct handling that logic behind the scenes :) (Also might be pretty neat with the project I saw combining StructOpt with a config crate for config files and env vars)

[TeXitoi]

Nightly is able to support? Is the requirements for it likely to arrive in stable this year?

Full support of error and warning is supported in nightly, but with a highly unstable API. I don't have the time to do all that properly if it will not work anymore in a couple of weeks.

The goal is to support that when the thing become less volatile, at worst when it become stable.

Not assigning an attribute would skip/ignore the field from StructOpt wouldn't

No, it will do an argument.

default_value gives a string to clap, and then the string will be parsed using the parse method. It's useless for the bool case as is_present is used, not value_of. That's why a default_value of any value will just block the bool case to true

did not get the my::method example, is it meant to be an alternative to assigning the value like Default::default()? I think the user can derive Default on the struct?

Yes. You can't implement Default for a type of another crate, and you may want something different that the default value, for example true for your bool.

I get that it would not work properly if not hiding and allowing CLI as the flag would still set the bool to true wouldn't?

You're right, if you put a default_value on a bool, the bool will always be true.

A structopt attribute could indicate a field(or all the fields in a nested struct) aren't for CLI?

The skip functionality that I present would allow that yes.

Also note that clap support env var, thus you can use env var easily in structopt:

#[structopt(short = "i", env = "INPUT", default_value = ".")]
input: String,

If there is anything that I forgot to answer, don't hesitate to ask again.

[polarathene]

No, it will do an argument.

#[derive(StructOpt, Debug)]
struct Opt {
    nonArgBool: bool,

    /// Disable spice client (due to CLI arg handling, it shouldn't use the C project useSpice variable)
    #[structopt(short = "s", long = "use-spice")]
    useSpice: bool,
}

use-spice is the only flag that'd show in help, not nonArgBool, although it's field is set to false. Oh.. I see what you mean, it's listed in the USAGE.

That's why a default_value of any value will just block the bool case to true

Ah ok, so this is really something that Clap should support? Even though it doesn't make much sense for Clap as it does with StructOpt?

Clap v3.x is set to have a feature for automating negations that generates --no- prefixed versions of bool flags. Though it's not exactly any better as it's just a different identifier with true still.

[TeXitoi]

Ah ok, so this is really something that Clap should support?

No, that just that using is_present when using default_value is meaningless as described in https://docs.rs/clap/2.31.2/clap/struct.Arg.html#method.default_value As a bool arg in structopt is created with is_present, we error on this case. An option as inverted_bool as you proposed, or parse(from_presence = "toggle_bool") as I proposed would be the clear solution (but the from_occurences trick is as powerful as these evolution, just less ergonomic).