Skip to content
This repository has been archived by the owner on Dec 18, 2023. It is now read-only.

cli booleans avoid negatives #156

Closed
faassen opened this issue Oct 10, 2022 · 4 comments
Closed

cli booleans avoid negatives #156

faassen opened this issue Oct 10, 2022 · 4 comments

Comments

@faassen
Copy link
Contributor

faassen commented Oct 10, 2022

b5 wrote on #179

Instead of --no-wrap I'd prefer --wrap, which defaults to true.
IMHO using negation in boolean flag labels is confusing:
--wrap=false is clearer than --no-wrap

I agree and would like to make it work this way, but Clap doesn't do this out of the box, so I've created a separate issue to figure this out.

What works is this (where the flag defaults to true):

/// Track metrics
#[clap(long, action = clap::ArgAction::Set, default_value_t=true)]
metrics: bool

Note that this does not support --metrics by itself, only with explicit =true and =false. I think that's acceptable for flags that default to true.

Documentation generated by Clap seems reasonable:

     --metrics <METRICS>  Track metrics [default: true] [possible values: true, false]

For flags that default to false we can use Clap's normal flag mechanism:

/// Verbose mode
#[clap(long, short = 'v')]
verbose: bool,

Documentation is like this:

 --verbose            Verbose mode

Note that this is a true Clap flag: it's only used to enable something, and you can't use an explicit true or false.

I wanted to highlight this difference, though I think it's acceptable. If we want to stick with true Clap flags for the "flag is turned on by default" use case, we have to take the --no- flags route instead.

Relevant Clap issues & PRs

The general explicit flag story:

clap-rs/clap#1649

Automatic support for --no flags. Isn't implemented.

clap-rs/clap#815

This misdirected me for a bit but has to do with environment variable parsing, so it not related:

clap-rs/clap#2664

@b5
Copy link
Member

b5 commented Oct 13, 2022

hmmm, now that I think about it, I'm not a massive fan of mixing "true clap flags" and "boolean clap flags". I'd prefer only "true clap flags", and you're saying the only way to do that is to go the --no route?

@faassen
Copy link
Contributor Author

faassen commented Oct 13, 2022

You either have a boolean option, ehich can default to true, and they always need a value, and have no single letter flag (as value is required)

Or clap flag which takes no value, defaults to false, and can have a single letter version.

@faassen
Copy link
Contributor Author

faassen commented Oct 14, 2022

I've gone for explicit --no-wrap as opposed to --wrap=false, with no single letter shortcut, because:

  • b5 is not a massive fan of mixing

  • true clap flags are easier to express with Clap

  • --wrap=true would never be used anyway, so we really only need one flag --no-wrap

  • It doesn't matter there's no shortcut because we didn't have one for --wrap=false either.

@b5
Copy link
Member

b5 commented Oct 17, 2022

At this point I think we've learned that using --no-wrap is more consistent with what clap wants to do under the hood, and while it might be a small UX hit, the lack of bugs caused by going against the grain of the library we're built on should be factored in as well. --no-wrap it is, and negation-worded flags should be allowed in the CLI, just avoided if possible.

@b5 b5 closed this as completed Oct 17, 2022
@dignifiedquire dignifiedquire transferred this issue from n0-computer/iroh Feb 17, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants