Re: [PATCH v5] diff: add diff.srcPrefix and diff.dstPrefix configuration variables

From: Dragan Simic <dsimic@manjaro.org>
Date: Mar 15, 2024, 19:13 UTC
Message-ID: <fab333caad7ffe3e4264e1fdf163511e@manjaro.org>
In-Reply-To: <xmqqy1ajqvkb.fsf@gitster.g>

On 2024-03-15 18:00, Junio C Hamano wrote:

Show 21 quoted lines

> Peter Hutterer <peter.hutterer@who-t.net> writes:
> 
>> Allow the default prefixes "a/" and "b/" to be tweaked by the
>> diff.srcPrefix and diff.dstPrefix configuration variables.
>> 
>> Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
>> ---
>> Changes to v4 (as pointed out by Dragan):
>> - copy/paste-o fixed in the dstprefix test
> 
> This one I understand is an improvement
> 
>> - reworded the description for the tests as suggested
> 
> Moving from "diff src/dstprefix" to "diff.*prefix" feels more like a
> regresison to me, when it is about interaction between {src,dst}prefix
> and other kind of prefix variables like {no,mnemonic}prefix.  I
> would have understood if the updated title were more like:
> 
>     test_expect_success 'diff.{src,dst}Prefix are ignored with 
> diff.noPrefix'

That would be even better, thank you for pointing it out. If possible, I'd suggest that your wording becomes merged.

> I am tempted to queue v4 with the z/ -> y/ fix from this round,
> without any other changes from v4 to v5.
> 
> Thanks, both.