gitweb: Refinement highlightning in combined diffs

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> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Michał Kiedrowicz committed Apr 11, 2012 at 23:18 UTC 51ef7a6e80013c9131c8751fca7c11b0b4bfba85
1 file changed +42 -13
gitweb/gitweb.perl
+42 -13
@@ -5064,7 +5064,7 @@ sub print_inline_diff_lines {
5064 # Format removed and added line, mark changed part and HTML-format them.
5065 # Implementation is based on contrib/diff-highlight
5066 sub format_rem_add_lines_pair {
5067 - my ($rem, $add) = @_;
5067 + my ($rem, $add, $num_parents) = @_;
5068
5069 # We need to untabify lines before split()'ing them;
5070 # otherwise offsets would be invalid.
@@ -5076,8 +5076,8 @@ sub format_rem_add_lines_pair {
5076 my @rem = split(//, $rem);
5077 my @add = split(//, $add);
5078 my ($esc_rem, $esc_add);
5079 - # Ignore +/- character, thus $prefix_len is set to 1.
5080 - my ($prefix_len, $suffix_len) = (1, 0);
5079 + # Ignore leading +/- characters for each parent.
5080 + my ($prefix_len, $suffix_len) = ($num_parents, 0);
5081 my ($prefix_has_nonspace, $suffix_has_nonspace);
5082
5083 my $shorter = (@rem < @add) ? @rem : @add;
@@ -5115,15 +5115,43 @@ sub format_rem_add_lines_pair {
5115
5116 # HTML-format diff context, removed and added lines.
5117 sub format_ctx_rem_add_lines {
5118 - my ($ctx, $rem, $add, $is_combined) = @_;
5118 + my ($ctx, $rem, $add, $num_parents) = @_;
5119 my (@new_ctx, @new_rem, @new_add);
5120 + my $can_highlight = 0;
5121 + my $is_combined = ($num_parents > 1);
5122
5123 # Highlight if every removed line has a corresponding added line.
5122 - # Combined diffs are not supported at this moment.
5123 - if (!$is_combined && @$add > 0 && @$add == @$rem) {
5124 + if (@$add > 0 && @$add == @$rem) {
5125 + $can_highlight = 1;
5126 +
5127 + # Highlight lines in combined diff only if the chunk contains
5128 + # diff between the same version, e.g.
5129 + #
5130 + # - a
5131 + # - b
5132 + # + c
5133 + # + d
5134 + #
5135 + # Otherwise the highlightling would be confusing.
5136 + if ($is_combined) {
5137 + for (my $i = 0; $i < @$add; $i++) {
5138 + my $prefix_rem = substr($rem->[$i], 0, $num_parents);
5139 + my $prefix_add = substr($add->[$i], 0, $num_parents);
5140 +
5141 + $prefix_rem =~ s/-/+/g;
5142 +
5143 + if ($prefix_rem ne $prefix_add) {
5144 + $can_highlight = 0;
5145 + last;
5146 + }
5147 + }
5148 + }
5149 + }
5150 +
5151 + if ($can_highlight) {
5152 for (my $i = 0; $i < @$add; $i++) {
5153 my ($line_rem, $line_add) = format_rem_add_lines_pair(
5126 - $rem->[$i], $add->[$i]);
5154 + $rem->[$i], $add->[$i], $num_parents);
5155 push @new_rem, $line_rem;
5156 push @new_add, $line_add;
5157 }
@@ -5139,10 +5167,11 @@ sub format_ctx_rem_add_lines {
5167
5168 # Print context lines and then rem/add lines.
5169 sub print_diff_lines {
5142 - my ($ctx, $rem, $add, $diff_style, $is_combined) = @_;
5170 + my ($ctx, $rem, $add, $diff_style, $num_parents) = @_;
5171 + my $is_combined = $num_parents > 1;
5172
5173 ($ctx, $rem, $add) = format_ctx_rem_add_lines($ctx, $rem, $add,
5145 - $is_combined);
5174 + $num_parents);
5175
5176 if ($diff_style eq 'sidebyside' && !$is_combined) {
5177 print_sidebyside_diff_lines($ctx, $rem, $add);
@@ -5153,7 +5182,7 @@ sub print_diff_lines {
5182 }
5183
5184 sub print_diff_chunk {
5156 - my ($diff_style, $is_combined, $from, $to, @chunk) = @_;
5185 + my ($diff_style, $num_parents, $from, $to, @chunk) = @_;
5186 my (@ctx, @rem, @add);
5187
5188 # The class of the previous line.
@@ -5188,7 +5217,7 @@ sub print_diff_chunk {
5217 if (!$class || ((@rem || @add) && $class eq 'ctx') ||
5218 (@rem && @add && $class ne $prev_class)) {
5219 print_diff_lines(\@ctx, \@rem, \@add,
5191 - $diff_style, $is_combined);
5220 + $diff_style, $num_parents);
5221 @ctx = @rem = @add = ();
5222 }
5223
@@ -5331,7 +5360,7 @@ sub git_patchset_body {
5360 my $class = diff_line_class($patch_line, \%from, \%to);
5361
5362 if ($class eq 'chunk_header') {
5334 - print_diff_chunk($diff_style, $is_combined, \%from, \%to, @chunk);
5363 + print_diff_chunk($diff_style, scalar @hash_parents, \%from, \%to, @chunk);
5364 @chunk = ();
5365 }
5366
@@ -5340,7 +5369,7 @@ sub git_patchset_body {
5369
5370 } continue {
5371 if (@chunk) {
5343 - print_diff_chunk($diff_style, $is_combined, \%from, \%to, @chunk);
5372 + print_diff_chunk($diff_style, scalar @hash_parents, \%from, \%to, @chunk);
5373 @chunk = ();
5374 }
5375 print "</div>\n"; # class="patch"