Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

🐛 Incorrect output when grepping in a branch with "/" and "-" in the name #1083

Closed
cffswb opened this issue May 20, 2022 · 8 comments · Fixed by #1634
Closed

🐛 Incorrect output when grepping in a branch with "/" and "-" in the name #1083

cffswb opened this issue May 20, 2022 · 8 comments · Fixed by #1634

Comments

@cffswb
Copy link

cffswb commented May 20, 2022

Context

OS: Linux
Delta revision: 427c9aa

Description

Tested in delta repository.

Run git grep alone:
$ GIT_CONFIG_GLOBAL=/dev/null git grep theme origin/gh-pages
image

Run git grep + delta:
$ GIT_CONFIG_GLOBAL=/dev/null git -c pager.grep='delta --no-gitconfig' grep theme origin/gh-pages
image
The gh-pageses become gh:pageses, and the themes aren't highlighted.

@dandavison
Copy link
Owner

Thanks @cffswb, will look into this.

By the way, until this is fixed, I just want to mention that piping rg --json into delta should never have parsing bugs (since delta receives a JSON format rather than having to parse grep output, which can't be parsed unambiguously, although I don't think we should be confusing an internal - character in a file name if that's what's happening here)

@mvf
Copy link

mvf commented Oct 11, 2022

This seems to also affect paths whose element just above the file has a - in it. Here's a shot of git grep hostmakedepends from the root of https://www.github.com/void-linux/void-packages. cyrus-sasl and d-feet are mangled:

delta2

The same command from the srcpkgs directory leaves d-feet intact, but the highlighting isn't applied anymore:

delta

This seems to happen when the path element with the - comes first and only has a single character before the first -.

@joshtriplett
Copy link
Contributor

@mvf Confirmed. Here's a reproducible example of that:

There are hits in src/zlib/configure and src/zlib-ng/configure, but notice that the latter gets printed as src/zlib:ng/configure:

image

@dandavison
Copy link
Owner

Hi all, currently the best answer here is to use rg --json to avoid any parse errors. Fundamentally the issue here is that traditional grep output is not unambiguously parseable. It would be nice if someone added JSON output to git grep. Copying from the other dup issues #1631 and #1259:

The delta manual contains an entry addressing this:

If you don't need special features of git grep, then for best results pipe rg --json output to delta: this avoids parsing ambiguities that are inevitable with the output of git grep and grep.

See the ~500 lines of unit tests here: https://github.com/dandavison/delta/blob/main/src/handlers/grep.rs#L653-L1177. If someone can improve the parsing while keeping all those tests passing (and hopefully adding a test for what you're fixing) that would be fantastic.

I'm going to close this since I have personally taken a fairly large stab at it and also implemented rg --json support for a fully robust solution.

@joshtriplett
Copy link
Contributor

@dandavison One idea for handling this unambiguously (asking first to find out if this would be an acceptable solution):

git grep emits color sequences, one of which is wrapped around the filename. delta could parse those color sequences and use them to determine when the filename ends, unambiguously. If that works, then the recommended delta configuration could include turning on color for grep. (If delta doesn't see the starting sequence at the start of the line, it could still fall back to today's ambiguous parsing.)

@dandavison
Copy link
Owner

@joshtriplett I think that's a good idea! I had thought that git would refuse to deliver the color escape codes when sending output to its pager, but it looks like it does, as can be verified with

GIT_PAGER='bat --plain -A' git grep foo

the recommended delta configuration could include turning on color for grep

It seems that git grep has color enabled by default, right?

@joshtriplett
Copy link
Contributor

joshtriplett commented Feb 24, 2024

@dandavison Yes, grep and other commands use color by default unless disabled. However, people can customize the colors/styles. So, delta might want to document what does and doesn't work.

@joshtriplett
Copy link
Contributor

@dandavison Fixed: #1634

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants