Subject: Re: [PATCH 2/3] parse-options: allow positivation of options starting, with no-

From: René Scharfe <[email-masked]>
Date: Mon, 27 Feb 2012 18:56:03 +0100

Am 27.02.2012 18:18, schrieb Junio C Hamano:
> Thomas Rast<[email-masked]>  writes:
>
>> Junio C Hamano<[email-masked]>  writes:
>>
>>> I would naïvely expect that it would be sufficient to update an existing
>>> definition for "--no-frotz" that uses PARSE_OPT_NONEG to instead define
>>> "--frotz" that by itself is a no-op, and "--no-frotz" would cause whatever
>>> the option currently means, with an update to the help text that says
>>> something to the effect that "--frotz by itself is meaningless and is
>>> always used as --no-frotz".
>>
>> Doesn't that last quote already answer your question?
>
> Yes, but only partly.  I would agree with the awkwardness in
>
>> It would be rather awkward to see, in 'git apply -h',
>>
>>    --add                 Also apply additions in the patch.  This is the
>>                          default; use --no-add to disable it.
>
> but it feeels somewhat questionable that the solution to get this:
>
>>
>> Compare to the current concise wording
>>
>>    --no-add              ignore additions made by the patch
>
> is to define OPT_BOOL("no-add") that does not have any hint (other than
> the fact that the option name begins with 3 character "no-") that this is
> an already negated boolean and the "no-" negation can be removed.

The parser already knows that the prefix "no-" negates an option.  It 
currenmtly only applies this knowledge if that prefix is added, but not 
if it is removed, which is inconsistent.

> This means an option "no-$foo" can never mean anything but "not foo".  Not
> that we would have to or necessarily want to support an option to give the
> number of foo as --no-foo=47, as --num-foo=47 is a perfectly good spelling
> for such an option.

With the patch, you can define a --no-foo option that doesn't accept 
--foo as its negation by specifying PARSE_OPT_NONEG.  That would also 
forbid --no-no-foo, though, but that's probably a good thing.

> If it were OPT_BOOL("no-foo", OPT_ISNEG | ...) that signals the parser
> that:
>
>   - the option name is already negative;
>   - the leading "no-" is to be removed to negate it; and
>   - no extra leading "no-", i.e. "--no-no-foo", is accepted.
>
> I probably wouldn't have felt this uneasy iffiness.

Teaching the parser to understand that removal of the prefix "no-" 
negates an option on top of its existing knowledge that adding it does 
the same just adds the other side of the same coin, which was curiously 
missing.

The patch does not forbid adding "no-" to an option that already starts 
with "no-".  This stricter rule would be easy to add, but since that is 
currently the only way to negate such options, it would break backwards 
compatibility and thus should be added in a separate patch, if at all.

With the patch, the following guidelines are followed:

	- "no-" means no, for both developers and users.
	- The user doesn't have to to say "no-no-".

The results feels simpler to me.

René