* [PATCH v2 1/8] t4051: rewrite, add more tests
2016-05-28 14:46 ` René Scharfe
@ 2016-05-28 14:57 ` René Scharfe
2016-05-29 23:55 ` Junio C Hamano
2016-05-28 14:58 ` [PATCH v2 2/8] xdiff: factor out match_func_rec() René Scharfe
` (6 subsequent siblings)
7 siblings, 1 reply; 27+ messages in thread
From: René Scharfe @ 2016-05-28 14:57 UTC (permalink / raw)
To: Junio C Hamano, git; +Cc: Ramsay Jones
Remove the tests that checked diff -W output against a fixed expected
result and replace them with more focused checks of desired properties
of the created diffs. That way we get more detailed and meaningful
diagnostics.
Store test file contents in files in a subdirectory in order to avoid
cluttering the test script with them.
Use tagged commits to store the changes to test diff -W against instead
of using changes to the worktree. Use the worktree instead to try and
apply the generated patch in order to validate it.
Document unwanted features: trailing empty lines, too much context for
appended functions, insufficient context at the end with -U0.
Signed-off-by: Rene Scharfe <l.s.r@web.de>
---
t/t4051-diff-function-context.sh | 216 +++++++++++++++++++++++++--------------
t/t4051/appended1.c | 15 +++
t/t4051/appended2.c | 35 +++++++
t/t4051/dummy.c | 7 ++
t/t4051/hello.c | 21 ++++
t/t4051/includes.c | 20 ++++
6 files changed, 240 insertions(+), 74 deletions(-)
create mode 100644 t/t4051/appended1.c
create mode 100644 t/t4051/appended2.c
create mode 100644 t/t4051/dummy.c
create mode 100644 t/t4051/hello.c
create mode 100644 t/t4051/includes.c
diff --git a/t/t4051-diff-function-context.sh b/t/t4051-diff-function-context.sh
index 001d678..a3adba4 100755
--- a/t/t4051-diff-function-context.sh
+++ b/t/t4051-diff-function-context.sh
@@ -3,90 +3,158 @@
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 () {
+ message=$1 &&
+ shift &&
+ git add $@ &&
+ test_tick &&
+ git commit -m $message &&
+ git tag $message
}
-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 -n '$ 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 "$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
+'
+
+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_failure ' 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_failure ' 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_failure ' 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_failure ' 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_failure ' 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)" != " "
'
test_done
diff --git a/t/t4051/appended1.c b/t/t4051/appended1.c
new file mode 100644
index 0000000..a9f56f1
--- /dev/null
+++ b/t/t4051/appended1.c
@@ -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
index 0000000..e651f71
--- /dev/null
+++ b/t/t4051/appended2.c
@@ -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
index 0000000..a43016e
--- /dev/null
+++ b/t/t4051/dummy.c
@@ -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
index 0000000..63b1a1e
--- /dev/null
+++ b/t/t4051/hello.c
@@ -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
index 0000000..efc68f8
--- /dev/null
+++ b/t/t4051/includes.c
@@ -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>
--
2.8.3
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCH v2 1/8] t4051: rewrite, add more tests
2016-05-28 14:57 ` [PATCH v2 1/8] t4051: rewrite, add more tests René Scharfe
@ 2016-05-29 23:55 ` Junio C Hamano
2016-05-30 20:55 ` René Scharfe
2016-05-31 20:00 ` [PATCH v2.5 " René Scharfe
0 siblings, 2 replies; 27+ messages in thread
From: Junio C Hamano @ 2016-05-29 23:55 UTC (permalink / raw)
To: René Scharfe; +Cc: git, Ramsay Jones
René Scharfe <l.s.r@web.de> writes:
> +commit_and_tag () {
> + message=$1 &&
> + shift &&
> + git add $@ &&
Lack of dq around $@ makes me wonder if there is something funny
going on (looking at the callers, there isn't, so we'd better quote
it to avoid wasting time, I think).
> + test_tick &&
> + git commit -m $message &&
> + git tag $message
> }
The use of $message as the sole argument to "git tag" makes the
readers guess that it must be a single token without any funny
character, so the readers would probably do not waste too much time
wondreing if the lack of dq around $message in the last two is
problematic.
> +last_context_line () {
> + sed -n '$ p'
> }
I have a vague recollection that some implementations of sed are
unhappy to see that space between the address and the operation; I'd
feel safer without it.
> +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^" &&
With the presence of ^ there, --detach is unnecessary; it would not
hurt, though.
> + git apply "$name.diff" &&
> + git diff --exit-code "$name"
Even though we may know that $name.diff" will never have a creation
of new paths, I'd feel safer if "apply" is run with "--index".
Thanks.
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH v2 1/8] t4051: rewrite, add more tests
2016-05-29 23:55 ` Junio C Hamano
@ 2016-05-30 20:55 ` René Scharfe
2016-05-31 20:00 ` [PATCH v2.5 " René Scharfe
1 sibling, 0 replies; 27+ messages in thread
From: René Scharfe @ 2016-05-30 20:55 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Ramsay Jones
Am 30.05.2016 um 01:55 schrieb Junio C Hamano:
> René Scharfe <l.s.r@web.de> writes:
>
>> +commit_and_tag () {
>> + message=$1 &&
>> + shift &&
>> + git add $@ &&
>
> Lack of dq around $@ makes me wonder if there is something funny
> going on (looking at the callers, there isn't, so we'd better quote
> it to avoid wasting time, I think).
OK.
>> + test_tick &&
>> + git commit -m $message &&
>> + git tag $message
>> }
>
> The use of $message as the sole argument to "git tag" makes the
> readers guess that it must be a single token without any funny
> character, so the readers would probably do not waste too much time
> wondreing if the lack of dq around $message in the last two is
> problematic.
Well, let's call it $tag; $message is a bit misleading here. The saved
letters can be invested in quotes. ;)
>> +last_context_line () {
>> + sed -n '$ p'
>> }
>
> I have a vague recollection that some implementations of sed are
> unhappy to see that space between the address and the operation; I'd
> feel safer without it.
Indeed most sed calls in t/ have no space there (found counter-examples
only in annotate-tests.sh, t4201-shortlog.sh, t9824-git-p4-git-lfs.sh).
>> +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^" &&
>
> With the presence of ^ there, --detach is unnecessary; it would not
> hurt, though.
Right. It's just there to make that intent clear.
>> + git apply "$name.diff" &&
>> + git diff --exit-code "$name"
>
> Even though we may know that $name.diff" will never have a creation
> of new paths, I'd feel safer if "apply" is run with "--index".
Makes sense; the less we assume about the diff to be checked the better.
Thanks a lot!
René
^ permalink raw reply [flat|nested] 27+ messages in thread* [PATCH v2.5 1/8] t4051: rewrite, add more tests
2016-05-29 23:55 ` Junio C Hamano
2016-05-30 20:55 ` René Scharfe
@ 2016-05-31 20:00 ` René Scharfe
2016-05-31 20:15 ` René Scharfe
2016-05-31 21:23 ` Junio C Hamano
1 sibling, 2 replies; 27+ messages in thread
From: René Scharfe @ 2016-05-31 20:00 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Ramsay Jones
Remove the tests that checked against a fixed result and replace them
with more focused checks of desired properties of the created diffs.
That way we get more detailed and meaningful diagnostics.
Store test file contents in files in a subdirectory in order to avoid
cluttering the test script with them.
Use tagged commits to store the changes to test diff -W against instead
of using changes to the worktree. Use the worktree instead to try and
apply the generated patch in order to validate it.
Document unwanted features: trailing empty lines, too much context for
appended functions, insufficient context at the end with -U0.
Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Rene Scharfe <l.s.r@web.de>
---
Replacement for the first patch addressing review comments, the other
seven patches are unchanged.
t/t4051-diff-function-context.sh | 216 +++++++++++++++++++++++++--------------
t/t4051/appended1.c | 15 +++
t/t4051/appended2.c | 35 +++++++
t/t4051/dummy.c | 7 ++
t/t4051/hello.c | 21 ++++
t/t4051/includes.c | 20 ++++
6 files changed, 240 insertions(+), 74 deletions(-)
create mode 100644 t/t4051/appended1.c
create mode 100644 t/t4051/appended2.c
create mode 100644 t/t4051/dummy.c
create mode 100644 t/t4051/hello.c
create mode 100644 t/t4051/includes.c
diff --git a/t/t4051-diff-function-context.sh b/t/t4051-diff-function-context.sh
index 001d678..ca01725 100755
--- a/t/t4051-diff-function-context.sh
+++ b/t/t4051-diff-function-context.sh
@@ -3,90 +3,158 @@
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
+'
+
+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_failure ' 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_failure ' 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_failure ' 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_failure ' 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_failure ' 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)" != " "
'
test_done
diff --git a/t/t4051/appended1.c b/t/t4051/appended1.c
new file mode 100644
index 0000000..a9f56f1
--- /dev/null
+++ b/t/t4051/appended1.c
@@ -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
index 0000000..e651f71
--- /dev/null
+++ b/t/t4051/appended2.c
@@ -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
index 0000000..a43016e
--- /dev/null
+++ b/t/t4051/dummy.c
@@ -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
index 0000000..63b1a1e
--- /dev/null
+++ b/t/t4051/hello.c
@@ -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
index 0000000..efc68f8
--- /dev/null
+++ b/t/t4051/includes.c
@@ -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>
--
2.8.3
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCH v2.5 1/8] t4051: rewrite, add more tests
2016-05-31 20:00 ` [PATCH v2.5 " René Scharfe
@ 2016-05-31 20:15 ` René Scharfe
2016-05-31 21:23 ` Junio C Hamano
1 sibling, 0 replies; 27+ messages in thread
From: René Scharfe @ 2016-05-31 20:15 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Ramsay Jones
Interdiff between v2 and v2.5:
- Rename $message to $tag, as that fits its purpose better, and quote it.
- Quote $@.
- Use the most common sed invocation from t/ for getting the last line of
a file, for consistency.
- Use apply --index to make sure we notice if the generated diff adds or
deletes files unintendedly.
---
t/t4051-diff-function-context.sh | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/t/t4051-diff-function-context.sh b/t/t4051-diff-function-context.sh
index 88a3308..b6bb04a 100755
--- a/t/t4051-diff-function-context.sh
+++ b/t/t4051-diff-function-context.sh
@@ -7,12 +7,12 @@ test_description='diff function context'
dir="$TEST_DIRECTORY/t4051"
commit_and_tag () {
- message=$1 &&
+ tag=$1 &&
shift &&
- git add $@ &&
+ git add "$@" &&
test_tick &&
- git commit -m $message &&
- git tag $message
+ git commit -m "$tag" &&
+ git tag "$tag"
}
first_context_line () {
@@ -23,7 +23,7 @@ first_context_line () {
}
last_context_line () {
- sed -n '$ p'
+ sed -ne \$p
}
check_diff () {
@@ -38,7 +38,7 @@ check_diff () {
test_expect_success ' diff applies' '
test_when_finished "git reset --hard" &&
git checkout --detach "$name^" &&
- git apply "$name.diff" &&
+ git apply --index "$name.diff" &&
git diff --exit-code "$name"
'
}
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCH v2.5 1/8] t4051: rewrite, add more tests
2016-05-31 20:00 ` [PATCH v2.5 " René Scharfe
2016-05-31 20:15 ` René Scharfe
@ 2016-05-31 21:23 ` Junio C Hamano
1 sibling, 0 replies; 27+ messages in thread
From: Junio C Hamano @ 2016-05-31 21:23 UTC (permalink / raw)
To: René Scharfe; +Cc: git, Ramsay Jones
Thanks.
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH v2 2/8] xdiff: factor out match_func_rec()
2016-05-28 14:46 ` René Scharfe
2016-05-28 14:57 ` [PATCH v2 1/8] t4051: rewrite, add more tests René Scharfe
@ 2016-05-28 14:58 ` René Scharfe
2016-05-28 15:00 ` [PATCH v2 3/8] xdiff: handle appended chunks better with -W René Scharfe
` (5 subsequent siblings)
7 siblings, 0 replies; 27+ messages in thread
From: René Scharfe @ 2016-05-28 14:58 UTC (permalink / raw)
To: Junio C Hamano, git; +Cc: Ramsay Jones
Add match_func_rec(), a helper that wraps accessing a record and calling
the appropriate function for checking if it contains a function line.
Signed-off-by: Rene Scharfe <l.s.r@web.de>
---
xdiff/xemit.c | 15 +++++++++++----
1 file changed, 11 insertions(+), 4 deletions(-)
diff --git a/xdiff/xemit.c b/xdiff/xemit.c
index 993724b..0c87637 100644
--- a/xdiff/xemit.c
+++ b/xdiff/xemit.c
@@ -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;
--
2.8.3
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH v2 3/8] xdiff: handle appended chunks better with -W
2016-05-28 14:46 ` René Scharfe
2016-05-28 14:57 ` [PATCH v2 1/8] t4051: rewrite, add more tests René Scharfe
2016-05-28 14:58 ` [PATCH v2 2/8] xdiff: factor out match_func_rec() René Scharfe
@ 2016-05-28 15:00 ` René Scharfe
2016-05-28 15:02 ` [PATCH v2 4/8] xdiff: ignore empty lines before added functions " René Scharfe
` (4 subsequent siblings)
7 siblings, 0 replies; 27+ messages in thread
From: René Scharfe @ 2016-05-28 15:00 UTC (permalink / raw)
To: Junio C Hamano, git; +Cc: Ramsay Jones
If lines are added at the end of a file, diff -W shows the whole file.
That's because get_func_line() only considers the pre-image and gives up
if it sees a record index beyond its end.
Consider the post-image as well to see if the added lines already make
up a full function. If it doesn't then search for the previous function
line by starting from the bottom of the pre-image, thereby avoiding to
confuse get_func_line().
Reuse the existing label called "again", as it's exactly where we need
to jump to when we're done handling the pre-context, but rename it to
"post_context_calculation" in order to document its new purpose better.
Reported-by: Junio C Hamano <gitster@pobox.com>
Initial-patch-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Rene Scharfe <l.s.r@web.de>
---
t/t4051-diff-function-context.sh | 2 +-
xdiff/xemit.c | 27 ++++++++++++++++++++++++---
2 files changed, 25 insertions(+), 4 deletions(-)
diff --git a/t/t4051-diff-function-context.sh b/t/t4051-diff-function-context.sh
index a3adba4..d78461d 100755
--- a/t/t4051-diff-function-context.sh
+++ b/t/t4051-diff-function-context.sh
@@ -131,7 +131,7 @@ test_expect_success ' context includes end' '
grep "^[+].*End of second part" extended.diff
'
-test_expect_failure ' context does not include other functions' '
+test_expect_success ' context does not include other functions' '
test $(grep -c "^[ +-].*Begin" extended.diff) -le 2
'
diff --git a/xdiff/xemit.c b/xdiff/xemit.c
index 0c87637..969100d 100644
--- a/xdiff/xemit.c
+++ b/xdiff/xemit.c
@@ -171,7 +171,28 @@ 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];
+
+ /*
+ * We don't need additional context if
+ * a whole function was added.
+ */
+ if (match_func_rec(&xe->xdf2, xecfg, xch->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) {
@@ -180,7 +201,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));
@@ -209,7 +230,7 @@ int xdl_emit_diff(xdfenv_t *xe, xdchange_t *xscr, xdemitcb_t *ecb,
if (l <= e1 ||
get_func_line(xe, xecfg, NULL, l, e1) < 0) {
xche = xche->next;
- goto again;
+ goto post_context_calculation;
}
}
}
--
2.8.3
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH v2 4/8] xdiff: ignore empty lines before added functions with -W
2016-05-28 14:46 ` René Scharfe
` (2 preceding siblings ...)
2016-05-28 15:00 ` [PATCH v2 3/8] xdiff: handle appended chunks better with -W René Scharfe
@ 2016-05-28 15:02 ` René Scharfe
2016-05-28 15:03 ` [PATCH v2 5/8] xdiff: -W: don't include common trailing empty lines in context René Scharfe
` (3 subsequent siblings)
7 siblings, 0 replies; 27+ messages in thread
From: René Scharfe @ 2016-05-28 15:02 UTC (permalink / raw)
To: Junio C Hamano, git; +Cc: Ramsay Jones
If a new function and a preceding empty line is appended, diff -W shows
the previous function in full in order to provide context for that empty
line. In most languages empty lines between sections are not
interesting in and off themselves and showing a whole extra function for
them is not what we want.
Skip empty lines when checking of the appended chunk starts with a
function line, thereby avoiding to extend the context just for them.
Helped-by: Ramsay Jones <ramsay@ramsayjones.plus.com>
Signed-off-by: Rene Scharfe <l.s.r@web.de>
---
t/t4051-diff-function-context.sh | 2 +-
xdiff/xemit.c | 22 ++++++++++++++++++++--
2 files changed, 21 insertions(+), 3 deletions(-)
diff --git a/t/t4051-diff-function-context.sh b/t/t4051-diff-function-context.sh
index d78461d..b191c655 100755
--- a/t/t4051-diff-function-context.sh
+++ b/t/t4051-diff-function-context.sh
@@ -117,7 +117,7 @@ test_expect_success ' context includes end' '
grep "^[+].*End of first part" appended.diff
'
-test_expect_failure ' context does not include other functions' '
+test_expect_success ' context does not include other functions' '
test $(grep -c "^[ +-].*Begin" appended.diff) -le 1
'
diff --git a/xdiff/xemit.c b/xdiff/xemit.c
index 969100d..29cec12 100644
--- a/xdiff/xemit.c
+++ b/xdiff/xemit.c
@@ -155,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;
@@ -176,12 +188,18 @@ int xdl_emit_diff(xdfenv_t *xe, xdchange_t *xscr, xdemitcb_t *ecb,
/* 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.
+ * a whole function was added, possibly
+ * starting with empty lines.
*/
- if (match_func_rec(&xe->xdf2, xecfg, xch->i2,
+ 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;
--
2.8.3
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH v2 5/8] xdiff: -W: don't include common trailing empty lines in context
2016-05-28 14:46 ` René Scharfe
` (3 preceding siblings ...)
2016-05-28 15:02 ` [PATCH v2 4/8] xdiff: ignore empty lines before added functions " René Scharfe
@ 2016-05-28 15:03 ` René Scharfe
2016-05-28 15:04 ` [PATCH v2 6/8] xdiff: don't trim common tail with -W René Scharfe
` (2 subsequent siblings)
7 siblings, 0 replies; 27+ messages in thread
From: René Scharfe @ 2016-05-28 15:03 UTC (permalink / raw)
To: Junio C Hamano, git; +Cc: Ramsay Jones
Empty lines between functions are shown by diff -W, as it considers them
to be part of the function preceding them. They are not interesting in
most languages. The previous patch stopped showing them in the special
case of a function added at the end of a file.
Stop extending context to those empty lines by skipping back over them
from the start of the next function.
Signed-off-by: Rene Scharfe <l.s.r@web.de>
---
t/t4051-diff-function-context.sh | 4 ++--
xdiff/xemit.c | 2 ++
2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/t/t4051-diff-function-context.sh b/t/t4051-diff-function-context.sh
index b191c655..9fe590f 100755
--- a/t/t4051-diff-function-context.sh
+++ b/t/t4051-diff-function-context.sh
@@ -85,7 +85,7 @@ test_expect_success ' context does not include preceding empty lines' '
test "$(first_context_line <changed_hello.diff)" != " "
'
-test_expect_failure ' context does not include trailing empty lines' '
+test_expect_success ' context does not include trailing empty lines' '
test "$(last_context_line <changed_hello.diff)" != " "
'
@@ -103,7 +103,7 @@ test_expect_success ' context does not include other functions' '
test $(grep -c "^[ +-].*Begin" changed_includes.diff) -le 1
'
-test_expect_failure ' context does not include trailing empty lines' '
+test_expect_success ' context does not include trailing empty lines' '
test "$(last_context_line <changed_includes.diff)" != " "
'
diff --git a/xdiff/xemit.c b/xdiff/xemit.c
index 29cec12..bfa53d3 100644
--- a/xdiff/xemit.c
+++ b/xdiff/xemit.c
@@ -231,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) {
--
2.8.3
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH v2 6/8] xdiff: don't trim common tail with -W
2016-05-28 14:46 ` René Scharfe
` (4 preceding siblings ...)
2016-05-28 15:03 ` [PATCH v2 5/8] xdiff: -W: don't include common trailing empty lines in context René Scharfe
@ 2016-05-28 15:04 ` René Scharfe
2016-05-28 15:05 ` [PATCH v2 7/8] t7810: add test for grep -W and trailing empty context lines René Scharfe
2016-05-28 15:06 ` [PATCH v2 8/8] grep: -W: don't extend context to trailing empty lines René Scharfe
7 siblings, 0 replies; 27+ messages in thread
From: René Scharfe @ 2016-05-28 15:04 UTC (permalink / raw)
To: Junio C Hamano, git; +Cc: Ramsay Jones
The function trim_common_tail() exits early if context lines are
requested. If -U0 and -W are specified together then it can still trim
context lines that might belong to a changed function. As a result
that function is shown incompletely.
Fix that by calling trim_common_tail() only if no function context or
fixed context is requested. The parameter ctx is no longer needed now;
remove it.
While at it fix an outdated comment as well.
Signed-off-by: Rene Scharfe <l.s.r@web.de>
---
t/t4051-diff-function-context.sh | 2 +-
xdiff-interface.c | 10 ++++------
2 files changed, 5 insertions(+), 7 deletions(-)
diff --git a/t/t4051-diff-function-context.sh b/t/t4051-diff-function-context.sh
index 9fe590f..88a3308 100755
--- a/t/t4051-diff-function-context.sh
+++ b/t/t4051-diff-function-context.sh
@@ -145,7 +145,7 @@ test_expect_success ' context includes begin' '
grep "^ .*Begin of first part" long_common_tail.diff
'
-test_expect_failure ' context includes end' '
+test_expect_success ' context includes end' '
grep "^ .*End of second part" long_common_tail.diff
'
diff --git a/xdiff-interface.c b/xdiff-interface.c
index 54236f2..f34ea76 100644
--- a/xdiff-interface.c
+++ b/xdiff-interface.c
@@ -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);
}
--
2.8.3
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH v2 7/8] t7810: add test for grep -W and trailing empty context lines
2016-05-28 14:46 ` René Scharfe
` (5 preceding siblings ...)
2016-05-28 15:04 ` [PATCH v2 6/8] xdiff: don't trim common tail with -W René Scharfe
@ 2016-05-28 15:05 ` René Scharfe
2016-05-28 15:06 ` [PATCH v2 8/8] grep: -W: don't extend context to trailing empty lines René Scharfe
7 siblings, 0 replies; 27+ messages in thread
From: René Scharfe @ 2016-05-28 15:05 UTC (permalink / raw)
To: Junio C Hamano, git; +Cc: Ramsay Jones
Add a test demonstrating that git grep -W prints empty lines following
the function context we're actually interested in. The modified test
file makes it necessary to adjust three unrelated test cases.
Signed-off-by: Rene Scharfe <l.s.r@web.de>
---
t/t7810-grep.sh | 19 ++++++++++++++++---
1 file changed, 16 insertions(+), 3 deletions(-)
diff --git a/t/t7810-grep.sh b/t/t7810-grep.sh
index 1e72971..26912dc 100755
--- a/t/t7810-grep.sh
+++ b/t/t7810-grep.sh
@@ -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");
@@ -741,6 +744,16 @@ test_expect_success 'grep -W' '
'
cat >expected <<EOF
+hello.c-#include <assert.h>
+hello.c:#include <stdio.h>
+EOF
+
+test_expect_failure '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;
hello.c- /* char ?? */
@@ -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");
--
2.8.3
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH v2 8/8] grep: -W: don't extend context to trailing empty lines
2016-05-28 14:46 ` René Scharfe
` (6 preceding siblings ...)
2016-05-28 15:05 ` [PATCH v2 7/8] t7810: add test for grep -W and trailing empty context lines René Scharfe
@ 2016-05-28 15:06 ` René Scharfe
7 siblings, 0 replies; 27+ messages in thread
From: René Scharfe @ 2016-05-28 15:06 UTC (permalink / raw)
To: Junio C Hamano, git; +Cc: Ramsay Jones
Empty lines between functions are shown by grep -W, as it considers them
to be part of the function preceding them. They are not interesting in
most languages. The previous patches stopped showing them for diff -W.
Stop showing empty lines trailing a function with grep -W. Grep scans
the lines of a buffer from top to bottom and prints matching lines
immediately. Thus we need to peek ahead in order to determine if an
empty line is part of a function body and worth showing or not.
Remember how far ahead we peeked in order to avoid having to do so
repeatedly when handling multiple consecutive empty lines.
Signed-off-by: Rene Scharfe <l.s.r@web.de>
---
grep.c | 28 ++++++++++++++++++++++++++--
t/t7810-grep.sh | 2 +-
2 files changed, 27 insertions(+), 3 deletions(-)
diff --git a/grep.c b/grep.c
index ec6f7ff..1e15b62 100644
--- 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,
diff --git a/t/t7810-grep.sh b/t/t7810-grep.sh
index 26912dc..960425a 100755
--- a/t/t7810-grep.sh
+++ b/t/t7810-grep.sh
@@ -748,7 +748,7 @@ hello.c-#include <assert.h>
hello.c:#include <stdio.h>
EOF
-test_expect_failure 'grep -W shows no trailing empty lines' '
+test_expect_success 'grep -W shows no trailing empty lines' '
git grep -W stdio >actual &&
test_cmp expected actual
'
--
2.8.3
^ permalink raw reply related [flat|nested] 27+ messages in thread