diff-highlight: avoid highlighting combined diffs
authorJeff King <peff@peff.net>
Wed, 31 Aug 2016 05:05:38 +0000 (01:05 -0400)
committerJunio C Hamano <gitster@pobox.com>
Wed, 31 Aug 2016 16:59:53 +0000 (09:59 -0700)
The algorithm in diff-highlight only understands how to look
at two sides of a diff; it cannot correctly handle combined
diffs with multiple preimages. Often highlighting does not
trigger at all for these diffs because the line counts do
not match up. E.g., if we see:

- ours
-theirs
++resolved

we would not bother highlighting; it naively looks like a
single line went away, and then a separate hunk added
another single line.

But of course there are exceptions. E.g., if the other side
deleted the line, we might see:

- ours
++resolved

which looks like we dropped " ours" and added "+resolved".
This is only a small highlighting glitch (we highlight the
space and the "+" along with the content), but it's also the
tip of the iceberg. Even if we learned to find the true
content here (by noticing we are in a 3-way combined diff
and marking _two_ characters from the front of the line as
uninteresting), there are other more complicated cases where
we really do need to handle a 3-way hunk.

Let's just punt for now; we can recognize combined diffs by
the presence of extra "@" symbols in the hunk header, and
treat them as non-diff content.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
contrib/diff-highlight/diff-highlight
contrib/diff-highlight/t/t9400-diff-highlight.sh
index 9280c88800942e748af254d62d84d781c9caca62..81bd8040e3cbd34a8b247f23a94eee891fc44296 100755 (executable)
@@ -36,7 +36,7 @@ $SIG{PIPE} = 'DEFAULT';
 while (<>) {
        if (!$in_hunk) {
                print;
-               $in_hunk = /^$GRAPH*$COLOR*\@/;
+               $in_hunk = /^$GRAPH*$COLOR*\@\@ /;
        }
        elsif (/^$GRAPH*$COLOR*-/) {
                push @removed, $_;
index b0fe7cc8aa294d0efae21400f034048666bce914..3b43dbed7488c5f4a5f05809725ffd0bcd7e61b5 100755 (executable)
@@ -256,4 +256,41 @@ test_expect_success 'diff-highlight works with the --graph option' '
        test_cmp graph.exp graph.act
 '
 
+# Most combined diffs won't meet diff-highlight's line-number filter. So we
+# create one here where one side drops a line and the other modifies it. That
+# should result in a diff like:
+#
+#    - modified content
+#    ++resolved content
+#
+# which naively looks like one side added "+resolved".
+test_expect_success 'diff-highlight ignores combined diffs' '
+       echo "content" >file &&
+       git add file &&
+       git commit -m base &&
+
+       >file &&
+       git commit -am master &&
+
+       git checkout -b other HEAD^ &&
+       echo "modified content" >file &&
+       git commit -am other &&
+
+       test_must_fail git merge master &&
+       echo "resolved content" >file &&
+       git commit -am resolved &&
+
+       cat >expect <<-\EOF &&
+       --- a/file
+       +++ b/file
+       @@@ -1,1 -1,0 +1,1 @@@
+       - modified content
+       ++resolved content
+       EOF
+
+       git show -c | "$DIFF_HIGHLIGHT" >actual.raw &&
+       sed -n "/^---/,\$p" <actual.raw >actual &&
+       test_cmp expect actual
+'
+
 test_done