[PATCH] grep: add option to show whole function as context

[PATCH] grep: add option to show whole function as context

[René Scharfe]

Add a new option, -S, to show the whole surrounding function of a match.

It uses the same regular expressions as -p and diff to find the beginning
of sections.

Currently it will not display comments in front of a function, but those
that are following one.  Despite this shortcoming it is already useful,
e.g. to simply see a more complete applicable context or to extract whole
functions.

Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>
---
Extending userdiff to allow comments to be recognized would be a bit more
involved, especially since we'd need to able to handle nested multi-line
comment blocks.

 Documentation/git-grep.txt |    6 ++++++
 builtin/grep.c             |    5 ++++-
 grep.c                     |   32 ++++++++++++++++++++++----------
 grep.h                     |    1 +
 t/t7810-grep.sh            |   14 ++++++++++++++
 5 files changed, 47 insertions(+), 11 deletions(-)

diff --git a/Documentation/git-grep.txt b/Documentation/git-grep.txt
index 07b3c6a..78e6fc4 100644
--- a/Documentation/git-grep.txt
+++ b/Documentation/git-grep.txt
@@ -172,6 +172,12 @@ OPTIONS
 	patch hunk headers (see 'Defining a custom hunk-header' in
 	linkgit:gitattributes[5]).
 
+-S::
+	Show the surrounding text from the previous line containing a
+	function name up to the one before the next function name,
+	effectively showing the whole function in which the match was
+	found.
+
 -f <file>::
 	Read patterns from <file>, one per line.
 
diff --git a/builtin/grep.c b/builtin/grep.c
index cccf8da..fc50859 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -838,6 +838,8 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
 			context_callback),
 		OPT_BOOLEAN('p', "show-function", &opt.funcname,
 			"show a line with the function name before matches"),
+		OPT_BOOLEAN('S', NULL, &opt.funcbody,
+			"show the surrounding function"),
 		OPT_GROUP(""),
 		OPT_CALLBACK('f', NULL, &opt, "file",
 			"read patterns from file", file_callback),
@@ -980,7 +982,8 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
 		use_threads = 0;
 
 	if (use_threads) {
-		if (opt.pre_context || opt.post_context || opt.file_break)
+		if (opt.pre_context || opt.post_context || opt.file_break ||
+		    opt.funcbody)
 			skip_first_line = 1;
 		start_threads(&opt);
 	}
diff --git a/grep.c b/grep.c
index 04e9ba4..26e8d8e 100644
--- a/grep.c
+++ b/grep.c
@@ -724,7 +724,7 @@ static void show_line(struct grep_opt *opt, char *bol, char *eol,
 	if (opt->file_break && opt->last_shown == 0) {
 		if (opt->show_hunk_mark)
 			opt->output(opt, "\n", 1);
-	} else if (opt->pre_context || opt->post_context) {
+	} else if (opt->pre_context || opt->post_context || opt->funcbody) {
 		if (opt->last_shown == 0) {
 			if (opt->show_hunk_mark) {
 				output_color(opt, "--", 2, opt->color_sep);
@@ -819,10 +819,13 @@ static void show_funcname_line(struct grep_opt *opt, const char *name,
 }
 
 static void show_pre_context(struct grep_opt *opt, const char *name, char *buf,
-			     char *bol, unsigned lno)
+			     char *bol, char *end, unsigned lno)
 {
 	unsigned cur = lno, from = 1, funcname_lno = 0;
-	int funcname_needed = opt->funcname;
+	int funcname_needed = !!opt->funcname;
+
+	if (opt->funcbody && !match_funcname(opt, bol, end))
+		funcname_needed = 2;
 
 	if (opt->pre_context < lno)
 		from = lno - opt->pre_context;
@@ -830,7 +833,8 @@ static void show_pre_context(struct grep_opt *opt, const char *name, char *buf,
 		from = opt->last_shown + 1;
 
 	/* Rewind. */
-	while (bol > buf && cur > from) {
+	while (bol > buf &&
+	       cur > (funcname_needed == 2 ? opt->last_shown + 1 : from)) {
 		char *eol = --bol;
 
 		while (bol > buf && bol[-1] != '\n')
@@ -942,13 +946,15 @@ static int grep_buffer_1(struct grep_opt *opt, const char *name,
 	int binary_match_only = 0;
 	unsigned count = 0;
 	int try_lookahead = 0;
+	int show_function = 0;
 	enum grep_context ctx = GREP_CONTEXT_HEAD;
 	xdemitconf_t xecfg;
 
 	if (!opt->output)
 		opt->output = std_output;
 
-	if (opt->pre_context || opt->post_context || opt->file_break) {
+	if (opt->pre_context || opt->post_context || opt->file_break ||
+	    opt->funcbody) {
 		/* Show hunk marks, except for the first file. */
 		if (opt->last_shown)
 			opt->show_hunk_mark = 1;
@@ -1004,7 +1010,8 @@ static int grep_buffer_1(struct grep_opt *opt, const char *name,
 		 */
 		if (try_lookahead
 		    && !(last_hit
-			 && lno <= last_hit + opt->post_context)
+			 && (show_function ||
+			     lno <= last_hit + opt->post_context))
 		    && look_ahead(opt, &left, &lno, &bol))
 			break;
 		eol = end_of_line(bol, &left);
@@ -1051,15 +1058,20 @@ static int grep_buffer_1(struct grep_opt *opt, const char *name,
 			/* Hit at this line.  If we haven't shown the
 			 * pre-context lines, we would need to show them.
 			 */
-			if (opt->pre_context)
-				show_pre_context(opt, name, buf, bol, lno);
+			if (opt->pre_context || opt->funcbody)
+				show_pre_context(opt, name, buf, bol, eol, lno);
 			else if (opt->funcname)
 				show_funcname_line(opt, name, buf, bol, lno);
 			show_line(opt, bol, eol, name, lno, ':');
 			last_hit = lno;
+			if (opt->funcbody)
+				show_function = 1;
+			goto next_line;
 		}
-		else if (last_hit &&
-			 lno <= last_hit + opt->post_context) {
+		if (show_function && match_funcname(opt, bol, eol))
+			show_function = 0;
+		if (show_function ||
+		    (last_hit && lno <= last_hit + opt->post_context)) {
 			/* If the last hit is within the post context,
 			 * we need to show this line.
 			 */
diff --git a/grep.h b/grep.h
index c568297..ae50c45 100644
--- a/grep.h
+++ b/grep.h
@@ -98,6 +98,7 @@ struct grep_opt {
 	int color;
 	int max_depth;
 	int funcname;
+	int funcbody;
 	char color_context[COLOR_MAXLEN];
 	char color_filename[COLOR_MAXLEN];
 	char color_function[COLOR_MAXLEN];
diff --git a/t/t7810-grep.sh b/t/t7810-grep.sh
index a29ae45..3f127ac 100755
--- a/t/t7810-grep.sh
+++ b/t/t7810-grep.sh
@@ -509,6 +509,20 @@ test_expect_success 'grep -p -B5' '
 	test_cmp expected actual
 '
 
+cat >expected <<EOF
+hello.c=int main(int argc, const char **argv)
+hello.c-{
+hello.c-	printf("Hello world.\n");
+hello.c:	return 0;
+hello.c-	/* char ?? */
+hello.c-}
+EOF
+
+test_expect_success 'grep -S' '
+	git grep -S return >actual &&
+	test_cmp expected actual
+'
+
 test_expect_success 'grep from a subdirectory to search wider area (1)' '
 	mkdir -p s &&
 	(
-- 
1.7.6

Re: [PATCH] grep: add option to show whole function as context

[Sverre Rabbelier]

Heya,

On Sun, Jul 31, 2011 at 17:59, René Scharfe <rene.scharfe@lsrfire.ath.cx> wrote:
> Add a new option, -S, to show the whole surrounding function of a match.

Perhaps a different option name so as not to conflict with the -S from
'git log'? It would be nice to have this available in 'git log -p'.

-- 
Cheers,

Sverre Rabbelier

Re: [PATCH] grep: add option to show whole function as context

[René Scharfe]

Am 31.07.2011 19:34, schrieb Sverre Rabbelier:
> On Sun, Jul 31, 2011 at 17:59, René Scharfe <rene.scharfe@lsrfire.ath.cx> wrote:
>> Add a new option, -S, to show the whole surrounding function of a match.
> 
> Perhaps a different option name so as not to conflict with the -S from
> 'git log'? It would be nice to have this available in 'git log -p'.

Do you mean a diff option to show whole affected functions as context
instead of the fixed number of context lines -U gives you?  That sounds
useful as well.

How about -W?  It's not used by GNU grep either.

René

Re: [PATCH] grep: add option to show whole function as context

[Sverre Rabbelier]

Heya,

On Mon, Aug 1, 2011 at 17:37, René Scharfe <rene.scharfe@lsrfire.ath.cx> wrote:
> Do you mean a diff option to show whole affected functions as context
> instead of the fixed number of context lines -U gives you?  That sounds
> useful as well.

Exactly.

> How about -W?  It's not used by GNU grep either.

Why not give it an easy-to-remember long name instead?
--show-function-context or such :)

-- 
Cheers,

Sverre Rabbelier

Re: [PATCH] grep: add option to show whole function as context

[René Scharfe]

Am 01.08.2011 17:39, schrieb Sverre Rabbelier:
>> How about -W?  It's not used by GNU grep either.
> 
> Why not give it an easy-to-remember long name instead?
> --show-function-context or such :)

Because I tend to forget them. ;)

René

[PATCH v2 1/2] grep: add option to show whole function as context

[René Scharfe]

Add a new option, -W, to show the whole surrounding function of a match.

It uses the same regular expressions as -p and diff to find the beginning
of sections.

Currently it will not display comments in front of a function, but those
that are following one.  Despite this shortcoming it is already useful,
e.g. to simply see a more complete applicable context or to extract whole
functions.

Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>
---
Changes since v1: use -W instead of -S to avoid clash with git log -S
and added second patch to add long options as well.

 Documentation/git-grep.txt |    6 ++++++
 builtin/grep.c             |    5 ++++-
 grep.c                     |   32 ++++++++++++++++++++++----------
 grep.h                     |    1 +
 t/t7810-grep.sh            |   14 ++++++++++++++
 5 files changed, 47 insertions(+), 11 deletions(-)

diff --git a/Documentation/git-grep.txt b/Documentation/git-grep.txt
index 07b3c6a..6cd0c50 100644
--- a/Documentation/git-grep.txt
+++ b/Documentation/git-grep.txt
@@ -172,6 +172,12 @@ OPTIONS
 	patch hunk headers (see 'Defining a custom hunk-header' in
 	linkgit:gitattributes[5]).
 
+-W::
+	Show the surrounding text from the previous line containing a
+	function name up to the one before the next function name,
+	effectively showing the whole function in which the match was
+	found.
+
 -f <file>::
 	Read patterns from <file>, one per line.
 
diff --git a/builtin/grep.c b/builtin/grep.c
index cccf8da..1fae662 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -838,6 +838,8 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
 			context_callback),
 		OPT_BOOLEAN('p', "show-function", &opt.funcname,
 			"show a line with the function name before matches"),
+		OPT_BOOLEAN('W', NULL, &opt.funcbody,
+			"show the surrounding function"),
 		OPT_GROUP(""),
 		OPT_CALLBACK('f', NULL, &opt, "file",
 			"read patterns from file", file_callback),
@@ -980,7 +982,8 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
 		use_threads = 0;
 
 	if (use_threads) {
-		if (opt.pre_context || opt.post_context || opt.file_break)
+		if (opt.pre_context || opt.post_context || opt.file_break ||
+		    opt.funcbody)
 			skip_first_line = 1;
 		start_threads(&opt);
 	}
diff --git a/grep.c b/grep.c
index 04e9ba4..26e8d8e 100644
--- a/grep.c
+++ b/grep.c
@@ -724,7 +724,7 @@ static void show_line(struct grep_opt *opt, char *bol, char *eol,
 	if (opt->file_break && opt->last_shown == 0) {
 		if (opt->show_hunk_mark)
 			opt->output(opt, "\n", 1);
-	} else if (opt->pre_context || opt->post_context) {
+	} else if (opt->pre_context || opt->post_context || opt->funcbody) {
 		if (opt->last_shown == 0) {
 			if (opt->show_hunk_mark) {
 				output_color(opt, "--", 2, opt->color_sep);
@@ -819,10 +819,13 @@ static void show_funcname_line(struct grep_opt *opt, const char *name,
 }
 
 static void show_pre_context(struct grep_opt *opt, const char *name, char *buf,
-			     char *bol, unsigned lno)
+			     char *bol, char *end, unsigned lno)
 {
 	unsigned cur = lno, from = 1, funcname_lno = 0;
-	int funcname_needed = opt->funcname;
+	int funcname_needed = !!opt->funcname;
+
+	if (opt->funcbody && !match_funcname(opt, bol, end))
+		funcname_needed = 2;
 
 	if (opt->pre_context < lno)
 		from = lno - opt->pre_context;
@@ -830,7 +833,8 @@ static void show_pre_context(struct grep_opt *opt, const char *name, char *buf,
 		from = opt->last_shown + 1;
 
 	/* Rewind. */
-	while (bol > buf && cur > from) {
+	while (bol > buf &&
+	       cur > (funcname_needed == 2 ? opt->last_shown + 1 : from)) {
 		char *eol = --bol;
 
 		while (bol > buf && bol[-1] != '\n')
@@ -942,13 +946,15 @@ static int grep_buffer_1(struct grep_opt *opt, const char *name,
 	int binary_match_only = 0;
 	unsigned count = 0;
 	int try_lookahead = 0;
+	int show_function = 0;
 	enum grep_context ctx = GREP_CONTEXT_HEAD;
 	xdemitconf_t xecfg;
 
 	if (!opt->output)
 		opt->output = std_output;
 
-	if (opt->pre_context || opt->post_context || opt->file_break) {
+	if (opt->pre_context || opt->post_context || opt->file_break ||
+	    opt->funcbody) {
 		/* Show hunk marks, except for the first file. */
 		if (opt->last_shown)
 			opt->show_hunk_mark = 1;
@@ -1004,7 +1010,8 @@ static int grep_buffer_1(struct grep_opt *opt, const char *name,
 		 */
 		if (try_lookahead
 		    && !(last_hit
-			 && lno <= last_hit + opt->post_context)
+			 && (show_function ||
+			     lno <= last_hit + opt->post_context))
 		    && look_ahead(opt, &left, &lno, &bol))
 			break;
 		eol = end_of_line(bol, &left);
@@ -1051,15 +1058,20 @@ static int grep_buffer_1(struct grep_opt *opt, const char *name,
 			/* Hit at this line.  If we haven't shown the
 			 * pre-context lines, we would need to show them.
 			 */
-			if (opt->pre_context)
-				show_pre_context(opt, name, buf, bol, lno);
+			if (opt->pre_context || opt->funcbody)
+				show_pre_context(opt, name, buf, bol, eol, lno);
 			else if (opt->funcname)
 				show_funcname_line(opt, name, buf, bol, lno);
 			show_line(opt, bol, eol, name, lno, ':');
 			last_hit = lno;
+			if (opt->funcbody)
+				show_function = 1;
+			goto next_line;
 		}
-		else if (last_hit &&
-			 lno <= last_hit + opt->post_context) {
+		if (show_function && match_funcname(opt, bol, eol))
+			show_function = 0;
+		if (show_function ||
+		    (last_hit && lno <= last_hit + opt->post_context)) {
 			/* If the last hit is within the post context,
 			 * we need to show this line.
 			 */
diff --git a/grep.h b/grep.h
index c568297..ae50c45 100644
--- a/grep.h
+++ b/grep.h
@@ -98,6 +98,7 @@ struct grep_opt {
 	int color;
 	int max_depth;
 	int funcname;
+	int funcbody;
 	char color_context[COLOR_MAXLEN];
 	char color_filename[COLOR_MAXLEN];
 	char color_function[COLOR_MAXLEN];
diff --git a/t/t7810-grep.sh b/t/t7810-grep.sh
index a29ae45..0d60016 100755
--- a/t/t7810-grep.sh
+++ b/t/t7810-grep.sh
@@ -509,6 +509,20 @@ test_expect_success 'grep -p -B5' '
 	test_cmp expected actual
 '
 
+cat >expected <<EOF
+hello.c=int main(int argc, const char **argv)
+hello.c-{
+hello.c-	printf("Hello world.\n");
+hello.c:	return 0;
+hello.c-	/* char ?? */
+hello.c-}
+EOF
+
+test_expect_success 'grep -W' '
+	git grep -W return >actual &&
+	test_cmp expected actual
+'
+
 test_expect_success 'grep from a subdirectory to search wider area (1)' '
 	mkdir -p s &&
 	(
-- 
1.7.6

[PATCH v2 2/2] grep: long context options

[René Scharfe]

Take long option names for -A (--after-context), -B (--before-context)
and -C (--context) from GNU grep and add a similar long option name
for -W (--function-context).

Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>
---
 Documentation/git-grep.txt |   26 +++++++++++++++++---------
 builtin/grep.c             |    8 ++++----
 2 files changed, 21 insertions(+), 13 deletions(-)

diff --git a/Documentation/git-grep.txt b/Documentation/git-grep.txt
index 6cd0c50..af501d2 100644
--- a/Documentation/git-grep.txt
+++ b/Documentation/git-grep.txt
@@ -155,15 +155,6 @@ OPTIONS
 	Show the filename above the matches in that file instead of
 	at the start of each shown line.
 
--[ABC] <context>::
-	Show `context` trailing (`A` -- after), or leading (`B`
-	-- before), or both (`C` -- context) lines, and place a
-	line containing `--` between contiguous groups of
-	matches.
-
--<num>::
-	A shortcut for specifying `-C<num>`.
-
 -p::
 --show-function::
 	Show the preceding line that contains the function name of
@@ -172,7 +163,24 @@ OPTIONS
 	patch hunk headers (see 'Defining a custom hunk-header' in
 	linkgit:gitattributes[5]).
 
+-<num>::
+-C <num>::
+--context <num>::
+	Show <num> leading and trailing lines, and place a line
+	containing `--` between contiguous groups of matches.
+
+-A <num>::
+--after-context <num>::
+	Show <num> trailing lines, and place a line containing
+	`--` between contiguous groups of matches.
+
+-B <num>::
+--before-context <num>::
+	Show <num> leading lines, and place a line containing
+	`--` between contiguous groups of matches.
+
 -W::
+--function-context::
 	Show the surrounding text from the previous line containing a
 	function name up to the one before the next function name,
 	effectively showing the whole function in which the match was
diff --git a/builtin/grep.c b/builtin/grep.c
index 1fae662..1851797 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -827,18 +827,18 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
 		OPT_BOOLEAN(0, "heading", &opt.heading,
 			"show filename only once above matches from same file"),
 		OPT_GROUP(""),
-		OPT_CALLBACK('C', NULL, &opt, "n",
+		OPT_CALLBACK('C', "context", &opt, "n",
 			"show <n> context lines before and after matches",
 			context_callback),
-		OPT_INTEGER('B', NULL, &opt.pre_context,
+		OPT_INTEGER('B', "before-context", &opt.pre_context,
 			"show <n> context lines before matches"),
-		OPT_INTEGER('A', NULL, &opt.post_context,
+		OPT_INTEGER('A', "after-context", &opt.post_context,
 			"show <n> context lines after matches"),
 		OPT_NUMBER_CALLBACK(&opt, "shortcut for -C NUM",
 			context_callback),
 		OPT_BOOLEAN('p', "show-function", &opt.funcname,
 			"show a line with the function name before matches"),
-		OPT_BOOLEAN('W', NULL, &opt.funcbody,
+		OPT_BOOLEAN('W', "function-context", &opt.funcbody,
 			"show the surrounding function"),
 		OPT_GROUP(""),
 		OPT_CALLBACK('f', NULL, &opt, "file",
-- 
1.7.6

Re: [PATCH v2 1/2] grep: add option to show whole function as context

[Junio C Hamano]

René Scharfe <rene.scharfe@lsrfire.ath.cx> writes:

> Add a new option, -W, to show the whole surrounding function of a match.

Thanks, will queue both patches.

It feels somewhat dirty to take the range between the previous "funcname"
and the next "funcname" and consider it the whole function, as if there is
nothing outside the function, though. I certainly understand that this is
a natural and unfortunate consequence that "funcname" is a mechanism
designed to mark only the _beginning_, and we didn't have any need for a
mechanism to mark the _end_.

I am not complaining; just making an observation.  I do not offhand have a
suggestion for improving this, and I think the obvious "then let's come up
with another configuration to mark the end" is not an improvement but
making things worse, so...

Re: [PATCH v2 2/2] grep: long context options

[Tait]

René Scharfe <rene.scharfe_lsrfire.ath.cx> said (on 2011/08/01):
> Take long option names for -A (--after-context), -B (--before-context)
> and -C (--context) from GNU grep and add a similar long option name
> for -W (--function-context).

Why not just add --context=function? Then when I want --context=indent
to give context based on the indent-level, it is an intuitive extension
of the existing options. (Of course, --context=<number> would still do
exactly what it does now.)

Re: [PATCH v2 2/2] grep: long context options

[René Scharfe]

Am 02.08.2011 03:01, schrieb Tait:
> René Scharfe <rene.scharfe_lsrfire.ath.cx> said (on 2011/08/01):
>> Take long option names for -A (--after-context), -B (--before-context)
>> and -C (--context) from GNU grep and add a similar long option name
>> for -W (--function-context).
> 
> Why not just add --context=function? Then when I want --context=indent
> to give context based on the indent-level, it is an intuitive extension
> of the existing options. (Of course, --context=<number> would still do
> exactly what it does now.)

With the current patches, you can use --function-context together with
--context=<num> to specify a minimum number of context lines to show,
even beyond function boundaries.

I'd expect a --context=function option to be equivalent to -W -C0 and
--context=<num> to be equivalent to -C<num> --no-function-context, so
the syntax would be limited in that regard (or be unintuitive to me).

We can still add it on top, though.

René

Re: [PATCH v2 1/2] grep: add option to show whole function as context

[René Scharfe]

Am 02.08.2011 01:17, schrieb Junio C Hamano:
> René Scharfe <rene.scharfe@lsrfire.ath.cx> writes:
> 
>> Add a new option, -W, to show the whole surrounding function of a match.
> 
> Thanks, will queue both patches.
> 
> It feels somewhat dirty to take the range between the previous "funcname"
> and the next "funcname" and consider it the whole function, as if there is
> nothing outside the function, though. I certainly understand that this is
> a natural and unfortunate consequence that "funcname" is a mechanism
> designed to mark only the _beginning_, and we didn't have any need for a
> mechanism to mark the _end_.
> 
> I am not complaining; just making an observation.  I do not offhand have a
> suggestion for improving this, and I think the obvious "then let's come up
> with another configuration to mark the end" is not an improvement but
> making things worse, so...

A certain amount of dirtyness is unavoidable unless we use a real
parser.  Apropos, labels are function boundaries as well, so there we
have another limitation (i.e. we won't get a function whole if it
contains labels without leading whitespace).

Ideally I'd like to see the whole function text up to the closing brace
but including any leading comments, if present.  Which is even more
complicated, of course.

For C-style and Shell files, /^}/ would probably suffice as a regex to
find function endings..

First I'm curious to see how useful the simple start-to-start heuristic
is, though.  Even the -[ABC] options are kind of fuzzy -- how do you
know the number is high enough before running grep?  Let's see how far
this quick and dirty approach to present a smarter context can take us.

René