Skip to content

Commit d19e556

Browse files
gitstergitster
committed
Merge branch 'jk/diff-highlight-graph-fix'
"diff-highlight" filter (in contrib/) learned to undertand "git log --graph" output better. * jk/diff-highlight-graph-fix: diff-highlight: detect --graph by indent diff-highlight: use flush() helper consistently diff-highlight: test graphs with --color diff-highlight: test interleaved parallel lines of history diff-highlight: prefer "echo" to "cat" in tests diff-highlight: use test_tick in graph test diff-highlight: correct test graph diagram
2 parents 0873c39 + 4551fbb commit d19e556

File tree

2 files changed

+129
-37
lines changed

2 files changed

+129
-37
lines changed

‎contrib/diff-highlight/DiffHighlight.pm

Lines changed: 66 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -21,37 +21,82 @@ my $RESET = "\x1b[m";
2121
my $COLOR = qr/\x1b\[[0-9;]*m/;
2222
my $BORING = qr/$COLOR|\s/;
2323

24-
# The patch portion of git log -p --graph should only ever have preceding | and
25-
# not / or \ as merge history only shows up on the commit line.
26-
my $GRAPH = qr/$COLOR?\|$COLOR?\s+/;
27-
2824
my @removed;
2925
my @added;
3026
my $in_hunk;
27+
my $graph_indent = 0;
3128

3229
our $line_cb = sub { print @_ };
3330
our $flush_cb = sub { local $| = 1 };
3431

35-
sub handle_line {
32+
# Count the visible width of a string, excluding any terminal color sequences.
33+
sub visible_width {
3634
local $_ = shift;
35+
my $ret = 0;
36+
while (length) {
37+
if (s/^$COLOR//) {
38+
# skip colors
39+
} elsif (s/^.//) {
40+
$ret++;
41+
}
42+
}
43+
return $ret;
44+
}
45+
46+
# Return a substring of $str, omitting $len visible characters from the
47+
# beginning, where terminal color sequences do not count as visible.
48+
sub visible_substr {
49+
my ($str, $len) = @_;
50+
while ($len > 0) {
51+
if ($str =~ s/^$COLOR//) {
52+
next
53+
}
54+
$str =~ s/^.//;
55+
$len--;
56+
}
57+
return $str;
58+
}
59+
60+
sub handle_line {
61+
my $orig = shift;
62+
local $_ = $orig;
63+
64+
# match a graph line that begins a commit
65+
if (/^(?:$COLOR?\|$COLOR?[ ])* # zero or more leading "|" with space
66+
$COLOR?\*$COLOR?[ ] # a "*" with its trailing space
67+
(?:$COLOR?\|$COLOR?[ ])* # zero or more trailing "|"
68+
[ ]* # trailing whitespace for merges
69+
/x) {
70+
my $graph_prefix = $&;
71+
72+
# We must flush before setting graph indent, since the
73+
# new commit may be indented differently from what we
74+
# queued.
75+
flush();
76+
$graph_indent = visible_width($graph_prefix);
77+
78+
} elsif ($graph_indent) {
79+
if (length($_) < $graph_indent) {
80+
$graph_indent = 0;
81+
} else {
82+
$_ = visible_substr($_, $graph_indent);
83+
}
84+
}
3785

3886
if (!$in_hunk) {
39-
$line_cb->($_);
40-
$in_hunk = /^$GRAPH*$COLOR*\@\@ /;
87+
$line_cb->($orig);
88+
$in_hunk = /^$COLOR*\@\@ /;
4189
}
42-
elsif (/^$GRAPH*$COLOR*-/) {
43-
push @removed, $_;
90+
elsif (/^$COLOR*-/) {
91+
push @removed, $orig;
4492
}
45-
elsif (/^$GRAPH*$COLOR*\+/) {
46-
push @added, $_;
93+
elsif (/^$COLOR*\+/) {
94+
push @added, $orig;
4795
}
4896
else {
49-
show_hunk(\@removed, \@added);
50-
@removed = ();
51-
@added = ();
52-
53-
$line_cb->($_);
54-
$in_hunk = /^$GRAPH*$COLOR*[\@ ]/;
97+
flush();
98+
$line_cb->($orig);
99+
$in_hunk = /^$COLOR*[\@ ]/;
55100
}
56101

57102
# Most of the time there is enough output to keep things streaming,
@@ -71,6 +116,8 @@ sub flush {
71116
# Flush any queued hunk (this can happen when there is no trailing
72117
# context in the final diff of the input).
73118
show_hunk(\@removed, \@added);
119+
@removed = ();
120+
@added = ();
74121
}
75122

76123
sub highlight_stdin {
@@ -226,8 +273,8 @@ sub is_pair_interesting {
226273
my $suffix_a = join('', @$a[($sa+1)..$#$a]);
227274
my $suffix_b = join('', @$b[($sb+1)..$#$b]);
228275

229-
return $prefix_a !~ /^$GRAPH*$COLOR*-$BORING*$/ ||
230-
$prefix_b !~ /^$GRAPH*$COLOR*\+$BORING*$/ ||
276+
return visible_substr($prefix_a, $graph_indent) !~ /^$COLOR*-$BORING*$/ ||
277+
visible_substr($prefix_b, $graph_indent) !~ /^$COLOR*\+$BORING*$/ ||
231278
$suffix_a !~ /^$BORING*$/ ||
232279
$suffix_b !~ /^$BORING*$/;
233280
}

‎contrib/diff-highlight/t/t9400-diff-highlight.sh

Lines changed: 63 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -52,40 +52,48 @@ test_strip_patch_header () {
5252
# dh_test_setup_history generates a contrived graph such that we have at least
5353
# 1 nesting (E) and 2 nestings (F).
5454
#
55-
# A branch
56-
# /
57-
# D---E---F master
55+
# A---B master
56+
# /
57+
# D---E---F branch
5858
#
5959
# git log --all --graph
6060
# * commit
61-
# | A
61+
# | B
6262
# | * commit
6363
# | | F
64+
# * | commit
65+
# | | A
6466
# | * commit
6567
# |/
6668
# | E
6769
# * commit
6870
# D
6971
#
7072
dh_test_setup_history () {
71-
echo "file1" >file1 &&
72-
echo "file2" >file2 &&
73-
echo "file3" >file3 &&
74-
75-
cat file1 >file &&
73+
echo file1 >file &&
7674
git add file &&
75+
test_tick &&
7776
git commit -m "D" &&
7877

7978
git checkout -b branch &&
80-
cat file2 >file &&
81-
git commit -a -m "A" &&
79+
echo file2 >file &&
80+
test_tick &&
81+
git commit -a -m "E" &&
8282

8383
git checkout master &&
84-
cat file2 >file &&
85-
git commit -a -m "E" &&
84+
echo file2 >file &&
85+
test_tick &&
86+
git commit -a -m "A" &&
8687

87-
cat file3 >file &&
88-
git commit -a -m "F"
88+
git checkout branch &&
89+
echo file3 >file &&
90+
test_tick &&
91+
git commit -a -m "F" &&
92+
93+
git checkout master &&
94+
echo file3 >file &&
95+
test_tick &&
96+
git commit -a -m "B"
8997
}
9098

9199
left_trim () {
@@ -246,16 +254,25 @@ test_expect_failure 'diff-highlight treats combining code points as a unit' '
246254
test_expect_success 'diff-highlight works with the --graph option' '
247255
dh_test_setup_history &&
248256
249-
# topo-order so that the order of the commits is the same as with --graph
257+
# date-order so that the commits are interleaved for both
250258
# trim graph elements so we can do a diff
251259
# trim leading space because our trim_graph is not perfect
252-
git log --branches -p --topo-order |
260+
git log --branches -p --date-order |
253261
"$DIFF_HIGHLIGHT" | left_trim >graph.exp &&
254-
git log --branches -p --graph |
262+
git log --branches -p --date-order --graph |
255263
"$DIFF_HIGHLIGHT" | trim_graph | left_trim >graph.act &&
256264
test_cmp graph.exp graph.act
257265
'
258266

267+
# Just reuse the previous graph test, but with --color. Our trimming
268+
# doesn't know about color, so just sanity check that something got
269+
# highlighted.
270+
test_expect_success 'diff-highlight works with color graph' '
271+
git log --branches -p --date-order --graph --color |
272+
"$DIFF_HIGHLIGHT" | trim_graph | left_trim >graph &&
273+
grep "\[7m" graph
274+
'
275+
259276
# Most combined diffs won't meet diff-highlight's line-number filter. So we
260277
# create one here where one side drops a line and the other modifies it. That
261278
# should result in a diff like:
@@ -293,4 +310,32 @@ test_expect_success 'diff-highlight ignores combined diffs' '
293310
test_cmp expect actual
294311
'
295312

313+
test_expect_success 'diff-highlight handles --graph with leading dash' '
314+
cat >file <<-\EOF &&
315+
before
316+
the old line
317+
-leading dash
318+
EOF
319+
git add file &&
320+
git commit -m before &&
321+
322+
sed s/old/new/ <file >file.tmp &&
323+
mv file.tmp file &&
324+
git add file &&
325+
git commit -m after &&
326+
327+
cat >expect <<-EOF &&
328+
--- a/file
329+
+++ b/file
330+
@@ -1,3 +1,3 @@
331+
before
332+
-the ${CW}old${CR} line
333+
+the ${CW}new${CR} line
334+
-leading dash
335+
EOF
336+
git log --graph -p -1 | "$DIFF_HIGHLIGHT" >actual.raw &&
337+
trim_graph <actual.raw | sed -n "/^---/,\$p" >actual &&
338+
test_cmp expect actual
339+
'
340+
296341
test_done

0 commit comments

Comments
 (0)