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@xxxxxxxxx>
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 &&
