Open kbknapp opened 7 years ago
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.
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.
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
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.
Random thoughts:
--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.no
format to be the default, so I'd want the no-
prefix stripped.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 …]
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.
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.)
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.
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.
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
.
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!
Oh, I had overlooked in that comment that you meant negations for all options and not just flags.
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.
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.
@dhruvkb the plan is to eventually support it but not yet.
Current blockers:
I'll subscribe to the issue for updates. In the meantime I've got a variant of @blyxxyz's snippet working for me. Thanks!
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
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
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.
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.
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.
Thank you! That looks helpful
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,
Would love to see this added!
I would love this as well. Like others in this thread, I don't think we should try to do this automatically; I'd be happy to see an explicit way for the builder and declarative APIs to add a --no-
option for a given option.
This option could work for a boolean, as well as for an Option<T>
that has a default of Some(default_value)
(with the --no-
option setting None
).
#[arg(long, negation)]
pub auto: bool,
#[arg(long, default_value = "hello", negation)]
pub value: Option<String>,
@joshtriplett interesting to also include this for options.
We also would need to define the builder API and what the semantics for this would be (parser, help, etc).
If negation
should not be automatically enabled, I would also argue that negation
should also not automatically add default_value_t = true
for booleans (even though I agree it would make a lot of sense). If we want to be explicit with unsurprising syntax, I expect the --auto/--no-auto
use-case to look like:
#[arg(long, default_value_t = true, negation)]
pub auto: bool,
In some way, negation
only make sense if long
is already present and if default_value
is already present. If long
is not implicitly added, then default_value
shouldn't either. And since we want to support Option
, then we can't implicitly add default-value
(unless we use the syntax support_negation_of_default = <default>
, in which case both long
and default_value
can be inferred).
If negation should not be automatically enabled, I would also argue that negation should also not automatically add default_value_t = true for booleans (even though I agree it would make a lot of sense).
default_value_t
(what to do when no flag is present) and default_missing_value_t
(what to do when the flag is present) are both customizable for flags. I don't think I'd want to say default_value_t
should be required though.
Option<bool>
(as mentioned earlier) is also a good way of handling the "not present" state.
In some way, negation only make sense if long is already present and if default_value is already present. If long is not implicitly added, then default_value shouldn't either. And since we want to support Option, then we can't implicitly add default-value (unless we use the syntax support_negation_of_default =
, in which case both long and default_value can be inferred).
I don't quite get this. Flags already require long/short. This is a modification to flag state. For someone to add negation
to an existing flag to now have to explicitly set flags that they were ok with before this doesn't make sense to me.
And since we want to support Option, then we can't implicitly add default-value (unless we use the syntax support_negation_of_default =
, in which case both long and default_value can be inferred).
There are lots of ways of implementing this and they don't necessarily preclude this:
Option
and implicit default_value_t
by checking the ValueSource
Option
is presentOverall though, this discussion has a fatal flaw: we are designing a feature around derive semantics and discussing what is or isn't possible. We need to focus on the builder semantics and then decide what automatic behavior we want to the derive semantics to have on top of that. There can be some back and forth on that (the derive influencing the builder design). Thats a big value-add of merging structopt into clap! We've run into many problems where the builder API made choices that work well in isolation but don't work well for builder and have worked to correct them.
I don't think I'd want to say
default_value_t
should be required though.
Maybe I'm misunderstanding what you imply. I didn't say that default_value_t
should be required. I said that negation
should not act as an implicit default_value_t
, and thus correct usage of negation
should specify a default_value_t
(it's not required, it's just probably wrong to not do it). Not sure about default_missing_value_t
though, some specified semantics (as you mention at the end of your comment) would indeed be useful. What I inferred from @joshtriplett comment would be a semantic like:
negation
is specified, then an additional conflicting flag is added.--no-LONG
where LONG
is the long name of the negated flag (which must thus have a long name).false
if it is a bool
and to None
if it is an Option
.
Option<bool>
(as mentioned earlier) is also a good way of handling the "not present" state.
This doesn't have the same user experience. The best workaround right now is to explictly define the additional --no-
flag and make it conflicting with the negated one (real-life example). Then in the code, check both flags and behave accordingly (real-life example). Note that in this example, the "default value" (i.e. when none of those flags is present) is context-dependent. In that sense, it is similar to the Option<bool>
except that it's implemented with 2 bool
s where (true, true)
is unreachable (thus having exactly the same number of values, namely 3).
I don't quite get this.
That's probably related to the misunderstanding above. What if @joshtriplett example was actually the following?
#[arg(negation)]
pub auto: bool,
What would it mean? What is the name of the added flag? And similarly if it was #[arg(short, negation)]
. But as you said at the end, we need a clear semantics, which I provided a candidate at the beginning of this comment. Hopefully it makes things clearer.
We need to focus on the builder semantics and then decide what automatic behavior we want to the derive semantics to have on top of that
I thought the builder and derive semantics were isomorphic since it seems derive attributes cover all builder functions. I never used the builder so maybe I'm missing some functionalities. That said, I agree that proposals need a clear semantics, and I hope the one I gave above is clear enough (for both builder and derive).
More generally it might be desirable to support multiple argument names controlling the same value (with different actions) that are all automatically exclusive with eachother, with some shorthands to better support more common use cases (bools, options).
Working out all of the semantics of this more general approach could make it much easier to define the semantics of the shorthand forms, as simply being equivalent to some usage of the general API.
Admittedly the current API is very value-centric, making it a bit hard to imagine how this feature could be used, but being able to assign multiple flag-action pairings to the same value would be your starting point.
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, theno
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:
New proposal:
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 doubleno
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 withno
and simply remove theno
, 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 apanic!
. 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
AppSettings::GenerateNegationFlags
which does the above, but automatically for all flags.AppSettings:GenerateHiddenNegationFlags
which hides all these negation flags.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.