jacktasia / dumb-jump

an Emacs "jump to definition" package for 50+ languages
GNU General Public License v3.0
1.57k stars 150 forks source link

Issue with git-grep #428

Open dpassen opened 1 year ago

dpassen commented 1 year ago

Recently, dumb-jump has failed to work for me when using the git-grep searcher. If I force ripgrep (setq dumb-jump-force-searcher 'rg), it works as I expect.

git 2.39.0 dumb-jump 20220620.2325 emacs 28.2

wrightgardner commented 1 year ago

I also ran into this with git 2.39.0. After setting debug logging with (setq dumb-jump-debug t) and examining the command, I played with it in a couple versions of git (the only 2 available on my system right now).

With git 2.37.1, git grep -E returns a result. With git 2.39.0, git grep -E does not return a result.

With git 2.39.0, git grep -P does return a result, but I believe that option needs to be compiled in, so it may not be universally available.

I could not find anything documenting a change in behavior for git grep, but I haven't had a lot of time to look or to examine any other versions.

wrightgardner commented 1 year ago

I had a little more time to look into this. git ships its own version of grep. Somewhere between 2.37 and 2.39, it appears that the handling of the boundary character class changed in its extended regex option. \\b works with GNU grep, but not BSD grep. Replacing \\b with \[\[:\<:\]\] (the BSD boundary), then results come back. Consulting http://mail-index.netbsd.org/tech-userlevel/2012/12/02/msg006954.html, it appears that there is not one form that works across OSs.

Forcing a searcher is ok, unless you're like me and use tramp to edit remote projects on machines that do not have that forced searcher installed. In this case, I'd like to use dumb-jump-prefer-searcher, but git grep is always used when in a git repo and not forcing a searcher.

One possible solution would be to move dumb-jump-prefer-searcher above git-grep here: https://github.com/jacktasia/dumb-jump/blob/master/dumb-jump.el#L2671. In this way, you could set dumb-jump-prefer-searcher to something you do have installed and essentially avoid the issue. However, this could cause a change in behavior for anyone who has it set now and is using git-grep over this preferred searcher.

In very limited experimentation, adding -w to the git grep command and removing \\b from the front of the search string seems to produce similar results with both git versions. I would have to look more into the existing unit tests to see if there is adequate coverage to be confident in such a change. If so, this could be a path forward.

-edited for grammar

wrightgardner commented 1 year ago

Unfortunately, it looks like \b used throughout many of the regular expressions, which would mean that the git grep -w flag won't do the trick because removing \b from the front won't be enough. You'd have to be able replace \b throughout the regular expressions.

wrightgardner commented 1 year ago

I think it may have been this commit that changed the git grep behavior: https://github.com/git/git/commit/1819ad327b7a1f19540a819813b70a0e8a7f798f

It notes changing to use the native regex library, which would account for the change from \b working to requiring BSD style word boundaries.

wrightgardner commented 1 year ago

I also tried moving dumb-jump-prefer-searcher above git grep in preference, but it finds my locally installed rg and is not operating on the remote, so it does not help in my situation with remote hosts.

gsingh93 commented 1 year ago

Not sure if it's just the boundary character. \s for whitespace doesn't seem to work either. I'm using git 2.40 on macOS.

I guess we should file a bug in the git repo?

wrightgardner commented 1 year ago

It might be worth a try, but I'm not sure if they'd consider it a bug or not. It's using the grep style native to the platform now. It just wasn't doing that on macOS before. If they're willing to take it on as it changed expected behavior (or at least the precedent set), that would be nice. It would normalize the behavior again across platforms.

Otherwise, It might be possible to detect the git version and the platform and sub the proper character classes in dumb-jump. It's already trying to detect the version and BSD/GNU for grep: https://github.com/jacktasia/dumb-jump/blob/ba4127336d897f5656032694bbe22c490ecbb000/dumb-jump.el#L1835

And also do some substitution of character classes based on gnu-grep being present: https://github.com/jacktasia/dumb-jump/blob/ba4127336d897f5656032694bbe22c490ecbb000/dumb-jump.el#L2843

wrightgardner commented 1 year ago

There are a couple of related bugs filed with git, but not specifically dealing with extended regex:

Suspected git grep regression in git 2.40.0 git bug: Perl compatible regular expressions do not work as expected

Filing a bug would at least get this on the radar as another side-effect.

wrightgardner commented 1 year ago

I did attempt to file a bug/question with git. I'll provide a link to it once I have one. It would be very nice if they fixed this on their end. Otherwise, it gets a bit complicated to detect the git version and os and sub in all the correct character classes.

wrightgardner commented 1 year ago

Conversation with Git: https://lore.kernel.org/git/xmqq355jbi6d.fsf@gitster.g/T/