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

From: Oleg Nesterov <oleg@redhat.com>
Date: Sep 11, 2023, 23:17 UTC
Message-ID: <20230911231756.GA2840@redhat.com>
In-Reply-To: <xmqq34zktk4h.fsf@gitster.g>

On 09/11, Junio C Hamano wrote:

Show 26 quoted lines

>
> Oleg Nesterov <oleg@redhat.com> writes:
>
> > 	$ git grep --untracked -pn xxx TEST.c
> >
> > before the patch:
> >
> > 	TEST.c=1=void func(void);
> > 	TEST.c:3:void func1(xxx)
> > 	TEST.c:5:       use1(xxx);
> > 	TEST.c:8:void func2(xxx)
> > 	TEST.c:10:      use2(xxx);
> >
> > after the patch:
> >
> > 	TEST.c=1=void func(void);
> > 	TEST.c:3:void func1(xxx)
> > 	TEST.c=3=void func1(xxx)
> > 	TEST.c:5:       use1(xxx);
> > 	TEST.c:8:void func2(xxx)
> > 	TEST.c=8=void func2(xxx)
> > 	TEST.c:10:      use2(xxx);
> >
> > which looks much better to me.
>
> The "better" is often subjective.

Sure. that is why I added "to me".

> The former is showing what is
> going on in the TEST.c code very clearly without wasting valuable
> vertical screen real estate, at least to me.

very clearly? As you probably understand this is subjective as well. But yes, you too added "at least to me" ;)

However, certainly this is not true when you use git-grep in scripts, please see 0/1.

Show 6 quoted lines

> If we want to adopt
> the proposed behaviour, which I would recommend against, the same
> patch should update the documentation, which currently says
>
>     Show the preceding line that contains the function name of the
>     match, unless the matching line is a function name itself.

And I still don't think this patch changes the documented behaviour. See my reply to Rene.

Again, if you do

	./git grep -pn --untracked func1 TEST.c

with this patch applied, the output is still

	TEST.c=1=void func(void);
	TEST.c:3:void func1(xxx)

which iiuc matches the documentation above.

Now,

	./git grep -pn --untracked xxx TEST.c

adds the additional

	TEST.c=3=void func1(xxx)
	...
	TEST.c=8=void func2(xxx)

but how does this contradict with the documentation above?

the matching lines are use1(xxx) and use2(xxx), there are NOT "the matching line is a function name itself".

> As René said, I think -p/--show-function is a rather less used
> option in modern Git where "--function-context", which back in
> 2944e4e6 did not exist, tend to be a much more useful option,

Well, not to me. And you know, I am a git user too ;)

> but it still is a backward
> incompatible behaviour change that needs to be handled with care.

And this is what I still don't understand.

> Thanks.

Thanks,

Oleg.