Skip to content

Commit

Permalink
diff-highlight: don't highlight whole lines
Browse files Browse the repository at this point in the history
If you have a change like:

  -foo
  +bar

we end up highlighting the entirety of both lines (since the
whole thing is changed). But the point of diff highlighting
is to pinpoint the specific change in a pair of lines that
are mostly identical. In this case, the highlighting is just
noise, since there is nothing to pinpoint, and we are better
off doing nothing.

The implementation looks for "interesting" pairs by checking
to see whether they actually have a matching prefix or
suffix that does not simply consist of colorization and
whitespace.  However, the implementation makes it easy to
plug in other heuristics, too, like:

  1. Depending on the source material, the set of "boring"
     characters could be tweaked to include language-specific
     stuff (like braces or semicolons for C).

  2. Instead of saying "an interesting line has at least one
     character of prefix or suffix", we could require that
     less than N percent of the line be highlighted.

The simple "ignore whitespace, and highlight if there are
any matched characters" implemented by this patch seems to
give good results on git.git. I'll leave experimentation
with other heuristics to somebody who has a dataset that
does not look good with the current code.

Based on an original idea and implementation by Michał
Kiedrowicz.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
  • Loading branch information
Jeff King authored and Junio C Hamano committed Feb 13, 2012
1 parent 2b21008 commit 097128d
Showing 1 changed file with 26 additions and 2 deletions.
28 changes: 26 additions & 2 deletions contrib/diff-highlight/diff-highlight
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ use strict;
my $HIGHLIGHT = "\x1b[7m";
my $UNHIGHLIGHT = "\x1b[27m";
my $COLOR = qr/\x1b\[[0-9;]*m/;
my $BORING = qr/$COLOR|\s/;

my @window;

Expand Down Expand Up @@ -104,8 +105,14 @@ sub show_pair {
}
}

print highlight(\@a, $pa, $sa);
print highlight(\@b, $pb, $sb);
if (is_pair_interesting(\@a, $pa, $sa, \@b, $pb, $sb)) {
print highlight(\@a, $pa, $sa);
print highlight(\@b, $pb, $sb);
}
else {
print join('', @a);
print join('', @b);
}
}

sub split_line {
Expand All @@ -125,3 +132,20 @@ sub highlight {
@{$line}[($suffix+1)..$#$line]
);
}

# Pairs are interesting to highlight only if we are going to end up
# highlighting a subset (i.e., not the whole line). Otherwise, the highlighting
# is just useless noise. We can detect this by finding either a matching prefix
# or suffix (disregarding boring bits like whitespace and colorization).
sub is_pair_interesting {
my ($a, $pa, $sa, $b, $pb, $sb) = @_;
my $prefix_a = join('', @$a[0..($pa-1)]);
my $prefix_b = join('', @$b[0..($pb-1)]);
my $suffix_a = join('', @$a[($sa+1)..$#$a]);
my $suffix_b = join('', @$b[($sb+1)..$#$b]);

return $prefix_a !~ /^$COLOR*-$BORING*$/ ||
$prefix_b !~ /^$COLOR*\+$BORING*$/ ||
$suffix_a !~ /^$BORING*$/ ||
$suffix_b !~ /^$BORING*$/;
}

0 comments on commit 097128d

Please sign in to comment.