[PATCH] diff: add diff.srcprefix and diff.dstprefix option support

[PATCH] diff: add diff.srcprefix and diff.dstprefix option support

[Peter Hutterer]

The git option equivalent to --src-prefix and --dst-prefix.
Both of these are of lower precedence than the diff.noprefix and
diff.mnemonicprefix option.

Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
---
As David already mentioned [1] my main motivation here is to use
a prefix of "./" because that is patch -p1 compatible *and* supports
double-click highlighting of the file path in most terminals.

The current approach of a/ and b/ fail at the latter and diff.noprefix
fails at the former.

[1] https://lore.kernel.org/git/f80aaf4a-ffea-48e6-b279-c3b7a6a53996@ixit.cz/

 Documentation/config/diff.txt |  6 ++++++
 diff.c                        | 18 ++++++++++++++++++
 diff.h                        |  1 +
 t/t4013-diff-various.sh       | 20 ++++++++++++++++++++
 4 files changed, 45 insertions(+)

diff --git a/Documentation/config/diff.txt b/Documentation/config/diff.txt
index 6c7e09a1ef5e..888632955b30 100644
--- a/Documentation/config/diff.txt
+++ b/Documentation/config/diff.txt
@@ -111,6 +111,12 @@ diff.mnemonicPrefix::
 diff.noprefix::
 	If set, 'git diff' does not show any source or destination prefix.
 
+diff.srcprefix::
+	If set, 'git diff' uses this source prefix.
+
+diff.dstprefix::
+	If set, 'git diff' uses this destination prefix.
+
 diff.relative::
 	If set to 'true', 'git diff' does not show changes outside of the directory
 	and show pathnames relative to the current directory.
diff --git a/diff.c b/diff.c
index e50def45383e..52a476737def 100644
--- a/diff.c
+++ b/diff.c
@@ -62,6 +62,8 @@ static const char *diff_order_file_cfg;
 int diff_auto_refresh_index = 1;
 static int diff_mnemonic_prefix;
 static int diff_no_prefix;
+static const char *diff_src_prefix;
+static const char *diff_dst_prefix;
 static int diff_relative;
 static int diff_stat_name_width;
 static int diff_stat_graph_width;
@@ -408,6 +410,12 @@ int git_diff_ui_config(const char *var, const char *value,
 		diff_no_prefix = git_config_bool(var, value);
 		return 0;
 	}
+	if (!strcmp(var, "diff.srcprefix")) {
+		return git_config_string(&diff_src_prefix, var, value);
+	}
+	if (!strcmp(var, "diff.dstprefix")) {
+		return git_config_string(&diff_dst_prefix, var, value);
+	}
 	if (!strcmp(var, "diff.relative")) {
 		diff_relative = git_config_bool(var, value);
 		return 0;
@@ -3429,6 +3437,14 @@ void diff_set_default_prefix(struct diff_options *options)
 	options->b_prefix = "b/";
 }
 
+void diff_set_custom_prefix(struct diff_options *options, const char *src_prefix, const char *dst_prefix)
+{
+	if (src_prefix)
+		options->a_prefix = src_prefix;
+	if (dst_prefix)
+		options->b_prefix = dst_prefix;
+}
+
 struct userdiff_driver *get_textconv(struct repository *r,
 				     struct diff_filespec *one)
 {
@@ -4736,6 +4752,8 @@ void repo_diff_setup(struct repository *r, struct diff_options *options)
 		diff_set_noprefix(options);
 	} else if (!diff_mnemonic_prefix) {
 		diff_set_default_prefix(options);
+		if (diff_src_prefix || diff_dst_prefix)
+			diff_set_custom_prefix(options, diff_src_prefix, diff_dst_prefix);
 	}
 
 	options->color_moved = diff_color_moved_default;
diff --git a/diff.h b/diff.h
index 66bd8aeb2936..ab4dd5ec70f3 100644
--- a/diff.h
+++ b/diff.h
@@ -499,6 +499,7 @@ void diff_tree_combined_merge(const struct commit *commit, struct rev_info *rev)
 void diff_set_mnemonic_prefix(struct diff_options *options, const char *a, const char *b);
 void diff_set_noprefix(struct diff_options *options);
 void diff_set_default_prefix(struct diff_options *options);
+void diff_set_custom_prefix(struct diff_options *options, const char *src_prefix, const char *dst_prefix);
 
 int diff_can_quit_early(struct diff_options *);
 
diff --git a/t/t4013-diff-various.sh b/t/t4013-diff-various.sh
index 1e3b2dbea484..86834186fdba 100755
--- a/t/t4013-diff-various.sh
+++ b/t/t4013-diff-various.sh
@@ -663,6 +663,26 @@ test_expect_success 'diff --default-prefix overrides diff.mnemonicprefix' '
 	check_prefix actual a/file0 b/file0
 '
 
+test_expect_success 'diff respects diff.srcprefix' '
+	git -c diff.srcprefix=x/ diff >actual &&
+	check_prefix actual x/file0 b/file0
+'
+
+test_expect_success 'diff respects diff.dstprefix' '
+	git -c diff.dstprefix=y/ diff >actual &&
+	check_prefix actual a/file0 y/file0
+'
+
+test_expect_success 'diff src/dstprefix ignored with diff.noprefix' '
+	git -c diff.dstprefix=y/ -c diff.srcprefix=x/ -c diff.noprefix diff >actual &&
+	check_prefix actual file0 file0
+'
+
+test_expect_success 'diff src/dstprefix ignored with diff.mnemonicprefix' '
+	git -c diff.dstprefix=x/ -c diff.srcprefix=y/ -c diff.mnemonicprefix diff >actual &&
+	check_prefix actual i/file0 w/file0
+'
+
 test_expect_success 'diff --no-renames cannot be abbreviated' '
 	test_expect_code 129 git diff --no-rename >actual 2>error &&
 	test_must_be_empty actual &&
-- 
2.44.0

Re: [PATCH] diff: add diff.srcprefix and diff.dstprefix option support

[Junio C Hamano]

Peter Hutterer <peter.hutterer@who-t.net> writes:

> Subject: Re: [PATCH] diff: add diff.srcprefix and diff.dstprefix option support

"option support" -> "configuration variables"

> The git option equivalent to --src-prefix and --dst-prefix.
> Both of these are of lower precedence than the diff.noprefix and
> diff.mnemonicprefix option.

I think it will become simpler to sell and explain if you do not
mention these options, and instead say that we are tweaking the
default prefixes used when none of the other options are used,
something like:

	Allow the default prefixes "a/" and "b/" to be tweaked by
	diff.srcprefix and diff.dstprefix configuration variables.
	
> Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
> ---
> +diff.srcprefix::
> +	If set, 'git diff' uses this source prefix.

Add "Defaults to 'a/'", perhaps?

> @@ -3429,6 +3437,14 @@ void diff_set_default_prefix(struct diff_options *options)
>  	options->b_prefix = "b/";
>  }
>  
> +void diff_set_custom_prefix(struct diff_options *options, const char *src_prefix, const char *dst_prefix)
> +{
> +	if (src_prefix)
> +		options->a_prefix = src_prefix;
> +	if (dst_prefix)
> +		options->b_prefix = dst_prefix;
> +}
> +
>  struct userdiff_driver *get_textconv(struct repository *r,
>  				     struct diff_filespec *one)
>  {
> @@ -4736,6 +4752,8 @@ void repo_diff_setup(struct repository *r, struct diff_options *options)
>  		diff_set_noprefix(options);
>  	} else if (!diff_mnemonic_prefix) {
>  		diff_set_default_prefix(options);
> +		if (diff_src_prefix || diff_dst_prefix)
> +			diff_set_custom_prefix(options, diff_src_prefix, diff_dst_prefix);
>  	}

This feels somewhat roundabout way to do this.  Instead of touching
this part at all, and not adding diff_set_custom_prefix() function,
how about just patching diff_set_default_prefix()?  The function
does not even have to be public and there is no need to touch the
header file.

Here is how I would simplify the code change part if I were doing
this patch.  It seems to pass t4013 (including your additional
ones).

Thanks.


 diff.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git i/diff.c w/diff.c
index e50def4538..4439b1a958 100644
--- i/diff.c
+++ w/diff.c
@@ -62,6 +62,8 @@ static const char *diff_order_file_cfg;
 int diff_auto_refresh_index = 1;
 static int diff_mnemonic_prefix;
 static int diff_no_prefix;
+static const char *diff_src_prefix = "a/";
+static const char *diff_dst_prefix = "b/";
 static int diff_relative;
 static int diff_stat_name_width;
 static int diff_stat_graph_width;
@@ -408,6 +410,12 @@ int git_diff_ui_config(const char *var, const char *value,
 		diff_no_prefix = git_config_bool(var, value);
 		return 0;
 	}
+	if (!strcmp(var, "diff.srcprefix")) {
+		return git_config_string(&diff_src_prefix, var, value);
+	}
+	if (!strcmp(var, "diff.dstprefix")) {
+		return git_config_string(&diff_dst_prefix, var, value);
+	}
 	if (!strcmp(var, "diff.relative")) {
 		diff_relative = git_config_bool(var, value);
 		return 0;
@@ -3425,8 +3433,8 @@ void diff_set_noprefix(struct diff_options *options)
 
 void diff_set_default_prefix(struct diff_options *options)
 {
-	options->a_prefix = "a/";
-	options->b_prefix = "b/";
+	options->a_prefix = diff_src_prefix;
+	options->b_prefix = diff_dst_prefix;
 }
 
 struct userdiff_driver *get_textconv(struct repository *r,

[PATCH v2] diff: add diff.srcprefix and diff.dstprefix configuration variables

[Peter Hutterer]

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 v1:
- note default in documentation
- drop the custom prefix function, change the defaults instead
- commit message: options -> configuration variables

Junio: all of your comments squashed in as requested, thanks for the
review and suggestions, much simpler now.

 Documentation/config/diff.txt |  6 ++++++
 diff.c                        | 12 ++++++++++--
 t/t4013-diff-various.sh       | 20 ++++++++++++++++++++
 3 files changed, 36 insertions(+), 2 deletions(-)

diff --git a/Documentation/config/diff.txt b/Documentation/config/diff.txt
index 6c7e09a1ef5e..43d94df19876 100644
--- a/Documentation/config/diff.txt
+++ b/Documentation/config/diff.txt
@@ -111,6 +111,12 @@ diff.mnemonicPrefix::
 diff.noprefix::
 	If set, 'git diff' does not show any source or destination prefix.
 
+diff.srcprefix::
+	If set, 'git diff' uses this source prefix. Defaults to 'a/'.
+
+diff.dstprefix::
+	If set, 'git diff' uses this destination prefix. Defaults to 'b/'.
+
 diff.relative::
 	If set to 'true', 'git diff' does not show changes outside of the directory
 	and show pathnames relative to the current directory.
diff --git a/diff.c b/diff.c
index e50def45383e..4439b1a95864 100644
--- a/diff.c
+++ b/diff.c
@@ -62,6 +62,8 @@ static const char *diff_order_file_cfg;
 int diff_auto_refresh_index = 1;
 static int diff_mnemonic_prefix;
 static int diff_no_prefix;
+static const char *diff_src_prefix = "a/";
+static const char *diff_dst_prefix = "b/";
 static int diff_relative;
 static int diff_stat_name_width;
 static int diff_stat_graph_width;
@@ -408,6 +410,12 @@ int git_diff_ui_config(const char *var, const char *value,
 		diff_no_prefix = git_config_bool(var, value);
 		return 0;
 	}
+	if (!strcmp(var, "diff.srcprefix")) {
+		return git_config_string(&diff_src_prefix, var, value);
+	}
+	if (!strcmp(var, "diff.dstprefix")) {
+		return git_config_string(&diff_dst_prefix, var, value);
+	}
 	if (!strcmp(var, "diff.relative")) {
 		diff_relative = git_config_bool(var, value);
 		return 0;
@@ -3425,8 +3433,8 @@ void diff_set_noprefix(struct diff_options *options)
 
 void diff_set_default_prefix(struct diff_options *options)
 {
-	options->a_prefix = "a/";
-	options->b_prefix = "b/";
+	options->a_prefix = diff_src_prefix;
+	options->b_prefix = diff_dst_prefix;
 }
 
 struct userdiff_driver *get_textconv(struct repository *r,
diff --git a/t/t4013-diff-various.sh b/t/t4013-diff-various.sh
index 1e3b2dbea484..86834186fdba 100755
--- a/t/t4013-diff-various.sh
+++ b/t/t4013-diff-various.sh
@@ -663,6 +663,26 @@ test_expect_success 'diff --default-prefix overrides diff.mnemonicprefix' '
 	check_prefix actual a/file0 b/file0
 '
 
+test_expect_success 'diff respects diff.srcprefix' '
+	git -c diff.srcprefix=x/ diff >actual &&
+	check_prefix actual x/file0 b/file0
+'
+
+test_expect_success 'diff respects diff.dstprefix' '
+	git -c diff.dstprefix=y/ diff >actual &&
+	check_prefix actual a/file0 y/file0
+'
+
+test_expect_success 'diff src/dstprefix ignored with diff.noprefix' '
+	git -c diff.dstprefix=y/ -c diff.srcprefix=x/ -c diff.noprefix diff >actual &&
+	check_prefix actual file0 file0
+'
+
+test_expect_success 'diff src/dstprefix ignored with diff.mnemonicprefix' '
+	git -c diff.dstprefix=x/ -c diff.srcprefix=y/ -c diff.mnemonicprefix diff >actual &&
+	check_prefix actual i/file0 w/file0
+'
+
 test_expect_success 'diff --no-renames cannot be abbreviated' '
 	test_expect_code 129 git diff --no-rename >actual 2>error &&
 	test_must_be_empty actual &&
-- 
2.44.0

Re: [PATCH v2] diff: add diff.srcprefix and diff.dstprefix configuration variables

[Junio C Hamano]

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 v1:
> - note default in documentation
> - drop the custom prefix function, change the defaults instead
> - commit message: options -> configuration variables

Looking good.  Will queue.  Thanks.

Re: [PATCH v2] diff: add diff.srcprefix and diff.dstprefix configuration variables

[Dragan Simic]

On 2024-03-12 20:23, Junio C Hamano wrote:
> 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.

If I may interject, albeit a bit late, we should use "diff.srcPrefix"
and "diff.dstPrefix" as the configuration option names.

>> Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
>> ---
>> Changes to v1:
>> - note default in documentation
>> - drop the custom prefix function, change the defaults instead
>> - commit message: options -> configuration variables
> 
> Looking good.  Will queue.  Thanks.

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

[Peter Hutterer]

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 v2;
- doc: change to camelcase diff.srcPrefix/diff.dstPrefix for
  consistency with diff.mnemonicPrefix and most other options
- git diff --default-prefix forces a/ and b/ regardless of configured
  prefix, see the 'diff_opt_default_prefix' hunk in the patch below.

The latter may be slightly controversial but: there are scripts out
there that rely on the a/ and b/ prefix (came across one last night).
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.

 Documentation/config/diff.txt |  6 ++++++
 diff.c                        | 14 ++++++++++++--
 t/t4013-diff-various.sh       | 35 +++++++++++++++++++++++++++++++++++
 3 files changed, 53 insertions(+), 2 deletions(-)

diff --git a/Documentation/config/diff.txt b/Documentation/config/diff.txt
index 6c7e09a1ef5e..afc23d7723b6 100644
--- a/Documentation/config/diff.txt
+++ b/Documentation/config/diff.txt
@@ -111,6 +111,12 @@ diff.mnemonicPrefix::
 diff.noprefix::
 	If set, 'git diff' does not show any source or destination prefix.
 
+diff.srcPrefix::
+	If set, 'git diff' uses this source prefix. Defaults to 'a/'.
+
+diff.dstPrefix::
+	If set, 'git diff' uses this destination prefix. Defaults to 'b/'.
+
 diff.relative::
 	If set to 'true', 'git diff' does not show changes outside of the directory
 	and show pathnames relative to the current directory.
diff --git a/diff.c b/diff.c
index e50def45383e..108c1875775d 100644
--- a/diff.c
+++ b/diff.c
@@ -62,6 +62,8 @@ static const char *diff_order_file_cfg;
 int diff_auto_refresh_index = 1;
 static int diff_mnemonic_prefix;
 static int diff_no_prefix;
+static const char *diff_src_prefix = "a/";
+static const char *diff_dst_prefix = "b/";
 static int diff_relative;
 static int diff_stat_name_width;
 static int diff_stat_graph_width;
@@ -408,6 +410,12 @@ int git_diff_ui_config(const char *var, const char *value,
 		diff_no_prefix = git_config_bool(var, value);
 		return 0;
 	}
+	if (!strcmp(var, "diff.srcprefix")) {
+		return git_config_string(&diff_src_prefix, var, value);
+	}
+	if (!strcmp(var, "diff.dstprefix")) {
+		return git_config_string(&diff_dst_prefix, var, value);
+	}
 	if (!strcmp(var, "diff.relative")) {
 		diff_relative = git_config_bool(var, value);
 		return 0;
@@ -3425,8 +3433,8 @@ void diff_set_noprefix(struct diff_options *options)
 
 void diff_set_default_prefix(struct diff_options *options)
 {
-	options->a_prefix = "a/";
-	options->b_prefix = "b/";
+	options->a_prefix = diff_src_prefix;
+	options->b_prefix = diff_dst_prefix;
 }
 
 struct userdiff_driver *get_textconv(struct repository *r,
@@ -5362,6 +5370,8 @@ static int diff_opt_default_prefix(const struct option *opt,
 
 	BUG_ON_OPT_NEG(unset);
 	BUG_ON_OPT_ARG(optarg);
+	diff_src_prefix = "a/";
+	diff_dst_prefix = "b/";
 	diff_set_default_prefix(options);
 	return 0;
 }
diff --git a/t/t4013-diff-various.sh b/t/t4013-diff-various.sh
index 1e3b2dbea484..e75f9f7d4cb2 100755
--- a/t/t4013-diff-various.sh
+++ b/t/t4013-diff-various.sh
@@ -663,6 +663,41 @@ test_expect_success 'diff --default-prefix overrides diff.mnemonicprefix' '
 	check_prefix actual a/file0 b/file0
 '
 
+test_expect_success 'diff respects diff.srcprefix' '
+	git -c diff.srcprefix=x/ diff >actual &&
+	check_prefix actual x/file0 b/file0
+'
+
+test_expect_success 'diff respects diff.dstprefix' '
+	git -c diff.dstprefix=y/ diff >actual &&
+	check_prefix actual a/file0 y/file0
+'
+
+test_expect_success 'diff --src-prefix overrides diff.srcprefix' '
+	git -c diff.srcprefix=z/ diff --src-prefix=z/ >actual &&
+	check_prefix actual z/file0 b/file0
+'
+
+test_expect_success 'diff --dst-prefix overrides diff.dstprefix' '
+	git -c diff.dstprefix=y/ diff --dst-prefix=z/ >actual &&
+	check_prefix actual a/file0 z/file0
+'
+
+test_expect_success 'diff src/dstprefix ignored with diff.noprefix' '
+	git -c diff.dstprefix=y/ -c diff.srcprefix=x/ -c diff.noprefix diff >actual &&
+	check_prefix actual file0 file0
+'
+
+test_expect_success 'diff src/dstprefix ignored with diff.mnemonicprefix' '
+	git -c diff.dstprefix=x/ -c diff.srcprefix=y/ -c diff.mnemonicprefix diff >actual &&
+	check_prefix actual i/file0 w/file0
+'
+
+test_expect_success 'diff src/dstprefix ignored with --default-prefix' '
+	git -c diff.dstprefix=x/ -c diff.srcprefix=y/ diff --default-prefix >actual &&
+	check_prefix actual a/file0 b/file0
+'
+
 test_expect_success 'diff --no-renames cannot be abbreviated' '
 	test_expect_code 129 git diff --no-rename >actual 2>error &&
 	test_must_be_empty actual &&
-- 
2.44.0

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

[Dragan Simic]

Hello Peter,

On 2024-03-13 00:15, Peter Hutterer wrote:
> Allow the default prefixes "a/" and "b/" to be tweaked by the
> diff.srcprefix and diff.dstprefix configuration variables.

The only thing that's left is to update the patch description
to use camel case. :)

> Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
> ---
> Changes to v2;
> - doc: change to camelcase diff.srcPrefix/diff.dstPrefix for
>   consistency with diff.mnemonicPrefix and most other options
> - git diff --default-prefix forces a/ and b/ regardless of configured
>   prefix, see the 'diff_opt_default_prefix' hunk in the patch below.
> 
> The latter may be slightly controversial but: there are scripts out
> there that rely on the a/ and b/ prefix (came across one last night).
> 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.
> 
>  Documentation/config/diff.txt |  6 ++++++
>  diff.c                        | 14 ++++++++++++--
>  t/t4013-diff-various.sh       | 35 +++++++++++++++++++++++++++++++++++
>  3 files changed, 53 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/config/diff.txt 
> b/Documentation/config/diff.txt
> index 6c7e09a1ef5e..afc23d7723b6 100644
> --- a/Documentation/config/diff.txt
> +++ b/Documentation/config/diff.txt
> @@ -111,6 +111,12 @@ diff.mnemonicPrefix::
>  diff.noprefix::
>  	If set, 'git diff' does not show any source or destination prefix.
> 
> +diff.srcPrefix::
> +	If set, 'git diff' uses this source prefix. Defaults to 'a/'.
> +
> +diff.dstPrefix::
> +	If set, 'git diff' uses this destination prefix. Defaults to 'b/'.
> +
>  diff.relative::
>  	If set to 'true', 'git diff' does not show changes outside of the 
> directory
>  	and show pathnames relative to the current directory.
> diff --git a/diff.c b/diff.c
> index e50def45383e..108c1875775d 100644
> --- a/diff.c
> +++ b/diff.c
> @@ -62,6 +62,8 @@ static const char *diff_order_file_cfg;
>  int diff_auto_refresh_index = 1;
>  static int diff_mnemonic_prefix;
>  static int diff_no_prefix;
> +static const char *diff_src_prefix = "a/";
> +static const char *diff_dst_prefix = "b/";
>  static int diff_relative;
>  static int diff_stat_name_width;
>  static int diff_stat_graph_width;
> @@ -408,6 +410,12 @@ int git_diff_ui_config(const char *var, const char 
> *value,
>  		diff_no_prefix = git_config_bool(var, value);
>  		return 0;
>  	}
> +	if (!strcmp(var, "diff.srcprefix")) {
> +		return git_config_string(&diff_src_prefix, var, value);
> +	}
> +	if (!strcmp(var, "diff.dstprefix")) {
> +		return git_config_string(&diff_dst_prefix, var, value);
> +	}
>  	if (!strcmp(var, "diff.relative")) {
>  		diff_relative = git_config_bool(var, value);
>  		return 0;
> @@ -3425,8 +3433,8 @@ void diff_set_noprefix(struct diff_options 
> *options)
> 
>  void diff_set_default_prefix(struct diff_options *options)
>  {
> -	options->a_prefix = "a/";
> -	options->b_prefix = "b/";
> +	options->a_prefix = diff_src_prefix;
> +	options->b_prefix = diff_dst_prefix;
>  }
> 
>  struct userdiff_driver *get_textconv(struct repository *r,
> @@ -5362,6 +5370,8 @@ static int diff_opt_default_prefix(const struct
> option *opt,
> 
>  	BUG_ON_OPT_NEG(unset);
>  	BUG_ON_OPT_ARG(optarg);
> +	diff_src_prefix = "a/";
> +	diff_dst_prefix = "b/";
>  	diff_set_default_prefix(options);
>  	return 0;
>  }
> diff --git a/t/t4013-diff-various.sh b/t/t4013-diff-various.sh
> index 1e3b2dbea484..e75f9f7d4cb2 100755
> --- a/t/t4013-diff-various.sh
> +++ b/t/t4013-diff-various.sh
> @@ -663,6 +663,41 @@ test_expect_success 'diff --default-prefix
> overrides diff.mnemonicprefix' '
>  	check_prefix actual a/file0 b/file0
>  '
> 
> +test_expect_success 'diff respects diff.srcprefix' '
> +	git -c diff.srcprefix=x/ diff >actual &&
> +	check_prefix actual x/file0 b/file0
> +'
> +
> +test_expect_success 'diff respects diff.dstprefix' '
> +	git -c diff.dstprefix=y/ diff >actual &&
> +	check_prefix actual a/file0 y/file0
> +'
> +
> +test_expect_success 'diff --src-prefix overrides diff.srcprefix' '
> +	git -c diff.srcprefix=z/ diff --src-prefix=z/ >actual &&
> +	check_prefix actual z/file0 b/file0
> +'
> +
> +test_expect_success 'diff --dst-prefix overrides diff.dstprefix' '
> +	git -c diff.dstprefix=y/ diff --dst-prefix=z/ >actual &&
> +	check_prefix actual a/file0 z/file0
> +'
> +
> +test_expect_success 'diff src/dstprefix ignored with diff.noprefix' '
> +	git -c diff.dstprefix=y/ -c diff.srcprefix=x/ -c diff.noprefix diff 
> >actual &&
> +	check_prefix actual file0 file0
> +'
> +
> +test_expect_success 'diff src/dstprefix ignored with 
> diff.mnemonicprefix' '
> +	git -c diff.dstprefix=x/ -c diff.srcprefix=y/ -c diff.mnemonicprefix
> diff >actual &&
> +	check_prefix actual i/file0 w/file0
> +'
> +
> +test_expect_success 'diff src/dstprefix ignored with --default-prefix' 
> '
> +	git -c diff.dstprefix=x/ -c diff.srcprefix=y/ diff --default-prefix 
> >actual &&
> +	check_prefix actual a/file0 b/file0
> +'
> +
>  test_expect_success 'diff --no-renames cannot be abbreviated' '
>  	test_expect_code 129 git diff --no-rename >actual 2>error &&
>  	test_must_be_empty actual &&

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

[Dragan Simic]

On 2024-03-13 03:15, Dragan Simic wrote:
> On 2024-03-13 00:15, Peter Hutterer wrote:
>> Allow the default prefixes "a/" and "b/" to be tweaked by the
>> diff.srcprefix and diff.dstprefix configuration variables.
> 
> The only thing that's left is to update the patch description
> to use camel case. :)

I've spotted some inconsistencies in the way camel case is already
used in some of the "diff.*" configuration option names, so I went
ahead and fixed those.  I'll send the patches after I figure out how
to best split the changes into patches for easier review, because
there are quite a few small changes.

>> Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
>> ---
>> Changes to v2;
>> - doc: change to camelcase diff.srcPrefix/diff.dstPrefix for
>>   consistency with diff.mnemonicPrefix and most other options
>> - git diff --default-prefix forces a/ and b/ regardless of configured
>>   prefix, see the 'diff_opt_default_prefix' hunk in the patch below.
>> 
>> The latter may be slightly controversial but: there are scripts out
>> there that rely on the a/ and b/ prefix (came across one last night).
>> 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.
>> 
>>  Documentation/config/diff.txt |  6 ++++++
>>  diff.c                        | 14 ++++++++++++--
>>  t/t4013-diff-various.sh       | 35 
>> +++++++++++++++++++++++++++++++++++
>>  3 files changed, 53 insertions(+), 2 deletions(-)
>> 
>> diff --git a/Documentation/config/diff.txt 
>> b/Documentation/config/diff.txt
>> index 6c7e09a1ef5e..afc23d7723b6 100644
>> --- a/Documentation/config/diff.txt
>> +++ b/Documentation/config/diff.txt
>> @@ -111,6 +111,12 @@ diff.mnemonicPrefix::
>>  diff.noprefix::
>>  	If set, 'git diff' does not show any source or destination prefix.
>> 
>> +diff.srcPrefix::
>> +	If set, 'git diff' uses this source prefix. Defaults to 'a/'.
>> +
>> +diff.dstPrefix::
>> +	If set, 'git diff' uses this destination prefix. Defaults to 'b/'.
>> +
>>  diff.relative::
>>  	If set to 'true', 'git diff' does not show changes outside of the 
>> directory
>>  	and show pathnames relative to the current directory.
>> diff --git a/diff.c b/diff.c
>> index e50def45383e..108c1875775d 100644
>> --- a/diff.c
>> +++ b/diff.c
>> @@ -62,6 +62,8 @@ static const char *diff_order_file_cfg;
>>  int diff_auto_refresh_index = 1;
>>  static int diff_mnemonic_prefix;
>>  static int diff_no_prefix;
>> +static const char *diff_src_prefix = "a/";
>> +static const char *diff_dst_prefix = "b/";
>>  static int diff_relative;
>>  static int diff_stat_name_width;
>>  static int diff_stat_graph_width;
>> @@ -408,6 +410,12 @@ int git_diff_ui_config(const char *var, const 
>> char *value,
>>  		diff_no_prefix = git_config_bool(var, value);
>>  		return 0;
>>  	}
>> +	if (!strcmp(var, "diff.srcprefix")) {
>> +		return git_config_string(&diff_src_prefix, var, value);
>> +	}
>> +	if (!strcmp(var, "diff.dstprefix")) {
>> +		return git_config_string(&diff_dst_prefix, var, value);
>> +	}
>>  	if (!strcmp(var, "diff.relative")) {
>>  		diff_relative = git_config_bool(var, value);
>>  		return 0;
>> @@ -3425,8 +3433,8 @@ void diff_set_noprefix(struct diff_options 
>> *options)
>> 
>>  void diff_set_default_prefix(struct diff_options *options)
>>  {
>> -	options->a_prefix = "a/";
>> -	options->b_prefix = "b/";
>> +	options->a_prefix = diff_src_prefix;
>> +	options->b_prefix = diff_dst_prefix;
>>  }
>> 
>>  struct userdiff_driver *get_textconv(struct repository *r,
>> @@ -5362,6 +5370,8 @@ static int diff_opt_default_prefix(const struct
>> option *opt,
>> 
>>  	BUG_ON_OPT_NEG(unset);
>>  	BUG_ON_OPT_ARG(optarg);
>> +	diff_src_prefix = "a/";
>> +	diff_dst_prefix = "b/";
>>  	diff_set_default_prefix(options);
>>  	return 0;
>>  }
>> diff --git a/t/t4013-diff-various.sh b/t/t4013-diff-various.sh
>> index 1e3b2dbea484..e75f9f7d4cb2 100755
>> --- a/t/t4013-diff-various.sh
>> +++ b/t/t4013-diff-various.sh
>> @@ -663,6 +663,41 @@ test_expect_success 'diff --default-prefix
>> overrides diff.mnemonicprefix' '
>>  	check_prefix actual a/file0 b/file0
>>  '
>> 
>> +test_expect_success 'diff respects diff.srcprefix' '
>> +	git -c diff.srcprefix=x/ diff >actual &&
>> +	check_prefix actual x/file0 b/file0
>> +'
>> +
>> +test_expect_success 'diff respects diff.dstprefix' '
>> +	git -c diff.dstprefix=y/ diff >actual &&
>> +	check_prefix actual a/file0 y/file0
>> +'
>> +
>> +test_expect_success 'diff --src-prefix overrides diff.srcprefix' '
>> +	git -c diff.srcprefix=z/ diff --src-prefix=z/ >actual &&
>> +	check_prefix actual z/file0 b/file0
>> +'
>> +
>> +test_expect_success 'diff --dst-prefix overrides diff.dstprefix' '
>> +	git -c diff.dstprefix=y/ diff --dst-prefix=z/ >actual &&
>> +	check_prefix actual a/file0 z/file0
>> +'
>> +
>> +test_expect_success 'diff src/dstprefix ignored with diff.noprefix' '
>> +	git -c diff.dstprefix=y/ -c diff.srcprefix=x/ -c diff.noprefix diff 
>> >actual &&
>> +	check_prefix actual file0 file0
>> +'
>> +
>> +test_expect_success 'diff src/dstprefix ignored with 
>> diff.mnemonicprefix' '
>> +	git -c diff.dstprefix=x/ -c diff.srcprefix=y/ -c diff.mnemonicprefix
>> diff >actual &&
>> +	check_prefix actual i/file0 w/file0
>> +'
>> +
>> +test_expect_success 'diff src/dstprefix ignored with 
>> --default-prefix' '
>> +	git -c diff.dstprefix=x/ -c diff.srcprefix=y/ diff --default-prefix 
>> >actual &&
>> +	check_prefix actual a/file0 b/file0
>> +'
>> +
>>  test_expect_success 'diff --no-renames cannot be abbreviated' '
>>  	test_expect_code 129 git diff --no-rename >actual 2>error &&
>>  	test_must_be_empty actual &&

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

[Phillip Wood]

Hi Peter

On 12/03/2024 23:15, Peter Hutterer wrote:
> 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 v2;
> - doc: change to camelcase diff.srcPrefix/diff.dstPrefix for
>    consistency with diff.mnemonicPrefix and most other options
> - git diff --default-prefix forces a/ and b/ regardless of configured
>    prefix, see the 'diff_opt_default_prefix' hunk in the patch below.
> 
> The latter may be slightly controversial but: there are scripts out
> there that rely on the a/ and b/ prefix (came across one last night).
> 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.

Best Wishes

Phillip

>   Documentation/config/diff.txt |  6 ++++++
>   diff.c                        | 14 ++++++++++++--
>   t/t4013-diff-various.sh       | 35 +++++++++++++++++++++++++++++++++++
>   3 files changed, 53 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/config/diff.txt b/Documentation/config/diff.txt
> index 6c7e09a1ef5e..afc23d7723b6 100644
> --- a/Documentation/config/diff.txt
> +++ b/Documentation/config/diff.txt
> @@ -111,6 +111,12 @@ diff.mnemonicPrefix::
>   diff.noprefix::
>   	If set, 'git diff' does not show any source or destination prefix.
>   
> +diff.srcPrefix::
> +	If set, 'git diff' uses this source prefix. Defaults to 'a/'.
> +
> +diff.dstPrefix::
> +	If set, 'git diff' uses this destination prefix. Defaults to 'b/'.
> +
>   diff.relative::
>   	If set to 'true', 'git diff' does not show changes outside of the directory
>   	and show pathnames relative to the current directory.
> diff --git a/diff.c b/diff.c
> index e50def45383e..108c1875775d 100644
> --- a/diff.c
> +++ b/diff.c
> @@ -62,6 +62,8 @@ static const char *diff_order_file_cfg;
>   int diff_auto_refresh_index = 1;
>   static int diff_mnemonic_prefix;
>   static int diff_no_prefix;
> +static const char *diff_src_prefix = "a/";
> +static const char *diff_dst_prefix = "b/";
>   static int diff_relative;
>   static int diff_stat_name_width;
>   static int diff_stat_graph_width;
> @@ -408,6 +410,12 @@ int git_diff_ui_config(const char *var, const char *value,
>   		diff_no_prefix = git_config_bool(var, value);
>   		return 0;
>   	}
> +	if (!strcmp(var, "diff.srcprefix")) {
> +		return git_config_string(&diff_src_prefix, var, value);
> +	}
> +	if (!strcmp(var, "diff.dstprefix")) {
> +		return git_config_string(&diff_dst_prefix, var, value);
> +	}
>   	if (!strcmp(var, "diff.relative")) {
>   		diff_relative = git_config_bool(var, value);
>   		return 0;
> @@ -3425,8 +3433,8 @@ void diff_set_noprefix(struct diff_options *options)
>   
>   void diff_set_default_prefix(struct diff_options *options)
>   {
> -	options->a_prefix = "a/";
> -	options->b_prefix = "b/";
> +	options->a_prefix = diff_src_prefix;
> +	options->b_prefix = diff_dst_prefix;
>   }
>   
>   struct userdiff_driver *get_textconv(struct repository *r,
> @@ -5362,6 +5370,8 @@ static int diff_opt_default_prefix(const struct option *opt,
>   
>   	BUG_ON_OPT_NEG(unset);
>   	BUG_ON_OPT_ARG(optarg);
> +	diff_src_prefix = "a/";
> +	diff_dst_prefix = "b/";
>   	diff_set_default_prefix(options);
>   	return 0;
>   }
> diff --git a/t/t4013-diff-various.sh b/t/t4013-diff-various.sh
> index 1e3b2dbea484..e75f9f7d4cb2 100755
> --- a/t/t4013-diff-various.sh
> +++ b/t/t4013-diff-various.sh
> @@ -663,6 +663,41 @@ test_expect_success 'diff --default-prefix overrides diff.mnemonicprefix' '
>   	check_prefix actual a/file0 b/file0
>   '
>   
> +test_expect_success 'diff respects diff.srcprefix' '
> +	git -c diff.srcprefix=x/ diff >actual &&
> +	check_prefix actual x/file0 b/file0
> +'
> +
> +test_expect_success 'diff respects diff.dstprefix' '
> +	git -c diff.dstprefix=y/ diff >actual &&
> +	check_prefix actual a/file0 y/file0
> +'
> +
> +test_expect_success 'diff --src-prefix overrides diff.srcprefix' '
> +	git -c diff.srcprefix=z/ diff --src-prefix=z/ >actual &&
> +	check_prefix actual z/file0 b/file0
> +'
> +
> +test_expect_success 'diff --dst-prefix overrides diff.dstprefix' '
> +	git -c diff.dstprefix=y/ diff --dst-prefix=z/ >actual &&
> +	check_prefix actual a/file0 z/file0
> +'
> +
> +test_expect_success 'diff src/dstprefix ignored with diff.noprefix' '
> +	git -c diff.dstprefix=y/ -c diff.srcprefix=x/ -c diff.noprefix diff >actual &&
> +	check_prefix actual file0 file0
> +'
> +
> +test_expect_success 'diff src/dstprefix ignored with diff.mnemonicprefix' '
> +	git -c diff.dstprefix=x/ -c diff.srcprefix=y/ -c diff.mnemonicprefix diff >actual &&
> +	check_prefix actual i/file0 w/file0
> +'
> +
> +test_expect_success 'diff src/dstprefix ignored with --default-prefix' '
> +	git -c diff.dstprefix=x/ -c diff.srcprefix=y/ diff --default-prefix >actual &&
> +	check_prefix actual a/file0 b/file0
> +'
> +
>   test_expect_success 'diff --no-renames cannot be abbreviated' '
>   	test_expect_code 129 git diff --no-rename >actual 2>error &&
>   	test_must_be_empty actual &&

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

[Phillip Wood]

On 13/03/2024 02:15, Dragan Simic wrote:
> Hello Peter,
> 
> On 2024-03-13 00:15, Peter Hutterer wrote:
>> Allow the default prefixes "a/" and "b/" to be tweaked by the
>> diff.srcprefix and diff.dstprefix configuration variables.
> 
> The only thing that's left is to update the patch description
> to use camel case. :)

If that's the only change that's required I don't think we should be 
asking for a re-roll. We do place a high value on good commit messages 
in this project but I don't think it is reasonable to require a re-roll 
for a purely aesthetic change.

Best Wishes

Phillip

>> Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
>> ---
>> Changes to v2;
>> - doc: change to camelcase diff.srcPrefix/diff.dstPrefix for
>>   consistency with diff.mnemonicPrefix and most other options
>> - git diff --default-prefix forces a/ and b/ regardless of configured
>>   prefix, see the 'diff_opt_default_prefix' hunk in the patch below.
>>
>> The latter may be slightly controversial but: there are scripts out
>> there that rely on the a/ and b/ prefix (came across one last night).
>> 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.
>>
>>  Documentation/config/diff.txt |  6 ++++++
>>  diff.c                        | 14 ++++++++++++--
>>  t/t4013-diff-various.sh       | 35 +++++++++++++++++++++++++++++++++++
>>  3 files changed, 53 insertions(+), 2 deletions(-)
>>
>> diff --git a/Documentation/config/diff.txt 
>> b/Documentation/config/diff.txt
>> index 6c7e09a1ef5e..afc23d7723b6 100644
>> --- a/Documentation/config/diff.txt
>> +++ b/Documentation/config/diff.txt
>> @@ -111,6 +111,12 @@ diff.mnemonicPrefix::
>>  diff.noprefix::
>>      If set, 'git diff' does not show any source or destination prefix.
>>
>> +diff.srcPrefix::
>> +    If set, 'git diff' uses this source prefix. Defaults to 'a/'.
>> +
>> +diff.dstPrefix::
>> +    If set, 'git diff' uses this destination prefix. Defaults to 'b/'.
>> +
>>  diff.relative::
>>      If set to 'true', 'git diff' does not show changes outside of the 
>> directory
>>      and show pathnames relative to the current directory.
>> diff --git a/diff.c b/diff.c
>> index e50def45383e..108c1875775d 100644
>> --- a/diff.c
>> +++ b/diff.c
>> @@ -62,6 +62,8 @@ static const char *diff_order_file_cfg;
>>  int diff_auto_refresh_index = 1;
>>  static int diff_mnemonic_prefix;
>>  static int diff_no_prefix;
>> +static const char *diff_src_prefix = "a/";
>> +static const char *diff_dst_prefix = "b/";
>>  static int diff_relative;
>>  static int diff_stat_name_width;
>>  static int diff_stat_graph_width;
>> @@ -408,6 +410,12 @@ int git_diff_ui_config(const char *var, const 
>> char *value,
>>          diff_no_prefix = git_config_bool(var, value);
>>          return 0;
>>      }
>> +    if (!strcmp(var, "diff.srcprefix")) {
>> +        return git_config_string(&diff_src_prefix, var, value);
>> +    }
>> +    if (!strcmp(var, "diff.dstprefix")) {
>> +        return git_config_string(&diff_dst_prefix, var, value);
>> +    }
>>      if (!strcmp(var, "diff.relative")) {
>>          diff_relative = git_config_bool(var, value);
>>          return 0;
>> @@ -3425,8 +3433,8 @@ void diff_set_noprefix(struct diff_options 
>> *options)
>>
>>  void diff_set_default_prefix(struct diff_options *options)
>>  {
>> -    options->a_prefix = "a/";
>> -    options->b_prefix = "b/";
>> +    options->a_prefix = diff_src_prefix;
>> +    options->b_prefix = diff_dst_prefix;
>>  }
>>
>>  struct userdiff_driver *get_textconv(struct repository *r,
>> @@ -5362,6 +5370,8 @@ static int diff_opt_default_prefix(const struct
>> option *opt,
>>
>>      BUG_ON_OPT_NEG(unset);
>>      BUG_ON_OPT_ARG(optarg);
>> +    diff_src_prefix = "a/";
>> +    diff_dst_prefix = "b/";
>>      diff_set_default_prefix(options);
>>      return 0;
>>  }
>> diff --git a/t/t4013-diff-various.sh b/t/t4013-diff-various.sh
>> index 1e3b2dbea484..e75f9f7d4cb2 100755
>> --- a/t/t4013-diff-various.sh
>> +++ b/t/t4013-diff-various.sh
>> @@ -663,6 +663,41 @@ test_expect_success 'diff --default-prefix
>> overrides diff.mnemonicprefix' '
>>      check_prefix actual a/file0 b/file0
>>  '
>>
>> +test_expect_success 'diff respects diff.srcprefix' '
>> +    git -c diff.srcprefix=x/ diff >actual &&
>> +    check_prefix actual x/file0 b/file0
>> +'
>> +
>> +test_expect_success 'diff respects diff.dstprefix' '
>> +    git -c diff.dstprefix=y/ diff >actual &&
>> +    check_prefix actual a/file0 y/file0
>> +'
>> +
>> +test_expect_success 'diff --src-prefix overrides diff.srcprefix' '
>> +    git -c diff.srcprefix=z/ diff --src-prefix=z/ >actual &&
>> +    check_prefix actual z/file0 b/file0
>> +'
>> +
>> +test_expect_success 'diff --dst-prefix overrides diff.dstprefix' '
>> +    git -c diff.dstprefix=y/ diff --dst-prefix=z/ >actual &&
>> +    check_prefix actual a/file0 z/file0
>> +'
>> +
>> +test_expect_success 'diff src/dstprefix ignored with diff.noprefix' '
>> +    git -c diff.dstprefix=y/ -c diff.srcprefix=x/ -c diff.noprefix 
>> diff >actual &&
>> +    check_prefix actual file0 file0
>> +'
>> +
>> +test_expect_success 'diff src/dstprefix ignored with 
>> diff.mnemonicprefix' '
>> +    git -c diff.dstprefix=x/ -c diff.srcprefix=y/ -c diff.mnemonicprefix
>> diff >actual &&
>> +    check_prefix actual i/file0 w/file0
>> +'
>> +
>> +test_expect_success 'diff src/dstprefix ignored with --default-prefix' '
>> +    git -c diff.dstprefix=x/ -c diff.srcprefix=y/ diff 
>> --default-prefix >actual &&
>> +    check_prefix actual a/file0 b/file0
>> +'
>> +
>>  test_expect_success 'diff --no-renames cannot be abbreviated' '
>>      test_expect_code 129 git diff --no-rename >actual 2>error &&
>>      test_must_be_empty actual &&
> 

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

[Dragan Simic]

Hello Phillip,

On 2024-03-13 16:06, Phillip Wood wrote:
> On 13/03/2024 02:15, Dragan Simic wrote:
>> Hello Peter,
>> 
>> On 2024-03-13 00:15, Peter Hutterer wrote:
>>> Allow the default prefixes "a/" and "b/" to be tweaked by the
>>> diff.srcprefix and diff.dstprefix configuration variables.
>> 
>> The only thing that's left is to update the patch description
>> to use camel case. :)
> 
> If that's the only change that's required I don't think we should be
> asking for a re-roll. We do place a high value on good commit messages
> in this project but I don't think it is reasonable to require a
> re-roll for a purely aesthetic change.

Well, I required nothing, I just noted it.  Such a small change can
also be performed by Junio while applying the patch.


>>> Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
>>> ---
>>> Changes to v2;
>>> - doc: change to camelcase diff.srcPrefix/diff.dstPrefix for
>>>   consistency with diff.mnemonicPrefix and most other options
>>> - git diff --default-prefix forces a/ and b/ regardless of configured
>>>   prefix, see the 'diff_opt_default_prefix' hunk in the patch below.
>>> 
>>> The latter may be slightly controversial but: there are scripts out
>>> there that rely on the a/ and b/ prefix (came across one last night).
>>> 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.
>>> 
>>>  Documentation/config/diff.txt |  6 ++++++
>>>  diff.c                        | 14 ++++++++++++--
>>>  t/t4013-diff-various.sh       | 35 
>>> +++++++++++++++++++++++++++++++++++
>>>  3 files changed, 53 insertions(+), 2 deletions(-)
>>> 
>>> diff --git a/Documentation/config/diff.txt 
>>> b/Documentation/config/diff.txt
>>> index 6c7e09a1ef5e..afc23d7723b6 100644
>>> --- a/Documentation/config/diff.txt
>>> +++ b/Documentation/config/diff.txt
>>> @@ -111,6 +111,12 @@ diff.mnemonicPrefix::
>>>  diff.noprefix::
>>>      If set, 'git diff' does not show any source or destination 
>>> prefix.
>>> 
>>> +diff.srcPrefix::
>>> +    If set, 'git diff' uses this source prefix. Defaults to 'a/'.
>>> +
>>> +diff.dstPrefix::
>>> +    If set, 'git diff' uses this destination prefix. Defaults to 
>>> 'b/'.
>>> +
>>>  diff.relative::
>>>      If set to 'true', 'git diff' does not show changes outside of 
>>> the directory
>>>      and show pathnames relative to the current directory.
>>> diff --git a/diff.c b/diff.c
>>> index e50def45383e..108c1875775d 100644
>>> --- a/diff.c
>>> +++ b/diff.c
>>> @@ -62,6 +62,8 @@ static const char *diff_order_file_cfg;
>>>  int diff_auto_refresh_index = 1;
>>>  static int diff_mnemonic_prefix;
>>>  static int diff_no_prefix;
>>> +static const char *diff_src_prefix = "a/";
>>> +static const char *diff_dst_prefix = "b/";
>>>  static int diff_relative;
>>>  static int diff_stat_name_width;
>>>  static int diff_stat_graph_width;
>>> @@ -408,6 +410,12 @@ int git_diff_ui_config(const char *var, const 
>>> char *value,
>>>          diff_no_prefix = git_config_bool(var, value);
>>>          return 0;
>>>      }
>>> +    if (!strcmp(var, "diff.srcprefix")) {
>>> +        return git_config_string(&diff_src_prefix, var, value);
>>> +    }
>>> +    if (!strcmp(var, "diff.dstprefix")) {
>>> +        return git_config_string(&diff_dst_prefix, var, value);
>>> +    }
>>>      if (!strcmp(var, "diff.relative")) {
>>>          diff_relative = git_config_bool(var, value);
>>>          return 0;
>>> @@ -3425,8 +3433,8 @@ void diff_set_noprefix(struct diff_options 
>>> *options)
>>> 
>>>  void diff_set_default_prefix(struct diff_options *options)
>>>  {
>>> -    options->a_prefix = "a/";
>>> -    options->b_prefix = "b/";
>>> +    options->a_prefix = diff_src_prefix;
>>> +    options->b_prefix = diff_dst_prefix;
>>>  }
>>> 
>>>  struct userdiff_driver *get_textconv(struct repository *r,
>>> @@ -5362,6 +5370,8 @@ static int diff_opt_default_prefix(const struct
>>> option *opt,
>>> 
>>>      BUG_ON_OPT_NEG(unset);
>>>      BUG_ON_OPT_ARG(optarg);
>>> +    diff_src_prefix = "a/";
>>> +    diff_dst_prefix = "b/";
>>>      diff_set_default_prefix(options);
>>>      return 0;
>>>  }
>>> diff --git a/t/t4013-diff-various.sh b/t/t4013-diff-various.sh
>>> index 1e3b2dbea484..e75f9f7d4cb2 100755
>>> --- a/t/t4013-diff-various.sh
>>> +++ b/t/t4013-diff-various.sh
>>> @@ -663,6 +663,41 @@ test_expect_success 'diff --default-prefix
>>> overrides diff.mnemonicprefix' '
>>>      check_prefix actual a/file0 b/file0
>>>  '
>>> 
>>> +test_expect_success 'diff respects diff.srcprefix' '
>>> +    git -c diff.srcprefix=x/ diff >actual &&
>>> +    check_prefix actual x/file0 b/file0
>>> +'
>>> +
>>> +test_expect_success 'diff respects diff.dstprefix' '
>>> +    git -c diff.dstprefix=y/ diff >actual &&
>>> +    check_prefix actual a/file0 y/file0
>>> +'
>>> +
>>> +test_expect_success 'diff --src-prefix overrides diff.srcprefix' '
>>> +    git -c diff.srcprefix=z/ diff --src-prefix=z/ >actual &&
>>> +    check_prefix actual z/file0 b/file0
>>> +'
>>> +
>>> +test_expect_success 'diff --dst-prefix overrides diff.dstprefix' '
>>> +    git -c diff.dstprefix=y/ diff --dst-prefix=z/ >actual &&
>>> +    check_prefix actual a/file0 z/file0
>>> +'
>>> +
>>> +test_expect_success 'diff src/dstprefix ignored with diff.noprefix' 
>>> '
>>> +    git -c diff.dstprefix=y/ -c diff.srcprefix=x/ -c diff.noprefix 
>>> diff >actual &&
>>> +    check_prefix actual file0 file0
>>> +'
>>> +
>>> +test_expect_success 'diff src/dstprefix ignored with 
>>> diff.mnemonicprefix' '
>>> +    git -c diff.dstprefix=x/ -c diff.srcprefix=y/ -c 
>>> diff.mnemonicprefix
>>> diff >actual &&
>>> +    check_prefix actual i/file0 w/file0
>>> +'
>>> +
>>> +test_expect_success 'diff src/dstprefix ignored with 
>>> --default-prefix' '
>>> +    git -c diff.dstprefix=x/ -c diff.srcprefix=y/ diff 
>>> --default-prefix >actual &&
>>> +    check_prefix actual a/file0 b/file0
>>> +'
>>> +
>>>  test_expect_success 'diff --no-renames cannot be abbreviated' '
>>>      test_expect_code 129 git diff --no-rename >actual 2>error &&
>>>      test_must_be_empty actual &&
>> 

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

[Junio C Hamano]

Dragan Simic <dsimic@manjaro.org> writes:

> Well, I required nothing, I just noted it.  Such a small change can
> also be performed by Junio while applying the patch.

Doing so is trivial.  Having to remember doing so when there are
many other patches in flight is a burden.  Don't put things on my
plate when you do not have to.

Thanks.

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

[Dragan Simic]

On 2024-03-13 16:24, Junio C Hamano wrote:
> Dragan Simic <dsimic@manjaro.org> writes:
> 
>> Well, I required nothing, I just noted it.  Such a small change can
>> also be performed by Junio while applying the patch.
> 
> Doing so is trivial.  Having to remember doing so when there are
> many other patches in flight is a burden.  Don't put things on my
> plate when you do not have to.

You're right, there are more important things.  Sorry.

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

[Junio C Hamano]

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 ;-)

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`.
 
 --line-prefix=<prefix>::
 	Prepend an additional prefix to every line of output.

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

[Phillip Wood]

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.

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

[Junio C Hamano]

Phillip Wood <phillip.wood123@gmail.com> writes:

> mention that one as well or just say something like "This is designed
> to be used to override the configuration variables `diff.*Prefix`.".

Excellent.

Thanks.  Peter, do you want to wrap things up with an updated patch
(no rush and no obligation to do so---we just want to know if it
will happen, in which case we will just wait, and otherwise somebody
else will do that)?

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

[Dragan Simic]

On 2024-03-13 16:29, Junio C Hamano wrote:
>  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`.

How about this instead:

     Ignore any configuration variables that control source and
     destination prefixes, which includes `diff.noPrefix`, 
`diff.srcPrefix`
     and `diff.dstPrefix` (see `git-config`(1)), and use the default
     prefixes `a/` and `b/`.

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

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

[Peter Hutterer]

On Wed, Mar 13, 2024 at 10:55:50AM -0700, Junio C Hamano wrote:
> Phillip Wood <phillip.wood123@gmail.com> writes:
> 
> > mention that one as well or just say something like "This is designed
> > to be used to override the configuration variables `diff.*Prefix`.".
> 
> Excellent.
> 
> Thanks.  Peter, do you want to wrap things up with an updated patch
> (no rush and no obligation to do so---we just want to know if it
> will happen, in which case we will just wait, and otherwise somebody
> else will do that)?

Happy to send out another patch (tomorrow, assuming the discussion has
settled fully).

Cheers,
  Peter

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

[Peter Hutterer]

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 v3:
- fix capitalization in the commit message
- quotes changed to " in the diff.txt hunk (for consistency with
  diff.mnemonicPrefix)
- reword the diff-options.txt entry to be more explicit/definitive

Dragan: I used the lowercase `noprefix` spelling here to be consistent
with the current state of the tree, can you please include the fix for
this in your pending patch? Thanks.

 Documentation/config/diff.txt  |  6 ++++++
 Documentation/diff-options.txt |  5 +++--
 diff.c                         | 14 ++++++++++++--
 t/t4013-diff-various.sh        | 35 ++++++++++++++++++++++++++++++++++
 4 files changed, 56 insertions(+), 4 deletions(-)

diff --git a/Documentation/config/diff.txt b/Documentation/config/diff.txt
index 6c7e09a1ef5e..afc23d7723b6 100644
--- a/Documentation/config/diff.txt
+++ b/Documentation/config/diff.txt
@@ -111,6 +111,12 @@ diff.mnemonicPrefix::
 diff.noprefix::
 	If set, 'git diff' does not show any source or destination prefix.
 
+diff.srcPrefix::
+	If set, 'git diff' uses this source prefix. Defaults to "a/".
+
+diff.dstPrefix::
+	If set, 'git diff' uses this destination prefix. Defaults to "b/".
+
 diff.relative::
 	If set to 'true', 'git diff' does not show changes outside of the directory
 	and show pathnames relative to the current directory.
diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
index aaaff0d46f0c..0e9456957e37 100644
--- a/Documentation/diff-options.txt
+++ b/Documentation/diff-options.txt
@@ -865,8 +865,9 @@ endif::git-format-patch[]
 
 --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`.
+	This overrides configuration variables such as `diff.noprefix`,
+	`diff.srcPrefix`, `diff.dstPrefix`, and `diff.mnemonicPrefix`
+	(see `git-config`(1)).
 
 --line-prefix=<prefix>::
 	Prepend an additional prefix to every line of output.
diff --git a/diff.c b/diff.c
index e50def45383e..108c1875775d 100644
--- a/diff.c
+++ b/diff.c
@@ -62,6 +62,8 @@ static const char *diff_order_file_cfg;
 int diff_auto_refresh_index = 1;
 static int diff_mnemonic_prefix;
 static int diff_no_prefix;
+static const char *diff_src_prefix = "a/";
+static const char *diff_dst_prefix = "b/";
 static int diff_relative;
 static int diff_stat_name_width;
 static int diff_stat_graph_width;
@@ -408,6 +410,12 @@ int git_diff_ui_config(const char *var, const char *value,
 		diff_no_prefix = git_config_bool(var, value);
 		return 0;
 	}
+	if (!strcmp(var, "diff.srcprefix")) {
+		return git_config_string(&diff_src_prefix, var, value);
+	}
+	if (!strcmp(var, "diff.dstprefix")) {
+		return git_config_string(&diff_dst_prefix, var, value);
+	}
 	if (!strcmp(var, "diff.relative")) {
 		diff_relative = git_config_bool(var, value);
 		return 0;
@@ -3425,8 +3433,8 @@ void diff_set_noprefix(struct diff_options *options)
 
 void diff_set_default_prefix(struct diff_options *options)
 {
-	options->a_prefix = "a/";
-	options->b_prefix = "b/";
+	options->a_prefix = diff_src_prefix;
+	options->b_prefix = diff_dst_prefix;
 }
 
 struct userdiff_driver *get_textconv(struct repository *r,
@@ -5362,6 +5370,8 @@ static int diff_opt_default_prefix(const struct option *opt,
 
 	BUG_ON_OPT_NEG(unset);
 	BUG_ON_OPT_ARG(optarg);
+	diff_src_prefix = "a/";
+	diff_dst_prefix = "b/";
 	diff_set_default_prefix(options);
 	return 0;
 }
diff --git a/t/t4013-diff-various.sh b/t/t4013-diff-various.sh
index 1e3b2dbea484..e75f9f7d4cb2 100755
--- a/t/t4013-diff-various.sh
+++ b/t/t4013-diff-various.sh
@@ -663,6 +663,41 @@ test_expect_success 'diff --default-prefix overrides diff.mnemonicprefix' '
 	check_prefix actual a/file0 b/file0
 '
 
+test_expect_success 'diff respects diff.srcprefix' '
+	git -c diff.srcprefix=x/ diff >actual &&
+	check_prefix actual x/file0 b/file0
+'
+
+test_expect_success 'diff respects diff.dstprefix' '
+	git -c diff.dstprefix=y/ diff >actual &&
+	check_prefix actual a/file0 y/file0
+'
+
+test_expect_success 'diff --src-prefix overrides diff.srcprefix' '
+	git -c diff.srcprefix=z/ diff --src-prefix=z/ >actual &&
+	check_prefix actual z/file0 b/file0
+'
+
+test_expect_success 'diff --dst-prefix overrides diff.dstprefix' '
+	git -c diff.dstprefix=y/ diff --dst-prefix=z/ >actual &&
+	check_prefix actual a/file0 z/file0
+'
+
+test_expect_success 'diff src/dstprefix ignored with diff.noprefix' '
+	git -c diff.dstprefix=y/ -c diff.srcprefix=x/ -c diff.noprefix diff >actual &&
+	check_prefix actual file0 file0
+'
+
+test_expect_success 'diff src/dstprefix ignored with diff.mnemonicprefix' '
+	git -c diff.dstprefix=x/ -c diff.srcprefix=y/ -c diff.mnemonicprefix diff >actual &&
+	check_prefix actual i/file0 w/file0
+'
+
+test_expect_success 'diff src/dstprefix ignored with --default-prefix' '
+	git -c diff.dstprefix=x/ -c diff.srcprefix=y/ diff --default-prefix >actual &&
+	check_prefix actual a/file0 b/file0
+'
+
 test_expect_success 'diff --no-renames cannot be abbreviated' '
 	test_expect_code 129 git diff --no-rename >actual 2>error &&
 	test_must_be_empty actual &&
-- 
2.44.0

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

[Dragan Simic]

Hello Peter,

On 2024-03-15 02:03, Peter Hutterer wrote:
> 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>

Looking good to me, except for some of the included tests, please
see my notes below.

Additionally, perhaps the documentation for "--default-prefix"
could use some wording improvements, but I'll try to address that
in my separate patch(es) mentioned below.  Of course, I can send
my suggestions now, if you prefer it that way?

> ---
> Changes to v3:
> - fix capitalization in the commit message
> - quotes changed to " in the diff.txt hunk (for consistency with
>   diff.mnemonicPrefix)
> - reword the diff-options.txt entry to be more explicit/definitive
> 
> Dragan: I used the lowercase `noprefix` spelling here to be consistent
> with the current state of the tree, can you please include the fix for
> this in your pending patch? Thanks.

Sure, I'll cover it later, after your patch is accepted.  Thanks
for the notice.

>  Documentation/config/diff.txt  |  6 ++++++
>  Documentation/diff-options.txt |  5 +++--
>  diff.c                         | 14 ++++++++++++--
>  t/t4013-diff-various.sh        | 35 ++++++++++++++++++++++++++++++++++
>  4 files changed, 56 insertions(+), 4 deletions(-)
> 
> diff --git a/Documentation/config/diff.txt 
> b/Documentation/config/diff.txt
> index 6c7e09a1ef5e..afc23d7723b6 100644
> --- a/Documentation/config/diff.txt
> +++ b/Documentation/config/diff.txt
> @@ -111,6 +111,12 @@ diff.mnemonicPrefix::
>  diff.noprefix::
>  	If set, 'git diff' does not show any source or destination prefix.
> 
> +diff.srcPrefix::
> +	If set, 'git diff' uses this source prefix. Defaults to "a/".
> +
> +diff.dstPrefix::
> +	If set, 'git diff' uses this destination prefix. Defaults to "b/".
> +
>  diff.relative::
>  	If set to 'true', 'git diff' does not show changes outside of the 
> directory
>  	and show pathnames relative to the current directory.
> diff --git a/Documentation/diff-options.txt 
> b/Documentation/diff-options.txt
> index aaaff0d46f0c..0e9456957e37 100644
> --- a/Documentation/diff-options.txt
> +++ b/Documentation/diff-options.txt
> @@ -865,8 +865,9 @@ endif::git-format-patch[]
> 
>  --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`.
> +	This overrides configuration variables such as `diff.noprefix`,
> +	`diff.srcPrefix`, `diff.dstPrefix`, and `diff.mnemonicPrefix`
> +	(see `git-config`(1)).
> 
>  --line-prefix=<prefix>::
>  	Prepend an additional prefix to every line of output.
> diff --git a/diff.c b/diff.c
> index e50def45383e..108c1875775d 100644
> --- a/diff.c
> +++ b/diff.c
> @@ -62,6 +62,8 @@ static const char *diff_order_file_cfg;
>  int diff_auto_refresh_index = 1;
>  static int diff_mnemonic_prefix;
>  static int diff_no_prefix;
> +static const char *diff_src_prefix = "a/";
> +static const char *diff_dst_prefix = "b/";
>  static int diff_relative;
>  static int diff_stat_name_width;
>  static int diff_stat_graph_width;
> @@ -408,6 +410,12 @@ int git_diff_ui_config(const char *var, const char 
> *value,
>  		diff_no_prefix = git_config_bool(var, value);
>  		return 0;
>  	}
> +	if (!strcmp(var, "diff.srcprefix")) {
> +		return git_config_string(&diff_src_prefix, var, value);
> +	}
> +	if (!strcmp(var, "diff.dstprefix")) {
> +		return git_config_string(&diff_dst_prefix, var, value);
> +	}
>  	if (!strcmp(var, "diff.relative")) {
>  		diff_relative = git_config_bool(var, value);
>  		return 0;
> @@ -3425,8 +3433,8 @@ void diff_set_noprefix(struct diff_options 
> *options)
> 
>  void diff_set_default_prefix(struct diff_options *options)
>  {
> -	options->a_prefix = "a/";
> -	options->b_prefix = "b/";
> +	options->a_prefix = diff_src_prefix;
> +	options->b_prefix = diff_dst_prefix;
>  }
> 
>  struct userdiff_driver *get_textconv(struct repository *r,
> @@ -5362,6 +5370,8 @@ static int diff_opt_default_prefix(const struct
> option *opt,
> 
>  	BUG_ON_OPT_NEG(unset);
>  	BUG_ON_OPT_ARG(optarg);
> +	diff_src_prefix = "a/";
> +	diff_dst_prefix = "b/";
>  	diff_set_default_prefix(options);
>  	return 0;
>  }
> diff --git a/t/t4013-diff-various.sh b/t/t4013-diff-various.sh
> index 1e3b2dbea484..e75f9f7d4cb2 100755
> --- a/t/t4013-diff-various.sh
> +++ b/t/t4013-diff-various.sh
> @@ -663,6 +663,41 @@ test_expect_success 'diff --default-prefix
> overrides diff.mnemonicprefix' '
>  	check_prefix actual a/file0 b/file0
>  '
> 
> +test_expect_success 'diff respects diff.srcprefix' '
> +	git -c diff.srcprefix=x/ diff >actual &&
> +	check_prefix actual x/file0 b/file0
> +'
> +
> +test_expect_success 'diff respects diff.dstprefix' '
> +	git -c diff.dstprefix=y/ diff >actual &&
> +	check_prefix actual a/file0 y/file0
> +'
> +
> +test_expect_success 'diff --src-prefix overrides diff.srcprefix' '
> +	git -c diff.srcprefix=z/ diff --src-prefix=z/ >actual &&

Shouldn't this be

	git -c diff.srcprefix=e/ diff --src-prefix=z/ >actual &&

instead (or something else for diff.srcPrefix, perhaps "y/"), to 
actually
perform the check for overriding?

> +	check_prefix actual z/file0 b/file0
> +'
> +
> +test_expect_success 'diff --dst-prefix overrides diff.dstprefix' '
> +	git -c diff.dstprefix=y/ diff --dst-prefix=z/ >actual &&
> +	check_prefix actual a/file0 z/file0
> +'
> +
> +test_expect_success 'diff src/dstprefix ignored with diff.noprefix' '

Shouldn't this be

     test_expect_success 'diff.*prefix ignored with diff.noprefix' '

instead, to describe the test better?

> +	git -c diff.dstprefix=y/ -c diff.srcprefix=x/ -c diff.noprefix diff 
> >actual &&
> +	check_prefix actual file0 file0
> +'
> +
> +test_expect_success 'diff src/dstprefix ignored with 
> diff.mnemonicprefix' '

Shouldn't this be

     test_expect_success 'diff.*prefix ignored with diff.mnemonicprefix' 
'

instead, to describe the test better?

> +	git -c diff.dstprefix=x/ -c diff.srcprefix=y/ -c diff.mnemonicprefix
> diff >actual &&
> +	check_prefix actual i/file0 w/file0
> +'
> +
> +test_expect_success 'diff src/dstprefix ignored with --default-prefix' 
> '

Shouldn't this be

     test_expect_success 'diff.*prefix ignored with --default-prefix' '

instead, to describe the test better?

> +	git -c diff.dstprefix=x/ -c diff.srcprefix=y/ diff --default-prefix 
> >actual &&
> +	check_prefix actual a/file0 b/file0
> +'
> +
>  test_expect_success 'diff --no-renames cannot be abbreviated' '
>  	test_expect_code 129 git diff --no-rename >actual 2>error &&
>  	test_must_be_empty actual &&

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

[Peter Hutterer]

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
- reworded the description for the tests as suggested

Dragan: good catch on the test, thanks for that. I think for the
rewording of --default-prefix it might be faster if you reword it?
Otherwise we might keep spinning this one for a quite a bit longer :)

 Documentation/config/diff.txt  |  6 ++++++
 Documentation/diff-options.txt |  5 +++--
 diff.c                         | 14 ++++++++++++--
 t/t4013-diff-various.sh        | 35 ++++++++++++++++++++++++++++++++++
 4 files changed, 56 insertions(+), 4 deletions(-)

diff --git a/Documentation/config/diff.txt b/Documentation/config/diff.txt
index 6c7e09a1ef5e..fea89291c675 100644
--- a/Documentation/config/diff.txt
+++ b/Documentation/config/diff.txt
@@ -111,6 +111,12 @@ diff.mnemonicPrefix::
 diff.noprefix::
 	If set, 'git diff' does not show any source or destination prefix.
 
+diff.srcPrefix::
+	If set, 'git diff' uses this source prefix. Defaults to "a/".
+
+diff.dstPrefix::
+	If set, 'git diff' uses this destination prefix. Defaults to "b/".
+
 diff.relative::
 	If set to 'true', 'git diff' does not show changes outside of the directory
 	and show pathnames relative to the current directory.
diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
index aaaff0d46f0c..0e9456957e37 100644
--- a/Documentation/diff-options.txt
+++ b/Documentation/diff-options.txt
@@ -865,8 +865,9 @@ endif::git-format-patch[]
 
 --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`.
+	This overrides configuration variables such as `diff.noprefix`,
+	`diff.srcPrefix`, `diff.dstPrefix`, and `diff.mnemonicPrefix`
+	(see `git-config`(1)).
 
 --line-prefix=<prefix>::
 	Prepend an additional prefix to every line of output.
diff --git a/diff.c b/diff.c
index e50def45383e..108c1875775d 100644
--- a/diff.c
+++ b/diff.c
@@ -62,6 +62,8 @@ static const char *diff_order_file_cfg;
 int diff_auto_refresh_index = 1;
 static int diff_mnemonic_prefix;
 static int diff_no_prefix;
+static const char *diff_src_prefix = "a/";
+static const char *diff_dst_prefix = "b/";
 static int diff_relative;
 static int diff_stat_name_width;
 static int diff_stat_graph_width;
@@ -408,6 +410,12 @@ int git_diff_ui_config(const char *var, const char *value,
 		diff_no_prefix = git_config_bool(var, value);
 		return 0;
 	}
+	if (!strcmp(var, "diff.srcprefix")) {
+		return git_config_string(&diff_src_prefix, var, value);
+	}
+	if (!strcmp(var, "diff.dstprefix")) {
+		return git_config_string(&diff_dst_prefix, var, value);
+	}
 	if (!strcmp(var, "diff.relative")) {
 		diff_relative = git_config_bool(var, value);
 		return 0;
@@ -3425,8 +3433,8 @@ void diff_set_noprefix(struct diff_options *options)
 
 void diff_set_default_prefix(struct diff_options *options)
 {
-	options->a_prefix = "a/";
-	options->b_prefix = "b/";
+	options->a_prefix = diff_src_prefix;
+	options->b_prefix = diff_dst_prefix;
 }
 
 struct userdiff_driver *get_textconv(struct repository *r,
@@ -5362,6 +5370,8 @@ static int diff_opt_default_prefix(const struct option *opt,
 
 	BUG_ON_OPT_NEG(unset);
 	BUG_ON_OPT_ARG(optarg);
+	diff_src_prefix = "a/";
+	diff_dst_prefix = "b/";
 	diff_set_default_prefix(options);
 	return 0;
 }
diff --git a/t/t4013-diff-various.sh b/t/t4013-diff-various.sh
index 1e3b2dbea484..6bf69888f258 100755
--- a/t/t4013-diff-various.sh
+++ b/t/t4013-diff-various.sh
@@ -663,6 +663,41 @@ test_expect_success 'diff --default-prefix overrides diff.mnemonicprefix' '
 	check_prefix actual a/file0 b/file0
 '
 
+test_expect_success 'diff respects diff.srcprefix' '
+	git -c diff.srcprefix=x/ diff >actual &&
+	check_prefix actual x/file0 b/file0
+'
+
+test_expect_success 'diff respects diff.dstprefix' '
+	git -c diff.dstprefix=y/ diff >actual &&
+	check_prefix actual a/file0 y/file0
+'
+
+test_expect_success 'diff --src-prefix overrides diff.srcprefix' '
+	git -c diff.srcprefix=y/ diff --src-prefix=z/ >actual &&
+	check_prefix actual z/file0 b/file0
+'
+
+test_expect_success 'diff --dst-prefix overrides diff.dstprefix' '
+	git -c diff.dstprefix=y/ diff --dst-prefix=z/ >actual &&
+	check_prefix actual a/file0 z/file0
+'
+
+test_expect_success 'diff.*prefix ignored with diff.noprefix' '
+	git -c diff.dstprefix=y/ -c diff.srcprefix=x/ -c diff.noprefix diff >actual &&
+	check_prefix actual file0 file0
+'
+
+test_expect_success 'diff.*prefix ignored with diff.mnemonicprefix' '
+	git -c diff.dstprefix=x/ -c diff.srcprefix=y/ -c diff.mnemonicprefix diff >actual &&
+	check_prefix actual i/file0 w/file0
+'
+
+test_expect_success 'diff.*prefix ignored with --default-prefix' '
+	git -c diff.dstprefix=x/ -c diff.srcprefix=y/ diff --default-prefix >actual &&
+	check_prefix actual a/file0 b/file0
+'
+
 test_expect_success 'diff --no-renames cannot be abbreviated' '
 	test_expect_code 129 git diff --no-rename >actual 2>error &&
 	test_must_be_empty actual &&
-- 
2.44.0

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

[Dragan Simic]

On 2024-03-15 06:54, Peter Hutterer wrote:
> 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>

Looking good to me!

Reviewed-by: Dragan Simic <dsimic@manjaro.org>

> ---
> Changes to v4 (as pointed out by Dragan):
> - copy/paste-o fixed in the dstprefix test
> - reworded the description for the tests as suggested
> 
> Dragan: good catch on the test, thanks for that.

Thanks for applying the suggestions in v5.

> I think for the
> rewording of --default-prefix it might be faster if you reword it?
> Otherwise we might keep spinning this one for a quite a bit longer :)

Agreed, I had exactly the same in mind. :)

>  Documentation/config/diff.txt  |  6 ++++++
>  Documentation/diff-options.txt |  5 +++--
>  diff.c                         | 14 ++++++++++++--
>  t/t4013-diff-various.sh        | 35 ++++++++++++++++++++++++++++++++++
>  4 files changed, 56 insertions(+), 4 deletions(-)
> 
> diff --git a/Documentation/config/diff.txt 
> b/Documentation/config/diff.txt
> index 6c7e09a1ef5e..fea89291c675 100644
> --- a/Documentation/config/diff.txt
> +++ b/Documentation/config/diff.txt
> @@ -111,6 +111,12 @@ diff.mnemonicPrefix::
>  diff.noprefix::
>  	If set, 'git diff' does not show any source or destination prefix.
> 
> +diff.srcPrefix::
> +	If set, 'git diff' uses this source prefix. Defaults to "a/".
> +
> +diff.dstPrefix::
> +	If set, 'git diff' uses this destination prefix. Defaults to "b/".
> +
>  diff.relative::
>  	If set to 'true', 'git diff' does not show changes outside of the 
> directory
>  	and show pathnames relative to the current directory.
> diff --git a/Documentation/diff-options.txt 
> b/Documentation/diff-options.txt
> index aaaff0d46f0c..0e9456957e37 100644
> --- a/Documentation/diff-options.txt
> +++ b/Documentation/diff-options.txt
> @@ -865,8 +865,9 @@ endif::git-format-patch[]
> 
>  --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`.
> +	This overrides configuration variables such as `diff.noprefix`,
> +	`diff.srcPrefix`, `diff.dstPrefix`, and `diff.mnemonicPrefix`
> +	(see `git-config`(1)).
> 
>  --line-prefix=<prefix>::
>  	Prepend an additional prefix to every line of output.
> diff --git a/diff.c b/diff.c
> index e50def45383e..108c1875775d 100644
> --- a/diff.c
> +++ b/diff.c
> @@ -62,6 +62,8 @@ static const char *diff_order_file_cfg;
>  int diff_auto_refresh_index = 1;
>  static int diff_mnemonic_prefix;
>  static int diff_no_prefix;
> +static const char *diff_src_prefix = "a/";
> +static const char *diff_dst_prefix = "b/";
>  static int diff_relative;
>  static int diff_stat_name_width;
>  static int diff_stat_graph_width;
> @@ -408,6 +410,12 @@ int git_diff_ui_config(const char *var, const char 
> *value,
>  		diff_no_prefix = git_config_bool(var, value);
>  		return 0;
>  	}
> +	if (!strcmp(var, "diff.srcprefix")) {
> +		return git_config_string(&diff_src_prefix, var, value);
> +	}
> +	if (!strcmp(var, "diff.dstprefix")) {
> +		return git_config_string(&diff_dst_prefix, var, value);
> +	}
>  	if (!strcmp(var, "diff.relative")) {
>  		diff_relative = git_config_bool(var, value);
>  		return 0;
> @@ -3425,8 +3433,8 @@ void diff_set_noprefix(struct diff_options 
> *options)
> 
>  void diff_set_default_prefix(struct diff_options *options)
>  {
> -	options->a_prefix = "a/";
> -	options->b_prefix = "b/";
> +	options->a_prefix = diff_src_prefix;
> +	options->b_prefix = diff_dst_prefix;
>  }
> 
>  struct userdiff_driver *get_textconv(struct repository *r,
> @@ -5362,6 +5370,8 @@ static int diff_opt_default_prefix(const struct
> option *opt,
> 
>  	BUG_ON_OPT_NEG(unset);
>  	BUG_ON_OPT_ARG(optarg);
> +	diff_src_prefix = "a/";
> +	diff_dst_prefix = "b/";
>  	diff_set_default_prefix(options);
>  	return 0;
>  }
> diff --git a/t/t4013-diff-various.sh b/t/t4013-diff-various.sh
> index 1e3b2dbea484..6bf69888f258 100755
> --- a/t/t4013-diff-various.sh
> +++ b/t/t4013-diff-various.sh
> @@ -663,6 +663,41 @@ test_expect_success 'diff --default-prefix
> overrides diff.mnemonicprefix' '
>  	check_prefix actual a/file0 b/file0
>  '
> 
> +test_expect_success 'diff respects diff.srcprefix' '
> +	git -c diff.srcprefix=x/ diff >actual &&
> +	check_prefix actual x/file0 b/file0
> +'
> +
> +test_expect_success 'diff respects diff.dstprefix' '
> +	git -c diff.dstprefix=y/ diff >actual &&
> +	check_prefix actual a/file0 y/file0
> +'
> +
> +test_expect_success 'diff --src-prefix overrides diff.srcprefix' '
> +	git -c diff.srcprefix=y/ diff --src-prefix=z/ >actual &&
> +	check_prefix actual z/file0 b/file0
> +'
> +
> +test_expect_success 'diff --dst-prefix overrides diff.dstprefix' '
> +	git -c diff.dstprefix=y/ diff --dst-prefix=z/ >actual &&
> +	check_prefix actual a/file0 z/file0
> +'
> +
> +test_expect_success 'diff.*prefix ignored with diff.noprefix' '
> +	git -c diff.dstprefix=y/ -c diff.srcprefix=x/ -c diff.noprefix diff 
> >actual &&
> +	check_prefix actual file0 file0
> +'
> +
> +test_expect_success 'diff.*prefix ignored with diff.mnemonicprefix' '
> +	git -c diff.dstprefix=x/ -c diff.srcprefix=y/ -c diff.mnemonicprefix
> diff >actual &&
> +	check_prefix actual i/file0 w/file0
> +'
> +
> +test_expect_success 'diff.*prefix ignored with --default-prefix' '
> +	git -c diff.dstprefix=x/ -c diff.srcprefix=y/ diff --default-prefix 
> >actual &&
> +	check_prefix actual a/file0 b/file0
> +'
> +
>  test_expect_success 'diff --no-renames cannot be abbreviated' '
>  	test_expect_code 129 git diff --no-rename >actual 2>error &&
>  	test_must_be_empty actual &&

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

[Junio C Hamano]

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'

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

Thanks, both.

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

[Dragan Simic]

On 2024-03-15 18:00, Junio C Hamano wrote:
> 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.

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

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

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

So, that is what I did before I pushed out today's integration
result.  I however have an "after the dust settles" clean-up patch
on top (not committed yet), which I am sending out for review.

------- >8 -------------- >8 -------------- >8 --------
Subject: diff.*Prefix: use camelCase in the doc and test titles

We added documentation for diff.srcPrefix and diff.dstPrefix with
their names properly camelCased, but the diff.noPrefix is listed
there in all lowercase.  Also these configuration variables, both
existing ones and the {src,dst}Prefix we recently added, were
spelled in all lowercase in the tests in t4013.

Now we are done with the main change, clean these up.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---

 * If we were early in the review cycle, we would strongly prefer to
   do a "preliminary clean-up" followed by the main change, as the
   clean-up step would be much less controversial and can be queued
   earlier before the main change solidifies.  But at v5 the main
   change is more or less perfect, so it is not worth rerolling to
   split the clean-up changes into preliminary ones and change to
   the main patch.  So this is written as an "on top, after the dust
   settles" clean-up patch.

 Documentation/config/diff.txt |  2 +-
 t/t4013-diff-various.sh       | 48 +++++++++++++++++++++----------------------
 2 files changed, 25 insertions(+), 25 deletions(-)

diff --git c/Documentation/config/diff.txt w/Documentation/config/diff.txt
index fea89291c6..5ce7b91f1d 100644
--- c/Documentation/config/diff.txt
+++ w/Documentation/config/diff.txt
@@ -108,7 +108,7 @@ diff.mnemonicPrefix::
 `git diff --no-index a b`;;
 	compares two non-git things (1) and (2).
 
-diff.noprefix::
+diff.noPrefix::
 	If set, 'git diff' does not show any source or destination prefix.
 
 diff.srcPrefix::
diff --git c/t/t4013-diff-various.sh w/t/t4013-diff-various.sh
index cfb5ad3d8d..3855d68dbc 100755
--- c/t/t4013-diff-various.sh
+++ w/t/t4013-diff-various.sh
@@ -633,8 +633,8 @@ check_prefix () {
 	test_cmp expect actual.paths
 }
 
-test_expect_success 'diff-files does not respect diff.noprefix' '
-	git -c diff.noprefix diff-files -p >actual &&
+test_expect_success 'diff-files does not respect diff.noPrefix' '
+	git -c diff.noPrefix diff-files -p >actual &&
 	check_prefix actual a/file0 b/file0
 '
 
@@ -643,58 +643,58 @@ test_expect_success 'diff-files respects --no-prefix' '
 	check_prefix actual file0 file0
 '
 
-test_expect_success 'diff respects diff.noprefix' '
-	git -c diff.noprefix diff >actual &&
+test_expect_success 'diff respects diff.noPrefix' '
+	git -c diff.noPrefix diff >actual &&
 	check_prefix actual file0 file0
 '
 
-test_expect_success 'diff --default-prefix overrides diff.noprefix' '
-	git -c diff.noprefix diff --default-prefix >actual &&
+test_expect_success 'diff --default-prefix overrides diff.noPrefix' '
+	git -c diff.noPrefix diff --default-prefix >actual &&
 	check_prefix actual a/file0 b/file0
 '
 
-test_expect_success 'diff respects diff.mnemonicprefix' '
-	git -c diff.mnemonicprefix diff >actual &&
+test_expect_success 'diff respects diff.mnemonicPrefix' '
+	git -c diff.mnemonicPrefix diff >actual &&
 	check_prefix actual i/file0 w/file0
 '
 
-test_expect_success 'diff --default-prefix overrides diff.mnemonicprefix' '
-	git -c diff.mnemonicprefix diff --default-prefix >actual &&
+test_expect_success 'diff --default-prefix overrides diff.mnemonicPrefix' '
+	git -c diff.mnemonicPrefix diff --default-prefix >actual &&
 	check_prefix actual a/file0 b/file0
 '
 
-test_expect_success 'diff respects diff.srcprefix' '
-	git -c diff.srcprefix=x/ diff >actual &&
+test_expect_success 'diff respects diff.srcPrefix' '
+	git -c diff.srcPrefix=x/ diff >actual &&
 	check_prefix actual x/file0 b/file0
 '
 
-test_expect_success 'diff respects diff.dstprefix' '
-	git -c diff.dstprefix=y/ diff >actual &&
+test_expect_success 'diff respects diff.dstPrefix' '
+	git -c diff.dstPrefix=y/ diff >actual &&
 	check_prefix actual a/file0 y/file0
 '
 
-test_expect_success 'diff --src-prefix overrides diff.srcprefix' '
-	git -c diff.srcprefix=y/ diff --src-prefix=z/ >actual &&
+test_expect_success 'diff --src-prefix overrides diff.srcPrefix' '
+	git -c diff.srcPrefix=y/ diff --src-prefix=z/ >actual &&
 	check_prefix actual z/file0 b/file0
 '
 
-test_expect_success 'diff --dst-prefix overrides diff.dstprefix' '
-	git -c diff.dstprefix=y/ diff --dst-prefix=z/ >actual &&
+test_expect_success 'diff --dst-prefix overrides diff.dstPrefix' '
+	git -c diff.dstPrefix=y/ diff --dst-prefix=z/ >actual &&
 	check_prefix actual a/file0 z/file0
 '
 
-test_expect_success 'diff.{src,dst}prefix ignored with diff.noprefix' '
-	git -c diff.dstprefix=y/ -c diff.srcprefix=x/ -c diff.noprefix diff >actual &&
+test_expect_success 'diff.{src,dst}Prefix ignored with diff.noPrefix' '
+	git -c diff.dstPrefix=y/ -c diff.srcPrefix=x/ -c diff.noPrefix diff >actual &&
 	check_prefix actual file0 file0
 '
 
-test_expect_success 'diff.{src,dst}prefix ignored with diff.mnemonicprefix' '
-	git -c diff.dstprefix=x/ -c diff.srcprefix=y/ -c diff.mnemonicprefix diff >actual &&
+test_expect_success 'diff.{src,dst}Prefix ignored with diff.mnemonicPrefix' '
+	git -c diff.dstPrefix=x/ -c diff.srcPrefix=y/ -c diff.mnemonicPrefix diff >actual &&
 	check_prefix actual i/file0 w/file0
 '
 
-test_expect_success 'diff.{src,dst}prefix ignored with --default-prefix' '
-	git -c diff.dstprefix=x/ -c diff.srcprefix=y/ diff --default-prefix >actual &&
+test_expect_success 'diff.{src,dst}Prefix ignored with --default-prefix' '
+	git -c diff.dstPrefix=x/ -c diff.srcPrefix=y/ diff --default-prefix >actual &&
 	check_prefix actual a/file0 b/file0
 '
 

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

[Dragan Simic]

On 2024-03-16 06:57, Junio C Hamano wrote:
> Junio C Hamano <gitster@pobox.com> writes:
> 
>> I am tempted to queue v4 with the z/ -> y/ fix from this round,
>> without any other changes from v4 to v5.
> 
> So, that is what I did before I pushed out today's integration
> result.  I however have an "after the dust settles" clean-up patch
> on top (not committed yet), which I am sending out for review.

Looking great to me!  Thanks a lot for the final touches.

Reviewed-by: Dragan Simic <dsimic@manjaro.org>

> ------- >8 -------------- >8 -------------- >8 --------
> Subject: diff.*Prefix: use camelCase in the doc and test titles
> 
> We added documentation for diff.srcPrefix and diff.dstPrefix with
> their names properly camelCased, but the diff.noPrefix is listed
> there in all lowercase.  Also these configuration variables, both
> existing ones and the {src,dst}Prefix we recently added, were
> spelled in all lowercase in the tests in t4013.
> 
> Now we are done with the main change, clean these up.
> 
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
> 
>  * If we were early in the review cycle, we would strongly prefer to
>    do a "preliminary clean-up" followed by the main change, as the
>    clean-up step would be much less controversial and can be queued
>    earlier before the main change solidifies.  But at v5 the main
>    change is more or less perfect, so it is not worth rerolling to
>    split the clean-up changes into preliminary ones and change to
>    the main patch.  So this is written as an "on top, after the dust
>    settles" clean-up patch.
> 
>  Documentation/config/diff.txt |  2 +-
>  t/t4013-diff-various.sh       | 48 
> +++++++++++++++++++++----------------------
>  2 files changed, 25 insertions(+), 25 deletions(-)
> 
> diff --git c/Documentation/config/diff.txt 
> w/Documentation/config/diff.txt
> index fea89291c6..5ce7b91f1d 100644
> --- c/Documentation/config/diff.txt
> +++ w/Documentation/config/diff.txt
> @@ -108,7 +108,7 @@ diff.mnemonicPrefix::
>  `git diff --no-index a b`;;
>  	compares two non-git things (1) and (2).
> 
> -diff.noprefix::
> +diff.noPrefix::
>  	If set, 'git diff' does not show any source or destination prefix.
> 
>  diff.srcPrefix::
> diff --git c/t/t4013-diff-various.sh w/t/t4013-diff-various.sh
> index cfb5ad3d8d..3855d68dbc 100755
> --- c/t/t4013-diff-various.sh
> +++ w/t/t4013-diff-various.sh
> @@ -633,8 +633,8 @@ check_prefix () {
>  	test_cmp expect actual.paths
>  }
> 
> -test_expect_success 'diff-files does not respect diff.noprefix' '
> -	git -c diff.noprefix diff-files -p >actual &&
> +test_expect_success 'diff-files does not respect diff.noPrefix' '
> +	git -c diff.noPrefix diff-files -p >actual &&
>  	check_prefix actual a/file0 b/file0
>  '
> 
> @@ -643,58 +643,58 @@ test_expect_success 'diff-files respects 
> --no-prefix' '
>  	check_prefix actual file0 file0
>  '
> 
> -test_expect_success 'diff respects diff.noprefix' '
> -	git -c diff.noprefix diff >actual &&
> +test_expect_success 'diff respects diff.noPrefix' '
> +	git -c diff.noPrefix diff >actual &&
>  	check_prefix actual file0 file0
>  '
> 
> -test_expect_success 'diff --default-prefix overrides diff.noprefix' '
> -	git -c diff.noprefix diff --default-prefix >actual &&
> +test_expect_success 'diff --default-prefix overrides diff.noPrefix' '
> +	git -c diff.noPrefix diff --default-prefix >actual &&
>  	check_prefix actual a/file0 b/file0
>  '
> 
> -test_expect_success 'diff respects diff.mnemonicprefix' '
> -	git -c diff.mnemonicprefix diff >actual &&
> +test_expect_success 'diff respects diff.mnemonicPrefix' '
> +	git -c diff.mnemonicPrefix diff >actual &&
>  	check_prefix actual i/file0 w/file0
>  '
> 
> -test_expect_success 'diff --default-prefix overrides 
> diff.mnemonicprefix' '
> -	git -c diff.mnemonicprefix diff --default-prefix >actual &&
> +test_expect_success 'diff --default-prefix overrides 
> diff.mnemonicPrefix' '
> +	git -c diff.mnemonicPrefix diff --default-prefix >actual &&
>  	check_prefix actual a/file0 b/file0
>  '
> 
> -test_expect_success 'diff respects diff.srcprefix' '
> -	git -c diff.srcprefix=x/ diff >actual &&
> +test_expect_success 'diff respects diff.srcPrefix' '
> +	git -c diff.srcPrefix=x/ diff >actual &&
>  	check_prefix actual x/file0 b/file0
>  '
> 
> -test_expect_success 'diff respects diff.dstprefix' '
> -	git -c diff.dstprefix=y/ diff >actual &&
> +test_expect_success 'diff respects diff.dstPrefix' '
> +	git -c diff.dstPrefix=y/ diff >actual &&
>  	check_prefix actual a/file0 y/file0
>  '
> 
> -test_expect_success 'diff --src-prefix overrides diff.srcprefix' '
> -	git -c diff.srcprefix=y/ diff --src-prefix=z/ >actual &&
> +test_expect_success 'diff --src-prefix overrides diff.srcPrefix' '
> +	git -c diff.srcPrefix=y/ diff --src-prefix=z/ >actual &&
>  	check_prefix actual z/file0 b/file0
>  '
> 
> -test_expect_success 'diff --dst-prefix overrides diff.dstprefix' '
> -	git -c diff.dstprefix=y/ diff --dst-prefix=z/ >actual &&
> +test_expect_success 'diff --dst-prefix overrides diff.dstPrefix' '
> +	git -c diff.dstPrefix=y/ diff --dst-prefix=z/ >actual &&
>  	check_prefix actual a/file0 z/file0
>  '
> 
> -test_expect_success 'diff.{src,dst}prefix ignored with diff.noprefix' 
> '
> -	git -c diff.dstprefix=y/ -c diff.srcprefix=x/ -c diff.noprefix diff 
> >actual &&
> +test_expect_success 'diff.{src,dst}Prefix ignored with diff.noPrefix' 
> '
> +	git -c diff.dstPrefix=y/ -c diff.srcPrefix=x/ -c diff.noPrefix diff 
> >actual &&
>  	check_prefix actual file0 file0
>  '
> 
> -test_expect_success 'diff.{src,dst}prefix ignored with 
> diff.mnemonicprefix' '
> -	git -c diff.dstprefix=x/ -c diff.srcprefix=y/ -c diff.mnemonicprefix
> diff >actual &&
> +test_expect_success 'diff.{src,dst}Prefix ignored with 
> diff.mnemonicPrefix' '
> +	git -c diff.dstPrefix=x/ -c diff.srcPrefix=y/ -c diff.mnemonicPrefix
> diff >actual &&
>  	check_prefix actual i/file0 w/file0
>  '
> 
> -test_expect_success 'diff.{src,dst}prefix ignored with 
> --default-prefix' '
> -	git -c diff.dstprefix=x/ -c diff.srcprefix=y/ diff --default-prefix 
> >actual &&
> +test_expect_success 'diff.{src,dst}Prefix ignored with 
> --default-prefix' '
> +	git -c diff.dstPrefix=x/ -c diff.srcPrefix=y/ diff --default-prefix 
> >actual &&
>  	check_prefix actual a/file0 b/file0
>  '

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

[Peter Hutterer]

On Fri, Mar 15, 2024 at 10:57:22PM -0700, Junio C Hamano wrote:
> Junio C Hamano <gitster@pobox.com> writes:
> 
> > I am tempted to queue v4 with the z/ -> y/ fix from this round,
> > without any other changes from v4 to v5.
> 
> So, that is what I did before I pushed out today's integration
> result.  I however have an "after the dust settles" clean-up patch
> on top (not committed yet), which I am sending out for review.
> 
> ------- >8 -------------- >8 -------------- >8 --------
> Subject: diff.*Prefix: use camelCase in the doc and test titles
> 
> We added documentation for diff.srcPrefix and diff.dstPrefix with
> their names properly camelCased, but the diff.noPrefix is listed
> there in all lowercase.  Also these configuration variables, both
> existing ones and the {src,dst}Prefix we recently added, were
> spelled in all lowercase in the tests in t4013.
> 
> Now we are done with the main change, clean these up.
> 
> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>

And thanks for merging the patch!

Cheers,
  Peter

> ---
> 
>  * If we were early in the review cycle, we would strongly prefer to
>    do a "preliminary clean-up" followed by the main change, as the
>    clean-up step would be much less controversial and can be queued
>    earlier before the main change solidifies.  But at v5 the main
>    change is more or less perfect, so it is not worth rerolling to
>    split the clean-up changes into preliminary ones and change to
>    the main patch.  So this is written as an "on top, after the dust
>    settles" clean-up patch.
> 
>  Documentation/config/diff.txt |  2 +-
>  t/t4013-diff-various.sh       | 48 +++++++++++++++++++++----------------------
>  2 files changed, 25 insertions(+), 25 deletions(-)
> 
> diff --git c/Documentation/config/diff.txt w/Documentation/config/diff.txt
> index fea89291c6..5ce7b91f1d 100644
> --- c/Documentation/config/diff.txt
> +++ w/Documentation/config/diff.txt
> @@ -108,7 +108,7 @@ diff.mnemonicPrefix::
>  `git diff --no-index a b`;;
>  	compares two non-git things (1) and (2).
>  
> -diff.noprefix::
> +diff.noPrefix::
>  	If set, 'git diff' does not show any source or destination prefix.
>  
>  diff.srcPrefix::
> diff --git c/t/t4013-diff-various.sh w/t/t4013-diff-various.sh
> index cfb5ad3d8d..3855d68dbc 100755
> --- c/t/t4013-diff-various.sh
> +++ w/t/t4013-diff-various.sh
> @@ -633,8 +633,8 @@ check_prefix () {
>  	test_cmp expect actual.paths
>  }
>  
> -test_expect_success 'diff-files does not respect diff.noprefix' '
> -	git -c diff.noprefix diff-files -p >actual &&
> +test_expect_success 'diff-files does not respect diff.noPrefix' '
> +	git -c diff.noPrefix diff-files -p >actual &&
>  	check_prefix actual a/file0 b/file0
>  '
>  
> @@ -643,58 +643,58 @@ test_expect_success 'diff-files respects --no-prefix' '
>  	check_prefix actual file0 file0
>  '
>  
> -test_expect_success 'diff respects diff.noprefix' '
> -	git -c diff.noprefix diff >actual &&
> +test_expect_success 'diff respects diff.noPrefix' '
> +	git -c diff.noPrefix diff >actual &&
>  	check_prefix actual file0 file0
>  '
>  
> -test_expect_success 'diff --default-prefix overrides diff.noprefix' '
> -	git -c diff.noprefix diff --default-prefix >actual &&
> +test_expect_success 'diff --default-prefix overrides diff.noPrefix' '
> +	git -c diff.noPrefix diff --default-prefix >actual &&
>  	check_prefix actual a/file0 b/file0
>  '
>  
> -test_expect_success 'diff respects diff.mnemonicprefix' '
> -	git -c diff.mnemonicprefix diff >actual &&
> +test_expect_success 'diff respects diff.mnemonicPrefix' '
> +	git -c diff.mnemonicPrefix diff >actual &&
>  	check_prefix actual i/file0 w/file0
>  '
>  
> -test_expect_success 'diff --default-prefix overrides diff.mnemonicprefix' '
> -	git -c diff.mnemonicprefix diff --default-prefix >actual &&
> +test_expect_success 'diff --default-prefix overrides diff.mnemonicPrefix' '
> +	git -c diff.mnemonicPrefix diff --default-prefix >actual &&
>  	check_prefix actual a/file0 b/file0
>  '
>  
> -test_expect_success 'diff respects diff.srcprefix' '
> -	git -c diff.srcprefix=x/ diff >actual &&
> +test_expect_success 'diff respects diff.srcPrefix' '
> +	git -c diff.srcPrefix=x/ diff >actual &&
>  	check_prefix actual x/file0 b/file0
>  '
>  
> -test_expect_success 'diff respects diff.dstprefix' '
> -	git -c diff.dstprefix=y/ diff >actual &&
> +test_expect_success 'diff respects diff.dstPrefix' '
> +	git -c diff.dstPrefix=y/ diff >actual &&
>  	check_prefix actual a/file0 y/file0
>  '
>  
> -test_expect_success 'diff --src-prefix overrides diff.srcprefix' '
> -	git -c diff.srcprefix=y/ diff --src-prefix=z/ >actual &&
> +test_expect_success 'diff --src-prefix overrides diff.srcPrefix' '
> +	git -c diff.srcPrefix=y/ diff --src-prefix=z/ >actual &&
>  	check_prefix actual z/file0 b/file0
>  '
>  
> -test_expect_success 'diff --dst-prefix overrides diff.dstprefix' '
> -	git -c diff.dstprefix=y/ diff --dst-prefix=z/ >actual &&
> +test_expect_success 'diff --dst-prefix overrides diff.dstPrefix' '
> +	git -c diff.dstPrefix=y/ diff --dst-prefix=z/ >actual &&
>  	check_prefix actual a/file0 z/file0
>  '
>  
> -test_expect_success 'diff.{src,dst}prefix ignored with diff.noprefix' '
> -	git -c diff.dstprefix=y/ -c diff.srcprefix=x/ -c diff.noprefix diff >actual &&
> +test_expect_success 'diff.{src,dst}Prefix ignored with diff.noPrefix' '
> +	git -c diff.dstPrefix=y/ -c diff.srcPrefix=x/ -c diff.noPrefix diff >actual &&
>  	check_prefix actual file0 file0
>  '
>  
> -test_expect_success 'diff.{src,dst}prefix ignored with diff.mnemonicprefix' '
> -	git -c diff.dstprefix=x/ -c diff.srcprefix=y/ -c diff.mnemonicprefix diff >actual &&
> +test_expect_success 'diff.{src,dst}Prefix ignored with diff.mnemonicPrefix' '
> +	git -c diff.dstPrefix=x/ -c diff.srcPrefix=y/ -c diff.mnemonicPrefix diff >actual &&
>  	check_prefix actual i/file0 w/file0
>  '
>  
> -test_expect_success 'diff.{src,dst}prefix ignored with --default-prefix' '
> -	git -c diff.dstprefix=x/ -c diff.srcprefix=y/ diff --default-prefix >actual &&
> +test_expect_success 'diff.{src,dst}Prefix ignored with --default-prefix' '
> +	git -c diff.dstPrefix=x/ -c diff.srcPrefix=y/ diff --default-prefix >actual &&
>  	check_prefix actual a/file0 b/file0
>  '
>  

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

[Junio C Hamano]

Peter Hutterer <peter.hutterer@who-t.net> writes:

> On Fri, Mar 15, 2024 at 10:57:22PM -0700, Junio C Hamano wrote:
>> Junio C Hamano <gitster@pobox.com> writes:
>> 
>> > I am tempted to queue v4 with the z/ -> y/ fix from this round,
>> > without any other changes from v4 to v5.
>> 
>> So, that is what I did before I pushed out today's integration
>> result.  I however have an "after the dust settles" clean-up patch
>> on top (not committed yet), which I am sending out for review.
>> 
>> ------- >8 -------------- >8 -------------- >8 --------
>> Subject: diff.*Prefix: use camelCase in the doc and test titles
>> 
>> We added documentation for diff.srcPrefix and diff.dstPrefix with
>> their names properly camelCased, but the diff.noPrefix is listed
>> there in all lowercase.  Also these configuration variables, both
>> existing ones and the {src,dst}Prefix we recently added, were
>> spelled in all lowercase in the tests in t4013.
>> 
>> Now we are done with the main change, clean these up.
>> 
>> Signed-off-by: Junio C Hamano <gitster@pobox.com>
>
> Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>
>
> And thanks for merging the patch!
>
> Cheers,
>   Peter

Thanks.