Re: [PATCH 1/1] git-grep: improve the --show-function behaviour
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Sep 11, 2023, 22:34 UTC
- Message-ID
- <xmqq34zktk4h.fsf@gitster.g>
- In-Reply-To
- <20230911121211.GA17401@redhat.com>
Oleg Nesterov <oleg@redhat.com> writes:
Show 21 quoted lines
> $ 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.
Show 25 quoted lines
> 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, '=');