Re: [RFC PATCH] parse-options: disallow double-negations of options starting with no-

[Jacob Keller <jacob.keller@gmail.com>]

On Wed, Apr 19, 2017 at 8:10 AM, Jeff King <peff@peff.net> wrote:
> On Wed, Apr 19, 2017 at 02:08:20AM -0700, Jacob Keller wrote:
>
>> From: Jacob Keller <jacob.keller@gmail.com>
>>
>> Many options can be negated by prefixing the option with "no-", for
>> example "--3way" can be prefixed with "--no-3way" to disable it. Since
>> 0f1930c58754 ("parse-options: allow positivation of options
>> starting, with no-", 2012-02-25) we have also had support to negate
>> options which start with "no-" by using the positive wording.
>>
>> This leads to the confusing (and non-documented) case that you can still
>> prefix options beginning with "no-" by a second "no-" to negate them.
>> That is, we allow "no-no-hardlinks" to negate the "no-hardlinks" option.
>>
>> This can be confusing to the user so lets just disallow the
>> double-negative forms. If the long_name begins with "no-" then we simply
>> don't allow the regular negation format, and only allow the option to be
>> negated by the positive form.
>>
>> Signed-off-by: Jacob Keller <jacob.keller@gmail.com>
>> ---
>> I started going about implementing an OPT_NEGBOOL as suggested by Peff,
>> but realized this might just be simpler, and we already support the
>> positive format for the negation, so we don't lose expressiveness. We
>> *might* want to tie this to an option flag instead so that it only kicks
>> in if the option specifically requests it. Thoughts?
>
> Yeah, if we are going to do anything, this is the right thing to do.
>
> I am on the fence on whether it actually needs addressing or not. Sure,
> --no-no-foo looks silly, but if the only way it happens is that the user
> typed it, it doesn't seem so bad to me to respect it. I am tempted to
> say we should support arbitrary levels of "no-" parsing as an easter
> egg, but that is probably silly. :)
>
> So I am fine with this patch, or without it.
>
> -Peff

This is why it's an RFC. I don't really feel that it's too much of a
problem. As for the reason why I thought it might want a flag itself
is because of concerns raised earlier that we might have something
liek

OPT_BOOL( ... "no-stage" ...)
OPT_INT(... "stage" ....)

or something already which might be broken and the only proper way to
disable no-stage is no-no-stage?

Is this actually a concern? I thought this was a comment raised by you
earlier as an objection to a change unless we specifically flagged
them as OPT_NEGBOOL()

Thanks,
Jake