[PATCH v4 8/8] gitweb: Refinement highlightning in combined diffs

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

The highlightning of combined diffs is currently disabled. This is because output from a combined diff is much harder to highlight because it is not obvious which removed and added lines should be compared.

Current code requires that the number of added lines is equal to the number of removed lines and only skips first +/- character, treating second +/- as a line content, Thus, it is not possible to simply use existing algorithm unchanged for combined diffs.

Let's start with a simple case: only highlight changes that come from one parent, i.e. when every removed line has a corresponding added line for the same parent. This way the highlightning cannot get wrong. For example, following diffs would be highlighted:

	- removed line for first parent
	+ added line for first parent
	  context line
	 -removed line for second parent
	 +added line for second parent

or

	- removed line for first parent
	 -removed line for second parent
	+ added line for first parent
	 +added line for second parent

but following output will not:

	- removed line for first parent
	 -removed line for second parent
	 +added line for second parent
	++added line for both parents

In other words, we require that pattern of '-'-es in pre-image matches pattern of '+'-es in post-image.

Further changes may introduce more intelligent approach that better handles combined diffs.

Signed-off-by: Michał Kiedrowicz <michal.kiedrowicz@gmail.com>
Acked-by: Jakub Narębski <jnareb@gmail.com>
---
 gitweb/gitweb.perl |   55 +++++++++++++++++++++++++++++++++++++++------------
 1 files changed, 42 insertions(+), 13 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index d5b3f04..f4feacc 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -5064,7 +5064,7 @@ sub print_inline_diff_lines {
 # 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) = @_;
+	my ($rem, $add, $num_parents) = @_;
 
 	# We need to untabify lines before split()'ing them;
 	# otherwise offsets would be invalid.
@@ -5076,8 +5076,8 @@ sub format_rem_add_lines_pair {
 	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);
+	# Ignore leading +/- characters for each parent.
+	my ($prefix_len, $suffix_len) = ($num_parents, 0);
 	my ($prefix_has_nonspace, $suffix_has_nonspace);
 
 	my $shorter = (@rem < @add) ? @rem : @add;
@@ -5115,15 +5115,43 @@ sub format_rem_add_lines_pair {
 
 # HTML-format diff context, removed and added lines.
 sub format_ctx_rem_add_lines {
-	my ($ctx, $rem, $add, $is_combined) = @_;
+	my ($ctx, $rem, $add, $num_parents) = @_;
 	my (@new_ctx, @new_rem, @new_add);
+	my $can_highlight = 0;
+	my $is_combined = ($num_parents > 1);
 
 	# 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) {
+	if (@$add > 0 && @$add == @$rem) {
+		$can_highlight = 1;
+
+		# Highlight lines in combined diff only if the chunk contains
+		# diff between the same version, e.g.
+		#
+		#    - a
+		#   -  b
+		#    + c
+		#   +  d
+		#
+		# Otherwise the highlightling would be confusing.
+		if ($is_combined) {
+			for (my $i = 0; $i < @$add; $i++) {
+				my $prefix_rem = substr($rem->[$i], 0, $num_parents);
+				my $prefix_add = substr($add->[$i], 0, $num_parents);
+
+				$prefix_rem =~ s/-/+/g;
+
+				if ($prefix_rem ne $prefix_add) {
+					$can_highlight = 0;
+					last;
+				}
+			}
+		}
+	}
+
+	if ($can_highlight) {
 		for (my $i = 0; $i < @$add; $i++) {
 			my ($line_rem, $line_add) = format_rem_add_lines_pair(
-			        $rem->[$i], $add->[$i]);
+			        $rem->[$i], $add->[$i], $num_parents);
 			push @new_rem, $line_rem;
 			push @new_add, $line_add;
 		}
@@ -5139,10 +5167,11 @@ sub format_ctx_rem_add_lines {
 
 # Print context lines and then rem/add lines.
 sub print_diff_lines {
-	my ($ctx, $rem, $add, $diff_style, $is_combined) = @_;
+	my ($ctx, $rem, $add, $diff_style, $num_parents) = @_;
+	my $is_combined = $num_parents > 1;
 
 	($ctx, $rem, $add) = format_ctx_rem_add_lines($ctx, $rem, $add,
-	        $is_combined);
+	        $num_parents);
 
 	if ($diff_style eq 'sidebyside' && !$is_combined) {
 		print_sidebyside_diff_lines($ctx, $rem, $add);
@@ -5153,7 +5182,7 @@ sub print_diff_lines {
 }
 
 sub print_diff_chunk {
-	my ($diff_style, $is_combined, $from, $to, @chunk) = @_;
+	my ($diff_style, $num_parents, $from, $to, @chunk) = @_;
 	my (@ctx, @rem, @add);
 
 	# The class of the previous line.
@@ -5188,7 +5217,7 @@ sub print_diff_chunk {
 		if (!$class || ((@rem || @add) && $class eq 'ctx') ||
 		    (@rem && @add && $class ne $prev_class)) {
 			print_diff_lines(\@ctx, \@rem, \@add,
-		                         $diff_style, $is_combined);
+		                         $diff_style, $num_parents);
 			@ctx = @rem = @add = ();
 		}
 
@@ -5331,7 +5360,7 @@ sub git_patchset_body {
 			my $class = diff_line_class($patch_line, \%from, \%to);
 
 			if ($class eq 'chunk_header') {
-				print_diff_chunk($diff_style, $is_combined, \%from, \%to, @chunk);
+				print_diff_chunk($diff_style, scalar @hash_parents, \%from, \%to, @chunk);
 				@chunk = ();
 			}
 
@@ -5340,7 +5369,7 @@ sub git_patchset_body {
 
 	} continue {
 		if (@chunk) {
-			print_diff_chunk($diff_style, $is_combined, \%from, \%to, @chunk);
+			print_diff_chunk($diff_style, scalar @hash_parents, \%from, \%to, @chunk);
 			@chunk = ();
 		}
 		print "</div>\n"; # class="patch"

-- 
1.7.8.4