Hi John
On 05/02/2023 03:46, John Cai via GitGitGadget wrote:
From: John Cai <johncai86@xxxxxxxxx>
It can be useful to specify diff algorithms per file type. For example,
one may want to use the minimal diff algorithm for .json files, another
for .c files, etc.
Have you got any examples of why this is useful? I find myself
occasionally changing the algorithm when the default gives a sub-optimal
diff but I've not really noticed any pattern with respect to file types.
Teach the diff machinery to check attributes for a diff algorithm.
Enforce precedence by favoring the command line option, then looking at
attributes, then finally the config.
To enforce precedence order, set the `xdl_opts_command_line` member
during options pasing to indicate the diff algorithm was set via command
line args.
I've only commented on the tests as it looks like Eric and had a careful
look at the code
diff --git a/t/lib-diff-alternative.sh b/t/lib-diff-alternative.sh
index 8d1e408bb58..630c98ea65a 100644
--- a/t/lib-diff-alternative.sh
+++ b/t/lib-diff-alternative.sh
@@ -107,8 +107,27 @@ EOF
STRATEGY=$1
+ test_expect_success "$STRATEGY diff from attributes" '
+ echo "file* diff-algorithm=$STRATEGY" >.gitattributes &&
+ test_must_fail git diff --no-index file1 file2 > output &&
+ test_cmp expect output
+ '
+
test_expect_success "$STRATEGY diff" '
- test_must_fail git diff --no-index "--$STRATEGY" file1 file2 > output &&
+ test_must_fail git diff --no-index "--diff-algorithm=$STRATEGY" file1 file2 > output &&
+ test_cmp expect output
+ '
+
+ test_expect_success "$STRATEGY diff command line precedence before attributes" '
+ echo "file* diff-algorithm=meyers" >.gitattributes &&
+ test_must_fail git diff --no-index "--diff-algorithm=$STRATEGY" file1 file2 > output &&
+ test_cmp expect output
+ '
+
+ test_expect_success "$STRATEGY diff attributes precedence before config" '
+ git config diff.algorithm default &&
+ echo "file* diff-algorithm=$STRATEGY" >.gitattributes &&
+ test_must_fail git diff --no-index "--diff-algorithm=$STRATEGY" file1 file2 > output &&
This test passes with or without the code changes in this patch. I think
you need to drop --diff-algorithm=$STRATEGY from the diff command.
test_cmp expect output
'
@@ -166,5 +185,11 @@ EOF
test_must_fail git diff --no-index "--$STRATEGY" uniq1 uniq2 > output &&
test_cmp expect output
'
+
+ test_expect_success "$STRATEGY diff from attributes" '
+ echo "file* diff-algorithm=$STRATEGY" >.gitattributes &&
+ test_must_fail git diff --no-index uniq1 uniq2 > output &&
+ test_cmp expect output
This test also passes with or without the code changes in this patch. It
is the same as the first test added above but with files that give the
same diff irrespective of the algorithm chosen so I don't think it is
doing anything useful. Unless I've missed something it should be dropped.
Best Wishes
Phillip
+ '
}
