{"thread":{"id":"47234","subject":"[PATCH 0/6] show non-empty lines before functions with diff/grep -W","startedAt":"2017-11-18T18:01:38Z","lastAt":"2017-11-21T23:35:39Z","messageCount":15,"participants":["René Scharfe","Eric Sunshine","Junio C Hamano","Stefan Beller"],"isPatch":true,"patchVersion":1,"patchTotal":6},"messages":[{"id":"332810","messageId":"790c2344-a71e-7089-9000-f9b37a4a5cd9@web.de","threadId":"47234","inReplyTo":null,"subject":"[PATCH 0/6] show non-empty lines before functions with diff/grep -W","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2017-11-18T18:01:19Z","receivedAt":"2017-11-18T18:01:38Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"The option -W/--function-context lets git diff and git grep show the\nwhole surrounding function as context.  For the sake of simplicity and\nperformance they don't fully parse the files, but as a heuristic show\nall lines from the preceding function line to the next one.\n\nThis series refines that heuristic and extends the context to include\nany non-empty lines before the preceding function line as well.  They\nmost likely contain comments related to that function and are thus\nrelevant for reviewing diffs and search results.\n\nIdea and original implementation for git diff by Vegard Nossum:\nhttps://public-inbox.org/git/1484324112-17773-2-git-send-email-vegard.nossum@oracle.com/\n\n  t4051: add test for comments preceeding function lines\n  xdiff: factor out is_func_rec()\n  xdiff: show non-empty lines before functions with -W\n  t7810: improve check of -W with user-defined function lines\n  grep: update boundary variable for pre-context\n  grep: show non-empty lines before functions with -W\n\n grep.c                           | 35 +++++++++++++++++++++++++++-------\n t/t4051-diff-function-context.sh |  4 ++++\n t/t4051/hello.c                  |  3 +++\n t/t7810-grep.sh                  | 41 ++++++++++++++++++++++++++++++----------\n xdiff/xemit.c                    | 13 ++++++++++---\n 5 files changed, 76 insertions(+), 20 deletions(-)\n\n-- \n2.15.0\n"},{"id":"332811","messageId":"b5815f62-3ed4-58ea-6d32-b156e44cbe3f@web.de","threadId":"47234","inReplyTo":"790c2344-a71e-7089-9000-f9b37a4a5cd9@web.de","subject":"[PATCH 1/6] t4051: add test for comments preceding function lines","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2017-11-18T18:04:00Z","receivedAt":"2017-11-18T18:04:14Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"When showing function context it would be helpful to show comments\nimmediately before declarations, as they are most likely relevant.  Add\na test for that.\n\nSigned-off-by: Rene Scharfe <l.s.r@web.de>\n---\n t/t4051-diff-function-context.sh | 4 ++++\n t/t4051/hello.c                  | 3 +++\n 2 files changed, 7 insertions(+)\n\ndiff --git a/t/t4051-diff-function-context.sh b/t/t4051-diff-function-context.sh\nindex 3e6b485ecb..30fc5bf2b3 100755\n--- a/t/t4051-diff-function-context.sh\n+++ b/t/t4051-diff-function-context.sh\n@@ -85,6 +85,10 @@ test_expect_success 'setup' '\n \n check_diff changed_hello 'changed function'\n \n+test_expect_failure ' context includes comment' '\n+\tgrep \"^ .*Hello comment\" changed_hello.diff\n+'\n+\n test_expect_success ' context includes begin' '\n \tgrep \"^ .*Begin of hello\" changed_hello.diff\n '\ndiff --git a/t/t4051/hello.c b/t/t4051/hello.c\nindex 63b1a1e4ef..73e767e178 100644\n--- a/t/t4051/hello.c\n+++ b/t/t4051/hello.c\n@@ -1,4 +1,7 @@\n \n+/*\n+ * Hello comment.\n+ */\n static void hello(void)\t// Begin of hello\n {\n \t/*\n-- \n2.15.0\n"},{"id":"332812","messageId":"3a304ace-05ee-2285-1790-627aa6674cba@web.de","threadId":"47234","inReplyTo":"790c2344-a71e-7089-9000-f9b37a4a5cd9@web.de","subject":"[PATCH 3/6] xdiff: show non-empty lines before functions with -W","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2017-11-18T18:05:19Z","receivedAt":"2017-11-18T18:05:42Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Non-empty lines before a function definition are most likely comments\nfor that function and thus relevant.  Include them in function context.\n\nSuch a non-empty line might also belong to the preceeding function if\nthere is no separating blank line.  Stop extending the context upwards\nalso at the next function line to make sure only one extra function body\nis shown at most.\n\nOriginal-patch-by: Vegard Nossum <vegard.nossum@oracle.com>\nSigned-off-by: Rene Scharfe <l.s.r@web.de>\n---\n t/t4051-diff-function-context.sh | 2 +-\n xdiff/xemit.c                    | 3 +++\n 2 files changed, 4 insertions(+), 1 deletion(-)\n\ndiff --git a/t/t4051-diff-function-context.sh b/t/t4051-diff-function-context.sh\nindex 30fc5bf2b3..2d76a971c4 100755\n--- a/t/t4051-diff-function-context.sh\n+++ b/t/t4051-diff-function-context.sh\n@@ -85,7 +85,7 @@ test_expect_success 'setup' '\n \n check_diff changed_hello 'changed function'\n \n-test_expect_failure ' context includes comment' '\n+test_expect_success ' context includes comment' '\n \tgrep \"^ .*Hello comment\" changed_hello.diff\n '\n \ndiff --git a/xdiff/xemit.c b/xdiff/xemit.c\nindex c2d5bd004c..7778dc2b19 100644\n--- a/xdiff/xemit.c\n+++ b/xdiff/xemit.c\n@@ -204,6 +204,9 @@ int xdl_emit_diff(xdfenv_t *xe, xdchange_t *xscr, xdemitcb_t *ecb,\n \t\t\t}\n \n \t\t\tfs1 = get_func_line(xe, xecfg, NULL, i1, -1);\n+\t\t\twhile (fs1 > 0 && !is_empty_rec(&xe->xdf1, fs1 - 1) &&\n+\t\t\t       !is_func_rec(&xe->xdf1, xecfg, fs1 - 1))\n+\t\t\t\tfs1--;\n \t\t\tif (fs1 < 0)\n \t\t\t\tfs1 = 0;\n \t\t\tif (fs1 < s1) {\n-- \n2.15.0\n"},{"id":"332813","messageId":"265400ac-ff2a-4a3f-795c-53f0274e8c7a@web.de","threadId":"47234","inReplyTo":"790c2344-a71e-7089-9000-f9b37a4a5cd9@web.de","subject":"[PATCH 5/6] grep: update boundary variable for pre-context","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2017-11-18T18:07:13Z","receivedAt":"2017-11-18T18:07:32Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Function context can be bigger than -A/-B/-C context.  To find the\nbeginning of the combined context we search backwards.  Currently we\ncheck at each loop iteration what we're looking for and determine the\neffective upper boundary based on that.\n\nSimplify this a bit by setting the variable \"from\" to the lowest unshown\nline number up front if we're looking for a function line and set it\nback to the required -B/-C context line number when we find one.  This\nprepares the ground for the next patch; no functional change intended.\n\nSigned-off-by: Rene Scharfe <l.s.r@web.de>\n---\n grep.c | 14 ++++++++------\n 1 file changed, 8 insertions(+), 6 deletions(-)\n\ndiff --git a/grep.c b/grep.c\nindex d0b9b6cdfa..2c55d10c55 100644\n--- a/grep.c\n+++ b/grep.c\n@@ -1479,20 +1479,21 @@ static void show_funcname_line(struct grep_opt *opt, struct grep_source *gs,\n static void show_pre_context(struct grep_opt *opt, struct grep_source *gs,\n \t\t\t     char *bol, char *end, unsigned lno)\n {\n-\tunsigned cur = lno, from = 1, funcname_lno = 0;\n+\tunsigned cur = lno, from = 1, funcname_lno = 0, orig_from;\n \tint funcname_needed = !!opt->funcname;\n \n-\tif (opt->funcbody && !match_funcname(opt, gs, bol, end))\n-\t\tfuncname_needed = 2;\n-\n \tif (opt->pre_context < lno)\n \t\tfrom = lno - opt->pre_context;\n \tif (from <= opt->last_shown)\n \t\tfrom = opt->last_shown + 1;\n+\torig_from = from;\n+\tif (opt->funcbody && !match_funcname(opt, gs, bol, end)) {\n+\t\tfuncname_needed = 1;\n+\t\tfrom = opt->last_shown + 1;\n+\t}\n \n \t/* Rewind. */\n-\twhile (bol > gs->buf &&\n-\t       cur > (funcname_needed == 2 ? opt->last_shown + 1 : from)) {\n+\twhile (bol > gs->buf && cur > from) {\n \t\tchar *eol = --bol;\n \n \t\twhile (bol > gs->buf && bol[-1] != '\\n')\n@@ -1501,6 +1502,7 @@ static void show_pre_context(struct grep_opt *opt, struct grep_source *gs,\n \t\tif (funcname_needed && match_funcname(opt, gs, bol, eol)) {\n \t\t\tfuncname_lno = cur;\n \t\t\tfuncname_needed = 0;\n+\t\t\tfrom = orig_from;\n \t\t}\n \t}\n \n-- \n2.15.0\n"},{"id":"332817","messageId":"c89b1e5f-0145-ec93-c7fa-53fac1d429da@web.de","threadId":"47234","inReplyTo":"790c2344-a71e-7089-9000-f9b37a4a5cd9@web.de","subject":"[PATCH 4/6] t7810: improve check of -W with user-defined function lines","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2017-11-18T18:06:16Z","receivedAt":"2017-11-18T18:10:40Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"The check for function context (-W) together with user-defined function\nline patterns reuses hello.c and pretends it's written in a language in\nwhich function lines contain either \"printf\" or a trailing curly brace.\nThat's a bit obscure.\n\nMake the test easier to read by adding a small PowerShell script, using\na simple, but meaningful expression, and separating out checks for\ndifferent aspects into dedicated tests instead of simply matching the\nwhole output byte for byte.\n\nAlso include a test for showing comments before function lines like git\ndiff -W does, which is currently failing because that functionality is\nnot implemented for git grep, yet.\n\nSigned-off-by: Rene Scharfe <l.s.r@web.de>\n---\n t/t7810-grep.sh | 41 +++++++++++++++++++++++++++++++----------\n 1 file changed, 31 insertions(+), 10 deletions(-)\n\ndiff --git a/t/t7810-grep.sh b/t/t7810-grep.sh\nindex 2a6679c2f5..632b905611 100755\n--- a/t/t7810-grep.sh\n+++ b/t/t7810-grep.sh\n@@ -60,6 +60,18 @@ test_expect_success setup '\n \t\techo \" line with leading space3\"\n \t\techo \"line without leading space2\"\n \t} >space &&\n+\tcat >hello.ps1 <<-\\EOF &&\n+\t# No-op.\n+\tfunction dummy() {}\n+\n+\t# Say hello.\n+\tfunction hello() {\n+\t  echo \"Hello world.\"\n+\t} # hello\n+\n+\t# Still a no-op.\n+\tfunction dummy() {}\n+\tEOF\n \tgit add . &&\n \ttest_tick &&\n \tgit commit -m initial\n@@ -766,18 +778,27 @@ test_expect_success 'grep -W shows no trailing empty lines' '\n \ttest_cmp expected actual\n '\n \n-cat >expected <<EOF\n-hello.c=\tprintf(\"Hello world.\\n\");\n-hello.c:\treturn 0;\n-hello.c-\t/* char ?? */\n-EOF\n-\n test_expect_success 'grep -W with userdiff' '\n \ttest_when_finished \"rm -f .gitattributes\" &&\n-\tgit config diff.custom.xfuncname \"(printf.*|})$\" &&\n-\techo \"hello.c diff=custom\" >.gitattributes &&\n-\tgit grep -W return >actual &&\n-\ttest_cmp expected actual\n+\tgit config diff.custom.xfuncname \"^function .*$\" &&\n+\techo \"hello.ps1 diff=custom\" >.gitattributes &&\n+\tgit grep -W echo >function-context-userdiff-actual\n+'\n+\n+test_expect_failure ' includes preceding comment' '\n+\tgrep \"# Say hello\" function-context-userdiff-actual\n+'\n+\n+test_expect_success ' includes function line' '\n+\tgrep \"=function hello\" function-context-userdiff-actual\n+'\n+\n+test_expect_success ' includes matching line' '\n+\tgrep \":  echo\" function-context-userdiff-actual\n+'\n+\n+test_expect_success ' includes last line of the function' '\n+\tgrep \"} # hello\" function-context-userdiff-actual\n '\n \n for threads in $(test_seq 0 10)\n-- \n2.15.0\n"},{"id":"332818","messageId":"4efcd033-ab8d-7b0c-0c2f-8e5b5cdd5e0c@web.de","threadId":"47234","inReplyTo":"790c2344-a71e-7089-9000-f9b37a4a5cd9@web.de","subject":"[PATCH 2/6] xdiff: factor out is_func_rec()","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2017-11-18T18:04:40Z","receivedAt":"2017-11-18T18:10:44Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Add a helper for checking if a given record is a function line.  It\nfrees callers from having to deal with the buffer arguments of\nmatch_func_rec().\n\nSigned-off-by: Rene Scharfe <l.s.r@web.de>\n---\n xdiff/xemit.c | 10 +++++++---\n 1 file changed, 7 insertions(+), 3 deletions(-)\n\ndiff --git a/xdiff/xemit.c b/xdiff/xemit.c\nindex 6881445e4a..c2d5bd004c 100644\n--- a/xdiff/xemit.c\n+++ b/xdiff/xemit.c\n@@ -121,6 +121,12 @@ static long match_func_rec(xdfile_t *xdf, xdemitconf_t const *xecfg, long ri,\n \treturn xecfg->find_func(rec, len, buf, sz, xecfg->find_func_priv);\n }\n \n+static int is_func_rec(xdfile_t *xdf, xdemitconf_t const *xecfg, long ri)\n+{\n+\tchar dummy[1];\n+\treturn match_func_rec(xdf, xecfg, ri, dummy, sizeof(dummy)) >= 0;\n+}\n+\n struct func_line {\n \tlong len;\n \tchar buf[80];\n@@ -178,7 +184,6 @@ int xdl_emit_diff(xdfenv_t *xe, xdchange_t *xscr, xdemitcb_t *ecb,\n \n \t\t\t/* Appended chunk? */\n \t\t\tif (i1 >= xe->xdf1.nrec) {\n-\t\t\t\tchar dummy[1];\n \t\t\t\tlong i2 = xch->i2;\n \n \t\t\t\t/*\n@@ -186,8 +191,7 @@ int xdl_emit_diff(xdfenv_t *xe, xdchange_t *xscr, xdemitcb_t *ecb,\n \t\t\t\t * a whole function was added.\n \t\t\t\t */\n \t\t\t\twhile (i2 < xe->xdf2.nrec) {\n-\t\t\t\t\tif (match_func_rec(&xe->xdf2, xecfg, i2,\n-\t\t\t\t\t\tdummy, sizeof(dummy)) >= 0)\n+\t\t\t\t\tif (is_func_rec(&xe->xdf2, xecfg, i2))\n \t\t\t\t\t\tgoto post_context_calculation;\n \t\t\t\t\ti2++;\n \t\t\t\t}\n-- \n2.15.0\n"},{"id":"332820","messageId":"9c3462e3-2d1d-05bc-9f7d-4bc93d32e7ff@web.de","threadId":"47234","inReplyTo":"790c2344-a71e-7089-9000-f9b37a4a5cd9@web.de","subject":"[PATCH 6/6] grep: show non-empty lines before functions with -W","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2017-11-18T18:08:08Z","receivedAt":"2017-11-18T18:13:01Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Non-empty lines before a function definition are most likely comments\nfor that function and thus relevant.  Include them in function context.\n\nSuch a non-empty line might also belong to the preceding function if\nthere is no separating blank line.  Stop extending the context upwards\nalso at the next function line to make sure only one extra function body\nis shown at most.\n\nSigned-off-by: Rene Scharfe <l.s.r@web.de>\n---\n grep.c          | 27 +++++++++++++++++++++++----\n t/t7810-grep.sh |  2 +-\n 2 files changed, 24 insertions(+), 5 deletions(-)\n\ndiff --git a/grep.c b/grep.c\nindex 2c55d10c55..a69c05edc2 100644\n--- a/grep.c\n+++ b/grep.c\n@@ -1476,33 +1476,52 @@ static void show_funcname_line(struct grep_opt *opt, struct grep_source *gs,\n \t}\n }\n \n+static int is_empty_line(const char *bol, const char *eol);\n+\n static void show_pre_context(struct grep_opt *opt, struct grep_source *gs,\n \t\t\t     char *bol, char *end, unsigned lno)\n {\n \tunsigned cur = lno, from = 1, funcname_lno = 0, orig_from;\n-\tint funcname_needed = !!opt->funcname;\n+\tint funcname_needed = !!opt->funcname, comment_needed = 0;\n \n \tif (opt->pre_context < lno)\n \t\tfrom = lno - opt->pre_context;\n \tif (from <= opt->last_shown)\n \t\tfrom = opt->last_shown + 1;\n \torig_from = from;\n-\tif (opt->funcbody && !match_funcname(opt, gs, bol, end)) {\n-\t\tfuncname_needed = 1;\n+\tif (opt->funcbody) {\n+\t\tif (match_funcname(opt, gs, bol, end))\n+\t\t\tcomment_needed = 1;\n+\t\telse\n+\t\t\tfuncname_needed = 1;\n \t\tfrom = opt->last_shown + 1;\n \t}\n \n \t/* Rewind. */\n \twhile (bol > gs->buf && cur > from) {\n+\t\tchar *next_bol = bol;\n \t\tchar *eol = --bol;\n \n \t\twhile (bol > gs->buf && bol[-1] != '\\n')\n \t\t\tbol--;\n \t\tcur--;\n+\t\tif (comment_needed && (is_empty_line(bol, eol) ||\n+\t\t\t\t       match_funcname(opt, gs, bol, eol))) {\n+\t\t\tcomment_needed = 0;\n+\t\t\tfrom = orig_from;\n+\t\t\tif (cur < from) {\n+\t\t\t\tcur++;\n+\t\t\t\tbol = next_bol;\n+\t\t\t\tbreak;\n+\t\t\t}\n+\t\t}\n \t\tif (funcname_needed && match_funcname(opt, gs, bol, eol)) {\n \t\t\tfuncname_lno = cur;\n \t\t\tfuncname_needed = 0;\n-\t\t\tfrom = orig_from;\n+\t\t\tif (opt->funcbody)\n+\t\t\t\tcomment_needed = 1;\n+\t\t\telse\n+\t\t\t\tfrom = orig_from;\n \t\t}\n \t}\n \ndiff --git a/t/t7810-grep.sh b/t/t7810-grep.sh\nindex 632b905611..c02ca735b9 100755\n--- a/t/t7810-grep.sh\n+++ b/t/t7810-grep.sh\n@@ -785,7 +785,7 @@ test_expect_success 'grep -W with userdiff' '\n \tgit grep -W echo >function-context-userdiff-actual\n '\n \n-test_expect_failure ' includes preceding comment' '\n+test_expect_success ' includes preceding comment' '\n \tgrep \"# Say hello\" function-context-userdiff-actual\n '\n \n-- \n2.15.0\n"},{"id":"332837","messageId":"CAPig+cQTzBYLqHw5TRYFajTK2Uq2fE=q=h=b=VA43zdJs0_CMA@mail.gmail.com","threadId":"47234","inReplyTo":"b5815f62-3ed4-58ea-6d32-b156e44cbe3f@web.de","subject":"Re: [PATCH 1/6] t4051: add test for comments preceding function lines","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2017-11-19T01:18:17Z","receivedAt":"2017-11-19T01:25:30Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Sat, Nov 18, 2017 at 1:04 PM, René Scharfe <l.s.r@web.de> wrote:\n> When showing function context it would be helpful to show comments\n> immediately before declarations, as they are most likely relevant.  Add\n> a test for that.\n>\n> Signed-off-by: Rene Scharfe <l.s.r@web.de>\n> ---\n> diff --git a/t/t4051-diff-function-context.sh b/t/t4051-diff-function-context.sh\n> @@ -85,6 +85,10 @@ test_expect_success 'setup' '\n>> +test_expect_failure ' context includes comment' '\n> +       grep \"^ .*Hello comment\" changed_hello.diff\n> +'\n\nThis shows only that the the \"* Hello comment.\" line is in the context\nbut says nothing about the entire comment block being included (which\npatch 3/6 implements). Perhaps the test could be more thorough?\n\n> diff --git a/t/t4051/hello.c b/t/t4051/hello.c\n> index 63b1a1e4ef..73e767e178 100644\n> --- a/t/t4051/hello.c\n> +++ b/t/t4051/hello.c\n> @@ -1,4 +1,7 @@\n>\n> +/*\n> + * Hello comment.\n> + */\n>  static void hello(void)        // Begin of hello\n>  {\n"},{"id":"332844","messageId":"bde81d14-a955-4ea4-5799-2a95511d8215@web.de","threadId":"47234","inReplyTo":"CAPig+cQTzBYLqHw5TRYFajTK2Uq2fE=q=h=b=VA43zdJs0_CMA@mail.gmail.com","subject":"Re: [PATCH 1/6] t4051: add test for comments preceding function lines","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2017-11-19T10:02:49Z","receivedAt":"2017-11-19T10:03:07Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 19.11.2017 um 02:18 schrieb Eric Sunshine:\n> On Sat, Nov 18, 2017 at 1:04 PM, René Scharfe <l.s.r@web.de> wrote:\n>> When showing function context it would be helpful to show comments\n>> immediately before declarations, as they are most likely relevant.  Add\n>> a test for that.\n>>\n>> Signed-off-by: Rene Scharfe <l.s.r@web.de>\n>> ---\n>> diff --git a/t/t4051-diff-function-context.sh b/t/t4051-diff-function-context.sh\n>> @@ -85,6 +85,10 @@ test_expect_success 'setup' '\n>>> +test_expect_failure ' context includes comment' '\n>> +       grep \"^ .*Hello comment\" changed_hello.diff\n>> +'\n> \n> This shows only that the the \"* Hello comment.\" line is in the context\n> but says nothing about the entire comment block being included (which\n> patch 3/6 implements). Perhaps the test could be more thorough?\n\nYou say that as if it was a bad thing.  That line represents 33% of\nyour suggested full-comment metric, i.e. more than nothing.  But more\nimportantly it's the actual comment payload.  The leading \"/*\" line is\nincluded as a consequence of the employed heuristic, but a more\nrefined one might omit it as it doesn't actually contain any comment.\n\nNot sure if we actually want to tweak our heuristic further, but the\ntest is meant to check only if the interesting part is present and\ngive leeway regarding other implementation details.\n\n> \n>> diff --git a/t/t4051/hello.c b/t/t4051/hello.c\n>> index 63b1a1e4ef..73e767e178 100644\n>> --- a/t/t4051/hello.c\n>> +++ b/t/t4051/hello.c\n>> @@ -1,4 +1,7 @@\n>>\n>> +/*\n>> + * Hello comment.\n>> + */\n>>   static void hello(void)        // Begin of hello\n>>   {\n"},{"id":"332883","messageId":"xmqqine5okeu.fsf@gitster.mtv.corp.google.com","threadId":"47234","inReplyTo":"bde81d14-a955-4ea4-5799-2a95511d8215@web.de","subject":"Re: [PATCH 1/6] t4051: add test for comments preceding function lines","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-11-20T00:36:57Z","receivedAt":"2017-11-20T00:39:09Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"René Scharfe <l.s.r@web.de> writes:\n\n> your suggested full-comment metric, i.e. more than nothing.  But more\n> importantly it's the actual comment payload.  The leading \"/*\" line is\n> included as a consequence of the employed heuristic, but a more\n> refined one might omit it as it doesn't actually contain any comment.\n\nI am slightly in favor of than against the above reasoning, but it\nprobably deserves to be recorded somewhere more readily accessible\nthan the mailing list archive.  The title of the test \"context\n*includes* comment\" can be read to hint it by not saying that the\nprecontext shows the *entire* comment, but that is a very weak hint\nthat will be missed by anybody unaware of the reasoning behind this\ndecision.\n\n    When showing function context it would be helpful to show comments\n    immediately before declarations, as they are most likely relevant.  Add\n    a test for that.\n\n... but without specifying the choice of lines too rigidly in the\ntest---we may want to stop before and not include \"/*\" in the\nfuture, for example.\n\nperhaps?\n"},{"id":"332922","messageId":"88f46ab2-03ff-3607-c776-25f34715ceda@web.de","threadId":"47234","inReplyTo":"xmqqine5okeu.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 1/6] t4051: add test for comments preceding function lines","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2017-11-20T17:28:54Z","receivedAt":"2017-11-20T17:29:09Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 20.11.2017 um 01:36 schrieb Junio C Hamano:\n> René Scharfe <l.s.r@web.de> writes:\n> \n>> your suggested full-comment metric, i.e. more than nothing.  But more\n>> importantly it's the actual comment payload.  The leading \"/*\" line is\n>> included as a consequence of the employed heuristic, but a more\n>> refined one might omit it as it doesn't actually contain any comment.\n> \n> I am slightly in favor of than against the above reasoning, but it\n> probably deserves to be recorded somewhere more readily accessible\n> than the mailing list archive.  The title of the test \"context\n> *includes* comment\" can be read to hint it by not saying that the\n> precontext shows the *entire* comment, but that is a very weak hint\n> that will be missed by anybody unaware of the reasoning behind this\n> decision.\n> \n>      When showing function context it would be helpful to show comments\n>      immediately before declarations, as they are most likely relevant.  Add\n>      a test for that.\n> \n> ... but without specifying the choice of lines too rigidly in the\n> test---we may want to stop before and not include \"/*\" in the\n> future, for example.\n> \n> perhaps?\n\nThat's fine with me.\n\nThanks,\nRené\n"},{"id":"332944","messageId":"CAGZ79kZxiEPE4LPEo-eb4Ta4bYXVr9xQM0dFeiJk9XXKU4AR+w@mail.gmail.com","threadId":"47234","inReplyTo":"9c3462e3-2d1d-05bc-9f7d-4bc93d32e7ff@web.de","subject":"Re: [PATCH 6/6] grep: show non-empty lines before functions with -W","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-11-20T20:39:15Z","receivedAt":"2017-11-20T20:39:21Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Sat, Nov 18, 2017 at 10:08 AM, René Scharfe <l.s.r@web.de> wrote:\n> Non-empty lines before a function definition are most likely comments\n> for that function and thus relevant.  Include them in function context.\n>\n> Such a non-empty line might also belong to the preceding function if\n> there is no separating blank line.  Stop extending the context upwards\n> also at the next function line to make sure only one extra function body\n> is shown at most.\n\nCan we add another heuristic, that checks for common function body ends, e.g.\nif the preceding line contains '}' but is not commented (the line doesn't\ncontain '*/' '//', '#'), we have a strong hint that it is a function, not an\nadditional comment.\n"},{"id":"332997","messageId":"55af5442-c22a-342e-edbd-02a776228345@web.de","threadId":"47234","inReplyTo":"CAGZ79kZxiEPE4LPEo-eb4Ta4bYXVr9xQM0dFeiJk9XXKU4AR+w@mail.gmail.com","subject":"Re: [PATCH 6/6] grep: show non-empty lines before functions with -W","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2017-11-20T22:07:32Z","receivedAt":"2017-11-20T22:07:49Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 20.11.2017 um 21:39 schrieb Stefan Beller:\n> On Sat, Nov 18, 2017 at 10:08 AM, René Scharfe <l.s.r@web.de> wrote:\n>> Non-empty lines before a function definition are most likely comments\n>> for that function and thus relevant.  Include them in function context.\n>>\n>> Such a non-empty line might also belong to the preceding function if\n>> there is no separating blank line.  Stop extending the context upwards\n>> also at the next function line to make sure only one extra function body\n>> is shown at most.\n> \n> Can we add another heuristic, that checks for common function body ends, e.g.\n> if the preceding line contains '}' but is not commented (the line doesn't\n> contain '*/' '//', '#'), we have a strong hint that it is a function, not an\n> additional comment.\n\nC comments containing \"}\" as part of the text would only be shown\npartially, e.g:\n\n\t/*\n\t * Not shown because of the curly closing brace in ${PATH}.\n\t * Shown.\n\t */\n\nTwo examples in git's repo are in refs.h and sha1-lookup.c.\n\nBefore diving deeper: Is it worth it?  Does the heuristic in this series\nproduce excessive context often?  Enough to be annoying?\n\nThanks,\nRené\n"},{"id":"333031","messageId":"xmqq7euklb5d.fsf@gitster.mtv.corp.google.com","threadId":"47234","inReplyTo":"88f46ab2-03ff-3607-c776-25f34715ceda@web.de","subject":"Re: [PATCH 1/6] t4051: add test for comments preceding function lines","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-11-21T00:37:34Z","receivedAt":"2017-11-21T00:37:41Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"René Scharfe <l.s.r@web.de> writes:\n\n>> I am slightly in favor of than against the above reasoning, but it\n>> probably deserves to be recorded somewhere more readily accessible\n>> than the mailing list archive...\n>> \n>> perhaps?\n>\n> That's fine with me.\n\nAs there seems to be no other reason for updating the series, I'll\njust amend it in locally.\n\nThanks.\n"},{"id":"333195","messageId":"CAGZ79kbL-oXPTgB+tzAUEVJ7K=F44883bW1u93UYgp4kp8r=fA@mail.gmail.com","threadId":"47234","inReplyTo":"55af5442-c22a-342e-edbd-02a776228345@web.de","subject":"Re: [PATCH 6/6] grep: show non-empty lines before functions with -W","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-11-21T23:35:34Z","receivedAt":"2017-11-21T23:35:39Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Mon, Nov 20, 2017 at 2:07 PM, René Scharfe <l.s.r@web.de> wrote:\n> Am 20.11.2017 um 21:39 schrieb Stefan Beller:\n>> On Sat, Nov 18, 2017 at 10:08 AM, René Scharfe <l.s.r@web.de> wrote:\n>>> Non-empty lines before a function definition are most likely comments\n>>> for that function and thus relevant.  Include them in function context.\n>>>\n>>> Such a non-empty line might also belong to the preceding function if\n>>> there is no separating blank line.  Stop extending the context upwards\n>>> also at the next function line to make sure only one extra function body\n>>> is shown at most.\n>>\n>> Can we add another heuristic, that checks for common function body ends, e.g.\n>> if the preceding line contains '}' but is not commented (the line doesn't\n>> contain '*/' '//', '#'), we have a strong hint that it is a function, not an\n>> additional comment.\n>\n> C comments containing \"}\" as part of the text would only be shown\n> partially, e.g:\n>\n>         /*\n>          * Not shown because of the curly closing brace in ${PATH}.\n>          * Shown.\n>          */\n>\n> Two examples in git's repo are in refs.h and sha1-lookup.c.\n>\n> Before diving deeper: Is it worth it?  Does the heuristic in this series\n> produce excessive context often?  Enough to be annoying?\n\nWe'll find out... I was just spurting out my thought of the day.\nSorry for the noise.\n"}]}