Merge branch 'rs/xdiff-hunk-with-func-line' into maint
authorJunio C Hamano <gitster@pobox.com>
Mon, 27 Jun 2016 16:56:24 +0000 (09:56 -0700)
committerJunio C Hamano <gitster@pobox.com>
Mon, 27 Jun 2016 16:56:24 +0000 (09:56 -0700)
"git show -W" (extend hunks to cover the entire function, delimited
by lines that match the "funcname" pattern) used to show the entire
file when a change added an entire function at the end of the file,
which has been fixed.

* rs/xdiff-hunk-with-func-line:
xdiff: fix merging of appended hunk with -W
grep: -W: don't extend context to trailing empty lines
t7810: add test for grep -W and trailing empty context lines
xdiff: don't trim common tail with -W
xdiff: -W: don't include common trailing empty lines in context
xdiff: ignore empty lines before added functions with -W
xdiff: handle appended chunks better with -W
xdiff: factor out match_func_rec()
t4051: rewrite, add more tests

grep.c
t/t4051-diff-function-context.sh
t/t4051/appended1.c [new file with mode: 0644]
t/t4051/appended2.c [new file with mode: 0644]
t/t4051/dummy.c [new file with mode: 0644]
t/t4051/hello.c [new file with mode: 0644]
t/t4051/includes.c [new file with mode: 0644]
t/t7810-grep.sh
xdiff-interface.c
xdiff/xemit.c
diff --git a/grep.c b/grep.c
index ec6f7ffa19622f1a63a4cdd51936d1061e90fc49..1e15b6292d768e9daf5c5e84f9346577abdf3939 100644 (file)
--- a/grep.c
+++ b/grep.c
@@ -1396,9 +1396,17 @@ static int fill_textconv_grep(struct userdiff_driver *driver,
        return 0;
 }
 
+static int is_empty_line(const char *bol, const char *eol)
+{
+       while (bol < eol && isspace(*bol))
+               bol++;
+       return bol == eol;
+}
+
 static int grep_source_1(struct grep_opt *opt, struct grep_source *gs, int collect_hits)
 {
        char *bol;
+       char *peek_bol = NULL;
        unsigned long left;
        unsigned lno = 1;
        unsigned last_hit = 0;
@@ -1543,8 +1551,24 @@ static int grep_source_1(struct grep_opt *opt, struct grep_source *gs, int colle
                                show_function = 1;
                        goto next_line;
                }
-               if (show_function && match_funcname(opt, gs, bol, eol))
-                       show_function = 0;
+               if (show_function && (!peek_bol || peek_bol < bol)) {
+                       unsigned long peek_left = left;
+                       char *peek_eol = eol;
+
+                       /*
+                        * Trailing empty lines are not interesting.
+                        * Peek past them to see if they belong to the
+                        * body of the current function.
+                        */
+                       peek_bol = bol;
+                       while (is_empty_line(peek_bol, peek_eol)) {
+                               peek_bol = peek_eol + 1;
+                               peek_eol = end_of_line(peek_bol, &peek_left);
+                       }
+
+                       if (match_funcname(opt, gs, peek_bol, peek_eol))
+                               show_function = 0;
+               }
                if (show_function ||
                    (last_hit && lno <= last_hit + opt->post_context)) {
                        /* If the last hit is within the post context,
index 001d678e09bfa6d3e1c7197c27925bb12a71b492..b79b87790b29f6f1193578fd725d22da7058c819 100755 (executable)
 test_description='diff function context'
 
 . ./test-lib.sh
-. "$TEST_DIRECTORY"/diff-lib.sh
 
+dir="$TEST_DIRECTORY/t4051"
 
-cat <<\EOF >hello.c
-#include <stdio.h>
-
-static int a(void)
-{
-       /*
-        * Dummy.
-        */
+commit_and_tag () {
+       tag=$1 &&
+       shift &&
+       git add "$@" &&
+       test_tick &&
+       git commit -m "$tag" &&
+       git tag "$tag"
 }
 
-static int hello_world(void)
-{
-       /* Classic. */
-       printf("Hello world.\n");
-
-       /* Success! */
-       return 0;
+first_context_line () {
+       awk '
+               found {print; exit}
+               /^@@/ {found = 1}
+       '
 }
-static int b(void)
-{
-       /*
-        * Dummy, too.
-        */
+
+last_context_line () {
+       sed -ne \$p
 }
 
-int main(int argc, char **argv)
-{
-       a();
-       b();
-       return hello_world();
+check_diff () {
+       name=$1
+       desc=$2
+       options="-W $3"
+
+       test_expect_success "$desc" '
+               git diff $options "$name^" "$name" >"$name.diff"
+       '
+
+       test_expect_success ' diff applies' '
+               test_when_finished "git reset --hard" &&
+               git checkout --detach "$name^" &&
+               git apply --index "$name.diff" &&
+               git diff --exit-code "$name"
+       '
 }
-EOF
 
 test_expect_success 'setup' '
-       git add hello.c &&
-       test_tick &&
-       git commit -m initial &&
-
-       grep -v Classic <hello.c >hello.c.new &&
-       mv hello.c.new hello.c
-'
-
-cat <<\EOF >expected
-diff --git a/hello.c b/hello.c
---- a/hello.c
-+++ b/hello.c
-@@ -10,8 +10,7 @@ static int a(void)
- static int hello_world(void)
- {
--      /* Classic. */
-       printf("Hello world.\n");
-       /* Success! */
-       return 0;
- }
-EOF
-
-test_expect_success 'diff -U0 -W' '
-       git diff -U0 -W >actual &&
-       compare_diff_patch actual expected
-'
-
-cat <<\EOF >expected
-diff --git a/hello.c b/hello.c
---- a/hello.c
-+++ b/hello.c
-@@ -9,9 +9,8 @@ static int a(void)
- static int hello_world(void)
- {
--      /* Classic. */
-       printf("Hello world.\n");
-       /* Success! */
-       return 0;
- }
-EOF
-
-test_expect_success 'diff -W' '
-       git diff -W >actual &&
-       compare_diff_patch actual expected
+       cat "$dir/includes.c" "$dir/dummy.c" "$dir/dummy.c" "$dir/hello.c" \
+               "$dir/dummy.c" "$dir/dummy.c" >file.c &&
+       commit_and_tag initial file.c &&
+
+       grep -v "delete me from hello" <file.c >file.c.new &&
+       mv file.c.new file.c &&
+       commit_and_tag changed_hello file.c &&
+
+       grep -v "delete me from includes" <file.c >file.c.new &&
+       mv file.c.new file.c &&
+       commit_and_tag changed_includes file.c &&
+
+       cat "$dir/appended1.c" >>file.c &&
+       commit_and_tag appended file.c &&
+
+       cat "$dir/appended2.c" >>file.c &&
+       commit_and_tag extended file.c &&
+
+       grep -v "Begin of second part" <file.c >file.c.new &&
+       mv file.c.new file.c &&
+       commit_and_tag long_common_tail file.c &&
+
+       git checkout initial &&
+       grep -v "delete me from hello" <file.c >file.c.new &&
+       mv file.c.new file.c &&
+       cat "$dir/appended1.c" >>file.c &&
+       commit_and_tag changed_hello_appended file.c
+'
+
+check_diff changed_hello 'changed function'
+
+test_expect_success ' context includes begin' '
+       grep "^ .*Begin of hello" changed_hello.diff
+'
+
+test_expect_success ' context includes end' '
+       grep "^ .*End of hello" changed_hello.diff
+'
+
+test_expect_success ' context does not include other functions' '
+       test $(grep -c "^[ +-].*Begin" changed_hello.diff) -le 1
+'
+
+test_expect_success ' context does not include preceding empty lines' '
+       test "$(first_context_line <changed_hello.diff)" != " "
+'
+
+test_expect_success ' context does not include trailing empty lines' '
+       test "$(last_context_line <changed_hello.diff)" != " "
+'
+
+check_diff changed_includes 'changed includes'
+
+test_expect_success ' context includes begin' '
+       grep "^ .*Begin.h" changed_includes.diff
+'
+
+test_expect_success ' context includes end' '
+       grep "^ .*End.h" changed_includes.diff
+'
+
+test_expect_success ' context does not include other functions' '
+       test $(grep -c "^[ +-].*Begin" changed_includes.diff) -le 1
+'
+
+test_expect_success ' context does not include trailing empty lines' '
+       test "$(last_context_line <changed_includes.diff)" != " "
+'
+
+check_diff appended 'appended function'
+
+test_expect_success ' context includes begin' '
+       grep "^[+].*Begin of first part" appended.diff
+'
+
+test_expect_success ' context includes end' '
+       grep "^[+].*End of first part" appended.diff
+'
+
+test_expect_success ' context does not include other functions' '
+       test $(grep -c "^[ +-].*Begin" appended.diff) -le 1
+'
+
+check_diff extended 'appended function part'
+
+test_expect_success ' context includes begin' '
+       grep "^ .*Begin of first part" extended.diff
+'
+
+test_expect_success ' context includes end' '
+       grep "^[+].*End of second part" extended.diff
+'
+
+test_expect_success ' context does not include other functions' '
+       test $(grep -c "^[ +-].*Begin" extended.diff) -le 2
+'
+
+test_expect_success ' context does not include preceding empty lines' '
+       test "$(first_context_line <extended.diff)" != " "
+'
+
+check_diff long_common_tail 'change with long common tail and no context' -U0
+
+test_expect_success ' context includes begin' '
+       grep "^ .*Begin of first part" long_common_tail.diff
+'
+
+test_expect_success ' context includes end' '
+       grep "^ .*End of second part" long_common_tail.diff
+'
+
+test_expect_success ' context does not include other functions' '
+       test $(grep -c "^[ +-].*Begin" long_common_tail.diff) -le 2
+'
+
+test_expect_success ' context does not include preceding empty lines' '
+       test "$(first_context_line <long_common_tail.diff.diff)" != " "
+'
+
+check_diff changed_hello_appended 'changed function plus appended function'
+
+test_expect_success ' context includes begin' '
+       grep "^ .*Begin of hello" changed_hello_appended.diff &&
+       grep "^[+].*Begin of first part" changed_hello_appended.diff
+'
+
+test_expect_success ' context includes end' '
+       grep "^ .*End of hello" changed_hello_appended.diff &&
+       grep "^[+].*End of first part" changed_hello_appended.diff
+'
+
+test_expect_success ' context does not include other functions' '
+       test $(grep -c "^[ +-].*Begin" changed_hello_appended.diff) -le 2
 '
 
 test_done
diff --git a/t/t4051/appended1.c b/t/t4051/appended1.c
new file mode 100644 (file)
index 0000000..a9f56f1
--- /dev/null
@@ -0,0 +1,15 @@
+
+int appended(void) // Begin of first part
+{
+       int i;
+       char *s = "a string";
+
+       printf("%s\n", s);
+
+       for (i = 99;
+            i >= 0;
+            i--) {
+               printf("%d bottles of beer on the wall\n", i);
+       }
+
+       printf("End of first part\n");
diff --git a/t/t4051/appended2.c b/t/t4051/appended2.c
new file mode 100644 (file)
index 0000000..e651f71
--- /dev/null
@@ -0,0 +1,35 @@
+       printf("Begin of second part\n");
+
+       /*
+        * Lorem ipsum dolor sit amet, consectetuer sadipscing elitr,
+        * sed diam nonumy eirmod tempor invidunt ut labore et dolore
+        * magna aliquyam erat, sed diam voluptua. At vero eos et
+        * accusam et justo duo dolores et ea rebum. Stet clita kasd
+        * gubergren, no sea takimata sanctus est Lorem ipsum dolor
+        * sit amet.
+        *
+        * Lorem ipsum dolor sit amet, consectetuer sadipscing elitr,
+        * sed diam nonumy eirmod tempor invidunt ut labore et dolore
+        * magna aliquyam erat, sed diam voluptua. At vero eos et
+        * accusam et justo duo dolores et ea rebum. Stet clita kasd
+        * gubergren, no sea takimata sanctus est Lorem ipsum dolor
+        * sit amet.
+        *
+        * Lorem ipsum dolor sit amet, consectetuer sadipscing elitr,
+        * sed diam nonumy eirmod tempor invidunt ut labore et dolore
+        * magna aliquyam erat, sed diam voluptua. At vero eos et
+        * accusam et justo duo dolores et ea rebum. Stet clita kasd
+        * gubergren, no sea takimata sanctus est Lorem ipsum dolor
+        * sit amet.
+        *
+        * Lorem ipsum dolor sit amet, consectetuer sadipscing elitr,
+        * sed diam nonumy eirmod tempor invidunt ut labore et dolore
+        * magna aliquyam erat, sed diam voluptua. At vero eos et
+        * accusam et justo duo dolores et ea rebum. Stet clita kasd
+        * gubergren, no sea takimata sanctus est Lorem ipsum dolor
+        * sit amet.
+        *
+        */
+
+       return 0;
+}      // End of second part
diff --git a/t/t4051/dummy.c b/t/t4051/dummy.c
new file mode 100644 (file)
index 0000000..a43016e
--- /dev/null
@@ -0,0 +1,7 @@
+
+static int dummy(void) // Begin of dummy
+{
+       int rc = 0;
+
+       return rc;
+}      // End of dummy
diff --git a/t/t4051/hello.c b/t/t4051/hello.c
new file mode 100644 (file)
index 0000000..63b1a1e
--- /dev/null
@@ -0,0 +1,21 @@
+
+static void hello(void)        // Begin of hello
+{
+       /*
+        * Classic.
+        */
+       putchar('H');
+       putchar('e');
+       putchar('l');
+       putchar('l');
+       putchar('o');
+       putchar(' ');
+       /* delete me from hello */
+       putchar('w');
+       putchar('o');
+       putchar('r');
+       putchar('l');
+       putchar('d');
+       putchar('.');
+       putchar('\n');
+}      // End of hello
diff --git a/t/t4051/includes.c b/t/t4051/includes.c
new file mode 100644 (file)
index 0000000..efc68f8
--- /dev/null
@@ -0,0 +1,20 @@
+#include <Begin.h>
+#include <unistd.h>
+#include <stdio.h>
+#include <sys/stat.h>
+#include <fcntl.h>
+#include <stddef.h>
+#include <stdlib.h>
+#include <stdarg.h>
+/* delete me from includes */
+#include <string.h>
+#include <sys/types.h>
+#include <dirent.h>
+#include <sys/time.h>
+#include <time.h>
+#include <signal.h>
+#include <assert.h>
+#include <regex.h>
+#include <utime.h>
+#include <syslog.h>
+#include <End.h>
index 1e72971a165efc127e6bda24fb0e4a3852d3c543..960425a4ec6229cd2d15dac74f6fdf6deda4ec6c 100755 (executable)
@@ -9,7 +9,9 @@ test_description='git grep various.
 . ./test-lib.sh
 
 cat >hello.c <<EOF
+#include <assert.h>
 #include <stdio.h>
+
 int main(int argc, const char **argv)
 {
        printf("Hello world.\n");
@@ -715,6 +717,7 @@ test_expect_success 'grep -p' '
 
 cat >expected <<EOF
 hello.c-#include <stdio.h>
+hello.c-
 hello.c=int main(int argc, const char **argv)
 hello.c-{
 hello.c-       printf("Hello world.\n");
@@ -740,6 +743,16 @@ test_expect_success 'grep -W' '
        test_cmp expected actual
 '
 
+cat >expected <<EOF
+hello.c-#include <assert.h>
+hello.c:#include <stdio.h>
+EOF
+
+test_expect_success 'grep -W shows no trailing empty lines' '
+       git grep -W stdio >actual &&
+       test_cmp expected actual
+'
+
 cat >expected <<EOF
 hello.c=       printf("Hello world.\n");
 hello.c:       return 0;
@@ -1232,8 +1245,8 @@ test_expect_success 'grep --heading' '
 
 cat >expected <<EOF
 <BOLD;GREEN>hello.c<RESET>
-2:int main(int argc, const <BLACK;BYELLOW>char<RESET> **argv)
-6:     /* <BLACK;BYELLOW>char<RESET> ?? */
+4:int main(int argc, const <BLACK;BYELLOW>char<RESET> **argv)
+8:     /* <BLACK;BYELLOW>char<RESET> ?? */
 
 <BOLD;GREEN>hello_world<RESET>
 3:Hel<BLACK;BYELLOW>lo_w<RESET>orld
@@ -1340,7 +1353,7 @@ test_expect_success 'grep --color -e A --and --not -e B with context' '
 '
 
 cat >expected <<EOF
-hello.c-#include <stdio.h>
+hello.c-
 hello.c=int main(int argc, const char **argv)
 hello.c-{
 hello.c:       pr<RED>int<RESET>f("<RED>Hello<RESET> world.\n");
index 54236f24b9786710f91650ac63f6004cdeb012e6..f34ea762e477d9874ef8d75e24f54230520a6e4b 100644 (file)
@@ -100,9 +100,9 @@ static int xdiff_outf(void *priv_, mmbuffer_t *mb, int nbuf)
 
 /*
  * Trim down common substring at the end of the buffers,
- * but leave at least ctx lines at the end.
+ * but end on a complete line.
  */
-static void trim_common_tail(mmfile_t *a, mmfile_t *b, long ctx)
+static void trim_common_tail(mmfile_t *a, mmfile_t *b)
 {
        const int blk = 1024;
        long trimmed = 0, recovered = 0;
@@ -110,9 +110,6 @@ static void trim_common_tail(mmfile_t *a, mmfile_t *b, long ctx)
        char *bp = b->ptr + b->size;
        long smaller = (a->size < b->size) ? a->size : b->size;
 
-       if (ctx)
-               return;
-
        while (blk + trimmed <= smaller && !memcmp(ap - blk, bp - blk, blk)) {
                trimmed += blk;
                ap -= blk;
@@ -134,7 +131,8 @@ int xdi_diff(mmfile_t *mf1, mmfile_t *mf2, xpparam_t const *xpp, xdemitconf_t co
        if (mf1->size > MAX_XDIFF_SIZE || mf2->size > MAX_XDIFF_SIZE)
                return -1;
 
-       trim_common_tail(&a, &b, xecfg->ctxlen);
+       if (!xecfg->ctxlen && !(xecfg->flags & XDL_EMIT_FUNCCONTEXT))
+               trim_common_tail(&a, &b);
 
        return xdl_diff(&a, &b, xpp, xecfg, xecb);
 }
index 993724b11c40bacffee8df927018e5790a265bd4..49aa16ff78d8c0f10942e0f519543cc6befabb9d 100644 (file)
@@ -120,6 +120,16 @@ static long def_ff(const char *rec, long len, char *buf, long sz, void *priv)
        return -1;
 }
 
+static long match_func_rec(xdfile_t *xdf, xdemitconf_t const *xecfg, long ri,
+                          char *buf, long sz)
+{
+       const char *rec;
+       long len = xdl_get_rec(xdf, ri, &rec);
+       if (!xecfg->find_func)
+               return def_ff(rec, len, buf, sz, xecfg->find_func_priv);
+       return xecfg->find_func(rec, len, buf, sz, xecfg->find_func_priv);
+}
+
 struct func_line {
        long len;
        char buf[80];
@@ -128,7 +138,6 @@ struct func_line {
 static long get_func_line(xdfenv_t *xe, xdemitconf_t const *xecfg,
                          struct func_line *func_line, long start, long limit)
 {
-       find_func_t ff = xecfg->find_func ? xecfg->find_func : def_ff;
        long l, size, step = (start > limit) ? -1 : 1;
        char *buf, dummy[1];
 
@@ -136,9 +145,7 @@ static long get_func_line(xdfenv_t *xe, xdemitconf_t const *xecfg,
        size = func_line ? sizeof(func_line->buf) : sizeof(dummy);
 
        for (l = start; l != limit && 0 <= l && l < xe->xdf1.nrec; l += step) {
-               const char *rec;
-               long reclen = xdl_get_rec(&xe->xdf1, l, &rec);
-               long len = ff(rec, reclen, buf, size, xecfg->find_func_priv);
+               long len = match_func_rec(&xe->xdf1, xecfg, l, buf, size);
                if (len >= 0) {
                        if (func_line)
                                func_line->len = len;
@@ -148,6 +155,18 @@ static long get_func_line(xdfenv_t *xe, xdemitconf_t const *xecfg,
        return -1;
 }
 
+static int is_empty_rec(xdfile_t *xdf, long ri)
+{
+       const char *rec;
+       long len = xdl_get_rec(xdf, ri, &rec);
+
+       while (len > 0 && XDL_ISSPACE(*rec)) {
+               rec++;
+               len--;
+       }
+       return !len;
+}
+
 int xdl_emit_diff(xdfenv_t *xe, xdchange_t *xscr, xdemitcb_t *ecb,
                  xdemitconf_t const *xecfg) {
        long s1, s2, e1, e2, lctx;
@@ -164,7 +183,34 @@ int xdl_emit_diff(xdfenv_t *xe, xdchange_t *xscr, xdemitcb_t *ecb,
                s2 = XDL_MAX(xch->i2 - xecfg->ctxlen, 0);
 
                if (xecfg->flags & XDL_EMIT_FUNCCONTEXT) {
-                       long fs1 = get_func_line(xe, xecfg, NULL, xch->i1, -1);
+                       long fs1, i1 = xch->i1;
+
+                       /* Appended chunk? */
+                       if (i1 >= xe->xdf1.nrec) {
+                               char dummy[1];
+                               long i2 = xch->i2;
+
+                               /*
+                                * We don't need additional context if
+                                * a whole function was added, possibly
+                                * starting with empty lines.
+                                */
+                               while (i2 < xe->xdf2.nrec &&
+                                      is_empty_rec(&xe->xdf2, i2))
+                                       i2++;
+                               if (i2 < xe->xdf2.nrec &&
+                                   match_func_rec(&xe->xdf2, xecfg, i2,
+                                                  dummy, sizeof(dummy)) >= 0)
+                                       goto post_context_calculation;
+
+                               /*
+                                * Otherwise get more context from the
+                                * pre-image.
+                                */
+                               i1 = xe->xdf1.nrec - 1;
+                       }
+
+                       fs1 = get_func_line(xe, xecfg, NULL, i1, -1);
                        if (fs1 < 0)
                                fs1 = 0;
                        if (fs1 < s1) {
@@ -173,7 +219,7 @@ int xdl_emit_diff(xdfenv_t *xe, xdchange_t *xscr, xdemitcb_t *ecb,
                        }
                }
 
again:
post_context_calculation:
                lctx = xecfg->ctxlen;
                lctx = XDL_MIN(lctx, xe->xdf1.nrec - (xche->i1 + xche->chg1));
                lctx = XDL_MIN(lctx, xe->xdf2.nrec - (xche->i2 + xche->chg2));
@@ -185,6 +231,8 @@ int xdl_emit_diff(xdfenv_t *xe, xdchange_t *xscr, xdemitcb_t *ecb,
                        long fe1 = get_func_line(xe, xecfg, NULL,
                                                 xche->i1 + xche->chg1,
                                                 xe->xdf1.nrec);
+                       while (fe1 > 0 && is_empty_rec(&xe->xdf1, fe1 - 1))
+                               fe1--;
                        if (fe1 < 0)
                                fe1 = xe->xdf1.nrec;
                        if (fe1 > e1) {
@@ -198,11 +246,12 @@ int xdl_emit_diff(xdfenv_t *xe, xdchange_t *xscr, xdemitcb_t *ecb,
                         * its new end.
                         */
                        if (xche->next) {
-                               long l = xche->next->i1;
+                               long l = XDL_MIN(xche->next->i1,
+                                                xe->xdf1.nrec - 1);
                                if (l <= e1 ||
                                    get_func_line(xe, xecfg, NULL, l, e1) < 0) {
                                        xche = xche->next;
-                                       goto again;
+                                       goto post_context_calculation;
                                }
                        }
                }