clap icon indicating copy to clipboard operation
clap copied to clipboard

Add support for automatic negation flags

Open kbknapp opened this issue 9 years ago • 35 comments

Add a way to automatically generate flags that override (or negate) other flags. This can be done manually already, but doing so for an entire CLI can be tedious, painful, and error prone. Manually doing so will also pollute the --help output.

This proposal offers a way to automatically have these negation flags generated on a case by case basis, or across all flags in the command. This proposal also offers a way to have these negation flags listed in the --help message or hidden.

Design

A negation flag would simply take the long version of the regular flag and pre-pend no; for exmaple --follow would get a --no-follow. If a flag only specifies a short version, the no would be prepended to the short such as -L gets --no-L.

When parsing occurs, if a negation flag is found, and the negated argument was used, it functions exactly like a override that is already supported.

Functionally the following two examples are equivilant:

app.arg(Arg::with_name("regular")
        .long("follow-links")
        .help("follows symlinks")
        .overrides_with("override"))
    .arg(Arg::with_name("override)
        .long("no-follow-links")
        .help("does not follow symlinks"))

New proposal:

app.arg(Arg::with_name("regular")
        .long("follow-links")
        .help("follows symlinks")
        .overridable(true))

Concerns

There are two primary concerns with this approach.

Flags that already contian "no"

A flag which already starts with no such as --no-ignore would end up getting a double no in the form of --no-no-ignore. This actually makes sense and is consistent, but looks strange at first glance. An alternative would be to check if a flag starts with no and simply remove the no, i.e. --no-ignore becomes --ignore but this has the downside of additional processing at runtime, becomes slightly more confusing, and has a higher chance of a conflict.

Conflicting names

If a user has selected to auto-generate a negation flag, and the negating flag long conflicts with a flag already in use, a panic! will occur. Example, --ignore and --no-ignore is already defined elsewhere, and the user has selected to automaticlly generate negation flags, this will cause --ignore to generate a --no-ignore flag which already exists causing a panic!. The fix is to either not use a sweeping setting that applies ot all flags indescriminantly, or to change/remove the already defined --no-ignore flag.


Progress

  • [ ] Add AppSettings::GenerateNegationFlags which does the above, but automatically for all flags.
    • [ ] docs
    • [ ] tests
  • [ ] Add AppSettings:GenerateHiddenNegationFlags which hides all these negation flags.
    • [ ] docs
    • [ ] tests

See the discussion in burntsushi/ripgrep#196

Relates to #748


Edit: Removed Arg::overridable(bool) because this can already be done manually by making another flag.

kbknapp avatar Jan 14 '17 02:01 kbknapp

Great proposal @kbknapp. Having a --no-* equivalent just feels natural when it comes to boolean values.

Perhaps this could even be the default, considering that v3 is still in beta? The user would then have to call .overridable(false) instead to get the old behavior.

jacobsvante avatar May 21 '21 08:05 jacobsvante

Not all single flags are need overridable flags. From looking at the clis, this is a minority use case. Which is why this won't be a default behaviour.

pksunkara avatar May 21 '21 13:05 pksunkara

It's actually quite useful if all flags have negation flags because you can effectively switch the default of a flag via aliases or wrappers without hardcoding the behavior, because the user can still override the choice. It even works with nested wrappers.

For some prior art, the very popular python command line argument library click pushes users to always define a positive and a negative flag (but allows having just one): https://click.palletsprojects.com/en/8.0.x/options/#boolean-flags

Emerentius avatar Jun 08 '21 18:06 Emerentius

I usually also always add a no variant for almost all my options, but it's that little "almost" that make me not want this to be automatic. If there was a simple opt-in argument you could set (toggleable(true) maybe?) then that would be enough for me.

Mange avatar Jun 08 '21 18:06 Mange

Random thoughts:

  • I don't think I normally see --no-L. Instead what I tend to see is either no short form or toggling of the case (e.g. -l vs -L). Having this be different makes this a bit more complicated.
  • Sometimes I want the no format to be the default, so I'd want the no- prefix stripped.

epage avatar Jul 22 '21 08:07 epage

There are also instances of complementary flags using +/- as their prefix. Aside from the bash shopt and Xwayland mentioned in #2468, xterm and set (shell builtin) have many such options:

usage:  xterm [-/+132] [-/+ah] [-/+ai] [-/+aw]
    [-/+bc] [-/+bdc] [-/+cb] [-/+cjk_width] [-/+cm] [-/+cn] 
    [-/+cu] [-/+dc] [-/+fbb] [-/+fbx] [-/+fullscreen]
    [-/+hm] [-/+hold] [-/+ie] [-/+im] [-/+itc]
    [-/+j] [-/+k8] [-/+l] [-/+lc] [-/+ls] [-/+maximized] [-/+mb]
    [-/+mesg] [-/+mk_width] [-/+nul] [-/+pc] [-/+pob] [-/+rv]
    [-/+rvc] [-/+rw] [-/+s] [-/+samename] [-/+sb] [-/+sf]
    [-/+si] [-/+sk] [-/+sm] [-/+sp] [-/+t] [-/+u8] [-/+uc]
    [-/+ulc] [-/+ulit] [-/+ut] [-/+vb] [-/+wc] [-/+wf]
    <omitted other options>
set [--abefhkmnptuvxBCEHPT] [-o option-name] [argument …]
set [+abefhkmnptuvxBCEHPT] [+o option-name] [argument …]

mkayaalp avatar Nov 15 '21 06:11 mkayaalp

Though that is a whole different argument scheme and is more related to #1210, especially with my comment.

However, that also points out the general challenge with trying to support automatic negation flags: there are a lot of different styles. I think the most important thing is we don't get in the way of people writing their CLI, even if it requires more boiler palte. After that is us making the core workflows and common / best practices easy to take advantage of. I think we should define the scope of what kind of automatic flags we support and limit ourselves to that or else I fear it'll either be too complicated by supporting all types or won't be present for anyone.

epage avatar Nov 15 '21 14:11 epage

EDIT: This can be done more cleanly nowadays, see https://github.com/clap-rs/clap/issues/815#issuecomment-1808282085

We're currently using this workaround to generate negation flags for all options:

    use once_cell::sync::OnceCell;

    let opts: Vec<_> = app.get_arguments().filter(|a| !a.is_positional()).collect();

    // We use a OnceCell to make strings 'static. Watch out that you don't
    // construct the app multiple times with different arguments because it'll
    // only be initialized once and reused thereafter.
    // Box::leak() would also work, but it makes valgrind unhappy.
    static ARG_STORAGE: OnceCell<Vec<String>> = OnceCell::new();
    let arg_storage = ARG_STORAGE.get_or_init(|| {
        opts.iter()
            .map(|opt| format!("--no-{}", opt.get_long().expect("long option")))
            .collect()
    });

    let negations: Vec<_> = opts
        .into_iter()
        .zip(arg_storage)
        .map(|(opt, flag)| {
            clap::Arg::new(&flag[2..])
                .long(flag)
                .hide(true)
                .overrides_with(opt.get_name())
        })
        .collect();

    app = app.args(negations);

(It also creates --no-help and --no-version, which is weird but harmless.)

blyxxyz avatar Feb 15 '22 22:02 blyxxyz

With the new ArgAction API, one approach we could take for this:

Arg::new("force")
    .action(clap::builder::SetBool)
    .long("force")
    .alias("no-force")

The SetBool action would effectively be

let alias: &str = ...;  // either `force` or `no-force`
let default_missing_value = if alias.starts_with("no-") {
    "false"
} else {
    "true"
};
...

The downside is you wouldn't be able to control whether shorts would set true or false.

epage avatar Jun 13 '22 17:06 epage

In xh we support negating all options, not just boolean options. --style=solarized --no-style becomes a no-op. I don't know how common that is (we imitated it from HTTPie), but it would be nice to have as a supported case.

The prefix check feels too magical, if I found that example in the wild I wouldn't know where to look for an explanation.

blyxxyz avatar Jun 13 '22 18:06 blyxxyz

In xh we support negating all options, not just boolean options. --style=solarized --no-style becomes a no-op. I don't know how common that is (we imitated it from HTTPie), but it would be nice to have as a supported case.

Thanks for bringing up this use case. I don't tend to see this too often. Even if we don't provide a native way of supporting it, there would always be overrides_with for clearing it out.

Speaking of, how are you implementing that today? I looked at your Parser didn't see a --no-style defined.

The prefix check feels too magical, if I found that example in the wild I wouldn't know where to look for an explanation.

I can understand. It is dependent on people thinking to check the the documentation for SetBool.

epage avatar Jun 13 '22 18:06 epage

Speaking of, how are you implementing that today?

See a few comments back: https://github.com/clap-rs/clap/issues/815#issuecomment-1040848452

The new reflection came in very handy!

blyxxyz avatar Jun 13 '22 19:06 blyxxyz

Oh, I had overlooked in that comment that you meant negations for all options and not just flags.

epage avatar Jun 13 '22 19:06 epage

Coming from the Python argparse camp, I heavily used the --*/--no-* pattern in my apps. This feature would be very helpful for me.

With the new ArgAction API, one approach we could take for this

@epage is it possible to define our own actions apart from the included standard ones? I couldn't find any examples for this so if you could point me to one, that'd be very helpful.

dhruvkb avatar Aug 07 '22 08:08 dhruvkb

is it possible to define our own actions apart from the included standard ones?

Not currently. but I think I saw something saying that was on the roadmap.

tmccombs avatar Aug 07 '22 18:08 tmccombs

@dhruvkb the plan is to eventually support it but not yet.

Current blockers:

  • Seeing how actions interact with the parser and other systems as the project evolves to make sure the we expose the right behavior.
  • Work through how to interact with parts of the system that are currently internal.

epage avatar Aug 08 '22 14:08 epage

I'll subscribe to the issue for updates. In the meantime I've got a variant of @blyxxyz's snippet working for me. Thanks!

dhruvkb avatar Aug 08 '22 14:08 dhruvkb

While I don't think this is the way we should go, I thought I'd record how CLI11 solves this problem: runtime parsing of a declaration string. See https://cliutils.github.io/CLI11/book/chapters/flags.html

epage avatar Aug 25 '22 21:08 epage

FWIW this is more or less what toggle_flag in bpaf is:

https://docs.rs/bpaf/0.5.6/bpaf/batteries/fn.toggle_flag.html

If all you care is autonegate for booleans you'd do something like this

fn autonegate(enabled: &'static str, disabled: &'static str, help: String) -> impl Parser<bool> {
    battery::toggle_flag(
        long(enabled).help(help), true, 
        long(disabled).help(format!("{help} disabled"), false
    ).map(|last_bool| last_bool.unwrap_or(true))
}

and then use it via external or directly in your logic as this: a boolean subparser that defaults to true and displays both options.

autonegate("follow-symlinks", "no-follow-symlinks", "follow symlinks".to_owned())

external requires to define a top level function at the moment, but I'll see if this can be changed.

pacak avatar Sep 04 '22 07:09 pacak

Hi, is there any updates? Thanks! Especially, it would be great to have one in Parser derivation mode, instead of the builder mode.

P.S. I found https://jwodder.github.io/kbits/posts/clap-bool-negate/ but the workaround is not super neat IMHO

fzyzcjy avatar Nov 13 '23 14:11 fzyzcjy

FWIW, in xh we've moved to a still cleaner implementation by enabling clap's string feature and doing this:

let negations: Vec<_> = app
    .get_arguments()
    .filter(|a| !a.is_positional())
    .map(|opt| {
        let long = opt.get_long().expect("long option");
        clap::Arg::new(format!("no-{}", long))
            .long(format!("no-{}", long))
            .hide(true)
            .action(ArgAction::SetTrue)
            // overrides_with is enough to make the flags take effect
            // We never have to check their values, they'll simply
            // unset previous occurrences of the original flag
            .overrides_with(opt.get_id())
    })
    .collect();

app.args(negations)
    .after_help("Each option can be reset with a --no-OPTION argument.")

It no longer feels like a hacky workaround so I'm pretty happy with it. It's cool how flexible clap has become.

blyxxyz avatar Nov 13 '23 14:11 blyxxyz

For my specific use-case, it would be perfect for me if I could use #[derive(Parser)] and use this feature to create an Option<bool> which would automatically have --foo and --no-foo combined as a single --[no-]foo in the help message. IMO --[no-]foo is a very readable and concise way to say "there are two mutually exclusive flags, and the app will decide if neither are specified." For example, --[no-]color to either force colors to be included or excluded in a CLI output, and the executable would try to detect color support if neither are specified.

spenserblack avatar Nov 13 '23 14:11 spenserblack

use this feature to create an Option<bool> which would automatically have

In my experience you only use Option<bool> if you want to handle case when user did not specify a value differently than from it being true or false, otherwise you just stick to bool and populate the default value from the parser. Option<bool> works on small examples but if you share configurations between apps or even use it in multiple places in the same app you can get into cases where different parts of the app treat None differently.

pacak avatar Nov 13 '23 15:11 pacak

Thank you! That looks helpful

fzyzcjy avatar Nov 13 '23 23:11 fzyzcjy

If the #[derive(Parser)] could implement a boolean flag like Python click, it would be perfect, combining both readability and flexibility.

@click.option('--color/--no-color', default=False)
@click.option('--enable-option/--disable-option', default=False)
@click.option('--with-something/--without-something', default=False)

Imagine:

    /// Given that the default value is true, providing a short option should be considered as false. And vice versa.
    #[arg(short = "C", long = "--color/--no-color", default_value = "true")]
    color_flag: bool,

    /// Explicitly providing a short option maps to true/false.
    #[arg(short = "o/O", long = "--enable-option/--disable-option", default_value = "true")]
    some_option_flag: bool,

kenchou avatar Nov 23 '23 10:11 kenchou