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

From: René Scharfe <l.s.r@web.de>
Date: Aug 5, 2023, 10:40 UTC
Message-ID: <67e28f23-abd7-01d2-a1a7-18befc86a21e@web.de>
In-Reply-To: <a9d51bb4-2897-c4ca-f8b3-85f50902b041@gmail.com>

Am 04.08.23 um 21:48 schrieb Phillip Wood:

Show 15 quoted lines

> On 04/08/2023 17:40, Junio C Hamano wrote:
>> Junio C Hamano <gitster@pobox.com> writes:
>>
>>> René Scharfe <l.s.r@web.de> writes:
>>>
>>>> A bit more verbose still: Document the negative form on its own line
>>>> with a generated description -- requires no new syntax:
>>>>
>>>>      -v, --invert-match         show non-matching lines
>>>>      --no-invert-match          opposite of --invert-match, default
>>>>      -I, --no-index             find in contents not managed by git
>>>>      --index                    opposite of --no-index, default
>>>
>>> I would expect _("opposite of %s, default") is acceptable by l10n
>>> folks, and assuming it is, the above would be a good approach.

The ", default" part is not correct in all cases, though, in particular when the default depends on the output being a terminal, or arguably when it's dependent on a config setting. We better drop it from the generated message.

Show 9 quoted lines

>> I was seeing what is likely to be in the -rc1 that will happen in
>> next week, and noticed that this discussion is left unconcluded.  I
>> am tempted to declare that the latest iteration that shows the
>> negation of "--no-foo" as "--[no-]no-foo" is "good enough" for now,
>> and leave the above improvement as one potential future direction.
>
> While it could certainly be improved in the future I think
> "--[no-]no-foo" is probably good enough. I definitely prefer it over
> "--[[no-]no]-foo"

Generating help lines for the opposite variant of all negatable options feels quite spammy. It almost doubles the length of the short help because we have so many of them. Doing that only for --no-... options is much more compatc because most options are positive:

    -v, --[no-]invert-match    show non-matching lines
    -I, --no-index             find in contents not managed by git
    --index                    opposite of --no-index

>> Objections?  Otherwise the 5-patch series will be in 'next'.

Patch 5 has a trivial merge conflict in t/t0040-parse-options.sh due to 448abbba63 (short help: allow multi-line opthelp, 2023-07-18), which is easily resolved by adding --longhelp, and a silent automatic mismerge of t/t1502/optionspec.help due to c512643e67 (short help: allow a gap smaller than USAGE_GAP, 2023-07-18), which requires removing three line newlines.

How about dropping patch 5 for now? We don't need to rush this pretty intrusive change of output. And the first four patches are worthwhile in their own right, I think.

René