Fix git-grep match-highlighting at line-start

[jdpopkin]

This commit fixes highlighting the part of the line of code that matches
the git grep query in cases where the match starts at the beginning of
the lines.

Fixes #1056.

[jdpopkin]

These variable-names aren't really applicable anymore. working_example was the one that worked before the changes were made to get_code_style_sections, broken_example was the one that didn't. That wouldn't actually be a useful distinction going forward; maybe we should rename them?

[jdpopkin]

If there are no escape-code-sequences separating the prefix-section from the code-section, the old approach would have produced the same value as the new approach. If there are escape-code-sequences between the two, the old approach would have skipped them, which isn't what we want. So I think this is okay?

[dandavison]

Hi @jdpopkin, I'm really sorry for being so slow here. I really appreciate you working on this bug.

I did a quick manual test and I'm thinking that there might be a tiny bit more work needed here. Try git grep match in the delta repo -- here's what I get:

On master

On this branch

So it looks like your branch is extending the highlighting too far in some cases. Do you agree / see that also?

[jdpopkin]

Yeah, I see the same thing on my end - it looks like my branch gets the highlighting wrong for all lines where the first non-whitespace character is + or -! I think I'll be able to look into this a bit tonight.

[jdpopkin]

So it looks like some of the weirdness here comes from git grep itself. Like, if I run GIT_PAGER=less git grep match and copy the escape codes out for that first bad line, I get this:

etc/examples/189-merge-conflict.2.diff^[[36m:^[[m10^[[36m:^[[m^[[32m +        let (style, non_emph_style) = ^[[1;31mmatch^[[m state {^[[m

With ANSI codes spelled out in words and some of the filename removed:

[...].diff${escape}${foreground=cyan}:${escape}${}10${escape}${foreground=cyan}:${escape}${}${escape}${foreground=green} +        let (style, non_emph_style) = ${escape}${bold=true,foreground=red}match${escape}${} state {${escape}${}

That looks normal in my standard green-on-black terminal, but not in this one:

So: for some reason, git grep is actually marking the start of these lines as green. And that's breaking match-detection because delta chooses between match_style and non_match_style based on whether or not this style-section has a non-default foreground-color. (Without my branch, delta doesn't have this problem because it can't detect a style-section that includes the first character of the code-section.)

We can work around that by checking for bold sections rather than non-default-foreground-color ones. I'm not really sure whether or not that's, like, a universal answer - there might be things other than pattern-matches that get formatted as bold, and maybe it'll run into trouble with users who customize git color config. If it makes sense to commit to default-git-colors-or-else, it could look for bold+red style-sections. If not, maybe the criteria used by get_code_style_sections could be configurable?

Anyway, I added a test that repros the scenario you ran into, along with a code change that makes get_code_style_sections's match-detection decide based on boldness rather than non-default foreground-color. It makes the tests pass, and it looked okay in my arbitrary noodling around with GIT_PAGER=target/release/delta git grep. I'll push that up in a moment - let me know what you think.

edit: forgot to run cargo fmt, sorry. re-pushing now!

[jdpopkin]

So: for some reason, git grep is actually marking the start of these lines as green.

As it turns out, the files where this is happening are example-files used for testing raw git output. In this case, these lines are green because they literally contain ANSI color codes making them green. And since ANSI color codes are what delta relies on to tell where the match-boundaries are in git grep output, it's getting confused. The general problem applies even without this branch's changes, though - the only difference is that the current implementation cuts out color codes if they start at the beginning of the line.

I think checking for bold or maybe red+bold (the default format for a grep match) instead of checking for a non-default foreground color makes sense here. That would fix the existing problems covering lines in etc/examples (though we'd still run into problems on lines that specifically included bold red text). I just pushed an --amend that checks for redness as well as boldness - what do you think?

[dandavison]

Thanks @jdpopkin! The code change is very clean and it seems to work so I think that's great and I've merged it.

(By the way, I agree that git grep is useful but I just wanted to point out for anyone reading that delta handles rg --json output, for which there should never be any parsing problems.)