Re: [PATCH] various: disallow --no-no-OPT for --no-opt options

[Junio C Hamano <gitster@pobox.com>]

Jeff King <peff@peff.net> 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
approach.

> 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
error.

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.

Thanks.