From eced93bcb8647ad7dad6511e0d58a4dad470f473 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Scharfe?= Date: Sat, 18 Nov 2017 19:04:00 +0100 Subject: [PATCH 1/6] t4051: add test for comments preceding function lines When showing function context it would be helpful to show comments immediately before declarations, as they are most likely relevant. Add a test for that, but without specifying the choice of lines too rigidly in the test---we may want to stop before and not include "/*" in the future, for example. Signed-off-by: Rene Scharfe Signed-off-by: Junio C Hamano --- t/t4051-diff-function-context.sh | 4 ++++ t/t4051/hello.c | 3 +++ 2 files changed, 7 insertions(+) diff --git a/t/t4051-diff-function-context.sh b/t/t4051-diff-function-context.sh index 3e6b485ecb..30fc5bf2b3 100755 --- a/t/t4051-diff-function-context.sh +++ b/t/t4051-diff-function-context.sh @@ -85,6 +85,10 @@ test_expect_success 'setup' ' check_diff changed_hello 'changed function' +test_expect_failure ' context includes comment' ' + grep "^ .*Hello comment" changed_hello.diff +' + test_expect_success ' context includes begin' ' grep "^ .*Begin of hello" changed_hello.diff ' diff --git a/t/t4051/hello.c b/t/t4051/hello.c index 63b1a1e4ef..73e767e178 100644 --- a/t/t4051/hello.c +++ b/t/t4051/hello.c @@ -1,4 +1,7 @@ +/* + * Hello comment. + */ static void hello(void) // Begin of hello { /* From cde32bf62f74758d3bad685f9fef6aea4e10d106 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Scharfe?= Date: Sat, 18 Nov 2017 19:04:40 +0100 Subject: [PATCH 2/6] xdiff: factor out is_func_rec() Add a helper for checking if a given record is a function line. It frees callers from having to deal with the buffer arguments of match_func_rec(). Signed-off-by: Rene Scharfe Signed-off-by: Junio C Hamano --- xdiff/xemit.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/xdiff/xemit.c b/xdiff/xemit.c index 6881445e4a..c2d5bd004c 100644 --- a/xdiff/xemit.c +++ b/xdiff/xemit.c @@ -121,6 +121,12 @@ static long match_func_rec(xdfile_t *xdf, xdemitconf_t const *xecfg, long ri, return xecfg->find_func(rec, len, buf, sz, xecfg->find_func_priv); } +static int is_func_rec(xdfile_t *xdf, xdemitconf_t const *xecfg, long ri) +{ + char dummy[1]; + return match_func_rec(xdf, xecfg, ri, dummy, sizeof(dummy)) >= 0; +} + struct func_line { long len; char buf[80]; @@ -178,7 +184,6 @@ 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; /* @@ -186,8 +191,7 @@ int xdl_emit_diff(xdfenv_t *xe, xdchange_t *xscr, xdemitcb_t *ecb, * a whole function was added. */ while (i2 < xe->xdf2.nrec) { - if (match_func_rec(&xe->xdf2, xecfg, i2, - dummy, sizeof(dummy)) >= 0) + if (is_func_rec(&xe->xdf2, xecfg, i2)) goto post_context_calculation; i2++; } From 5c3ed90f3f56757f2af9cc327313e9df2e38cdd5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Scharfe?= Date: Sat, 18 Nov 2017 19:05:19 +0100 Subject: [PATCH 3/6] xdiff: show non-empty lines before functions with -W Non-empty lines before a function definition are most likely comments for that function and thus relevant. Include them in function context. Such a non-empty line might also belong to the preceeding function if there is no separating blank line. Stop extending the context upwards also at the next function line to make sure only one extra function body is shown at most. Original-patch-by: Vegard Nossum Signed-off-by: Rene Scharfe Signed-off-by: Junio C Hamano --- t/t4051-diff-function-context.sh | 2 +- xdiff/xemit.c | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/t/t4051-diff-function-context.sh b/t/t4051-diff-function-context.sh index 30fc5bf2b3..2d76a971c4 100755 --- a/t/t4051-diff-function-context.sh +++ b/t/t4051-diff-function-context.sh @@ -85,7 +85,7 @@ test_expect_success 'setup' ' check_diff changed_hello 'changed function' -test_expect_failure ' context includes comment' ' +test_expect_success ' context includes comment' ' grep "^ .*Hello comment" changed_hello.diff ' diff --git a/xdiff/xemit.c b/xdiff/xemit.c index c2d5bd004c..7778dc2b19 100644 --- a/xdiff/xemit.c +++ b/xdiff/xemit.c @@ -204,6 +204,9 @@ int xdl_emit_diff(xdfenv_t *xe, xdchange_t *xscr, xdemitcb_t *ecb, } fs1 = get_func_line(xe, xecfg, NULL, i1, -1); + while (fs1 > 0 && !is_empty_rec(&xe->xdf1, fs1 - 1) && + !is_func_rec(&xe->xdf1, xecfg, fs1 - 1)) + fs1--; if (fs1 < 0) fs1 = 0; if (fs1 < s1) { From 76e650d7d95662749fa5cbcc9f4faefb510334b0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Scharfe?= Date: Sat, 18 Nov 2017 19:06:16 +0100 Subject: [PATCH 4/6] t7810: improve check of -W with user-defined function lines The check for function context (-W) together with user-defined function line patterns reuses hello.c and pretends it's written in a language in which function lines contain either "printf" or a trailing curly brace. That's a bit obscure. Make the test easier to read by adding a small PowerShell script, using a simple, but meaningful expression, and separating out checks for different aspects into dedicated tests instead of simply matching the whole output byte for byte. Also include a test for showing comments before function lines like git Signed-off-by: Junio C Hamano --- t/t7810-grep.sh | 41 +++++++++++++++++++++++++++++++---------- 1 file changed, 31 insertions(+), 10 deletions(-) diff --git a/t/t7810-grep.sh b/t/t7810-grep.sh index 2a6679c2f5..632b905611 100755 --- a/t/t7810-grep.sh +++ b/t/t7810-grep.sh @@ -60,6 +60,18 @@ test_expect_success setup ' echo " line with leading space3" echo "line without leading space2" } >space && + cat >hello.ps1 <<-\EOF && + # No-op. + function dummy() {} + + # Say hello. + function hello() { + echo "Hello world." + } # hello + + # Still a no-op. + function dummy() {} + EOF git add . && test_tick && git commit -m initial @@ -766,18 +778,27 @@ test_expect_success 'grep -W shows no trailing empty lines' ' test_cmp expected actual ' -cat >expected <.gitattributes && - git grep -W return >actual && - test_cmp expected actual + git config diff.custom.xfuncname "^function .*$" && + echo "hello.ps1 diff=custom" >.gitattributes && + git grep -W echo >function-context-userdiff-actual +' + +test_expect_failure ' includes preceding comment' ' + grep "# Say hello" function-context-userdiff-actual +' + +test_expect_success ' includes function line' ' + grep "=function hello" function-context-userdiff-actual +' + +test_expect_success ' includes matching line' ' + grep ": echo" function-context-userdiff-actual +' + +test_expect_success ' includes last line of the function' ' + grep "} # hello" function-context-userdiff-actual ' for threads in $(test_seq 0 10) From 6653a01bf2dd1e2aabbcf1f83269bd75737acc94 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Scharfe?= Date: Sat, 18 Nov 2017 19:07:13 +0100 Subject: [PATCH 5/6] grep: update boundary variable for pre-context Function context can be bigger than -A/-B/-C context. To find the beginning of the combined context we search backwards. Currently we check at each loop iteration what we're looking for and determine the effective upper boundary based on that. Simplify this a bit by setting the variable "from" to the lowest unshown line number up front if we're looking for a function line and set it back to the required -B/-C context line number when we find one. This prepares the ground for the next patch; no functional change intended. Signed-off-by: Rene Scharfe Signed-off-by: Junio C Hamano --- grep.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/grep.c b/grep.c index d0b9b6cdfa..2c55d10c55 100644 --- a/grep.c +++ b/grep.c @@ -1479,20 +1479,21 @@ static void show_funcname_line(struct grep_opt *opt, struct grep_source *gs, static void show_pre_context(struct grep_opt *opt, struct grep_source *gs, char *bol, char *end, unsigned lno) { - unsigned cur = lno, from = 1, funcname_lno = 0; + unsigned cur = lno, from = 1, funcname_lno = 0, orig_from; int funcname_needed = !!opt->funcname; - if (opt->funcbody && !match_funcname(opt, gs, bol, end)) - funcname_needed = 2; - if (opt->pre_context < lno) from = lno - opt->pre_context; if (from <= opt->last_shown) from = opt->last_shown + 1; + orig_from = from; + if (opt->funcbody && !match_funcname(opt, gs, bol, end)) { + funcname_needed = 1; + from = opt->last_shown + 1; + } /* Rewind. */ - while (bol > gs->buf && - cur > (funcname_needed == 2 ? opt->last_shown + 1 : from)) { + while (bol > gs->buf && cur > from) { char *eol = --bol; while (bol > gs->buf && bol[-1] != '\n') @@ -1501,6 +1502,7 @@ static void show_pre_context(struct grep_opt *opt, struct grep_source *gs, if (funcname_needed && match_funcname(opt, gs, bol, eol)) { funcname_lno = cur; funcname_needed = 0; + from = orig_from; } } From a5dc20b0701cee53b2c37a4aa3a339b48d5bb298 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Scharfe?= Date: Sat, 18 Nov 2017 19:08:08 +0100 Subject: [PATCH 6/6] grep: show non-empty lines before functions with -W Non-empty lines before a function definition are most likely comments for that function and thus relevant. Include them in function context. Such a non-empty line might also belong to the preceding function if there is no separating blank line. Stop extending the context upwards also at the next function line to make sure only one extra function body is shown at most. Signed-off-by: Rene Scharfe Signed-off-by: Junio C Hamano --- grep.c | 27 +++++++++++++++++++++++---- t/t7810-grep.sh | 2 +- 2 files changed, 24 insertions(+), 5 deletions(-) diff --git a/grep.c b/grep.c index 2c55d10c55..a69c05edc2 100644 --- a/grep.c +++ b/grep.c @@ -1476,33 +1476,52 @@ static void show_funcname_line(struct grep_opt *opt, struct grep_source *gs, } } +static int is_empty_line(const char *bol, const char *eol); + static void show_pre_context(struct grep_opt *opt, struct grep_source *gs, char *bol, char *end, unsigned lno) { unsigned cur = lno, from = 1, funcname_lno = 0, orig_from; - int funcname_needed = !!opt->funcname; + int funcname_needed = !!opt->funcname, comment_needed = 0; if (opt->pre_context < lno) from = lno - opt->pre_context; if (from <= opt->last_shown) from = opt->last_shown + 1; orig_from = from; - if (opt->funcbody && !match_funcname(opt, gs, bol, end)) { - funcname_needed = 1; + if (opt->funcbody) { + if (match_funcname(opt, gs, bol, end)) + comment_needed = 1; + else + funcname_needed = 1; from = opt->last_shown + 1; } /* Rewind. */ while (bol > gs->buf && cur > from) { + char *next_bol = bol; char *eol = --bol; while (bol > gs->buf && bol[-1] != '\n') bol--; cur--; + if (comment_needed && (is_empty_line(bol, eol) || + match_funcname(opt, gs, bol, eol))) { + comment_needed = 0; + from = orig_from; + if (cur < from) { + cur++; + bol = next_bol; + break; + } + } if (funcname_needed && match_funcname(opt, gs, bol, eol)) { funcname_lno = cur; funcname_needed = 0; - from = orig_from; + if (opt->funcbody) + comment_needed = 1; + else + from = orig_from; } } diff --git a/t/t7810-grep.sh b/t/t7810-grep.sh index 632b905611..c02ca735b9 100755 --- a/t/t7810-grep.sh +++ b/t/t7810-grep.sh @@ -785,7 +785,7 @@ test_expect_success 'grep -W with userdiff' ' git grep -W echo >function-context-userdiff-actual ' -test_expect_failure ' includes preceding comment' ' +test_expect_success ' includes preceding comment' ' grep "# Say hello" function-context-userdiff-actual '