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

From: Junio C Hamano <gitster@pobox.com>
Date: Jul 24, 2023, 18:51 UTC
Message-ID: <xmqqr0oxnnx4.fsf@gitster.g>
In-Reply-To: <1535f30e-3cf9-1a0a-04af-4ba4a7c46d15@web.de>

René Scharfe <l.s.r@web.de> writes:

Show 13 quoted lines

> Am 21.07.23 um 22:09 schrieb Junio C Hamano:
>> René Scharfe <l.s.r@web.de> 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".

Show 21 quoted lines

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