Skip to content

Commit 176841f

Browse files
stefanbellergitster
authored andcommitted
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 <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent 2e2d5ac commit 176841f

File tree

3 files changed

+59
-3
lines changed

3 files changed

+59
-3
lines changed

diff.c

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -256,12 +256,14 @@ static int parse_color_moved(const char *arg)
256256

257257
if (!strcmp(arg, "no"))
258258
return COLOR_MOVED_NO;
259+
else if (!strcmp(arg, "plain"))
260+
return COLOR_MOVED_PLAIN;
259261
else if (!strcmp(arg, "zebra"))
260262
return COLOR_MOVED_ZEBRA;
261263
else if (!strcmp(arg, "default"))
262264
return COLOR_MOVED_DEFAULT;
263265
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'"));
265267
}
266268

267269
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,
879881
}
880882

881883
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) {
883886
for (i = 0; i < block_length + 1; i++) {
884887
l = &o->emitted_symbols->buf[n - i];
885888
l->flags &= ~DIFF_SYMBOL_MOVED_LINE;
@@ -893,6 +896,9 @@ static void mark_color_as_moved(struct diff_options *o,
893896
l->flags |= DIFF_SYMBOL_MOVED_LINE;
894897
block_length++;
895898

899+
if (o->color_moved == COLOR_MOVED_PLAIN)
900+
continue;
901+
896902
/* Check any potential block runs, advance each or nullify */
897903
for (i = 0; i < pmb_nr; i++) {
898904
struct moved_entry *p = pmb[i];

diff.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -190,6 +190,7 @@ struct diff_options {
190190
struct emitted_diff_symbols *emitted_symbols;
191191
enum {
192192
COLOR_MOVED_NO = 0,
193+
COLOR_MOVED_PLAIN = 1,
193194
COLOR_MOVED_ZEBRA = 2,
194195
} color_moved;
195196
#define COLOR_MOVED_DEFAULT COLOR_MOVED_ZEBRA

t/t4015-diff-whitespace.sh

Lines changed: 50 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -986,7 +986,7 @@ test_expect_success 'detect moved code, complete file' '
986986
git mv test.c main.c &&
987987
test_config color.diff.oldMoved "normal red" &&
988988
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 &&
990990
cat >expected <<-\EOF &&
991991
<BOLD>diff --git a/main.c b/main.c<RESET>
992992
<BOLD>new file mode 100644<RESET>
@@ -1130,6 +1130,55 @@ test_expect_success 'detect malicious moved code, inside file' '
11301130
test_cmp expected actual
11311131
'
11321132

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+
11331182
test_expect_success 'no effect from --color-moved with --word-diff' '
11341183
cat <<-\EOF >text.txt &&
11351184
Lorem Ipsum is simply dummy text of the printing and typesetting industry.

0 commit comments

Comments
 (0)