git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH 4/5] diff-highlight: match multi-line hunks

From
Jeff King <peff@peff.net>
Date
Feb 13, 2012, 22:36 UTC
Message-ID
<20120213223636.GD19521@sigill.intra.peff.net>
In-Reply-To
<20120213222702.GA19393@sigill.intra.peff.net>

Currently we only bother highlighting single-line hunks. The rationale was that the purpose of highlighting is to point out small changes between two similar lines that are otherwise hard to see. However, that meant we missed similar cases where two lines were changed together, like:

   -foo(buf);
   -bar(buf);
   +foo(obj->buf);
   +bar(obj->buf);

Each of those changes is simple, and would benefit from highlighting (the "obj->" parts in this case).

This patch considers whole hunks at a time. For now, we consider only the case where the hunk has the same number of removed and added lines, and assume that the lines from each segment correspond one-to-one. While this is just a heuristic, in practice it seems to generate sensible results (especially because we now omit highlighting on completely-changed lines, so when our heuristic is wrong, we tend to avoid highlighting at all).

Based on an original idea and implementation by Michał Kiedrowicz.

Signed-off-by: Jeff King <peff@peff.net>
---
Same attribution statement applies as to patch 2 (in fact, patches 1 and
3 could be attributed to you, too).

This version has the missing documentation fixes. The implementation is a little different than yours. I rearranged the parsing in a manner that was a little more obvious to me, and I pulled out the "don't highlight if the number of lines don't match" case into its own conditional, which makes it more obvious where to work if somebody wants to try doing something fancier.

 contrib/diff-highlight/README         |   16 ++++---
 contrib/diff-highlight/diff-highlight |   70 ++++++++++++++++++++-------------
 2 files changed, 52 insertions(+), 34 deletions(-)
diff --git a/contrib/diff-highlight/README b/contrib/diff-highlight/README
index 1b7b6df..4a58579 100644
--- a/contrib/diff-highlight/README
+++ b/contrib/diff-highlight/README
@@ -14,13 +14,15 @@ Instead, this script post-processes the line-oriented diff, finds pairs
 of lines, and highlights the differing segments.  It's currently very
 simple and stupid about doing these tasks. In particular:
 
-  1. It will only highlight a pair of lines if they are the only two
-     lines in a hunk.  It could instead try to match up "before" and
-     "after" lines for a given hunk into pairs of similar lines.
-     However, this may end up visually distracting, as the paired
-     lines would have other highlighted lines in between them. And in
-     practice, the lines which most need attention called to their
-     small, hard-to-see changes are touching only a single line.
+  1. It will only highlight hunks in which the number of removed and
+     added lines is the same, and it will pair lines within the hunk by
+     position (so the first removed line is compared to the first added
+     line, and so forth). This is simple and tends to work well in
+     practice. More complex changes don't highlight well, so we tend to
+     exclude them due to the "same number of removed and added lines"
+     restriction. Or even if we do try to highlight them, they end up
+     not highlighting because of our "don't highlight if the whole line
+     would be highlighted" rule.
 
   2. It will find the common prefix and suffix of two lines, and
      consider everything in the middle to be "different". It could
diff --git a/contrib/diff-highlight/diff-highlight b/contrib/diff-highlight/diff-highlight
index 279d211..c4404d4 100755
--- a/contrib/diff-highlight/diff-highlight
+++ b/contrib/diff-highlight/diff-highlight
@@ -10,23 +10,28 @@ my $UNHIGHLIGHT = "\x1b[27m";
 my $COLOR = qr/\x1b\[[0-9;]*m/;
 my $BORING = qr/$COLOR|\s/;
 
-my @window;
+my @removed;
+my @added;
+my $in_hunk;
 
 while (<>) {
-	# We highlight only single-line changes, so we need
-	# a 4-line window to make a decision on whether
-	# to highlight.
-	push @window, $_;
-	next if @window < 4;
-	if ($window[0] =~ /^$COLOR*(\@| )/ &&
-	    $window[1] =~ /^$COLOR*-/ &&
-	    $window[2] =~ /^$COLOR*\+/ &&
-	    $window[3] !~ /^$COLOR*\+/) {
-		print shift @window;
-		show_hunk(shift @window, shift @window);
+	if (!$in_hunk) {
+		print;
+		$in_hunk = /^$COLOR*\@/;
+	}
+	elsif (/^$COLOR*-/) {
+		push @removed, $_;
+	}
+	elsif (/^$COLOR*\+/) {
+		push @added, $_;
 	}
 	else {
-		print shift @window;
+		show_hunk(\@removed, \@added);
+		@removed = ();
+		@added = ();
+
+		print;
+		$in_hunk = /^$COLOR*[\@ ]/;
 	}
 
 	# Most of the time there is enough output to keep things streaming,
@@ -42,26 +47,37 @@ while (<>) {
 	}
 }
 
-# Special case a single-line hunk at the end of file.
-if (@window == 3 &&
-    $window[0] =~ /^$COLOR*(\@| )/ &&
-    $window[1] =~ /^$COLOR*-/ &&
-    $window[2] =~ /^$COLOR*\+/) {
-	print shift @window;
-	show_hunk(shift @window, shift @window);
-}
-
-# And then flush any remaining lines.
-while (@window) {
-	print shift @window;
-}
+# Flush any queued hunk (this can happen when there is no trailing context in
+# the final diff of the input).
+show_hunk(\@removed, \@added);
 
 exit 0;
 
 sub show_hunk {
 	my ($a, $b) = @_;
 
-	print highlight_pair($a, $b);
+	# If one side is empty, then there is nothing to compare or highlight.
+	if (!@$a || !@$b) {
+		print @$a, @$b;
+		return;
+	}
+
+	# If we have mismatched numbers of lines on each side, we could try to
+	# be clever and match up similar lines. But for now we are simple and
+	# stupid, and only handle multi-line hunks that remove and add the same
+	# number of lines.
+	if (@$a != @$b) {
+		print @$a, @$b;
+		return;
+	}
+
+	my @queue;
+	for (my $i = 0; $i < @$a; $i++) {
+		my ($rm, $add) = highlight_pair($a->[$i], $b->[$i]);
+		print $rm;
+		push @queue, $add;
+	}
+	print @queue;
 }
 
 sub highlight_pair {

-- 
1.7.8.4.17.g2df81
Previous: Jeff KingNext: Jeff King
Message 38 of 67 in “gitweb: Highlight interesting parts of diff”
  1. 0/8 gitweb: Highlight interesting parts of diffMichał Kiedrowicz, Feb 10, 2012
  2. 1/8 gitweb: Extract print_sidebyside_diff_lines()Michał Kiedrowicz, Feb 10, 2012
  3. Jakub NarebskiFeb 11, 2012
  4. Michał KiedrowiczFeb 11, 2012
  5. 2/8 gitweb: Use print_diff_chunk() for both side-by-side and inline diffsMichał Kiedrowicz, Feb 10, 2012
  6. Jakub NarebskiFeb 11, 2012
  7. Michał KiedrowiczFeb 11, 2012
  8. Michał KiedrowiczFeb 25, 2012
  9. 3/8 gitweb: Move HTML-formatting diff line back to process_diff_line()Michał Kiedrowicz, Feb 10, 2012
  10. Jakub NarebskiFeb 11, 2012
  11. 4/8 gitweb: Push formatting diff lines to print_diff_chunk()Michał Kiedrowicz, Feb 10, 2012
  12. Jakub NarebskiFeb 11, 2012
  13. Michał KiedrowiczFeb 11, 2012
  14. Michał KiedrowiczFeb 11, 2012
  15. 5/8 gitweb: Format diff lines just before printingMichał Kiedrowicz, Feb 10, 2012
  16. Jakub NarebskiFeb 11, 2012
  17. Michał KiedrowiczFeb 11, 2012
  18. 6/8 gitweb: Highlight interesting parts of diffMichał Kiedrowicz, Feb 10, 2012
  19. Jakub NarebskiFeb 10, 2012
  20. Michał KiedrowiczFeb 10, 2012
  21. Jakub NarebskiFeb 10, 2012
  22. Michał KiedrowiczFeb 10, 2012
  23. Jakub NarebskiFeb 10, 2012
  24. Jeff KingFeb 10, 2012
  25. Michal KiedrowiczFeb 14, 2012
  26. Junio C HamanoFeb 14, 2012
  27. Jeff KingFeb 14, 2012
  28. Jeff KingFeb 10, 2012
  29. Michał KiedrowiczFeb 10, 2012
  30. Jeff KingFeb 10, 2012
  31. Michał KiedrowiczFeb 10, 2012
  32. diff-highlight: Work for multiline changes tooMichał Kiedrowicz, Feb 10, 2012
  33. Jeff KingFeb 13, 2012
  34. 1/5 diff-highlight: make perl strict and warnings fatalJeff King, Feb 13, 2012
  35. 2/5 diff-highlight: don't highlight whole linesJeff King, Feb 13, 2012
  36. Michal KiedrowiczFeb 14, 2012
  37. 3/5 diff-highlight: refactor to prepare for multi-line hunksJeff King, Feb 13, 2012
  38. 4/5 diff-highlight: match multi-line hunksJeff King, Feb 13, 2012
  39. 5/5 diff-highlight: document some non-optimal casesJeff King, Feb 13, 2012
  40. Michal KiedrowiczFeb 14, 2012
  41. Junio C HamanoFeb 14, 2012
  42. Jeff KingFeb 14, 2012
  43. Junio C HamanoFeb 14, 2012
  44. Jeff KingFeb 14, 2012
  45. Michal KiedrowiczFeb 14, 2012
  46. Jakub NarebskiFeb 10, 2012
  47. Jakub NarebskiFeb 11, 2012
  48. Jakub NarebskiFeb 12, 2012
  49. Michal KiedrowiczFeb 13, 2012
  50. Jakub NarebskiFeb 13, 2012
  51. Michał KiedrowiczFeb 13, 2012
  52. Michal KiedrowiczFeb 13, 2012
  53. Jakub NarebskiFeb 13, 2012
  54. Michał KiedrowiczFeb 13, 2012
  55. Jakub NarebskiFeb 14, 2012
  56. Michał KiedrowiczFeb 14, 2012
  57. Jeff KingFeb 14, 2012
  58. Michał KiedrowiczFeb 14, 2012
  59. Jeff KingFeb 14, 2012
  60. 7/8 gitweb: Use different colors to present marked changesMichał Kiedrowicz, Feb 10, 2012
  61. Jakub NarebskiFeb 12, 2012
  62. Michal KiedrowiczFeb 13, 2012
  63. 8/8 gitweb: Highlight combined diffsMichał Kiedrowicz, Feb 10, 2012
  64. Jakub NarebskiFeb 12, 2012
  65. Michal KiedrowiczFeb 13, 2012
  66. Jakub NarebskiFeb 11, 2012
  67. Michał KiedrowiczFeb 11, 2012

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.