diff.c: color moved lines differently, plain mode

Add the 'plain' mode for move detection of code. This omits the checking for adjacent blocks, so it is not as useful. If you have a lot of the same blocks moved in the same patch, the 'Zebra' would end up slow as it is O(n^2) (n is number of same blocks). So this may be useful there and is generally easy to add. Instead be very literal at the move detection, do not skip over short blocks here. Signed-off-by: Stefan Beller <sbeller@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Stefan Beller committed Jun 30, 2017 at 13:53 UTC 176841f0c9b470b008c95eb50b7bb9424321d540
3 files changed +59 -3
diff.c
+8 -2
@@ -256,12 +256,14 @@ static int parse_color_moved(const char *arg)
256
257 if (!strcmp(arg, "no"))
258 return COLOR_MOVED_NO;
259 + else if (!strcmp(arg, "plain"))
260 + return COLOR_MOVED_PLAIN;
261 else if (!strcmp(arg, "zebra"))
262 return COLOR_MOVED_ZEBRA;
263 else if (!strcmp(arg, "default"))
264 return COLOR_MOVED_DEFAULT;
265 else
264 - return error(_("color moved setting must be one of 'no', 'default', 'zebra'"));
266 + return error(_("color moved setting must be one of 'no', 'default', 'zebra', 'plain'"));
267 }
268
269 int git_diff_ui_config(const char *var, const char *value, void *cb)
@@ -879,7 +881,8 @@ static void mark_color_as_moved(struct diff_options *o,
881 }
882
883 if (!match) {
882 - if (block_length < COLOR_MOVED_MIN_BLOCK_LENGTH) {
884 + if (block_length < COLOR_MOVED_MIN_BLOCK_LENGTH &&
885 + o->color_moved != COLOR_MOVED_PLAIN) {
886 for (i = 0; i < block_length + 1; i++) {
887 l = &o->emitted_symbols->buf[n - i];
888 l->flags &= ~DIFF_SYMBOL_MOVED_LINE;
@@ -893,6 +896,9 @@ static void mark_color_as_moved(struct diff_options *o,
896 l->flags |= DIFF_SYMBOL_MOVED_LINE;
897 block_length++;
898
899 + if (o->color_moved == COLOR_MOVED_PLAIN)
900 + continue;
901 +
902 /* Check any potential block runs, advance each or nullify */
903 for (i = 0; i < pmb_nr; i++) {
904 struct moved_entry *p = pmb[i];
diff.h
+1
@@ -190,6 +190,7 @@ struct diff_options {
190 struct emitted_diff_symbols *emitted_symbols;
191 enum {
192 COLOR_MOVED_NO = 0,
193 + COLOR_MOVED_PLAIN = 1,
194 COLOR_MOVED_ZEBRA = 2,
195 } color_moved;
196 #define COLOR_MOVED_DEFAULT COLOR_MOVED_ZEBRA
t/t4015-diff-whitespace.sh
+50 -1
@@ -986,7 +986,7 @@ test_expect_success 'detect moved code, complete file' '
986 git mv test.c main.c &&
987 test_config color.diff.oldMoved "normal red" &&
988 test_config color.diff.newMoved "normal green" &&
989 - git diff HEAD --color-moved --no-renames | test_decode_color >actual &&
989 + git diff HEAD --color-moved=zebra --no-renames | test_decode_color >actual &&
990 cat >expected <<-\EOF &&
991 <BOLD>diff --git a/main.c b/main.c<RESET>
992 <BOLD>new file mode 100644<RESET>
@@ -1130,6 +1130,55 @@ test_expect_success 'detect malicious moved code, inside file' '
1130 test_cmp expected actual
1131 '
1132
1133 +test_expect_success 'plain moved code, inside file' '
1134 + test_config color.diff.oldMoved "normal red" &&
1135 + test_config color.diff.newMoved "normal green" &&
1136 + test_config color.diff.oldMovedAlternative "blue" &&
1137 + test_config color.diff.newMovedAlternative "yellow" &&
1138 + # needs previous test as setup
1139 + git diff HEAD --no-renames --color-moved=plain| test_decode_color >actual &&
1140 + cat <<-\EOF >expected &&
1141 + <BOLD>diff --git a/main.c b/main.c<RESET>
1142 + <BOLD>index 27a619c..7cf9336 100644<RESET>
1143 + <BOLD>--- a/main.c<RESET>
1144 + <BOLD>+++ b/main.c<RESET>
1145 + <CYAN>@@ -5,13 +5,6 @@<RESET> <RESET>printf("Hello ");<RESET>
1146 + printf("World\n");<RESET>
1147 + }<RESET>
1148 + <RESET>
1149 + <BRED>-int secure_foo(struct user *u)<RESET>
1150 + <BRED>-{<RESET>
1151 + <BRED>-if (!u->is_allowed_foo)<RESET>
1152 + <BRED>-return;<RESET>
1153 + <BRED>-foo(u);<RESET>
1154 + <BRED>-}<RESET>
1155 + <BRED>-<RESET>
1156 + int main()<RESET>
1157 + {<RESET>
1158 + foo();<RESET>
1159 + <BOLD>diff --git a/test.c b/test.c<RESET>
1160 + <BOLD>index 1dc1d85..2bedec9 100644<RESET>
1161 + <BOLD>--- a/test.c<RESET>
1162 + <BOLD>+++ b/test.c<RESET>
1163 + <CYAN>@@ -4,6 +4,13 @@<RESET> <RESET>int bar()<RESET>
1164 + printf("Hello World, but different\n");<RESET>
1165 + }<RESET>
1166 + <RESET>
1167 + <BGREEN>+<RESET><BGREEN>int secure_foo(struct user *u)<RESET>
1168 + <BGREEN>+<RESET><BGREEN>{<RESET>
1169 + <BGREEN>+<RESET><BGREEN>foo(u);<RESET>
1170 + <BGREEN>+<RESET><BGREEN>if (!u->is_allowed_foo)<RESET>
1171 + <BGREEN>+<RESET><BGREEN>return;<RESET>
1172 + <BGREEN>+<RESET><BGREEN>}<RESET>
1173 + <BGREEN>+<RESET>
1174 + int another_function()<RESET>
1175 + {<RESET>
1176 + bar();<RESET>
1177 + EOF
1178 +
1179 + test_cmp expected actual
1180 +'
1181 +
1182 test_expect_success 'no effect from --color-moved with --word-diff' '
1183 cat <<-\EOF >text.txt &&
1184 Lorem Ipsum is simply dummy text of the printing and typesetting industry.