Re: [RFC PATCH] parse-options: disallow double-negations of options starting with no-
- Date: Wed, 19 Apr 2017 13:54:06 -0700
- From: Jacob Keller <jacob.keller@xxxxxxxxx>
- Subject: Re: [RFC PATCH] parse-options: disallow double-negations of options starting with no-
On Wed, Apr 19, 2017 at 8:10 AM, Jeff King <peff@xxxxxxxx> wrote:
> On Wed, Apr 19, 2017 at 02:08:20AM -0700, Jacob Keller wrote:
>> From: Jacob Keller <jacob.keller@xxxxxxxxx>
>> 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@xxxxxxxxx>
>> 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.
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
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()