Skip to content

Commit b0a2ba4

Browse files
phillipwoodgitster
phillipwood
authored and
gitster
committed
diff --color-moved=zebra: be stricter with color alternation
Currently when using --color-moved=zebra the color of moved blocks depends on the number of lines separating them. This means that adding an odd number of unmoved lines between blocks that are already separated by one or more unmoved lines will change the color of subsequent moved blocks. This does not make much sense as the blocks were already separated by unmoved lines and causes problems when adding lines to test cases. Fix this by only using the alternate colors for adjacent moved blocks. Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Reviewed-by: Stefan Beller <sbeller@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
1 parent 2034b47 commit b0a2ba4

File tree

2 files changed

+22
-11
lines changed

2 files changed

+22
-11
lines changed

‎diff.c‎

Lines changed: 19 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1040,26 +1040,30 @@ static int shrink_potential_moved_blocks(struct moved_block *pmb,
10401040
* The last block consists of the (n - block_length)'th line up to but not
10411041
* including the nth line.
10421042
*
1043+
* Returns 0 if the last block is empty or is unset by this function, non zero
1044+
* otherwise.
1045+
*
10431046
* NEEDSWORK: This uses the same heuristic as blame_entry_score() in blame.c.
10441047
* Think of a way to unify them.
10451048
*/
1046-
static void adjust_last_block(struct diff_options *o, int n, int block_length)
1049+
static int adjust_last_block(struct diff_options *o, int n, int block_length)
10471050
{
10481051
int i, alnum_count = 0;
10491052
if (o->color_moved == COLOR_MOVED_PLAIN)
1050-
return;
1053+
return block_length;
10511054
for (i = 1; i < block_length + 1; i++) {
10521055
const char *c = o->emitted_symbols->buf[n - i].line;
10531056
for (; *c; c++) {
10541057
if (!isalnum(*c))
10551058
continue;
10561059
alnum_count++;
10571060
if (alnum_count >= COLOR_MOVED_MIN_ALNUM_COUNT)
1058-
return;
1061+
return 1;
10591062
}
10601063
}
10611064
for (i = 1; i < block_length + 1; i++)
10621065
o->emitted_symbols->buf[n - i].flags &= ~DIFF_SYMBOL_MOVED_LINE;
1066+
return 0;
10631067
}
10641068

10651069
/* Find blocks of moved code, delegate actual coloring decision to helper */
@@ -1069,14 +1073,15 @@ static void mark_color_as_moved(struct diff_options *o,
10691073
{
10701074
struct moved_block *pmb = NULL; /* potentially moved blocks */
10711075
int pmb_nr = 0, pmb_alloc = 0;
1072-
int n, flipped_block = 1, block_length = 0;
1076+
int n, flipped_block = 0, block_length = 0;
10731077

10741078

10751079
for (n = 0; n < o->emitted_symbols->nr; n++) {
10761080
struct hashmap *hm = NULL;
10771081
struct moved_entry *key;
10781082
struct moved_entry *match = NULL;
10791083
struct emitted_diff_symbol *l = &o->emitted_symbols->buf[n];
1084+
enum diff_symbol last_symbol = 0;
10801085

10811086
switch (l->s) {
10821087
case DIFF_SYMBOL_PLUS:
@@ -1092,7 +1097,7 @@ static void mark_color_as_moved(struct diff_options *o,
10921097
free(key);
10931098
break;
10941099
default:
1095-
flipped_block = 1;
1100+
flipped_block = 0;
10961101
}
10971102

10981103
if (!match) {
@@ -1103,10 +1108,13 @@ static void mark_color_as_moved(struct diff_options *o,
11031108
moved_block_clear(&pmb[i]);
11041109
pmb_nr = 0;
11051110
block_length = 0;
1111+
flipped_block = 0;
1112+
last_symbol = l->s;
11061113
continue;
11071114
}
11081115

11091116
if (o->color_moved == COLOR_MOVED_PLAIN) {
1117+
last_symbol = l->s;
11101118
l->flags |= DIFF_SYMBOL_MOVED_LINE;
11111119
continue;
11121120
}
@@ -1137,19 +1145,22 @@ static void mark_color_as_moved(struct diff_options *o,
11371145
}
11381146
}
11391147

1140-
flipped_block = (flipped_block + 1) % 2;
1148+
if (adjust_last_block(o, n, block_length) &&
1149+
pmb_nr && last_symbol != l->s)
1150+
flipped_block = (flipped_block + 1) % 2;
1151+
else
1152+
flipped_block = 0;
11411153

1142-
adjust_last_block(o, n, block_length);
11431154
block_length = 0;
11441155
}
11451156

11461157
if (pmb_nr) {
11471158
block_length++;
1148-
11491159
l->flags |= DIFF_SYMBOL_MOVED_LINE;
11501160
if (flipped_block && o->color_moved != COLOR_MOVED_BLOCKS)
11511161
l->flags |= DIFF_SYMBOL_MOVED_LINE_ALT;
11521162
}
1163+
last_symbol = l->s;
11531164
}
11541165
adjust_last_block(o, n, block_length);
11551166

‎t/t4015-diff-whitespace.sh‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1802,14 +1802,14 @@ test_expect_success 'only move detection ignores white spaces' '
18021802
<BOLD;MAGENTA>-a long line to exceed per-line minimum<RESET>
18031803
<BOLD;MAGENTA>-another long line to exceed per-line minimum<RESET>
18041804
<RED>-original file<RESET>
1805-
<BOLD;YELLOW>+<RESET>Q<BOLD;YELLOW>a long line to exceed per-line minimum<RESET>
1806-
<BOLD;YELLOW>+<RESET>Q<BOLD;YELLOW>another long line to exceed per-line minimum<RESET>
1805+
<BOLD;CYAN>+<RESET>Q<BOLD;CYAN>a long line to exceed per-line minimum<RESET>
1806+
<BOLD;CYAN>+<RESET>Q<BOLD;CYAN>another long line to exceed per-line minimum<RESET>
18071807
<GREEN>+<RESET><GREEN>new file<RESET>
18081808
EOF
18091809
test_cmp expected actual
18101810
'
18111811

1812-
test_expect_failure 'compare whitespace delta across moved blocks' '
1812+
test_expect_success 'compare whitespace delta across moved blocks' '
18131813
18141814
git reset --hard &&
18151815
q_to_tab <<-\EOF >text.txt &&

0 commit comments

Comments
 (0)