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>

diff --git a/diff.c b/diff.c
index 084ecc5678cc0e01d34d75544124dfc4152cc0ed..79ea592388375d1e1167382e1d31246cfcd021fb 100644 (file)
--- a/diff.c
+++ b/diff.c
@@ -256,12 +256,14 @@ static int parse_color_moved(const char *arg)
 
        if (!strcmp(arg, "no"))
                return COLOR_MOVED_NO;
 
        if (!strcmp(arg, "no"))
                return COLOR_MOVED_NO;

+       else if (!strcmp(arg, "plain"))
+               return COLOR_MOVED_PLAIN;

        else if (!strcmp(arg, "zebra"))
                return COLOR_MOVED_ZEBRA;
        else if (!strcmp(arg, "default"))
                return COLOR_MOVED_DEFAULT;
        else
        else if (!strcmp(arg, "zebra"))
                return COLOR_MOVED_ZEBRA;
        else if (!strcmp(arg, "default"))
                return COLOR_MOVED_DEFAULT;
        else

-               return error(_("color moved setting must be one of 'no', 'default', 'zebra'"));
+               return error(_("color moved setting must be one of 'no', 'default', 'zebra', 'plain'"));

 }
 
 int git_diff_ui_config(const char *var, const char *value, void *cb)
 }
 
 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,
                }
 
                if (!match) {
                }
 
                if (!match) {

-                       if (block_length < COLOR_MOVED_MIN_BLOCK_LENGTH) {
+                       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;
                                for (i = 0; i < block_length + 1; i++) {
                                        l = &o->emitted_symbols->buf[n - i];
                                        l->flags &= ~DIFF_SYMBOL_MOVED_LINE;


@@ -893,6 +896,9 @@ static void mark_color_as_moved(struct diff_options *o,
                l->flags |= DIFF_SYMBOL_MOVED_LINE;
                block_length++;
 
                l->flags |= DIFF_SYMBOL_MOVED_LINE;
                block_length++;
 

+               if (o->color_moved == COLOR_MOVED_PLAIN)
+                       continue;
+

                /* Check any potential block runs, advance each or nullify */
                for (i = 0; i < pmb_nr; i++) {
                        struct moved_entry *p = pmb[i];
                /* Check any potential block runs, advance each or nullify */
                for (i = 0; i < pmb_nr; i++) {
                        struct moved_entry *p = pmb[i];

diff --git a/diff.h b/diff.h
index 3196802673694dbc656a8c7b61f21d48baeeb32f..4cfd609c54db4fcfc913f5bdff4233f8e5a12ea4 100644 (file)
--- a/diff.h
+++ b/diff.h
@@ -190,6 +190,7 @@ struct diff_options {
        struct emitted_diff_symbols *emitted_symbols;
        enum {
                COLOR_MOVED_NO = 0,
        struct emitted_diff_symbols *emitted_symbols;
        enum {
                COLOR_MOVED_NO = 0,

+               COLOR_MOVED_PLAIN = 1,

                COLOR_MOVED_ZEBRA = 2,
        } color_moved;
        #define COLOR_MOVED_DEFAULT COLOR_MOVED_ZEBRA
                COLOR_MOVED_ZEBRA = 2,
        } color_moved;
        #define COLOR_MOVED_DEFAULT COLOR_MOVED_ZEBRA

diff --git a/t/t4015-diff-whitespace.sh b/t/t4015-diff-whitespace.sh
index 29704ae14e27b9f4053464cf9e78ff7cb243f087..d1d7b9999052bfe9cc5c5b443ad6f5d641cac845 100755 (executable)
--- a/t/t4015-diff-whitespace.sh
+++ b/t/t4015-diff-whitespace.sh
@@ -986,7 +986,7 @@ test_expect_success 'detect moved code, complete file' '
        git mv test.c main.c &&
        test_config color.diff.oldMoved "normal red" &&
        test_config color.diff.newMoved "normal green" &&
        git mv test.c main.c &&
        test_config color.diff.oldMoved "normal red" &&
        test_config color.diff.newMoved "normal green" &&

-       git diff HEAD --color-moved --no-renames | test_decode_color >actual &&
+       git diff HEAD --color-moved=zebra --no-renames | test_decode_color >actual &&

        cat >expected <<-\EOF &&
        <BOLD>diff --git a/main.c b/main.c<RESET>
        <BOLD>new file mode 100644<RESET>
        cat >expected <<-\EOF &&
        <BOLD>diff --git a/main.c b/main.c<RESET>
        <BOLD>new file mode 100644<RESET>


@@ -1130,6 +1130,55 @@ test_expect_success 'detect malicious moved code, inside file' '
        test_cmp expected actual
 '
 
        test_cmp expected actual
 '
 

+test_expect_success 'plain moved code, inside file' '
+       test_config color.diff.oldMoved "normal red" &&
+       test_config color.diff.newMoved "normal green" &&
+       test_config color.diff.oldMovedAlternative "blue" &&
+       test_config color.diff.newMovedAlternative "yellow" &&
+       # needs previous test as setup
+       git diff HEAD --no-renames --color-moved=plain| test_decode_color >actual &&
+       cat <<-\EOF >expected &&
+       <BOLD>diff --git a/main.c b/main.c<RESET>
+       <BOLD>index 27a619c..7cf9336 100644<RESET>
+       <BOLD>--- a/main.c<RESET>
+       <BOLD>+++ b/main.c<RESET>
+       <CYAN>@@ -5,13 +5,6 @@<RESET> <RESET>printf("Hello ");<RESET>
+        printf("World\n");<RESET>
+        }<RESET>
+        <RESET>
+       <BRED>-int secure_foo(struct user *u)<RESET>
+       <BRED>-{<RESET>
+       <BRED>-if (!u->is_allowed_foo)<RESET>
+       <BRED>-return;<RESET>
+       <BRED>-foo(u);<RESET>
+       <BRED>-}<RESET>
+       <BRED>-<RESET>
+        int main()<RESET>
+        {<RESET>
+        foo();<RESET>
+       <BOLD>diff --git a/test.c b/test.c<RESET>
+       <BOLD>index 1dc1d85..2bedec9 100644<RESET>
+       <BOLD>--- a/test.c<RESET>
+       <BOLD>+++ b/test.c<RESET>
+       <CYAN>@@ -4,6 +4,13 @@<RESET> <RESET>int bar()<RESET>
+        printf("Hello World, but different\n");<RESET>
+        }<RESET>
+        <RESET>
+       <BGREEN>+<RESET><BGREEN>int secure_foo(struct user *u)<RESET>
+       <BGREEN>+<RESET><BGREEN>{<RESET>
+       <BGREEN>+<RESET><BGREEN>foo(u);<RESET>
+       <BGREEN>+<RESET><BGREEN>if (!u->is_allowed_foo)<RESET>
+       <BGREEN>+<RESET><BGREEN>return;<RESET>
+       <BGREEN>+<RESET><BGREEN>}<RESET>
+       <BGREEN>+<RESET>
+        int another_function()<RESET>
+        {<RESET>
+        bar();<RESET>
+       EOF
+
+       test_cmp expected actual
+'
+

 test_expect_success 'no effect from --color-moved with --word-diff' '
        cat <<-\EOF >text.txt &&
        Lorem Ipsum is simply dummy text of the printing and typesetting industry.
 test_expect_success 'no effect from --color-moved with --word-diff' '
        cat <<-\EOF >text.txt &&
        Lorem Ipsum is simply dummy text of the printing and typesetting industry.