Re: [PATCH 8/8] gitweb: Highlight combined diffs
From: Jakub Narebski <hidden>
Date: 2016-06-15 22:53:03
Michał Kiedrowicz [off-list ref] writes:
The highlightning of combined diffs is currently disabled. This is because output from a combined diff is much harder to highlight because it's not obvious which removed and added lines should be compared. Moreover, code that compares added and removed lines doesn't care about combined diffs. It only skips first +/- character, treating second +/- as a line content. 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
That is a very reasonable approach.
quoted hunk ↗ jump to hunk
Further changes may introduce more intelligent approach that better handles combined diffs. Signed-off-by: Michał Kiedrowicz <redacted> --- gitweb/gitweb.perl | 40 +++++++++++++++++++++++++++++++++++++--- 1 files changed, 37 insertions(+), 3 deletions(-)diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl index 1a5b454..2b6cb9e 100755 --- a/gitweb/gitweb.perl +++ b/gitweb/gitweb.perl@@ -4944,13 +4944,16 @@ sub esc_html_mark_range { # Format removed and added line, mark changed part and HTML-format them. sub format_rem_add_line { - my ($rem, $add) = @_; + my ($rem, $add, $is_combined) = @_; my @r = split(//, $rem); my @a = split(//, $add); my ($esc_rem, $esc_add); my ($prefix, $suffix_rem, $suffix_add) = (1, $#r, $#a); my ($prefix_is_space, $suffix_is_space) = (1, 1); + # In combined diff we must ignore two +/- characters. + $prefix = 2 if ($is_combined); +
Errr... actually number of prefix is equalt to number of parents, so it might be in case of octopus merge more than 2.
quoted hunk ↗ jump to hunk
while ($prefix < @r && $prefix < @a) { last if ($r[$prefix] ne $a[$prefix]);@@ -4988,11 +4991,42 @@ sub format_ctx_rem_add_lines { my ($ctx, $rem, $add, $is_combined) = @_; my (@new_ctx, @new_rem, @new_add); my $num_add_lines = @$add; + my $can_highlight; + + # Highlight if every removed line has a corresponding added line. + if ($num_add_lines > 0 && $num_add_lines == @$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 < $num_add_lines; $i++) { + my $prefix_rem = substr($rem->[$i], 0, 2); + my $prefix_add = substr($add->[$i], 0, 2); + + $prefix_rem =~ s/-/+/g; + + if ($prefix_rem ne $prefix_add) { + $can_highlight = 0; + last;
Nb. this assumes that we cannot refine and highlight something like this: # - a # - b # + c # ++ d But perhaps this is better left for future improvemnt.
+ }
+ }
+ }
+ } else {
+ $can_highlight = 0;
+ }This 'else' would be not necessary if $can_highlight was initialized to 0.
- if (!$is_combined && $num_add_lines > 0 && $num_add_lines == @$rem) {
+ if ($can_highlight) {
for (my $i = 0; $i < $num_add_lines; $i++) {
my ($line_rem, $line_add) = format_rem_add_line(
- $rem->[$i], $add->[$i]);
+ $rem->[$i], $add->[$i], $is_combined);
push @new_rem, $line_rem;
push @new_add, $line_add;O.K. -- Jakub Narębski