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

[Phillip Wood <phillip.wood123@gmail.com>]

On 13/03/2024 15:29, Junio C Hamano wrote:
> Phillip Wood <phillip.wood123@gmail.com> writes:
> 
>>> With a custom prefix those scripts will break, having an option that
>>> forces the a/ and b/ prefix helps. Plus the man page explicitly says:
>>>     Use the default source and destination prefixes ("a/" and "b/").
>>> So let's stick with that behaviour then.
>>
>> As I understand it the purpose of --default-prefix is to override all
>> the prefix related config settings so this seems like a very sensible
>> choice.
> 
> It would be nice to update the description of '--default-prefix' so
> that nobody has to say "As I understand it" anymore ;-)

That's a good idea

> As we are selling .{src,dst}Prefix as a thing that sets the default
> prefix, we'd need to break the loop somehow, and "hardcoded" below
> is my attempt to do so, but I am not sure if I succeeded.
> 
>   Documentation/diff-options.txt | 7 ++++---
>   1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git c/Documentation/diff-options.txt w/Documentation/diff-options.txt
> index aaaff0d46f..62eaa46d84 100644
> --- c/Documentation/diff-options.txt
> +++ w/Documentation/diff-options.txt
> @@ -864,9 +864,10 @@ endif::git-format-patch[]
>   	Do not show any source or destination prefix.
>   
>   --default-prefix::
> -	Use the default source and destination prefixes ("a/" and "b/").
> -	This is usually the default already, but may be used to override
> -	config such as `diff.noprefix`.
> +	Use the hardcoded default source and destination prefixes
> +	("a/" and "b/").  This is designed to be used to override
> +	configuration variables such as `diff.noprefix` and
> +	`diff.srcPrefix`.

That looks clear to me. I think the only other config variable that 
affects the prefix is "diff.mnemonicPrefix" so if we're going to update 
the description to mention "diff.srcPrefix" maybe we should mention that 
one as well or just say something like "This is designed to be used to 
override the configuration variables `diff.*Prefix`.".

Best Wishes

Phillip


>   --line-prefix=<prefix>::
>   	Prepend an additional prefix to every line of output.