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

🚀 syntax-highlight 'git blame' output #426

Closed
phil-blain opened this issue Dec 4, 2020 · 12 comments · Fixed by #705
Closed

🚀 syntax-highlight 'git blame' output #426

phil-blain opened this issue Dec 4, 2020 · 12 comments · Fixed by #705

Comments

@phil-blain
Copy link
Contributor

Hi @dandavison ! searching "blame" in the issues brougt me to this one, but here is one (crazy?) idea:

delta could syntax-highlight git blame output ! that would be really great. What do you think (I can open a new issue for that if you like) ?

Originally posted by @phil-blain in #291 (comment)

@phil-blain phil-blain changed the title syntax-highlight 'git blame' output 🚀 syntax-highlight 'git blame' output Dec 4, 2020
@dandavison
Copy link
Owner

One thing we could do here is, if the user has hyperlinks = true have the commit hash be a hyperlink. By default, this could be to a link to the correct line number of the correct commit in the remote web-hosted repo.

dandavison added a commit that referenced this issue Aug 29, 2021
dandavison added a commit that referenced this issue Aug 29, 2021
dandavison added a commit that referenced this issue Aug 29, 2021
dandavison added a commit that referenced this issue Aug 29, 2021
dandavison added a commit that referenced this issue Aug 29, 2021
dandavison added a commit that referenced this issue Aug 29, 2021
@dandavison
Copy link
Owner

I've merged initial support for this into master. Thanks @phil-blain for the feature request a while back!

It's controlled by the new options below. At the moment, each commit gets assigned a color from blame-palette and every line gets shaded with the commit's color as its background color. So you'll probably want to play with blame-palette, especially if you're using a dark background, because the defaults are currently light. I'll probably improve the defaults and other things related to the colors before releasing this, e.g. add a light default and dark default, etc.

This is still rough so suggestions and feedback welcome.

cc @Kr1ss-XD @ulwlu @davidmnoll

--blame-format <blame-format>
    Format string for git blame commit metadata. Available placeholders are "{timestamp}", "{author}", and
    "{commit}" [default: {timestamp:<15} {author:<15} {commit:<8} │ ]
--blame-palette <blame-palette>
    Background colors used for git blame lines (space-separated string). Lines added by the same commit are
    painted with the same color; colors are recycled as needed [default: #FFFFFF #DDDDDD #BBBBBB]
--blame-timestamp-format <blame-timestamp-format>
    Format of `git blame` timestamp in raw git output received by delta [default: %Y-%m-%d %H:%M:%S %z]
image

@Kr1ss-XD
Copy link
Contributor

Thanks a lot @dandavison. It's amazing to see the pace the tool is evolving with ! 👍

I really like this feature, although some details would still be nice to have, such as

  • time formats for output, other than relative (perhaps even localized ?),
  • highlighting according to the commit's age, like vanilla git blame does when blame.coloring = highlightRecent is set.

I'm aware both points would require delta to parse the timestamps, so I'm not sure how much of a hassle that would be.

As another note, it looks like I can't get git blame output to be formatted by delta when using blame.date formats other than iso, even though I'm adjusting delta.blame-timestamp-format accordingly. This might well be me doing something wrong though; I haven't really investigated that yet.

@michaelblyons
Copy link

  • time formats for output, other than relative (perhaps even localized ?),

Yes, please. I am really quite fond of the --date=human format.

  • highlighting according to the commit's age, like vanilla git blame does when blame.coloring = highlightRecent is set.

This is neat. I didn't know you could do that. The config setting is documented, but --color-by-age is missing from the git-blame docs.

@Kr1ss-XD
Copy link
Contributor

[...] --color-by-age is missing from the git-blame docs.

That flag's something I didn't know it's existing. 😆

@dandavison
Copy link
Owner

@michaelblyons @Kr1ss-XD thanks for the points above. Now that I look at what git offers in blame.coloring, color.blame.highlightRecent, blame.date, blame.showEmail, etc, I'm starting to think that my first pass at this (in master) is doing too much.

Would it make sense to backtrack and say that delta's job here is just the following:

  1. Emit the metadata section of the line unchanged, with all ANSI colors intact, and
  2. Syntax highlight the code?

@michaelblyons
Copy link

michaelblyons commented Sep 1, 2021

Good questions, Dan. There are two things in your demo that I would miss if you went the minimal route:

  • The background coloring of the code area itself.

  • Trimming names.

    We have one guy with a 28-char name and it really throws the blame code waaaaay to the right. It's fine in log statements since I've moved the name to EOL, but for blame output, the name is first. If there was a bonus .mailmap for blame, I could fix it locally without offending him, but I can't check in a mailmap that tinkers with his name.

Edit: I won't be offended if you drop back to just code highlighting. I just wanted to remark on the utility of what you've already done.

@Kr1ss-XD
Copy link
Contributor

Kr1ss-XD commented Sep 1, 2021

I agree to @michaelblyons with everything, including both bulletpoints, and the closing post scriptum.

  1. The (background) coloring of the code area gives a nice overview on the first glance, which lines of code stem from which commits. This point is not too important for me personally though, since we can still define custom color highlighting for the metadata column via blame's own settings.
  2. On the other hand, regarding the metadata formatting git blame only allows us to use different date formats. Therefore I'd greatly appreciate it if the implementation you'll eventually go with would at least keep the delta.blame-format setting.

@dandavison
Copy link
Owner

Unfortunately I think this is not going to go out with the next release, due to a security bug in a time library used to format the blame timestamps. #746 Let me know if you know of a good workaround. Perhaps that's best, in view of the discussion above; what we have so far for blame is preliminary. (Next release is imminent, with the new line-wrapping side-by-side mode).

@zachriggle
Copy link

I was just going to file an issue for this! Thanks @dandavison and @phil-blain!

@dandavison
Copy link
Owner

Hi @zachriggle, no problem! The current blame work is in a branch if you'd like to try it: #761. There are quite a lot of open design questions -- discussed above in this issue -- due to the fact that git itself offers many options controlling blame output. But the changes in that branch at least should offer a new enhanced blame view for users that do not have a customized blame config. Let's have discussion in this issue. All thoughts appreciated.

dandavison added a commit that referenced this issue Nov 11, 2021
Fixes #426

Partial versions of these changes were previously in master and then
reverted multiple times.

See #746
0745f85
3aab5d1
dandavison added a commit that referenced this issue Nov 11, 2021
Fixes #426

Partial versions of these changes were previously in master and then
reverted multiple times.

See #746
0745f85
3aab5d1
dandavison added a commit that referenced this issue Nov 12, 2021
Fixes #426

Partial versions of these changes were previously in master and then
reverted multiple times.

See #746
0745f85
3aab5d1
dandavison added a commit that referenced this issue Nov 14, 2021
Fixes #426

Partial versions of these changes were previously in master and then
reverted multiple times.

See #746
0745f85
3aab5d1
@michaelblyons
Copy link

  • highlighting according to the commit's age, like vanilla git blame does when blame.coloring = highlightRecent is set.

This is neat. I didn't know you could do that. The config setting is documented, but --color-by-age is missing from the git-blame docs.

Hahaha, added in 2.34.0.

dandavison added a commit that referenced this issue Nov 15, 2021
Fixes #426

Partial versions of these changes were previously in master and then
reverted multiple times.

See #746
0745f85
3aab5d1
dandavison added a commit that referenced this issue Nov 16, 2021
Fixes #426

Partial versions of these changes were previously in master and then
reverted multiple times.

See #746
0745f85
3aab5d1
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.

5 participants