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

[Junio C Hamano <gitster@pobox.com>]

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.  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.  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.

which has allowed the users to depend on the current behaviour for
practically forever since the feature was introduced by 2944e4e6
(grep: add option -p/--show-function, 2009-07-02).

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, so the
fallout from such a change may be small, but it still is a backward
incompatible behaviour change that needs to be handled with care.

Thanks.

> Signed-off-by: Oleg Nesterov <oleg@redhat.com>
> ---
>  grep.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/grep.c b/grep.c
> index 0904d55b24..7cad8352f4 100644
> --- a/grep.c
> +++ b/grep.c
> @@ -1350,12 +1350,11 @@ static void show_funcname_line(struct grep_opt *opt, struct grep_source *gs,
>  	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, '=');