Skip to content

Commit

Permalink
gitweb: Refinement highlightning in combined diffs
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
Michał Kiedrowicz authored and Junio C Hamano committed Apr 11, 2012
1 parent 5fb6ddf commit 51ef7a6
Showing 1 changed file with 42 additions and 13 deletions.
55 changes: 42 additions & 13 deletions gitweb/gitweb.perl
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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;
Expand Down Expand Up @@ -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;
}
Expand All @@ -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);
Expand All @@ -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.
Expand Down Expand Up @@ -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 = ();
}

Expand Down Expand Up @@ -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 = ();
}

Expand All @@ -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"
Expand Down

0 comments on commit 51ef7a6

Please sign in to comment.