Handle grep output by dandavison · Pull Request #774 · dandavison/delta

dandavison · 2021-11-14T23:45:33Z

This PR adds the ability to display grep output. Thanks @zachriggle for the proposal! cc @th1000s @Kr1ss-XD
Fixes #769

Handle standard filepath:code and filepath:line_number:code output as produced by git grep, rg -Hn --color=always, grep -Hn --color=always, etc.
Retain the match highlighting as produced by the grep tool, and expose it in delta's color output styled with grep-match-style. (Note that --color=always is needed to retain the color if piping into delta, but not for git grep when delta is configured as git's pager)
grep-match-file-style and grep-match-line-number-style to style the filepath:line_number construct, similar to the hunk-header-* styles (to which values the grep-* variants default)
Special handling of -p, and -W options of git grep: these display the function context in which the matches occur.
navigate keybindings jump between match function contexts under git grep -p and between matching lines under git grep -W.

`git grep -nW`

git grep -W shows the entire "function context", including non matching lines. It makes sense to use navigate to jump between matches. Here,navigate is enabled, yielding the bullet markers at the beginning of the lines.

`git grep -np`

git grep -p shows the header of the function context (i.e. the function/class definition, if you're lucky or have configured git's frag detection algorithm well ([1] [2]). Delta re-uses its "hunk header" from git output to display the "function" header in this mode. Therefore if navigate is enabled, as here, you can navigate between "functions"

`rg -Hn --color=always`

Piped into delta with

    grep-match-style = syntax "#444444"
    grep-match-file-style = cyan
    grep-match-line-number-style = blue

dandavison · 2021-11-16T17:17:57Z

If anybody would like to test the grep handling in this branch that would be great. I intend to add support for ripgrep's --json output. (Parsing of standard grep output is ambiguous so the implementation here just makes an attempt and can probably be improved).

zachriggle · 2021-11-17T08:51:07Z

Played around with this and it's pretty nifty! (And almost there!)

Here's what the feature looks like currently:

Here is a screenshot of my oh-god-oh-god-why-did-I-write-this-in-Zsh wrapper that parses the git grep -W output and feeds it directly to bat with -H to highlight the matching lines, and -r start:stop to show the context.

This might inspire additional configuration points to get the native performance speed of Rust, and the snazzy output of bat --style=full -H 26 -r 15:34 src/delta.rs (though mine uses a custom theme to look nice in the terminal).

dandavison · 2021-11-17T20:33:10Z

Thanks @zachriggle!

This might inspire additional configuration points to get the native performance speed of Rust, and the snazzy output of bat --style=full -H 26 -r 15:34 src/delta.rs

Can you be explicit about what more you'd like to see from delta? One difference I see in your screenshots is that delta is highlighting just the word matches themselves, whereas your script is highlighting the entire line. I'm going to make that possible in delta by exposing styles options. Tentative names:

grep-match-line-style   # style for matching line of code     
grep-match-word-style   # style for matching word (overrides grep-match-line-style)
grep-context-line-style # style for context line of code
grep-file-style         # style for file path
grep-line-number-style  # style for line number (if -n flag was passed)

(Or should grep-match-line-style and grep-match-word-style be called grep-match-style and grep-match-emph-style?)

But beyond that, what else? Did you have in mind the way bat displays ruled margin lines and doesn't repeat the file path in the left margin?

dandavison · 2021-11-17T21:23:24Z

Ok, I've pushed the latest to this branch. That includes support for ripgrep rg --json output (one advantage of which is it can handle parsing ambiguities such as a file named my-7-file or even my:7:file; if anyone feels like improving the parsing heuristics in this branch be my guest!)

Here's my attempt at recreating your example @zachriggle:

[delta]
    syntax-theme = Solarized (dark)
    grep-match-line-style = syntax "#24D7FD"
    grep-match-word-style = syntax "#24D7FD"

dandavison · 2021-11-17T22:07:37Z

One more change @zachriggle: by default it's now going to use : as the separator character everywhere -- i.e. for match lines, context lines, and context header lines alike. To retain the originals (i.e. :, -, =) use grep-separator-symbol = keep.

I am thinking that this is most useful to users since (a) terminal emulators and other applications often recognize the /my/file:7: construct, but do not recognize - and =, and (b) I believe that - and = are intended for machine parsing, whereas delta's raison d'être is to provide output for human eyes: delta uses color styling to indicate match vs context lines.

dandavison · 2021-11-18T17:52:55Z

I don't think I personally have any more TODOs here. If anyone gets a chance to play around with this branch then that would be great, and further suggestions would be very welcome. Otherwise I'll probably release this (and the git blame support) fairly soon.

Kr1ss-XD · 2021-11-18T18:58:36Z

Thank you @dandavison for implementing this. While playing around with this branch, I noticed that output from some commands other than git grep might be affected by the grep highlighting.

For example, git log --graph master.. inside the working tree of this very branch yields :

The colons seem to trigger this.

dandavison · 2021-11-18T19:41:10Z

I noticed that output from some commands other than git grep might be affected by the grep highlighting.

Thank you @Kr1ss-XD!

dandavison · 2021-11-18T20:54:40Z

I noticed that output from some commands other than git grep might be affected by the grep highlighting.

I pushed a temporary fix, but I wonder whether the real fix is going to involve inspecting the parent process. For example, we could only trigger grep handling if the parent process appears to be grep, git grep, rg, etc.

Alternatively we tighten up the file path regex (how?) that identifies grep lines and be more careful about other situations. For example, git show commit:./my-file can output completely arbitrary content. We probably need to actively detect that case using the parent process (are there other examples as pathological as that?)

Kr1ss-XD · 2021-11-18T21:14:40Z

I wonder whether the real fix is going to involve inspecting the parent process. For example, we could only trigger grep handling if the parent process appears to be grep, git grep, rg, etc.

This could in fact be the more robust way. Catching every possible case with a regex is probably not as trivial as it seems.

For example, git show commit:./my-file can output completely arbitrary content. We probably need to actively detect that case using the parent process (are there other examples as pathological as that?)

I don't have something specific from the top of my head. Although I would imagine that potentially every git command that outputs file contents could unintendedly trigger the regex.

Kr1ss-XD · 2021-11-18T21:25:30Z

git reflog show seems to be affected, too :

with the release version of delta :

with delta built from this branch (including the latest commit, that is) :

Kr1ss-XD · 2021-11-19T01:05:18Z

Guess I've encountered another bug with this branch. I ran git commit on a modified worktree, having forgotten to add my modifications to the staging area. commit.pager = delta is set in my git configuraton.

Not sure if this is related to the regex issue, but the empty commit command resulted in a Rust panic :

With the release version of delta, I get what I'd expect :

In case it helps :

stack backtrace

$ RUST_BACKTRACE=full git commit
thread 'main' panicked at 'String mismatch encountered while superimposing style sections: ' ' vs 'r'', src/paint.rs:891:17                                                                                                              
stack backtrace:                                                                                                                                                                                                                         
   0:     0x562c0ff8078c - std::backtrace_rs::backtrace::libunwind::trace::h793e05efd273d0f4                                                                                                                                             
                               at /rustc/82af160c2cb9c349a0373cba98d8ad7f911f0d34/library/std/src/../../backtrace/src/backtrace/libunwind.rs:93:5                                                                                        
   1:     0x562c0ff8078c - std::backtrace_rs::backtrace::trace_unsynchronized::h640b7b86ff610c77                                                                                                                                         
                               at /rustc/82af160c2cb9c349a0373cba98d8ad7f911f0d34/library/std/src/../../backtrace/src/backtrace/mod.rs:66:5                                                                                              
   2:     0x562c0ff8078c - std::sys_common::backtrace::_print_fmt::h362fa2a4f354f877                                                                                                                                                     
                               at /rustc/82af160c2cb9c349a0373cba98d8ad7f911f0d34/library/std/src/sys_common/backtrace.rs:67:5                                                                                                           
   3:     0x562c0ff8078c - <std::sys_common::backtrace::_print::DisplayBacktrace as core::fmt::Display>::fmt::hf439e5ed84c74abd                                                                                                          
                               at /rustc/82af160c2cb9c349a0373cba98d8ad7f911f0d34/library/std/src/sys_common/backtrace.rs:46:22                                                                                                          
   4:     0x562c0ffa578c - core::fmt::write::h72801a82c94e6ff1                                                                                                                                                                           
                               at /rustc/82af160c2cb9c349a0373cba98d8ad7f911f0d34/library/core/src/fmt/mod.rs:1149:17                                                                                                                    
   5:     0x562c0ff7c555 - std::io::Write::write_fmt::h5562a8b6da0f0339                                                                                                                                                                  
                               at /rustc/82af160c2cb9c349a0373cba98d8ad7f911f0d34/library/std/src/io/mod.rs:1697:15                                                                                                                      
   6:     0x562c0ff821e0 - std::sys_common::backtrace::_print::hb29ddd998d02631c
                               at /rustc/82af160c2cb9c349a0373cba98d8ad7f911f0d34/library/std/src/sys_common/backtrace.rs:49:5
   7:     0x562c0ff821e0 - std::sys_common::backtrace::print::h81965e3d7c90fbb6
                               at /rustc/82af160c2cb9c349a0373cba98d8ad7f911f0d34/library/std/src/sys_common/backtrace.rs:36:9
   8:     0x562c0ff821e0 - std::panicking::default_hook::{{closure}}::h84db205ab6674b38
                               at /rustc/82af160c2cb9c349a0373cba98d8ad7f911f0d34/library/std/src/panicking.rs:211:50
   9:     0x562c0ff81d8b - std::panicking::default_hook::h1bf8bb4159936bca
                               at /rustc/82af160c2cb9c349a0373cba98d8ad7f911f0d34/library/std/src/panicking.rs:228:9
  10:     0x562c0ff82894 - std::panicking::rust_panic_with_hook::hf8e86850fbbd03b1
                               at /rustc/82af160c2cb9c349a0373cba98d8ad7f911f0d34/library/std/src/panicking.rs:606:17
  11:     0x562c0ff82370 - std::panicking::begin_panic_handler::{{closure}}::h590a0d6060ff866e
                               at /rustc/82af160c2cb9c349a0373cba98d8ad7f911f0d34/library/std/src/panicking.rs:502:13
  12:     0x562c0ff80c44 - std::sys_common::backtrace::__rust_end_short_backtrace::h260b8bd1c848a03c
                               at /rustc/82af160c2cb9c349a0373cba98d8ad7f911f0d34/library/std/src/sys_common/backtrace.rs:139:18
  13:     0x562c0ff822d9 - rust_begin_unwind
                               at /rustc/82af160c2cb9c349a0373cba98d8ad7f911f0d34/library/std/src/panicking.rs:498:5 
  14:     0x562c0fd64e71 - core::panicking::panic_fmt::h7b8580d81fcbbacd
                               at /rustc/82af160c2cb9c349a0373cba98d8ad7f911f0d34/library/core/src/panicking.rs:106:14
  15:     0x562c0fd821f8 - delta::paint::superimpose_style_sections::superimpose_style_sections::h0a2423a61ba38759
  16:     0x562c0fda86e8 - delta::paint::Painter::paint_line::hdfdb33dcb68e36ae
  17:     0x562c0fda6c3a - delta::paint::Painter::paint_lines::h9e8ad34bce69f0ed
  18:     0x562c0fda756e - delta::paint::Painter::syntax_highlight_and_paint_line::hbb0697d1034d6c9d
  19:     0x562c0fdc5779 - delta::handlers::grep::<impl delta::delta::StateMachine>::handle_grep_line::hd9c3f67364fb815c
  20:     0x562c0fdc113c - delta::delta::delta::ha53caae0cc686754
  21:     0x562c0fdad0e8 - delta::main::hfc9d3bbdc5bc8f0c
  22:     0x562c0fe17cf3 - std::sys_common::backtrace::__rust_begin_short_backtrace::hd718b01b6b0147e8
  23:     0x562c0fdffb69 - std::rt::lang_start::{{closure}}::h003cc26f8ed5d39a
  24:     0x562c0ff7ffd1 - core::ops::function::impls::<impl core::ops::function::FnOnce<A> for &F>::call_once::h42d3791e66d196c0
                               at /rustc/82af160c2cb9c349a0373cba98d8ad7f911f0d34/library/core/src/ops/function.rs:259:13
  25:     0x562c0ff7ffd1 - std::panicking::try::do_call::hc3bcb188e535517f
                               at /rustc/82af160c2cb9c349a0373cba98d8ad7f911f0d34/library/std/src/panicking.rs:406:40
  26:     0x562c0ff7ffd1 - std::panicking::try::hb1edf04d4a7097ab
                               at /rustc/82af160c2cb9c349a0373cba98d8ad7f911f0d34/library/std/src/panicking.rs:370:19
  27:     0x562c0ff7ffd1 - std::panic::catch_unwind::h75ed941ac0d36d57
                               at /rustc/82af160c2cb9c349a0373cba98d8ad7f911f0d34/library/std/src/panic.rs:133:14
  28:     0x562c0ff7ffd1 - std::rt::lang_start_internal::{{closure}}::hc702b3389af4cbea
                               at /rustc/82af160c2cb9c349a0373cba98d8ad7f911f0d34/library/std/src/rt.rs:128:48
  29:     0x562c0ff7ffd1 - std::panicking::try::do_call::h53276a03c7fe2ebc
                               at /rustc/82af160c2cb9c349a0373cba98d8ad7f911f0d34/library/std/src/panicking.rs:406:40
  30:     0x562c0ff7ffd1 - std::panicking::try::ha9e37f45dc542517
                               at /rustc/82af160c2cb9c349a0373cba98d8ad7f911f0d34/library/std/src/panicking.rs:370:19

dandavison · 2021-11-19T17:22:38Z

Thanks again @Kr1ss-XD for those bugs. I've updated this branch so that delta only attempts to parse input as grep output if either

it has evidence that the relevant parent process is one of: git grep, rg, grep, ack, ag, pt, sift, ucg or
the input looks like rg --json.

That change should fix all 3 of the bugs that you encountered.

Note that (1) is not guaranteed to work when output from a grep tool is piped to delta, and also that you must use --color=always if you are using a pipe and you want delta to be able to highlight individual matching substrings.

So I'd recommend either:

Use git grep with pager.grep = delta in gitconfig.
Use rg --json | delta

Both of those should always work. With git grep there are still some file parsing edge cases. rg --json has no parsing edge cases, but doesn't have the -p and -W options that git grep has that attempt to identify the function/class context for matching lines.

There are some unit test failures, pending integration of #783.

dandavison · 2021-11-21T21:29:18Z

@th1000s if you have a moment would you be able to look at the failures of test_parse_grep_parent_command_is_not_grep_1 and test_parse_grep_parent_command_is_not_grep_2? I've updated this branch to make use of your one-shot-thread-local fake parent command magic, which basically appears to be fantastic. It was all working as I expected, but: these two tests pass when run individually but fail when run with the other tests in the module. I'm wondering whether there's an interaction between threads? My understanding of what you'd done was that there should not be any such interaction.

th1000s · 2021-11-21T22:56:27Z

This happens because the args set by WithArgs::new() are not read by the next parent_grep_command() as the call is cached by the (global!) lazy_static PARENT_GREP_COMMAND - and as mentioned in #779 that behavior has yet to be documented :)

I suggest abstracting access to that variable and not use lazy_static when testing.

dandavison · 2021-11-22T00:31:53Z

+    {
+        determine_calling_process()
+    }
+}


I suggest abstracting access to that variable and not use lazy_static when testing.

@th1000s Ah of course (but I bet it would have taken me some time to realise), thanks!

Is this roughly what you had in mind? What would be the right approach for avoiding the clone() here? My first thought was to return Option<Cow<'static, CallingProcess>>, but I'm not sure how to deal with

the trait bound std::borrow::Cow<'_, _>: std::convert::From<&utils::process::CallingProcess> is not satisfied

(I'm thinking Cow is the right approach and have the Cow version compiling now)

Nice, I would have reached for a macro there :)

dandavison · 2021-11-22T01:58:49Z

This PR also adds syntax-highlighting for git show $revision:./path/to/file.lang

@zachriggle

- Handle standard filepath:code and filepath:line_number:code output as produced by `git grep`, `rg -H`, `grep -H`, etc (with -n for line numbers). - Retain the match highlighting as produced by the grep tool, and expose it in delta's color output styled with grep-match-style. (Note that --color=always is needed to retain the color if piping into delta, but not for `git grep` when delta is configured as git's pager) - Special handling of -p, and -W options of `git grep`: these display the function context in which the matches occur. - `navigate` keybindings jump between match function contexts under `git grep -p` and between matching lines under `git grep -W`. Thanks @zachriggle for the proposal. Fixes #769

Fix broken output for my "gg" (git-grep with option alias) possibly related: dandavison/delta#774

dandavison force-pushed the 769-git-grep branch 5 times, most recently from 86ee3ea to 5e8c209 Compare November 15, 2021 01:41

dandavison mentioned this pull request Nov 15, 2021

Util: check parent process for filename #770

Merged

dandavison force-pushed the blame-resurrection branch 2 times, most recently from 7170f9f to 95f62d2 Compare November 16, 2021 01:56

Base automatically changed from blame-resurrection to master November 16, 2021 02:01

dandavison force-pushed the 769-git-grep branch 4 times, most recently from 2460313 to 8395f5b Compare November 16, 2021 17:06

dandavison mentioned this pull request Nov 16, 2021

🚀 Support for "git grep" colorization, specifically with the -W / --function-context flag #769

Closed

dandavison force-pushed the 769-git-grep branch from 8395f5b to 7b9958a Compare November 17, 2021 21:15

dandavison marked this pull request as ready for review November 17, 2021 21:24

dandavison force-pushed the 769-git-grep branch 2 times, most recently from 27d5479 to 9614924 Compare November 17, 2021 22:02

dandavison force-pushed the 769-git-grep branch 3 times, most recently from 60d1380 to c2767a5 Compare November 18, 2021 17:44

dandavison force-pushed the 769-git-grep branch from fc16f88 to 3a3adc6 Compare November 19, 2021 17:09

dandavison mentioned this pull request Nov 19, 2021

Make process utils more generic #779

Merged

dandavison force-pushed the 769-git-grep branch from 3a3adc6 to 43d7c4f Compare November 21, 2021 21:24

dandavison mentioned this pull request Nov 21, 2021

Enable faking extract_args() parameters from process utils #783

Merged

dandavison commented Nov 22, 2021

View reviewed changes

dandavison force-pushed the 769-git-grep branch from 37b2136 to cb03c5f Compare November 22, 2021 00:35

dandavison added 6 commits November 21, 2021 19:36

Parse styles from a string containing ANSI sequences

2c160b0

Clarify tests of ansi_preserving_slice

b656da8

Function to compute text index in string containing ANSI sequences

8fe01d0

Refactor: StyleSectionSpecifier enum for syntax_highlight_and_paint_line

8938d82

Refactor: get_painted_file_with_line_number function

f24bf49

Obtain parent git grep command line options

f9bc4a7

dandavison force-pushed the 769-git-grep branch from cb03c5f to 0d541dd Compare November 22, 2021 00:36

dandavison force-pushed the 769-git-grep branch 2 times, most recently from 2e6fbaa to e108629 Compare November 22, 2021 15:06

dandavison added 2 commits November 22, 2021 10:22

Syntax-highlight git show $revision:./path/to/file.ext output

eb51193

dandavison force-pushed the 769-git-grep branch from e108629 to eb51193 Compare November 22, 2021 15:23

dandavison merged commit d000e40 into master Nov 22, 2021

dandavison deleted the 769-git-grep branch November 22, 2021 18:18

renovate Bot mentioned this pull request Nov 25, 2021

chore(deps): update dependency dandavison/delta to v0.10.0 aquaproj/aqua-registry#757

Merged

1 task

misoobu added a commit to misoobu/dotfiles that referenced this pull request Dec 6, 2021

Use less pager for git grep

1428a4e

Fix broken output for my "gg" (git-grep with option alias) possibly related: dandavison/delta#774

Conversation

dandavison commented Nov 14, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

git grep -nW

git grep -np

rg -Hn --color=always

Uh oh!

dandavison commented Nov 16, 2021

Uh oh!

zachriggle commented Nov 17, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dandavison commented Nov 17, 2021

Uh oh!

dandavison commented Nov 17, 2021

Uh oh!

dandavison commented Nov 17, 2021

Uh oh!

dandavison commented Nov 18, 2021

Uh oh!

Kr1ss-XD commented Nov 18, 2021

Uh oh!

dandavison commented Nov 18, 2021

Uh oh!

dandavison commented Nov 18, 2021

Uh oh!

Kr1ss-XD commented Nov 18, 2021

Uh oh!

Kr1ss-XD commented Nov 18, 2021

Uh oh!

Kr1ss-XD commented Nov 19, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dandavison commented Nov 19, 2021

Uh oh!

dandavison commented Nov 21, 2021

Uh oh!

th1000s commented Nov 21, 2021

Uh oh!

dandavison Nov 22, 2021

Choose a reason for hiding this comment

Uh oh!

dandavison Nov 22, 2021

Choose a reason for hiding this comment

Uh oh!

th1000s Nov 27, 2021

Choose a reason for hiding this comment

Uh oh!

dandavison commented Nov 22, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

dandavison commented Nov 14, 2021 •

edited

Loading

`git grep -nW`

`git grep -np`

`rg -Hn --color=always`

zachriggle commented Nov 17, 2021 •

edited

Loading

Kr1ss-XD commented Nov 19, 2021 •

edited

Loading