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

[PATCH v4 7/8] gitweb: Highlight interesting parts of diff

From
Michał Kiedrowicz <michal.kiedrowicz@gmail.com>
Date
Apr 11, 2012, 21:18 UTC
Message-ID
<1334179124-14258-8-git-send-email-michal.kiedrowicz@gmail.com>
In-Reply-To
<1334179124-14258-1-git-send-email-michal.kiedrowicz@gmail.com>

Reading diff output is sometimes very hard, even if it's colored, especially if lines differ only in few characters. This is often true when a commit fixes a typo or renames some variables or functions.

This commit teaches gitweb to highlight characters that are different between old and new line with a light green/red background. This should work in the similar manner as in Trac or GitHub.

The algorithm that compares lines is based on contrib/diff-highlight. Basically, it works by determining common prefix/suffix of corresponding lines and highlightning only the middle part of lines. For more information, see contrib/diff-highlight/README.

Combined diffs are not supported but a following commit will change it.

Since we need to pass esc_html()'ed or esc_html_hl_regions()'ed lines to format_diff_lines(), so it was taught to accept preformatted lines passed as a reference.

Signed-off-by: Michał Kiedrowicz <michal.kiedrowicz@gmail.com>
Acked-by: Jakub Narębski <jnareb@gmail.com>
---
 gitweb/gitweb.perl       |  107 ++++++++++++++++++++++++++++++++++++++++-----
 gitweb/static/gitweb.css |    8 +++
 2 files changed, 103 insertions(+), 12 deletions(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 390774e..d5b3f04 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -2430,19 +2430,25 @@ sub format_cc_diff_chunk_header {
 }
 
 # process patch (diff) line (not to be used for diff headers),
-# returning HTML-formatted (but not wrapped) line
+# returning HTML-formatted (but not wrapped) line.
+# If the line is passed as a reference, it is treated as HTML and not
+# esc_html()'ed.
 sub format_diff_line {
 	my ($line, $diff_class, $from, $to) = @_;
 
-	chomp $line;
-	$line = untabify($line);
-
-	if ($from && $to && $line =~ m/^\@{2} /) {
-		$line = format_unidiff_chunk_header($line, $from, $to);
-	} elsif ($from && $to && $line =~ m/^\@{3}/) {
-		$line = format_cc_diff_chunk_header($line, $from, $to);
+	if (ref($line)) {
+		$line = $$line;
 	} else {
-		$line = esc_html($line, -nbsp=>1);
+		chomp $line;
+		$line = untabify($line);
+
+		if ($from && $to && $line =~ m/^\@{2} /) {
+			$line = format_unidiff_chunk_header($line, $from, $to);
+		} elsif ($from && $to && $line =~ m/^\@{3}/) {
+			$line = format_cc_diff_chunk_header($line, $from, $to);
+		} else {
+			$line = esc_html($line, -nbsp=>1);
+		}
 	}
 
 	my $diff_classes = "diff";
@@ -5055,10 +5061,89 @@ sub print_inline_diff_lines {
 	print @$ctx, @$rem, @$add;
 }
 
+# Format removed and added line, mark changed part and HTML-format them.
+# Implementation is based on contrib/diff-highlight
+sub format_rem_add_lines_pair {
+	my ($rem, $add) = @_;
+
+	# We need to untabify lines before split()'ing them;
+	# otherwise offsets would be invalid.
+	chomp $rem;
+	chomp $add;
+	$rem = untabify($rem);
+	$add = untabify($add);
+
+	my @rem = split(//, $rem);
+	my @add = split(//, $add);
+	my ($esc_rem, $esc_add);
+	# Ignore +/- character, thus $prefix_len is set to 1.
+	my ($prefix_len, $suffix_len) = (1, 0);
+	my ($prefix_has_nonspace, $suffix_has_nonspace);
+
+	my $shorter = (@rem < @add) ? @rem : @add;
+	while ($prefix_len < $shorter) {
+		last if ($rem[$prefix_len] ne $add[$prefix_len]);
+
+		$prefix_has_nonspace = 1 if ($rem[$prefix_len] !~ /\s/);
+		$prefix_len++;
+	}
+
+	while ($prefix_len + $suffix_len < $shorter) {
+		last if ($rem[-1 - $suffix_len] ne $add[-1 - $suffix_len]);
+
+		$suffix_has_nonspace = 1 if ($rem[-1 - $suffix_len] !~ /\s/);
+		$suffix_len++;
+	}
+
+	# Mark lines that are different from each other, but have some common
+	# part that isn't whitespace.  If lines are completely different, don't
+	# mark them because that would make output unreadable, especially if
+	# diff consists of multiple lines.
+	if ($prefix_has_nonspace || $suffix_has_nonspace) {
+		$esc_rem = esc_html_hl_regions($rem, 'marked',
+		        [$prefix_len, @rem - $suffix_len], -nbsp=>1);
+		$esc_add = esc_html_hl_regions($add, 'marked',
+		        [$prefix_len, @add - $suffix_len], -nbsp=>1);
+	} else {
+		$esc_rem = esc_html($rem, -nbsp=>1);
+		$esc_add = esc_html($add, -nbsp=>1);
+	}
+
+	return format_diff_line(\$esc_rem, 'rem'),
+	       format_diff_line(\$esc_add, 'add');
+}
+
+# HTML-format diff context, removed and added lines.
+sub format_ctx_rem_add_lines {
+	my ($ctx, $rem, $add, $is_combined) = @_;
+	my (@new_ctx, @new_rem, @new_add);
+
+	# Highlight if every removed line has a corresponding added line.
+	# Combined diffs are not supported at this moment.
+	if (!$is_combined && @$add > 0 && @$add == @$rem) {
+		for (my $i = 0; $i < @$add; $i++) {
+			my ($line_rem, $line_add) = format_rem_add_lines_pair(
+			        $rem->[$i], $add->[$i]);
+			push @new_rem, $line_rem;
+			push @new_add, $line_add;
+		}
+	} else {
+		@new_rem = map { format_diff_line($_, 'rem') } @$rem;
+		@new_add = map { format_diff_line($_, 'add') } @$add;
+	}
+
+	@new_ctx = map { format_diff_line($_, 'ctx') } @$ctx;
+
+	return (\@new_ctx, \@new_rem, \@new_add);
+}
+
 # Print context lines and then rem/add lines.
 sub print_diff_lines {
 	my ($ctx, $rem, $add, $diff_style, $is_combined) = @_;
 
+	($ctx, $rem, $add) = format_ctx_rem_add_lines($ctx, $rem, $add,
+	        $is_combined);
+
 	if ($diff_style eq 'sidebyside' && !$is_combined) {
 		print_sidebyside_diff_lines($ctx, $rem, $add);
 	} else {
@@ -5090,11 +5175,9 @@ sub print_diff_chunk {
 	foreach my $line_info (@chunk) {
 		my ($class, $line) = @$line_info;
 
-		$line = format_diff_line($line, $class, $from, $to);
-
 		# print chunk headers
 		if ($class && $class eq 'chunk_header') {
-			print $line;
+			print format_diff_line($line, $class, $from, $to);
 			next;
 		}
 
diff --git a/gitweb/static/gitweb.css b/gitweb/static/gitweb.css
index c530355..cb86d2d 100644
--- a/gitweb/static/gitweb.css
+++ b/gitweb/static/gitweb.css
@@ -438,6 +438,10 @@ div.diff.add {
 	color: #008800;
 }
 
+div.diff.add span.marked {
+	background-color: #aaffaa;
+}
+
 div.diff.from_file a.path,
 div.diff.from_file {
 	color: #aa0000;
@@ -447,6 +451,10 @@ div.diff.rem {
 	color: #cc0000;
 }
 
+div.diff.rem span.marked {
+	background-color: #ffaaaa;
+}
+
 div.diff.chunk_header a,
 div.diff.chunk_header {
 	color: #990099;
-- 
1.7.8.4
Previous: Michał KiedrowiczNext: Michał Kiedrowicz
Message 8 of 10 in “Highlight interesting parts of diff”
  1. 0/8 Highlight interesting parts of diffMichał Kiedrowicz, Apr 11, 2012
  2. 1/8 gitweb: Use descriptive names in esc_html_hl_regions()Michał Kiedrowicz, Apr 11, 2012
  3. 2/8 gitweb: esc_html_hl_regions(): Don't create empty <span> elementsMichał Kiedrowicz, Apr 11, 2012
  4. 3/8 gitweb: Pass esc_html_hl_regions() options to esc_html()Michał Kiedrowicz, Apr 11, 2012
  5. 4/8 gitweb: Extract print_sidebyside_diff_lines()Michał Kiedrowicz, Apr 11, 2012
  6. 5/8 gitweb: Use print_diff_chunk() for both side-by-side and inline diffsMichał Kiedrowicz, Apr 11, 2012
  7. 6/8 gitweb: Push formatting diff lines to print_diff_chunk()Michał Kiedrowicz, Apr 11, 2012
  8. 7/8 gitweb: Highlight interesting parts of diffMichał Kiedrowicz, Apr 11, 2012
  9. 8/8 gitweb: Refinement highlightning in combined diffsMichał Kiedrowicz, Apr 11, 2012
  10. Junio C HamanoApr 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.