summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--Documentation/diff-options.txt8
-rw-r--r--diff.c47
-rw-r--r--diff.h2
-rwxr-xr-xt/t4015-diff-whitespace.sh261
4 files changed, 236 insertions, 82 deletions
diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
index b1ab964..a88c767 100644
--- a/Documentation/diff-options.txt
+++ b/Documentation/diff-options.txt
@@ -254,13 +254,11 @@ plain::
moved line, but it is not very useful in a review to determine
if a block of code was moved without permutation.
zebra::
- Blocks of moved code are detected greedily. The detected blocks are
+ Blocks of moved text of at least 20 alphanumeric characters
+ are detected greedily. The detected blocks are
painted using either the 'color.diff.{old,new}Moved' color or
'color.diff.{old,new}MovedAlternative'. The change between
- the two colors indicates that a new block was detected. If there
- are fewer than 3 adjacent moved lines, they are not marked up
- as moved, but the regular colors 'color.diff.{old,new}' will be
- used.
+ the two colors indicates that a new block was detected.
dimmed_zebra::
Similar to 'zebra', but additional dimming of uninteresting parts
of moved code is performed. The bordering lines of two adjacent
diff --git a/diff.c b/diff.c
index 8c7e360..ac4023d 100644
--- a/diff.c
+++ b/diff.c
@@ -855,6 +855,38 @@ static int shrink_potential_moved_blocks(struct moved_entry **pmb,
return rp + 1;
}
+/*
+ * If o->color_moved is COLOR_MOVED_PLAIN, this function does nothing.
+ *
+ * Otherwise, if the last block has fewer alphanumeric characters than
+ * COLOR_MOVED_MIN_ALNUM_COUNT, unset DIFF_SYMBOL_MOVED_LINE on all lines in
+ * that block.
+ *
+ * The last block consists of the (n - block_length)'th line up to but not
+ * including the nth line.
+ *
+ * NEEDSWORK: This uses the same heuristic as blame_entry_score() in blame.c.
+ * Think of a way to unify them.
+ */
+static void adjust_last_block(struct diff_options *o, int n, int block_length)
+{
+ int i, alnum_count = 0;
+ if (o->color_moved == COLOR_MOVED_PLAIN)
+ return;
+ for (i = 1; i < block_length + 1; i++) {
+ const char *c = o->emitted_symbols->buf[n - i].line;
+ for (; *c; c++) {
+ if (!isalnum(*c))
+ continue;
+ alnum_count++;
+ if (alnum_count >= COLOR_MOVED_MIN_ALNUM_COUNT)
+ return;
+ }
+ }
+ for (i = 1; i < block_length + 1; i++)
+ o->emitted_symbols->buf[n - i].flags &= ~DIFF_SYMBOL_MOVED_LINE;
+}
+
/* Find blocks of moved code, delegate actual coloring decision to helper */
static void mark_color_as_moved(struct diff_options *o,
struct hashmap *add_lines,
@@ -890,20 +922,13 @@ static void mark_color_as_moved(struct diff_options *o,
}
if (!match) {
- if (block_length < COLOR_MOVED_MIN_BLOCK_LENGTH &&
- o->color_moved != COLOR_MOVED_PLAIN) {
- for (i = 0; i < block_length + 1; i++) {
- l = &o->emitted_symbols->buf[n - i];
- l->flags &= ~DIFF_SYMBOL_MOVED_LINE;
- }
- }
+ adjust_last_block(o, n, block_length);
pmb_nr = 0;
block_length = 0;
continue;
}
l->flags |= DIFF_SYMBOL_MOVED_LINE;
- block_length++;
if (o->color_moved == COLOR_MOVED_PLAIN)
continue;
@@ -933,11 +958,17 @@ static void mark_color_as_moved(struct diff_options *o,
}
flipped_block = (flipped_block + 1) % 2;
+
+ adjust_last_block(o, n, block_length);
+ block_length = 0;
}
+ block_length++;
+
if (flipped_block)
l->flags |= DIFF_SYMBOL_MOVED_LINE_ALT;
}
+ adjust_last_block(o, n, block_length);
free(pmb);
}
diff --git a/diff.h b/diff.h
index 5755f46..aca150b 100644
--- a/diff.h
+++ b/diff.h
@@ -195,7 +195,7 @@ struct diff_options {
COLOR_MOVED_ZEBRA_DIM = 3,
} color_moved;
#define COLOR_MOVED_DEFAULT COLOR_MOVED_ZEBRA
- #define COLOR_MOVED_MIN_BLOCK_LENGTH 3
+ #define COLOR_MOVED_MIN_ALNUM_COUNT 20
};
void diff_emit_submodule_del(struct diff_options *o, const char *line);
diff --git a/t/t4015-diff-whitespace.sh b/t/t4015-diff-whitespace.sh
index c3b6974..12d182d 100755
--- a/t/t4015-diff-whitespace.sh
+++ b/t/t4015-diff-whitespace.sh
@@ -1101,9 +1101,9 @@ test_expect_success 'detect malicious moved code, inside file' '
<BRED>-{<RESET>
<BLUE>-if (!u->is_allowed_foo)<RESET>
<BLUE>-return;<RESET>
- <BRED>-foo(u);<RESET>
- <BLUE>-}<RESET>
- <BLUE>-<RESET>
+ <RED>-foo(u);<RESET>
+ <RED>-}<RESET>
+ <RED>-<RESET>
int main()<RESET>
{<RESET>
foo();<RESET>
@@ -1117,11 +1117,11 @@ test_expect_success 'detect malicious moved code, inside file' '
<RESET>
<BGREEN>+<RESET><BGREEN>int secure_foo(struct user *u)<RESET>
<BGREEN>+<RESET><BGREEN>{<RESET>
- <YELLOW>+<RESET><YELLOW>foo(u);<RESET>
+ <GREEN>+<RESET><GREEN>foo(u);<RESET>
<BGREEN>+<RESET><BGREEN>if (!u->is_allowed_foo)<RESET>
<BGREEN>+<RESET><BGREEN>return;<RESET>
- <YELLOW>+<RESET><YELLOW>}<RESET>
- <YELLOW>+<RESET>
+ <GREEN>+<RESET><GREEN>}<RESET>
+ <GREEN>+<RESET>
int another_function()<RESET>
{<RESET>
bar();<RESET>
@@ -1182,9 +1182,9 @@ test_expect_success 'plain moved code, inside file' '
test_expect_success 'detect permutations inside moved code -- dimmed_zebra' '
git reset --hard &&
cat <<-\EOF >lines.txt &&
- line 1
- line 2
- line 3
+ long line 1
+ long line 2
+ long line 3
line 4
line 5
line 6
@@ -1195,9 +1195,9 @@ test_expect_success 'detect permutations inside moved code -- dimmed_zebra' '
line 11
line 12
line 13
- line 14
- line 15
- line 16
+ long line 14
+ long line 15
+ long line 16
EOF
git add lines.txt &&
git commit -m "add poetry" &&
@@ -1208,12 +1208,12 @@ test_expect_success 'detect permutations inside moved code -- dimmed_zebra' '
line 7
line 8
line 9
- line 1
- line 2
- line 3
- line 14
- line 15
- line 16
+ long line 1
+ long line 2
+ long line 3
+ long line 14
+ long line 15
+ long line 16
line 10
line 11
line 12
@@ -1227,35 +1227,36 @@ test_expect_success 'detect permutations inside moved code -- dimmed_zebra' '
test_config color.diff.newMovedDimmed "normal cyan" &&
test_config color.diff.oldMovedAlternativeDimmed "normal blue" &&
test_config color.diff.newMovedAlternativeDimmed "normal yellow" &&
- git diff HEAD --no-renames --color-moved=dimmed_zebra| test_decode_color >actual &&
+ git diff HEAD --no-renames --color-moved=dimmed_zebra |
+ grep -v "index" |
+ test_decode_color >actual &&
cat <<-\EOF >expected &&
<BOLD>diff --git a/lines.txt b/lines.txt<RESET>
- <BOLD>index 47ea9c3..ba96a38 100644<RESET>
<BOLD>--- a/lines.txt<RESET>
<BOLD>+++ b/lines.txt<RESET>
<CYAN>@@ -1,16 +1,16 @@<RESET>
- <BMAGENTA>-line 1<RESET>
- <BMAGENTA>-line 2<RESET>
- <BMAGENTA>-line 3<RESET>
+ <BMAGENTA>-long line 1<RESET>
+ <BMAGENTA>-long line 2<RESET>
+ <BMAGENTA>-long line 3<RESET>
line 4<RESET>
line 5<RESET>
line 6<RESET>
line 7<RESET>
line 8<RESET>
line 9<RESET>
- <BCYAN>+<RESET><BCYAN>line 1<RESET>
- <BCYAN>+<RESET><BCYAN>line 2<RESET>
- <CYAN>+<RESET><CYAN>line 3<RESET>
- <YELLOW>+<RESET><YELLOW>line 14<RESET>
- <BYELLOW>+<RESET><BYELLOW>line 15<RESET>
- <BYELLOW>+<RESET><BYELLOW>line 16<RESET>
+ <BCYAN>+<RESET><BCYAN>long line 1<RESET>
+ <BCYAN>+<RESET><BCYAN>long line 2<RESET>
+ <CYAN>+<RESET><CYAN>long line 3<RESET>
+ <YELLOW>+<RESET><YELLOW>long line 14<RESET>
+ <BYELLOW>+<RESET><BYELLOW>long line 15<RESET>
+ <BYELLOW>+<RESET><BYELLOW>long line 16<RESET>
line 10<RESET>
line 11<RESET>
line 12<RESET>
line 13<RESET>
- <BMAGENTA>-line 14<RESET>
- <BMAGENTA>-line 15<RESET>
- <BMAGENTA>-line 16<RESET>
+ <BMAGENTA>-long line 14<RESET>
+ <BMAGENTA>-long line 15<RESET>
+ <BMAGENTA>-long line 16<RESET>
EOF
test_cmp expected actual
'
@@ -1270,35 +1271,36 @@ test_expect_success 'cmd option assumes configured colored-moved' '
test_config color.diff.oldMovedAlternativeDimmed "normal blue" &&
test_config color.diff.newMovedAlternativeDimmed "normal yellow" &&
test_config diff.colorMoved zebra &&
- git diff HEAD --no-renames --color-moved| test_decode_color >actual &&
+ git diff HEAD --no-renames --color-moved |
+ grep -v "index" |
+ test_decode_color >actual &&
cat <<-\EOF >expected &&
<BOLD>diff --git a/lines.txt b/lines.txt<RESET>
- <BOLD>index 47ea9c3..ba96a38 100644<RESET>
<BOLD>--- a/lines.txt<RESET>
<BOLD>+++ b/lines.txt<RESET>
<CYAN>@@ -1,16 +1,16 @@<RESET>
- <MAGENTA>-line 1<RESET>
- <MAGENTA>-line 2<RESET>
- <MAGENTA>-line 3<RESET>
+ <MAGENTA>-long line 1<RESET>
+ <MAGENTA>-long line 2<RESET>
+ <MAGENTA>-long line 3<RESET>
line 4<RESET>
line 5<RESET>
line 6<RESET>
line 7<RESET>
line 8<RESET>
line 9<RESET>
- <CYAN>+<RESET><CYAN>line 1<RESET>
- <CYAN>+<RESET><CYAN>line 2<RESET>
- <CYAN>+<RESET><CYAN>line 3<RESET>
- <YELLOW>+<RESET><YELLOW>line 14<RESET>
- <YELLOW>+<RESET><YELLOW>line 15<RESET>
- <YELLOW>+<RESET><YELLOW>line 16<RESET>
+ <CYAN>+<RESET><CYAN>long line 1<RESET>
+ <CYAN>+<RESET><CYAN>long line 2<RESET>
+ <CYAN>+<RESET><CYAN>long line 3<RESET>
+ <YELLOW>+<RESET><YELLOW>long line 14<RESET>
+ <YELLOW>+<RESET><YELLOW>long line 15<RESET>
+ <YELLOW>+<RESET><YELLOW>long line 16<RESET>
line 10<RESET>
line 11<RESET>
line 12<RESET>
line 13<RESET>
- <MAGENTA>-line 14<RESET>
- <MAGENTA>-line 15<RESET>
- <MAGENTA>-line 16<RESET>
+ <MAGENTA>-long line 14<RESET>
+ <MAGENTA>-long line 15<RESET>
+ <MAGENTA>-long line 16<RESET>
EOF
test_cmp expected actual
'
@@ -1324,16 +1326,16 @@ line 1
line 2
line 3
line 4
-line 5
-line 6
-line 7
+long line 5
+long line 6
+long line 7
EOF
git add lines.txt &&
git commit -m "add poetry" &&
cat <<\EOF >lines.txt &&
- line 5
- line 6
- line 7
+ long line 5
+ long line 6
+ long line 7
line 1
line 2
line 3
@@ -1341,44 +1343,167 @@ line 4
EOF
test_config color.diff.oldMoved "magenta" &&
test_config color.diff.newMoved "cyan" &&
- git diff HEAD --no-renames --color-moved| test_decode_color >actual &&
+ git diff HEAD --no-renames --color-moved |
+ grep -v "index" |
+ test_decode_color >actual &&
cat <<-\EOF >expected &&
<BOLD>diff --git a/lines.txt b/lines.txt<RESET>
- <BOLD>index 734156d..eb89ead 100644<RESET>
<BOLD>--- a/lines.txt<RESET>
<BOLD>+++ b/lines.txt<RESET>
<CYAN>@@ -1,7 +1,7 @@<RESET>
- <GREEN>+<RESET> <GREEN>line 5<RESET>
- <GREEN>+<RESET> <GREEN>line 6<RESET>
- <GREEN>+<RESET> <GREEN>line 7<RESET>
+ <GREEN>+<RESET> <GREEN>long line 5<RESET>
+ <GREEN>+<RESET> <GREEN>long line 6<RESET>
+ <GREEN>+<RESET> <GREEN>long line 7<RESET>
line 1<RESET>
line 2<RESET>
line 3<RESET>
line 4<RESET>
- <RED>-line 5<RESET>
- <RED>-line 6<RESET>
- <RED>-line 7<RESET>
+ <RED>-long line 5<RESET>
+ <RED>-long line 6<RESET>
+ <RED>-long line 7<RESET>
EOF
test_cmp expected actual &&
- git diff HEAD --no-renames -w --color-moved| test_decode_color >actual &&
+ git diff HEAD --no-renames -w --color-moved |
+ grep -v "index" |
+ test_decode_color >actual &&
cat <<-\EOF >expected &&
<BOLD>diff --git a/lines.txt b/lines.txt<RESET>
- <BOLD>index 734156d..eb89ead 100644<RESET>
<BOLD>--- a/lines.txt<RESET>
<BOLD>+++ b/lines.txt<RESET>
<CYAN>@@ -1,7 +1,7 @@<RESET>
- <CYAN>+<RESET> <CYAN>line 5<RESET>
- <CYAN>+<RESET> <CYAN>line 6<RESET>
- <CYAN>+<RESET> <CYAN>line 7<RESET>
+ <CYAN>+<RESET> <CYAN>long line 5<RESET>
+ <CYAN>+<RESET> <CYAN>long line 6<RESET>
+ <CYAN>+<RESET> <CYAN>long line 7<RESET>
line 1<RESET>
line 2<RESET>
line 3<RESET>
line 4<RESET>
- <MAGENTA>-line 5<RESET>
- <MAGENTA>-line 6<RESET>
- <MAGENTA>-line 7<RESET>
+ <MAGENTA>-long line 5<RESET>
+ <MAGENTA>-long line 6<RESET>
+ <MAGENTA>-long line 7<RESET>
+ EOF
+ test_cmp expected actual
+'
+
+test_expect_success '--color-moved block at end of diff output respects MIN_ALNUM_COUNT' '
+ git reset --hard &&
+ >bar &&
+ cat <<-\EOF >foo &&
+ irrelevant_line
+ line1
+ EOF
+ git add foo bar &&
+ git commit -m x &&
+
+ cat <<-\EOF >bar &&
+ line1
+ EOF
+ cat <<-\EOF >foo &&
+ irrelevant_line
+ EOF
+
+ git diff HEAD --color-moved=zebra --no-renames |
+ grep -v "index" |
+ test_decode_color >actual &&
+ cat >expected <<-\EOF &&
+ <BOLD>diff --git a/bar b/bar<RESET>
+ <BOLD>--- a/bar<RESET>
+ <BOLD>+++ b/bar<RESET>
+ <CYAN>@@ -0,0 +1 @@<RESET>
+ <GREEN>+<RESET><GREEN>line1<RESET>
+ <BOLD>diff --git a/foo b/foo<RESET>
+ <BOLD>--- a/foo<RESET>
+ <BOLD>+++ b/foo<RESET>
+ <CYAN>@@ -1,2 +1 @@<RESET>
+ irrelevant_line<RESET>
+ <RED>-line1<RESET>
+ EOF
+
+ test_cmp expected actual
+'
+
+test_expect_success '--color-moved respects MIN_ALNUM_COUNT' '
+ git reset --hard &&
+ cat <<-\EOF >foo &&
+ nineteen chars 456789
+ irrelevant_line
+ twenty chars 234567890
EOF
+ >bar &&
+ git add foo bar &&
+ git commit -m x &&
+
+ cat <<-\EOF >foo &&
+ irrelevant_line
+ EOF
+ cat <<-\EOF >bar &&
+ twenty chars 234567890
+ nineteen chars 456789
+ EOF
+
+ git diff HEAD --color-moved=zebra --no-renames |
+ grep -v "index" |
+ test_decode_color >actual &&
+ cat >expected <<-\EOF &&
+ <BOLD>diff --git a/bar b/bar<RESET>
+ <BOLD>--- a/bar<RESET>
+ <BOLD>+++ b/bar<RESET>
+ <CYAN>@@ -0,0 +1,2 @@<RESET>
+ <BOLD;CYAN>+<RESET><BOLD;CYAN>twenty chars 234567890<RESET>
+ <GREEN>+<RESET><GREEN>nineteen chars 456789<RESET>
+ <BOLD>diff --git a/foo b/foo<RESET>
+ <BOLD>--- a/foo<RESET>
+ <BOLD>+++ b/foo<RESET>
+ <CYAN>@@ -1,3 +1 @@<RESET>
+ <RED>-nineteen chars 456789<RESET>
+ irrelevant_line<RESET>
+ <BOLD;MAGENTA>-twenty chars 234567890<RESET>
+ EOF
+
+ test_cmp expected actual
+'
+
+test_expect_success '--color-moved treats adjacent blocks as separate for MIN_ALNUM_COUNT' '
+ git reset --hard &&
+ cat <<-\EOF >foo &&
+ 7charsA
+ irrelevant_line
+ 7charsB
+ 7charsC
+ EOF
+ >bar &&
+ git add foo bar &&
+ git commit -m x &&
+
+ cat <<-\EOF >foo &&
+ irrelevant_line
+ EOF
+ cat <<-\EOF >bar &&
+ 7charsB
+ 7charsC
+ 7charsA
+ EOF
+
+ git diff HEAD --color-moved=zebra --no-renames | grep -v "index" | test_decode_color >actual &&
+ cat >expected <<-\EOF &&
+ <BOLD>diff --git a/bar b/bar<RESET>
+ <BOLD>--- a/bar<RESET>
+ <BOLD>+++ b/bar<RESET>
+ <CYAN>@@ -0,0 +1,3 @@<RESET>
+ <GREEN>+<RESET><GREEN>7charsB<RESET>
+ <GREEN>+<RESET><GREEN>7charsC<RESET>
+ <GREEN>+<RESET><GREEN>7charsA<RESET>
+ <BOLD>diff --git a/foo b/foo<RESET>
+ <BOLD>--- a/foo<RESET>
+ <BOLD>+++ b/foo<RESET>
+ <CYAN>@@ -1,4 +1 @@<RESET>
+ <RED>-7charsA<RESET>
+ irrelevant_line<RESET>
+ <RED>-7charsB<RESET>
+ <RED>-7charsC<RESET>
+ EOF
+
test_cmp expected actual
'