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

Parse filename unambiguously using color escape sequences #1634

Merged
merged 2 commits into from Mar 2, 2024

Conversation

joshtriplett
Copy link
Contributor

Parse filename unambiguously using color escape sequences, to handle cases where separators like - appear in the filename.

git grep, by default, emits ANSI color escape sequences around the filename, separator, and line number. Parse these if available.

Add tests for filenames that previously failed to parse correctly.

Fixes: #1083


Before:
delta-before

After:
delta-after


This currently assumes the default colors, and will fall back to the previous parsing if any of the colors have been changed. This is an issue elsewhere in delta as well; for instance, if color.grep.match has been changed then delta won't highlight matches. I think this would be easy to fix, by parsing git's color configuration (which I implemented in anstyle-git), rendering those color sequences to ANSI escapes, and using those in the regex rather than hardcoded ones. However, I think that should be a separate PR.

Rather than indenting the entire body of the function twice, return
early if the conditions aren't met.
`git grep`, by default, emits ANSI color escape sequences around the
filename, separator, and line number. Parse these if available.

This currently assumes the default colors, and will fall back to the
previous parsing if any of the colors have been changed.

Add tests for filenames that previously failed to parse correctly.
@dandavison dandavison merged commit f7ae06a into dandavison:main Mar 2, 2024
10 of 12 checks passed
@dandavison
Copy link
Owner

dandavison commented Mar 2, 2024

Brilliant! This is a great advance. I had switched to using rg --json exclusively but I know people will want to use git grep and other greps. (And the git grep -W option is interesting; delta tries to be helpful with it in combination with --navigate, allowing jumping between search hits with n/N)

@joshtriplett joshtriplett deleted the parse-filename-using-color branch March 3, 2024 08:57
@imbrish
Copy link
Contributor

imbrish commented Mar 4, 2024

Hey @joshtriplett, thanks for this.

I was also thinking about reading custom git grep colors from .gitconfig to parse the output correctly.

Do you plan a follow up pull request for that?

@dandavison
Copy link
Owner

If anyone does work on reading colors from .gitconfig, I just want to point out this hack that's been in the codebase for a long time. What it's doing is checking whether git has added any colors other than the default added/removed colors; if the answer is "yes" then we output a raw line e.g. because this might be git outputting "color-moved" colors. The hack is that the added/removed colors are hardcoded as red/green. Just mentioning in case it can also be made more robust (although I don't think anyone's ever complained).

delta/src/style.rs

Lines 359 to 366 in dcae5bc

pub static ref GIT_DEFAULT_MINUS_STYLE: Style = Style {
ansi_term_style: ansi_term::Color::Red.normal(),
..Style::new()
};
pub static ref GIT_DEFAULT_PLUS_STYLE: Style = Style {
ansi_term_style: ansi_term::Color::Green.normal(),
..Style::new()
};

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 this pull request may close these issues.

🐛 Incorrect output when grepping in a branch with "/" and "-" in the name
3 participants