Handle git blame #761

dandavison · 2021-11-04T00:36:41Z

dandavison · 2021-11-04T20:02:10Z

cc @th1000s @Kr1ss-XD FYI in case you'd like to give thoughts. I may merge this to master, but please don't let that dissuade you from commenting if you feel like it.

This PR resurrects and extends the git blame work initially discussed in #426.

There's quite a bit of discussion in that issue: basically, the implementation here is intended to work for users with vanilla git configs i.e. who are not making use of git settings such as blame.coloring, color.blame.highlightRecent, blame.date, blame.showEmail. But a different approach might be required if delta is to enhance blame for users making use of those git settings.

I've made two changes to what I showed in #426

I'm only showing blame info for the first line when there would be multiple identical successive lines.
Added default palettes (3 colors) for delta's light and dark "modes" (--light --dark).

Kr1ss-XD · 2021-11-04T22:10:16Z

I'm only showing blame info for the first line when there would be multiple identical successive lines.

This is a great idea. The output looks much more clearly arranged like that.

I'm looking forward to check this branch out for testing, most likely the upcoming weekend 😃

zachriggle · 2021-11-09T02:35:32Z

@dandavison I downloaded this branch and built it (per #426, after nuking ~/.cargo to get the build to work) and get good results with Rust files (i.e. delta's own source)...

... but it does not appear to work on C\C++ source code (e.g. torvalds/linux file init/main.c).

Kr1ss-XD · 2021-11-09T18:16:55Z

I'm seeing this too, however I don't necessarily think it has to do with the files being C source code vs. Rust. I rather suspect this to be caused by large sized repositories, more precise large repo histories.

For example, I have cloned this repository, which only contains a handful of .c and .h files, the history of [master] is about 100 commits long. git blame output is formatted by delta here, as expected. The same goes for a test repo I've set up with two commits to a test.c file :

On the other hand, if I do git blame on any file in the John repo or the Linux sources (which both have an immense git history), the output looks as if delta would just pass it through. (git blame <file> seems to yield the same output as git -P blame <file> | less)

dandavison · 2021-11-09T19:07:09Z

Thanks @zachriggle @Kr1ss-XD: yes, as @Kr1ss-XD says the problem was due to history size: in my initial implementation I had hardcoded an assumption that the commit hash would be truncated to 8 characters, and then forgotten to go back and do something actually reasonable. It appears that in repos with more history, git automatically displays more characters of the hash to avoid the possibility of displaying two distinct commits with identical formatting. So, we assume 7-40 characters now.

-    [0-9a-f]{8}    # commit hash
+    [0-9a-f]{7,40} # commit hash

dandavison · 2021-11-09T19:13:27Z

One change I'm planning to make to this branch is the following:

Currently, due to recycling of colors from the palette, it's possible that two distinct commits appearing next to each other would be assigned the same color. I'm thinking of choosing a different color in that situation. In other words I would give more importance to "give neighboring commits different colors" than to "ensure that a given commit always receives the same color".

zachriggle · 2021-11-09T19:15:10Z

That sounds like a great idea! How can one customize the palette of available colors?

dandavison · 2021-11-09T19:19:53Z

How can one customize the palette of available colors?

That's the blame-palette option.

Kr1ss-XD · 2021-11-09T19:22:39Z

I would give more importance to "give neighboring commits different colors" than to "ensure that a given commit always receives the same color".

I like the idea, too. It would be even better if this could be toggled on or off by a config setting.

Kr1ss-XD · 2021-11-09T19:31:42Z

It appears that in repos with more history, git automatically displays more characters of the hash to avoid the possibility of displaying two distinct commits with identical formatting.

That's correct. It can also be configured by core.abbrev (which may be auto).

So, we assume 7-40 characters now.

The smallest allowed length is 4 though, according to the documentation. Maybe it would be better to use that as a minimum for the regex ?

dandavison · 2021-11-09T20:02:01Z

The smallest allowed length is 4 though, according to the documentation. Maybe it would be better to use that as a minimum for the regex ?

Thanks; done. (Btw for anyone building this branch you may have to put up with force-pushes to it, so git fetch origin blame-resurrection && git reset --hard origin/blame-resurrection )

th1000s · 2021-11-09T22:41:55Z

What is the current state and reasoning for/against syntax highlighting? I see the first pair of screenshot still have it, but later ones and my local build do not.

dandavison · 2021-11-10T03:31:28Z

What is the current state and reasoning for/against syntax highlighting? I see the first pair of screenshot still have it, but later ones and my local build do not.

Thank you, I had entirely forgotten about this. In 0b8f145 I added a default-language option. blame handling currently relies upon this to know what language to use, because the blame output itself does not contain the usual file extensions used for this purpose. On the plus side, this can be set on a per-repository basis, so if your repo is monolingual then we have a reasonable solution:

delta(blame-resurrection) head -n2 .git/config
[delta]
    default-language = rs

Anyone have any better suggestions here?

One thing that occurs to me is that it if someone wanted to create a rust tool to automatically predict language from source code, then that could make a nice rust crate. It really seems that it is not a problem pushing at the frontiers of machine learning :) Github has a tool that does this: https://github.com/github/linguist/blob/master/docs/how-linguist-works.md I believe that it wouldn't be hard to use the data used by their classifier (naive Bayes FWIW) and make a Rust version, i.e. skipping all the hard work of collecting the dataset. But, something of a tangent for a branch trying to add blame handling.

Kr1ss-XD · 2021-11-10T17:29:16Z

When the blame-format setting contains "constant" characters, for instance I have set it to "{author:<18} ({commit:>7}) ┊{timestamp:^16}┊ " (note the parenthesises around commit), delta prints them out on consecutive lines of the same commit.

I think it would be nicer to suppress those characters, too. @dandavison ?

EDIT/ I just realized that it would probably look worse if delta did that, because then also the ┊ (or whatever users define as limiter lines) would be omitted in those lines ...
Maybe it's better to just not use something like () or [] in this setting.

dandavison · 2021-11-10T21:25:47Z

@Kr1ss-XD hm, yes as you say, we can either have those lines blank for variable content only (as currently) or we can have them completely blank. I went for the former because at the very least, users are going to want a separating character at the right-hand end to be present in a blank line.

Incidentally, hyperlinks makes a lot of sense with git blame. I see you're using tmux. Are you building tmux with the hyperlinks patch applied, and if so have you managed to apply it against a recent tmux version? I changed laptops and there seemed to be a problem with the older tmux commit I was using on my new laptop (apple M1).

(I propose languid as the name for the language-identifying crate... although perhaps if it's a straight port of linguist it should retain that name.)

th1000s · 2021-11-10T22:38:44Z

blame handling / which language to use

One quick solution would be to check the command line of the parent process, then git blame hello_world.rs with #770 will return rs.

dandavison · 2021-11-12T01:48:48Z

One quick solution would be to check the command line of the parent process, then git blame hello_world.rs with #770 will return rs.

Fantastic. This PR is now using @th1000s's parent process inspection utility, so there should no longer be any need for default-language, if the file you are blameing has a standard file extension for the language.

Kr1ss-XD · 2021-11-12T17:40:41Z

Are you building tmux with the hyperlinks patch applied, and if so have you managed to apply it against a recent tmux version? I changed laptops and there seemed to be a problem with the older tmux commit I was using on my new laptop (apple M1).

I have actually tried that a while ago, and I kind of got hyperlinks working in tmux, in combination with your open-in-editor script. Although there was something that really annoyed me, which I don't remember any more. In any case, I've switched back to the tmux binary package from the Archlinux repos.

dandavison · 2021-11-05T09:08:52Z

Cargo.toml

@@ -15,6 +15,8 @@ name = "delta"
 path = "src/main.rs"

 [dependencies]
+chrono = "0.4.19"


There's a security bug in this library (flagged by cargo audit). But I think there's nothing we can do about our exposure other than wait for upstream changes because syntect uses the library, and we can't replace syntect.

trishume/syntect#385

dandavison · 2021-11-05T09:16:22Z

src/handlers/blame.rs

+        if let Some(field) = field {
+            let field = pad(&field);
+            if is_repeat {
+                s.push_str(&" ".repeat(measure_text_width(&field)));


TODO: it should be possible to determine the required width once only and thereafter construct the blank line more efficiently.

Fixes #426 Partial versions of these changes were previously in master and then reverted multiple times. See #746 0745f85 3aab5d1

dandavison force-pushed the blame-resurrection branch from bf91c6e to 298f4f7 Compare November 4, 2021 00:41

dandavison changed the title ~~Blame resurrection~~ Handle git blame Nov 4, 2021

dandavison force-pushed the blame-resurrection branch from 71f1ede to 6c07c1f Compare November 7, 2021 15:40

dandavison mentioned this pull request Nov 8, 2021

🚀 syntax-highlight 'git blame' output #426

Closed

dandavison force-pushed the blame-resurrection branch from ddafe23 to cb88ab2 Compare November 9, 2021 19:58

dandavison force-pushed the blame-resurrection branch from f467620 to 80e1ca7 Compare November 10, 2021 13:37

dandavison force-pushed the blame-resurrection branch 4 times, most recently from 776b746 to e423ecf Compare November 12, 2021 01:46

dandavison force-pushed the blame-resurrection branch from e423ecf to 8f0d357 Compare November 14, 2021 19:35

dandavison mentioned this pull request Nov 15, 2021

Util: check parent process for filename #770

Merged

dandavison added 3 commits November 15, 2021 17:28

Fix test of parent process when run under cargo-tarpaulin

4e58aab

Clean up

e8637bc

Support precision specifier in run-time format strings

8018d39

dandavison force-pushed the blame-resurrection branch from 21b8b1e to 7170f9f Compare November 15, 2021 22:28

dandavison commented Nov 16, 2021

View reviewed changes

Handle git blame output

95f62d2

Fixes #426 Partial versions of these changes were previously in master and then reverted multiple times. See #746 0745f85 3aab5d1

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

dandavison merged commit 24a07ff into master Nov 16, 2021

dandavison deleted the blame-resurrection branch November 16, 2021 02:01

dandavison mentioned this pull request Nov 18, 2021

Support --color-moved #72

Closed

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

zachriggle mentioned this pull request Dec 21, 2021

🚀 "git blame ... | delta" will colorize based on commit date, even if only "--blame-format '{author} |'" is specified #868

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Handle git blame #761

Handle git blame #761

dandavison commented Nov 4, 2021

dandavison commented Nov 4, 2021

Kr1ss-XD commented Nov 4, 2021

zachriggle commented Nov 9, 2021

Kr1ss-XD commented Nov 9, 2021 •

edited

Loading

dandavison commented Nov 9, 2021

dandavison commented Nov 9, 2021

zachriggle commented Nov 9, 2021 via email •

edited by dandavison

Loading

dandavison commented Nov 9, 2021

Kr1ss-XD commented Nov 9, 2021

Kr1ss-XD commented Nov 9, 2021

dandavison commented Nov 9, 2021

th1000s commented Nov 9, 2021

dandavison commented Nov 10, 2021

Kr1ss-XD commented Nov 10, 2021 •

edited

Loading

dandavison commented Nov 10, 2021 •

edited

Loading

th1000s commented Nov 10, 2021

dandavison commented Nov 12, 2021

Kr1ss-XD commented Nov 12, 2021

dandavison Nov 5, 2021

dandavison Nov 5, 2021

Handle git blame #761

Handle git blame #761

Conversation

dandavison commented Nov 4, 2021

dandavison commented Nov 4, 2021

Kr1ss-XD commented Nov 4, 2021

zachriggle commented Nov 9, 2021

Kr1ss-XD commented Nov 9, 2021 • edited Loading

dandavison commented Nov 9, 2021

dandavison commented Nov 9, 2021

zachriggle commented Nov 9, 2021 via email • edited by dandavison Loading

dandavison commented Nov 9, 2021

Kr1ss-XD commented Nov 9, 2021

Kr1ss-XD commented Nov 9, 2021

dandavison commented Nov 9, 2021

th1000s commented Nov 9, 2021

dandavison commented Nov 10, 2021

Kr1ss-XD commented Nov 10, 2021 • edited Loading

dandavison commented Nov 10, 2021 • edited Loading

th1000s commented Nov 10, 2021

dandavison commented Nov 12, 2021

Kr1ss-XD commented Nov 12, 2021

dandavison Nov 5, 2021

Choose a reason for hiding this comment

dandavison Nov 5, 2021

Choose a reason for hiding this comment

Kr1ss-XD commented Nov 9, 2021 •

edited

Loading

zachriggle commented Nov 9, 2021 via email •

edited by dandavison

Loading

Kr1ss-XD commented Nov 10, 2021 •

edited

Loading

dandavison commented Nov 10, 2021 •

edited

Loading