Am 11.09.23 um 14:12 schrieb Oleg Nesterov:
> show_funcname_line() returns when "lno <= opt->last_shown" and this
> is not right in that the ->last_shown line (which matched the pattern)
> can also have the actual function name we need to report.
>
> Change this code to check "lno < opt->last_shown". While at it, move
> this check up to avoid the unnecessary "find the previous bol" loop.
>
> Note that --lno can't underflow, lno==0 is not possible in this loop.
>
> Simple test-case:
>
> $ cat TEST.c
> void func(void);
>
> void func1(xxx)
> {
> use1(xxx);
> }
>
> void func2(xxx)
> {
> use2(xxx);
> }
>
> $ 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.
Interesting idea to treat function lines as annotations of matches
instead of as special context. Showing lines twice feels wasteful, but
at least for -p it might be justifiable from that angle. Wouldn't you
have to repeat function line 3 before the match in line 8, though?
The reason for not repeating a matched function line was that it
doesn't add much information under the assumption that it's easy to
identify function lines visually. I assume this still holds, but
perhaps it doesn't for more complicated languages?
I have to admit that I almost never use --show-function, but instead
use the related --function-context a lot. So my practical experience
with the former option is very limited.
> grep.c | 7 +++----
> 1 file changed, 3 insertions(+), 4 deletions(-)
The patch would need to update Documentation/git-grep.txt as well to
reflect the changed output.
René
