Subject: Re: [PATCH] ls-tree: fix --no-full-name

From: Junio C Hamano <[email-masked]>
Date: Mon, 24 Jul 2023 11:51:19 -0700

René Scharfe <[email-masked]> writes:

> Am 21.07.23 um 22:09 schrieb Junio C Hamano:
>> René Scharfe <[email-masked]> writes:
>>
>>> -    -D, --no-doubt        begins with 'no-'
>>> +    -D, --[no-]no-doubt   begins with 'no-'
>>
>> Hmph, I really really loved the neat trick to allow "no-doubt"
>> option to be "positivised" by _dropping_ the leading "no-" at around
>> 0f1930c5 (parse-options: allow positivation of options starting,
>> with no-, 2012-02-25).
>
> Yeah, if there is a better way to document A) that the "no-" is optional
> and B) whether it's present by default, I'm all ears.

Some options take "no-" prefix while some others do not, so
indicating that "this can take negative forms" vs "this do not take
negative forms" by "--[no-]xyzzy" and "--frotz" makes sense.

Yikes.  There are tons of options whose names begin with "no-" and
marked PARSE_OPT_NONEG, so "an option '--no-nitfol' that does not
have the 'no-' part in [brackets] can drop 'no-' to make it
positive" would not fly as a rule/convention.

If we do not mind getting longer, we could say

	-D, --no-doubt, --doubt

and explain in the description that --no-doubt is the same as -D and
--doubt is the default.  It is making the developers responsible for
clarify, which is not very satisfying.

We may not reject "--no-no-doubt" but with the positivization
support, double negation is not something we'd encourage without
feeling embarrassed.

> Hard to say for me -- these are synthetic test cases and I lack context
> to make that decision.  In t0040 (t/helper/test-parse-options.c rather)
> we do have a few PARSE_OPT_NONEG uses already.  In t1502 we need to add
> some...

True.  The test coverage will be hurt if we start futzing with
OPT_NONEG bit "randomly".

>>> diff --git a/t/t1502-rev-parse-parseopt.sh b/t/t1502-rev-parse-parseopt.sh
>>> index dd811b7fb4..0a67e2dd4f 100755
>>> --- a/t/t1502-rev-parse-parseopt.sh
>>> +++ b/t/t1502-rev-parse-parseopt.sh
>>> @@ -64,33 +64,38 @@ test_expect_success 'test --parseopt help output' '
>>>  |
>>>  |    some-command does foo and bar!
>>>  |
>>> -|    -h, --help            show the help
>>> -|    --foo                 some nifty option --foo
>>> -|    --bar ...             some cool option --bar with an argument
>>> -|    -b, --baz             a short and long option
>>> +|    -h, --[no-]help       show the help
>>
>> Indeed it is amusing, but we probably should give PARSE_OPT_NONEG
>> appropriately, instead of changing the expectations, for many of the
>> changes we see here, I think.
>
> ... and --help is the one obvious choice for me, because --no-help is
> not supported, of course.  But we can use some more dedicated tests of
> negation and double-negation.

Yeah.