René, sorry for late reply, On 09/14, René Scharfe wrote: > > Am 13.09.23 um 11:46 schrieb Oleg Nesterov: > > > > I have another opinion. To me the 2nd "=..." marker does help to > > understand the hit location. But this doesn't matter. > > You see it as another layer of information, as an annotation, an > additional line containing meta-information. I saw them as context > lines, i.e. lines from the original file shown in the original order > without duplication, like - lines, with the only place for meta- > information being the marker character itself. Yes, > > But without my patch, in this case I get > > > > TEST.c 1 void func1(struct pid *); > > TEST.c 3 void func2(struct pid *pid) > > TEST.c 5 use1(pid); > > TEST.c 8 void func3(struct pid *pid) > > TEST.c 10 use2(pid); > > > > because the output from git-grep > > > > $ git grep --untracked -pn pid TEST.c > > TEST.c:1:void func1(struct pid *); > > TEST.c:3:void func2(struct pid *pid) > > TEST.c:5: use1(pid); > > TEST.c:8:void func3(struct pid *pid) > > TEST.c:10: use2(pid); > > > > doesn't have the "=..." markers at all. > > Sure, that's a problem. You could easily check whether a match is also > a function line according to the default heuristic Yes, but... > there are some impressive regexes in userdiff.c > and the script would have to figure out which language the file is > configured to be for Git in the first place. Yes, and this is what I'd like to avoid, I do not want to duplicate the builtin_drivers[] logic. > > in my editor without this patch, I get > > > > kernel/sys.c 224 sys_setpriority struct pid *pgrp; [...snip...] > Well, your script turns "SYSCALL_DEFINE3(setpriority, [...]" into > "sys_setpriority" etc., so it is already knows a lot about function lines. No, not a lot ;) But yes sure, I can adapt this script to the current behaviour. In fact I can even change it to not use "-p", the script can read the file backwards itself (but of course I'd prefer to not do this). Nevermind. Thanks for discussion. If I can't convince maintainers - lets forget this patch. Although I will probably keep it (and another one I had from the very begginning) for myself, it works for me. > Can we use two markers, i.e. both : and =? No idea what that might break. ... > So with the patch below this would look like this: ... > kernel/sys.c#1073#SYSCALL_DEFINE2(setpgid, pid_t, pid, pid_t, pgid) This works for me too. So please CC me if you ever push this change ;) Thanks, Oleg.
