From ce61fb968f9b86f5d889eeb7b80da8ec3d6bfb9b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Kiedrowicz?= Date: Wed, 11 Apr 2012 23:18:37 +0200 Subject: [PATCH 1/8] gitweb: Use descriptive names in esc_html_hl_regions() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The $s->[0] and $s->[1] variables look a bit cryptic. Let's rename them to $begin and $end so that it's clear what they do. Suggested-by: Jakub Narębski Signed-off-by: Michał Kiedrowicz Signed-off-by: Junio C Hamano --- gitweb/gitweb.perl | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl index a8b5fad266..1c54301582 100755 --- a/gitweb/gitweb.perl +++ b/gitweb/gitweb.perl @@ -1738,12 +1738,15 @@ sub esc_html_hl_regions { my $pos = 0; for my $s (@sel) { - $out .= esc_html(substr($str, $pos, $s->[0] - $pos)) - if ($s->[0] - $pos > 0); - $out .= $cgi->span({-class => $css_class}, - esc_html(substr($str, $s->[0], $s->[1] - $s->[0]))); + my ($begin, $end) = @$s; - $pos = $s->[1]; + my $escaped = esc_html(substr($str, $begin, $end - $begin)); + + $out .= esc_html(substr($str, $pos, $begin - $pos)) + if ($begin - $pos > 0); + $out .= $cgi->span({-class => $css_class}, $escaped); + + $pos = $end; } $out .= esc_html(substr($str, $pos)) if ($pos < length($str)); From cbbea3dfc1a1cfe382e58488c389471b4ab21876 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Kiedrowicz?= Date: Wed, 11 Apr 2012 23:18:38 +0200 Subject: [PATCH 2/8] gitweb: esc_html_hl_regions(): Don't create empty elements MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If $end is equal to or less than $begin, esc_html_hl_regions() generates an empty element. It normally shouldn't be visible in the web browser, but it doesn't look good when looking at page source. It also minimally increases generated page size for no special reason. Signed-off-by: Michał Kiedrowicz Acked-by: Jakub Narębski Signed-off-by: Junio C Hamano --- gitweb/gitweb.perl | 3 +++ 1 file changed, 3 insertions(+) diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl index 1c54301582..588b87d234 100755 --- a/gitweb/gitweb.perl +++ b/gitweb/gitweb.perl @@ -1740,6 +1740,9 @@ sub esc_html_hl_regions { for my $s (@sel) { my ($begin, $end) = @$s; + # Don't create empty elements. + next if $end <= $begin; + my $escaped = esc_html(substr($str, $begin, $end - $begin)); $out .= esc_html(substr($str, $pos, $begin - $pos)) From 9768a9d884ee293548c0435df7645fd82d8d942e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Nar=C4=99bski?= Date: Wed, 11 Apr 2012 23:18:39 +0200 Subject: [PATCH 3/8] gitweb: Pass esc_html_hl_regions() options to esc_html() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit With this change, esc_html_hl_regions() accepts options and passes them down to esc_html(). This may be needed if a caller wants to pass -nbsp=>1 to esc_html(). The idea and implementation example of this change was described in 337da8d2 (gitweb: Introduce esc_html_match_hl and esc_html_hl_regions, 2012-02-27). While other suggestions may be more useful in some cases, there is no need to implement them at the moment. The esc_html_hl_regions() interface may be changed later if it's needed. [mk: extracted from larger patch and wrote commit message] Signed-off-by: Jakub Narębski Signed-off-by: Michał Kiedrowicz Signed-off-by: Junio C Hamano --- gitweb/gitweb.perl | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl index 588b87d234..db1f698bfe 100755 --- a/gitweb/gitweb.perl +++ b/gitweb/gitweb.perl @@ -1732,7 +1732,9 @@ sub chop_and_escape_str { # 'foobar' sub esc_html_hl_regions { my ($str, $css_class, @sel) = @_; - return esc_html($str) unless @sel; + my %opts = grep { ref($_) ne 'ARRAY' } @sel; + @sel = grep { ref($_) eq 'ARRAY' } @sel; + return esc_html($str, %opts) unless @sel; my $out = ''; my $pos = 0; @@ -1743,15 +1745,16 @@ sub esc_html_hl_regions { # Don't create empty elements. next if $end <= $begin; - my $escaped = esc_html(substr($str, $begin, $end - $begin)); + my $escaped = esc_html(substr($str, $begin, $end - $begin), + %opts); - $out .= esc_html(substr($str, $pos, $begin - $pos)) + $out .= esc_html(substr($str, $pos, $begin - $pos), %opts) if ($begin - $pos > 0); $out .= $cgi->span({-class => $css_class}, $escaped); $pos = $end; } - $out .= esc_html(substr($str, $pos)) + $out .= esc_html(substr($str, $pos), %opts) if ($pos < length($str)); return $out; From d21102c9ffd6beb0408693b6f1b633f8e494c921 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Kiedrowicz?= Date: Wed, 11 Apr 2012 23:18:40 +0200 Subject: [PATCH 4/8] gitweb: Extract print_sidebyside_diff_lines() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Currently, print_sidebyside_diff_chunk() does two things: it accumulates diff lines and prints them. Accumulation may be used to perform additional operations on diff lines, so it makes sense to split these two things. Thus, whole code that formats and prints diff lines in the 'side-by-side' manner is moved out of print_sidebyside_diff_chunk() to a separate subroutine and two conditions that control printing diff liens are merged. Thanks to that, we can easily (in later patches) replace call to that subroutine with a call to more generic print_diff_lines() that will control whether 'inline' or 'side-by-side' diff should be printed. As a side effect, context lines are printed just before printing added and removed lines, and at the end of chunk (previously, they were printed immediately on the class change). However, this doesn't change gitweb output. The outcome of this patch is that print_sidebyside_diff_chunk() is now much shorter and easier to read. While at it, drop the '# assume that it is change' comment. According to Jakub Narębski: What I meant here when I was writing it that they are lines that changed between two versions, like '!' in original (not unified) context format. We can omit this comment. Signed-off-by: Michał Kiedrowicz Acked-by: Jakub Narębski Signed-off-by: Junio C Hamano --- gitweb/gitweb.perl | 96 ++++++++++++++++++++++++---------------------- 1 file changed, 51 insertions(+), 45 deletions(-) diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl index db1f698bfe..82717490c1 100755 --- a/gitweb/gitweb.perl +++ b/gitweb/gitweb.perl @@ -5002,6 +5002,52 @@ sub git_difftree_body { print "\n"; } +# Print context lines and then rem/add lines in a side-by-side manner. +sub print_sidebyside_diff_lines { + my ($ctx, $rem, $add) = @_; + + # print context block before add/rem block + if (@$ctx) { + print join '', + '
', + '
', + @$ctx, + '
', + '
', + @$ctx, + '
', + '
'; + } + + if (!@$add) { + # pure removal + print join '', + '
', + '
', + @$rem, + '
', + '
'; + } elsif (!@$rem) { + # pure addition + print join '', + '
', + '
', + @$add, + '
', + '
'; + } else { + print join '', + '
', + '
', + @$rem, + '
', + '
', + @$add, + '
', + '
'; + } +} + sub print_sidebyside_diff_chunk { my @chunk = @_; my (@ctx, @rem, @add); @@ -5028,51 +5074,11 @@ sub print_sidebyside_diff_chunk { next; } - ## print from accumulator when type of class of lines change - # empty contents block on start rem/add block, or end of chunk - if (@ctx && (!$class || $class eq 'rem' || $class eq 'add')) { - print join '', - '
', - '
', - @ctx, - '
', - '
', - @ctx, - '
', - '
'; - @ctx = (); - } - # empty add/rem block on start context block, or end of chunk - if ((@rem || @add) && (!$class || $class eq 'ctx')) { - if (!@add) { - # pure removal - print join '', - '
', - '
', - @rem, - '
', - '
'; - } elsif (!@rem) { - # pure addition - print join '', - '
', - '
', - @add, - '
', - '
'; - } else { - # assume that it is change - print join '', - '
', - '
', - @rem, - '
', - '
', - @add, - '
', - '
'; - } - @rem = @add = (); + ## print from accumulator when have some add/rem lines or end + # of chunk (flush context lines) + if (!$class || ((@rem || @add) && $class eq 'ctx')) { + print_sidebyside_diff_lines(\@ctx, \@rem, \@add); + @ctx = @rem = @add = (); } ## adding lines to accumulator From 44185f93f46af7077585ef7ccf5339334a4ab487 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Kiedrowicz?= Date: Wed, 11 Apr 2012 23:18:41 +0200 Subject: [PATCH 5/8] gitweb: Use print_diff_chunk() for both side-by-side and inline diffs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This renames print_sidebyside_diff_chunk() to print_diff_chunk() and makes use of it for both side-by-side and inline diffs. Now diff lines are always accumulated before they are printed. This opens the possibility to preprocess diff output before it's printed, which is needed for diff refinement highlightning (implemented in incoming patches). If print_diff_chunk() was left as is, the new function print_inline_diff_lines() could reorder diff lines. It first prints all context lines, then all removed lines and finally all added lines. If the diff output consisted of mixed added and removed lines, gitweb would reorder these lines. This is true for combined diff output, for example: - removed line for first parent + added line for first parent -removed line for second parent ++added line for both parents would be rendered as: - removed line for first parent -removed line for second parent + added line for first parent ++added line for both parents To prevent gitweb from reordering lines, print_diff_chunk() calls print_diff_lines() as soon as it detects that both added and removed lines are present and there was a class change, and at the end of chunk. Signed-off-by: Michał Kiedrowicz Signed-off-by: Junio C Hamano --- gitweb/gitweb.perl | 55 ++++++++++++++++++++++++++++++++-------------- 1 file changed, 39 insertions(+), 16 deletions(-) diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl index 82717490c1..90836e633c 100755 --- a/gitweb/gitweb.perl +++ b/gitweb/gitweb.perl @@ -5048,10 +5048,32 @@ sub print_sidebyside_diff_lines { } } -sub print_sidebyside_diff_chunk { - my @chunk = @_; +# Print context lines and then rem/add lines in inline manner. +sub print_inline_diff_lines { + my ($ctx, $rem, $add) = @_; + + print @$ctx, @$rem, @$add; +} + +# Print context lines and then rem/add lines. +sub print_diff_lines { + my ($ctx, $rem, $add, $diff_style, $is_combined) = @_; + + if ($diff_style eq 'sidebyside' && !$is_combined) { + print_sidebyside_diff_lines($ctx, $rem, $add); + } else { + # default 'inline' style and unknown styles + print_inline_diff_lines($ctx, $rem, $add); + } +} + +sub print_diff_chunk { + my ($diff_style, $is_combined, @chunk) = @_; my (@ctx, @rem, @add); + # The class of the previous line. + my $prev_class = ''; + return unless @chunk; # incomplete last line might be among removed or added lines, @@ -5075,9 +5097,13 @@ sub print_sidebyside_diff_chunk { } ## print from accumulator when have some add/rem lines or end - # of chunk (flush context lines) - if (!$class || ((@rem || @add) && $class eq 'ctx')) { - print_sidebyside_diff_lines(\@ctx, \@rem, \@add); + # of chunk (flush context lines), or when have add and rem + # lines and new block is reached (otherwise add/rem lines could + # be reordered) + if (!$class || ((@rem || @add) && $class eq 'ctx') || + (@rem && @add && $class ne $prev_class)) { + print_diff_lines(\@ctx, \@rem, \@add, + $diff_style, $is_combined); @ctx = @rem = @add = (); } @@ -5094,6 +5120,8 @@ sub print_sidebyside_diff_chunk { if ($class eq 'ctx') { push @ctx, $line; } + + $prev_class = $class; } } @@ -5220,22 +5248,17 @@ sub git_patchset_body { $diff_classes .= " $class" if ($class); $line = "
$line
\n"; - if ($diff_style eq 'sidebyside' && !$is_combined) { - if ($class eq 'chunk_header') { - print_sidebyside_diff_chunk(@chunk); - @chunk = ( [ $class, $line ] ); - } else { - push @chunk, [ $class, $line ]; - } - } else { - # default 'inline' style and unknown styles - print $line; + if ($class eq 'chunk_header') { + print_diff_chunk($diff_style, $is_combined, @chunk); + @chunk = (); } + + push @chunk, [ $class, $line ]; } } continue { if (@chunk) { - print_sidebyside_diff_chunk(@chunk); + print_diff_chunk($diff_style, $is_combined, @chunk); @chunk = (); } print "\n"; # class="patch" From f4a81026508a48a129bd936aef2c75ac1f1cdd97 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Kiedrowicz?= Date: Wed, 11 Apr 2012 23:18:42 +0200 Subject: [PATCH 6/8] gitweb: Push formatting diff lines to print_diff_chunk() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Now lines are formatted closer to place where we actually use HTML formatted output. This means that we put raw lines in the @chunk accumulator, rather than formatted lines. Because we still need to know class (type) of line when accumulating data to post-process and print, process_diff_line() subroutine was retired and replaced by diff_line_class() used in git_patchset_body() and new restructured format_diff_line() used in print_diff_chunk(). As a side effect, we have to pass \%from and \%to down to callstack. This is a preparation patch for diff refinement highlightning. It's not meant to change gitweb output. [jn: wrote commit message] Signed-off-by: Michał Kiedrowicz Acked-by: Jakub Narębski Signed-off-by: Junio C Hamano --- gitweb/gitweb.perl | 37 ++++++++++++++++++------------------- 1 file changed, 18 insertions(+), 19 deletions(-) diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl index 90836e633c..390774ed93 100755 --- a/gitweb/gitweb.perl +++ b/gitweb/gitweb.perl @@ -2430,26 +2430,26 @@ sub format_cc_diff_chunk_header { } # process patch (diff) line (not to be used for diff headers), -# returning class and HTML-formatted (but not wrapped) line -sub process_diff_line { - my $line = shift; - my ($from, $to) = @_; - - my $diff_class = diff_line_class($line, $from, $to); +# returning HTML-formatted (but not wrapped) line +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); - return $diff_class, $line; - } elsif ($from && $to && $line =~ m/^\@{3}/) { $line = format_cc_diff_chunk_header($line, $from, $to); - return $diff_class, $line; - + } else { + $line = esc_html($line, -nbsp=>1); } - return $diff_class, esc_html($line, -nbsp=>1); + + my $diff_classes = "diff"; + $diff_classes .= " $diff_class" if ($diff_class); + $line = "
$line
\n"; + + return $line; } # Generates undef or something like "_snapshot_" or "snapshot (_tbz2_ _zip_)", @@ -5068,7 +5068,7 @@ sub print_diff_lines { } sub print_diff_chunk { - my ($diff_style, $is_combined, @chunk) = @_; + my ($diff_style, $is_combined, $from, $to, @chunk) = @_; my (@ctx, @rem, @add); # The class of the previous line. @@ -5090,6 +5090,8 @@ 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; @@ -5243,22 +5245,19 @@ sub git_patchset_body { next PATCH if ($patch_line =~ m/^diff /); - my ($class, $line) = process_diff_line($patch_line, \%from, \%to); - my $diff_classes = "diff"; - $diff_classes .= " $class" if ($class); - $line = "
$line
\n"; + my $class = diff_line_class($patch_line, \%from, \%to); if ($class eq 'chunk_header') { - print_diff_chunk($diff_style, $is_combined, @chunk); + print_diff_chunk($diff_style, $is_combined, \%from, \%to, @chunk); @chunk = (); } - push @chunk, [ $class, $line ]; + push @chunk, [ $class, $patch_line ]; } } continue { if (@chunk) { - print_diff_chunk($diff_style, $is_combined, @chunk); + print_diff_chunk($diff_style, $is_combined, \%from, \%to, @chunk); @chunk = (); } print "\n"; # class="patch" From 5fb6ddf67afb3d0dc9b1fce64543f20b28a81b4b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Kiedrowicz?= Date: Wed, 11 Apr 2012 23:18:43 +0200 Subject: [PATCH 7/8] gitweb: Highlight interesting parts of diff MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Acked-by: Jakub Narębski Signed-off-by: Junio C Hamano --- 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 390774ed93..d5b3f04e11 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 c530355a7c..cb86d2d029 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; From 51ef7a6e80013c9131c8751fca7c11b0b4bfba85 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Kiedrowicz?= Date: Wed, 11 Apr 2012 23:18:44 +0200 Subject: [PATCH 8/8] gitweb: Refinement highlightning in combined diffs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Acked-by: Jakub Narębski Signed-off-by: Junio C Hamano --- gitweb/gitweb.perl | 55 +++++++++++++++++++++++++++++++++++----------- 1 file changed, 42 insertions(+), 13 deletions(-) diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl index d5b3f04e11..f4feaccfda 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 "\n"; # class="patch"