Re: [PATCH 1/1] git-grep: improve the --show-function behaviour

[Oleg Nesterov <oleg@redhat.com>]

On 09/12, Oleg Nesterov wrote:
>
> René, Junio,
>
> I don't like the fact we can't understand each other ;) Could you
> please explain why do you think this patch should update the docs?
>
> Please forget about my patch for the moment. Lets start from the very
> beginning:
>
> 	-p::
> 	--show-function::
> 		Show the preceding line that contains the function name of
> 		the match, unless the matching line is a function name itself.
>
> and in my opinion, it is the current behaviour that doesn't match the
> documentation.
>
> -------------------------------------------------------------------------
>
> 	$ cat TEST1.c
> 	void func1()
> 	{
> 	}
> 	void func2()
> 	{
> 	}
>
> 	$ git grep --untracked -pn func2 TEST1.c
> 	TEST1.c=1=void func1()
> 	TEST1.c:4:void func2()
>
> in this case the matching line is "void func2()" and it is also a function
> name itself, in this case git-grep should not show "=void func1()" which is
> "the preceding line that contains the function name of the match.
>
> But it does. So perhaps git-grep needs another change, something like
>
> 	if (match_funcname(opt, gs, bol, end_of_line(...)))
> 		return;
>
> at the start of show_funcname_line(), but my patch does not change this
> behaviour.
>
> --------------------------------------------------------------------------
>
> 	$ cat TEST2.c
> 	void func(xxx)
> 	{
> 		use(xxx);
> 	}
>
> 	$ git grep --untracked -pn xxx TEST2.c
> 	TEST2.c:1:void func(xxx)
> 	TEST2.c:3:      use(xxx)
>
> the 2nd match is use(xxx) and it is not a function name itself, in this
> case git-grep should "Show the preceding line that contains the function
> name of the match.
>
> But it doesn't. To me, this behaviour looks as
>
> 		Show the preceding line that contains the function name of
> 		the match, unless the _PREVIOUS_ matching line is a function
> 		name itself.
>
> Now, with my patch we have
>
> 	$ ./git grep --untracked -pn xxx TEST2.c
> 	TEST2.c:1:void func(xxx)
> 	TEST2.c=1=void func(xxx)
> 	TEST2.c:3:      use(xxx);
>
> and unless I am totatlly confused this does match the documentation.

So, just in case, please see V2 below. In my opinion it _fixes_ the
current behaviour. With this patch

	$ ./git grep --untracked -pn func2 TEST1.c
	TEST1.c:4:void func2()

	$ ./git grep --untracked -pn xxx TEST2.c
	TEST2.c:1:void func(xxx)
	TEST2.c=1=void func(xxx)
	TEST2.c:3:      use(xxx);

Or I am totally confused?

Oleg.
---

diff --git a/grep.c b/grep.c
index 0904d55b24..c240c4bfa1 100644
--- a/grep.c
+++ b/grep.c
@@ -1347,15 +1347,19 @@ static int match_funcname(struct grep_opt *opt, struct grep_source *gs,
 static void show_funcname_line(struct grep_opt *opt, struct grep_source *gs,
 			       const char *bol, unsigned lno)
 {
+	unsigned long left = bol - gs->buf;
+
+	if (match_funcname(opt, gs, bol, end_of_line(bol, &left)))
+		return;
+
 	while (bol > gs->buf) {
 		const char *eol = --bol;
 
+		if (--lno < opt->last_shown)
+			break;
+
 		while (bol > gs->buf && bol[-1] != '\n')
 			bol--;
-		lno--;
-
-		if (lno <= opt->last_shown)
-			break;
 
 		if (match_funcname(opt, gs, bol, eol)) {
 			show_line(opt, bol, eol, gs->name, lno, 0, '=');