Re: [PATCH] various: disallow --no-no-OPT for --no-opt options
- Date: Tue, 18 Apr 2017 19:40:37 -0700
- From: Junio C Hamano <gitster@xxxxxxxxx>
- Subject: Re: [PATCH] various: disallow --no-no-OPT for --no-opt options
Jeff King <peff@xxxxxxxx> writes:
> On Wed, Apr 19, 2017 at 12:29:18AM +0200, René Scharfe wrote:
>> PARSE_OPT_NONEG should only be used for options where a negation doesn't
>> make sense, e.g. for the --stage option of checkout-index.
> I think we do strive to avoid "--no-no-foo", and instead have "--no-foo"
> and "--foo" to cover both sides. So for example:
>> > - OPT_BOOL(0, "no-add", &state->no_add,
>> > + OPT_BOOL_NONEG(0, "no-add", &state->no_add,
>> > N_("ignore additions made by the patch")),
> This could be more like:
> OPT_NEGBOOL(0, "add", &state->no_add, ...)
> where NEGBOOL would be smart enough to show "--no-add" in the help as
> the primary.
I very much appreciate that this topic to avoid --no-no-OPT
nonsense, but just disabling --no-no-OPT without giving --OPT the
meaning the user who would have used --no-no-OPT wanted does not
sound like a good solution. Your NEGBOOL looks like a better
> It might even be possible to detect the existing line and
> have parse-options automatically respect "--foo" when "--no-foo" is
> present. But that may run afoul of callers that add both "--foo" and
> "--no-foo" manually.
True but wouldn't that something we would want to avoid anyway?
That is, "git cmd [--OPT | --no-OPT | --no-no-OPT]" from the end
user's point of view should be an error because it is unclear what
difference there are between --OPT and --no-no-OPT. And we should
be able to add a rule to parse_options_check() to catch such an
Having said that, I am not sure if we want to go the route of
"existing line that begins with 'no-' behaves magical". For
boolean, I suspect we may be get away with such a magic without
confusing ourselves too much, though.