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>

Phillip Wood committed Nov 23, 2018 at 11:16 UTC b0a2ba47761fa7bffb5a33e5a76f85da50a00ba5
2 files changed +22 -11
diff.c
+19 -8
@@ -1040,14 +1040,17 @@ static int shrink_potential_moved_blocks(struct moved_block *pmb,
1040 * The last block consists of the (n - block_length)'th line up to but not
1041 * including the nth line.
1042 *
1043 + * Returns 0 if the last block is empty or is unset by this function, non zero
1044 + * otherwise.
1045 + *
1046 * NEEDSWORK: This uses the same heuristic as blame_entry_score() in blame.c.
1047 * Think of a way to unify them.
1048 */
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)
1050 {
1051 int i, alnum_count = 0;
1052 if (o->color_moved == COLOR_MOVED_PLAIN)
1050 - return;
1053 + return block_length;
1054 for (i = 1; i < block_length + 1; i++) {
1055 const char *c = o->emitted_symbols->buf[n - i].line;
1056 for (; *c; c++) {
@@ -1055,11 +1058,12 @@ static void adjust_last_block(struct diff_options *o, int n, int block_length)
1058 continue;
1059 alnum_count++;
1060 if (alnum_count >= COLOR_MOVED_MIN_ALNUM_COUNT)
1058 - return;
1061 + return 1;
1062 }
1063 }
1064 for (i = 1; i < block_length + 1; i++)
1065 o->emitted_symbols->buf[n - i].flags &= ~DIFF_SYMBOL_MOVED_LINE;
1066 + return 0;
1067 }
1068
1069 /* Find blocks of moved code, delegate actual coloring decision to helper */
@@ -1069,7 +1073,7 @@ static void mark_color_as_moved(struct diff_options *o,
1073 {
1074 struct moved_block *pmb = NULL; /* potentially moved blocks */
1075 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;
1077
1078
1079 for (n = 0; n < o->emitted_symbols->nr; n++) {
@@ -1077,6 +1081,7 @@ static void mark_color_as_moved(struct diff_options *o,
1081 struct moved_entry *key;
1082 struct moved_entry *match = NULL;
1083 struct emitted_diff_symbol *l = &o->emitted_symbols->buf[n];
1084 + enum diff_symbol last_symbol = 0;
1085
1086 switch (l->s) {
1087 case DIFF_SYMBOL_PLUS:
@@ -1092,7 +1097,7 @@ static void mark_color_as_moved(struct diff_options *o,
1097 free(key);
1098 break;
1099 default:
1095 - flipped_block = 1;
1100 + flipped_block = 0;
1101 }
1102
1103 if (!match) {
@@ -1103,10 +1108,13 @@ static void mark_color_as_moved(struct diff_options *o,
1108 moved_block_clear(&pmb[i]);
1109 pmb_nr = 0;
1110 block_length = 0;
1111 + flipped_block = 0;
1112 + last_symbol = l->s;
1113 continue;
1114 }
1115
1116 if (o->color_moved == COLOR_MOVED_PLAIN) {
1117 + last_symbol = l->s;
1118 l->flags |= DIFF_SYMBOL_MOVED_LINE;
1119 continue;
1120 }
@@ -1137,19 +1145,22 @@ static void mark_color_as_moved(struct diff_options *o,
1145 }
1146 }
1147
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;
1153
1142 - adjust_last_block(o, n, block_length);
1154 block_length = 0;
1155 }
1156
1157 if (pmb_nr) {
1158 block_length++;
1148 -
1159 l->flags |= DIFF_SYMBOL_MOVED_LINE;
1160 if (flipped_block && o->color_moved != COLOR_MOVED_BLOCKS)
1161 l->flags |= DIFF_SYMBOL_MOVED_LINE_ALT;
1162 }
1163 + last_symbol = l->s;
1164 }
1165 adjust_last_block(o, n, block_length);
1166
t/t4015-diff-whitespace.sh
+3 -3
@@ -1802,14 +1802,14 @@ test_expect_success 'only move detection ignores white spaces' '
1802 <BOLD;MAGENTA>-a long line to exceed per-line minimum<RESET>
1803 <BOLD;MAGENTA>-another long line to exceed per-line minimum<RESET>
1804 <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>
1807 <GREEN>+<RESET><GREEN>new file<RESET>
1808 EOF
1809 test_cmp expected actual
1810 '
1811
1812 -test_expect_failure 'compare whitespace delta across moved blocks' '
1812 +test_expect_success 'compare whitespace delta across moved blocks' '
1813
1814 git reset --hard &&
1815 q_to_tab <<-\EOF >text.txt &&