Skip to content

Commit eb31545

Browse files
phillipwoodgitster
phillipwood
authored and
gitster
committed
diff --color-moved=zebra: fix alternate coloring
b0a2ba4 ("diff --color-moved=zebra: be stricter with color alternation", 2018-11-23) sought to avoid using the alternate colors unless there are two adjacent moved blocks of the same sign. Unfortunately it contains two bugs that prevented it from fixing the problem properly. Firstly `last_symbol` is reset at the start of each iteration of the loop losing the symbol of the last line and secondly when deciding whether to use the alternate color it should be checking if the current line is the same sign of the last line, not a different sign. The combination of the two errors means that we still use the alternate color when we should do but we also use it when we shouldn't. This is most noticable when using --color-moved-ws=allow-indentation-change with hunks like -this line gets indented + this line gets indented where the post image is colored with newMovedAlternate rather than newMoved. While this does not matter much, the next commit will change the coloring to be correct in this case, so lets fix the bug here to make it clear why the output is changing and add a regression test. Signed-off-by: Phillip Wood <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent 0990658 commit eb31545

File tree

2 files changed

+74
-2
lines changed

2 files changed

+74
-2
lines changed

‎diff.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1176,14 +1176,14 @@ static void mark_color_as_moved(struct diff_options *o,
11761176
struct moved_block *pmb = NULL; /* potentially moved blocks */
11771177
int pmb_nr = 0, pmb_alloc = 0;
11781178
int n, flipped_block = 0, block_length = 0;
1179+
enum diff_symbol last_symbol = 0;
11791180

11801181

11811182
for (n = 0; n < o->emitted_symbols->nr; n++) {
11821183
struct hashmap *hm = NULL;
11831184
struct moved_entry *key;
11841185
struct moved_entry *match = NULL;
11851186
struct emitted_diff_symbol *l = &o->emitted_symbols->buf[n];
1186-
enum diff_symbol last_symbol = 0;
11871187

11881188
switch (l->s) {
11891189
case DIFF_SYMBOL_PLUS:
@@ -1251,7 +1251,7 @@ static void mark_color_as_moved(struct diff_options *o,
12511251
&pmb, &pmb_alloc,
12521252
&pmb_nr);
12531253

1254-
if (contiguous && pmb_nr && last_symbol != l->s)
1254+
if (contiguous && pmb_nr && last_symbol == l->s)
12551255
flipped_block = (flipped_block + 1) % 2;
12561256
else
12571257
flipped_block = 0;

‎t/t4015-diff-whitespace.sh

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1442,6 +1442,78 @@ test_expect_success 'detect permutations inside moved code -- dimmed-zebra' '
14421442
test_cmp expected actual
14431443
'
14441444

1445+
test_expect_success 'zebra alternate color is only used when necessary' '
1446+
cat >old.txt <<-\EOF &&
1447+
line 1A should be marked as oldMoved newMovedAlternate
1448+
line 1B should be marked as oldMoved newMovedAlternate
1449+
unchanged
1450+
line 2A should be marked as oldMoved newMovedAlternate
1451+
line 2B should be marked as oldMoved newMovedAlternate
1452+
line 3A should be marked as oldMovedAlternate newMoved
1453+
line 3B should be marked as oldMovedAlternate newMoved
1454+
unchanged
1455+
line 4A should be marked as oldMoved newMovedAlternate
1456+
line 4B should be marked as oldMoved newMovedAlternate
1457+
line 5A should be marked as oldMovedAlternate newMoved
1458+
line 5B should be marked as oldMovedAlternate newMoved
1459+
line 6A should be marked as oldMoved newMoved
1460+
line 6B should be marked as oldMoved newMoved
1461+
EOF
1462+
cat >new.txt <<-\EOF &&
1463+
line 1A should be marked as oldMoved newMovedAlternate
1464+
line 1B should be marked as oldMoved newMovedAlternate
1465+
unchanged
1466+
line 3A should be marked as oldMovedAlternate newMoved
1467+
line 3B should be marked as oldMovedAlternate newMoved
1468+
line 2A should be marked as oldMoved newMovedAlternate
1469+
line 2B should be marked as oldMoved newMovedAlternate
1470+
unchanged
1471+
line 6A should be marked as oldMoved newMoved
1472+
line 6B should be marked as oldMoved newMoved
1473+
line 4A should be marked as oldMoved newMovedAlternate
1474+
line 4B should be marked as oldMoved newMovedAlternate
1475+
line 5A should be marked as oldMovedAlternate newMoved
1476+
line 5B should be marked as oldMovedAlternate newMoved
1477+
EOF
1478+
test_expect_code 1 git diff --no-index --color --color-moved=zebra \
1479+
--color-moved-ws=allow-indentation-change \
1480+
old.txt new.txt >output &&
1481+
grep -v index output | test_decode_color >actual &&
1482+
cat >expected <<-\EOF &&
1483+
<BOLD>diff --git a/old.txt b/new.txt<RESET>
1484+
<BOLD>--- a/old.txt<RESET>
1485+
<BOLD>+++ b/new.txt<RESET>
1486+
<CYAN>@@ -1,14 +1,14 @@<RESET>
1487+
<BOLD;MAGENTA>-line 1A should be marked as oldMoved newMovedAlternate<RESET>
1488+
<BOLD;MAGENTA>-line 1B should be marked as oldMoved newMovedAlternate<RESET>
1489+
<BOLD;CYAN>+<RESET><BOLD;CYAN> line 1A should be marked as oldMoved newMovedAlternate<RESET>
1490+
<BOLD;CYAN>+<RESET><BOLD;CYAN> line 1B should be marked as oldMoved newMovedAlternate<RESET>
1491+
unchanged<RESET>
1492+
<BOLD;MAGENTA>-line 2A should be marked as oldMoved newMovedAlternate<RESET>
1493+
<BOLD;MAGENTA>-line 2B should be marked as oldMoved newMovedAlternate<RESET>
1494+
<BOLD;BLUE>-line 3A should be marked as oldMovedAlternate newMoved<RESET>
1495+
<BOLD;BLUE>-line 3B should be marked as oldMovedAlternate newMoved<RESET>
1496+
<BOLD;CYAN>+<RESET><BOLD;CYAN> line 3A should be marked as oldMovedAlternate newMoved<RESET>
1497+
<BOLD;CYAN>+<RESET><BOLD;CYAN> line 3B should be marked as oldMovedAlternate newMoved<RESET>
1498+
<BOLD;YELLOW>+<RESET><BOLD;YELLOW> line 2A should be marked as oldMoved newMovedAlternate<RESET>
1499+
<BOLD;YELLOW>+<RESET><BOLD;YELLOW> line 2B should be marked as oldMoved newMovedAlternate<RESET>
1500+
unchanged<RESET>
1501+
<BOLD;MAGENTA>-line 4A should be marked as oldMoved newMovedAlternate<RESET>
1502+
<BOLD;MAGENTA>-line 4B should be marked as oldMoved newMovedAlternate<RESET>
1503+
<BOLD;BLUE>-line 5A should be marked as oldMovedAlternate newMoved<RESET>
1504+
<BOLD;BLUE>-line 5B should be marked as oldMovedAlternate newMoved<RESET>
1505+
<BOLD;MAGENTA>-line 6A should be marked as oldMoved newMoved<RESET>
1506+
<BOLD;MAGENTA>-line 6B should be marked as oldMoved newMoved<RESET>
1507+
<BOLD;CYAN>+<RESET><BOLD;CYAN> line 6A should be marked as oldMoved newMoved<RESET>
1508+
<BOLD;CYAN>+<RESET><BOLD;CYAN> line 6B should be marked as oldMoved newMoved<RESET>
1509+
<BOLD;YELLOW>+<RESET><BOLD;YELLOW> line 4A should be marked as oldMoved newMovedAlternate<RESET>
1510+
<BOLD;YELLOW>+<RESET><BOLD;YELLOW> line 4B should be marked as oldMoved newMovedAlternate<RESET>
1511+
<BOLD;CYAN>+<RESET><BOLD;CYAN> line 5A should be marked as oldMovedAlternate newMoved<RESET>
1512+
<BOLD;CYAN>+<RESET><BOLD;CYAN> line 5B should be marked as oldMovedAlternate newMoved<RESET>
1513+
EOF
1514+
test_cmp expected actual
1515+
'
1516+
14451517
test_expect_success 'cmd option assumes configured colored-moved' '
14461518
test_config color.diff.oldMoved "magenta" &&
14471519
test_config color.diff.newMoved "cyan" &&

0 commit comments

Comments
 (0)