Delta: Support --color-moved

[dwijnand]

Git recently added --color-moved support, to colour moved code: https://stackoverflow.com/a/48166435/463761

Would it be possible to add support for that in delta?

[dandavison]

Thanks @dwijnand, I didn't know about this feature in git. Here are some thoughts, let me know if this sounds right to you.

From a quick look at the docs and play around with the feature, it looks to me like git uses color alone to distinguish "moved" code from "changed" code: i.e. git does not, for example, use special - and + markers for moved blocks.

Now, delta cannot assume that it has access to the original color codes from git. So that suggests that we would have to re-implement git's algorithms for moved code detection. That in itself sounds like a blocker: it would be fragile to attempt to track the details of their algorithm.

However, if git emitted (or could be configured to emit) special +/- markers for moved code blocks then the situation would be entirely different and we could definitely have delta apply special colors to them. I do wonder whether it would make sense for git to do that. After all, without that, nothing can programmatically obtain the information from git _output_.

Currently delta only uses the input text and does not read anything from the git repo (i.e. delta does not use a library like libgit2, whereas e.g. bat does). So in fact delta doesn't know or care what repo the input comes from and may not even have access to that repo. I don't know if something like libgit2 might supply information about moved code blocks now or in the future.

In addition to being fragile to reimplement git's algorithms it would also be a potential perfomance hit: delta is currently implemented in a streaming fashion: as it moves through the input, it only stores a small amount of local context in memory. But for delta to do moved code detection, it seems to me that in the worst case it would have to read the diff for one file into memory and do some computation before starting to emit any output (e.g. if it encountered a - block at the beginning of the diff, it would need to read to the end of the file before knowing whether to color that as deleted or moved. I don't know how often that would result in a noticeable performance hit for delta users, but it would be a non-trivial code change. Git has the repo available to it and thus probably can do this more efficiently.

What do you think, am I understanding the situation correctly?

[dwijnand]

Thanks for the detailed write-up! I didn't spend time thinking about it too much, but I can share my thoughts based on your analysis.

Now, delta cannot assume that it has access to the original color codes from git.

What if:

1. it uses the colour codes you've separately configured in delta; or
2. it uses the .git/config/~/.gitconfig-configured colour codes, provided it can access them; or
3. it just uses git's defaults (or bombs)

?

[dandavison]

Ah-ha, I see. So currently, delta ignores any ANSI color codes in the input it receives it from git. But we could in fact inspect the color codes coming from git, and compare them to what we believe git is using for moved code, and thus figure out when we are in a special moved code block rather than a normal added/deleted block.

This wouldn't work if someone were to do git diff | delta because by default git does not write color codes into a pipe, but it would work for the most important case where delta is configured officially as the git pager via git config's core.pager variable.

Does that sound right?

[dwijnand]

Yeah, that was the setup that I had in mind. And you're right to point out the piping caveat, for the alternative usage.

[dandavison]

OK great, this seems very doable. We can always add an option to disable it / make it non-default if the color-sniffing is unreliable for some reason.

So the next question might be: what is a good way for delta to display moved code blocks? Is it simply a case of adding a third and fourth background colour? Any advice on the actual design / colour selection would be appreciated; I found it quite hard to come up with good defaults, especially for dark terminal backgrounds, and that was with just two background colours. cc @nkouevda @fdcds.

[dwijnand]

Yeah, that might be a challenge, given you're already adding both syntax highlighting and within-line highlighting... I don't have any suggestions, sorry.

[0xC0FFEE]

So the next question might be: what is a good way for delta to display moved code blocks? Is it simply a case of adding a third and fourth background colour? Any advice on the actual design / colour selection would be appreciated; I found it quite hard to come up with good defaults, especially for dark terminal backgrounds, and that was with just two background colours. cc @nkouevda @fdcds.

Choosing the right™ colors is always hard (spoiler: there will always be someone saying that color X is better). I've personally been using some variation of red/green for moved code for quite some time and I'm total used to this by now. So much, that currently this prevents me for using delta on a day to day basis :-).

Having said that, I would make the colors configurable and do not worry too much about the defaults in the beginning. IMHO defaults can be optimized after this feature has seen some real world tests and once they are good enough, the feature can be enabled by default.

Some possibilities to indicate moved lines could be:

• Make the background color configurable
• Prefix lines with some marker (e.g. +N/-N where N is the number of the moved block in this hunk)
• Maybe disable syntax highlighting for moved code and make the foreground color configurable
• Using the same background color as added/deleted lines, but draw the whole line in italics/underlined/...

[zx8]

This is the _single_ missing feature holding me back from making the final switch to git-delta – in every other regard, git-delta seems far superior to any of the alternatives from my initial testing, but I've come to rely on quickly identifying and filtering out moved lines so much that I keep having to revert back to diff-so-fancy, solely for --color-moved support.

[dandavison]

Hey @zx8, thanks for that. I did see your comment when you made it a couple of weeks ago and I have started work on this. Hopefully there'll be something soon!

[dandavison]

Support for color-moved is now in the master branch and I'll release it soon. Meanwhile, if anyone's able to build on master and report back that would be fantastic.

Below are some implementation notes: the TL;DR is that Delta supports color-moved by emitting moved lines with exactly the same color styling as they have under Git. This means that all Git's nuances associated with the values of --color-moved and --color-moved-ws are available in Delta, but that we are unable to apply syntax highlighting to moved lines.

Since this issue was opened a long time ago, I'll just mention that Delta has evolved quite a bit since then. In particular, the most convenient way to configure Delta is now with a [delta] section in ~.gitconfig, and Delta supports git style strings everywhere. There are also various new features such as line numbers, side-by-side view, and diff-highlight/diff-so-fancy emulation. (README.md).

--

I considered 3 ways to implement this (recall that the only way for Delta to infer that Git has identified a line as moved is by inspecting ANSI color escape sequences):

1. If the line does not have the ANSI escapes expected for a typical removed/added hunk line, then emit the raw line with its ANSI escapes intact.
   _This is what I chose, but it means we cannot syntax-highlight moved lines._

2. If the line does not have the ANSI escapes expected for a typical removed/added hunk line, then interpret it as a moved line and style it with a special Delta style for moved lines.
   _This would give us syntax-highlighting for moved lines, but it loses Git's color-moved option nuances and it is fragile._

3. If the line has ANSI escapes expected for one of Git's 8 moved color styles, then identify it as such and apply a corresponding special Delta style.
   _This would give us syntax-highlighting and all Git's color-moved nuances but it is complex and fragile._

(2) is fragile in that Git might emit colors other than the canonical diff.old/diff.new for hunk lines in situations for reasons other than color-moved. I think (?) it doesn't today, but it seems plausible that a future git feature will do this.

(3) is fragile in that a user might configure another git style to be the same as one of the moved styles. If that were so, Delta would have no way to avoid the ensuing error.

A challenge faced by all 3 options is that two things influence the colors emitted by Git: color values configured in gitconfig, and git's hard-coded default colors. Since a few months ago, Delta does read values from gitconfig, so that's not a blocker. Git's hard-coded color defaults are (AFAIK) private and could change with any release.

So I went with (1): if Delta sees something that doesn't look like a typical removed/added hunk line, it emits the raw line unaltered. For this it was still necessary to bake into Delta the values of Git's default red/green colors for removed/added lines. But if you have set color.diff.{old,new} to something different in gitconfig, Delta will spot that.

If for some reason this color-sniffing goes wrong, it's somewhat disastrous: Delta won't give any of its usual output. So, color-sniffing is the default :) There is however a kill-switch: inspect-raw-lines = false.

As one additional safety measure, if Delta sees the standard red/green foreground styles, it will always assume this is a typical removed/added hunk line. That's true even if you've actually set one of the colorMoved styles to red and set diff.old to something else. I did that out of paranoia; in principle it's unnecessary, so let me know if it's restrictive.

[dandavison]

By the way, the implementation notes above were in mainly in case anyone has ideas about how Delta can support color-moved better, so please feel free to suggest improvements / ways we could support syntax highlighting, etc.

cc @navarroaxel @Kr1ss-XD @ryuta69 if you're able to run Delta from the master branch in case you encounter bugs in the color-moved support that would be great (I just found a bug, triggered by using the blink attribute in color.diff.{old,new}).

In case anyone else feels like doing some testing, configuration would be something like

[diff]
    colorMoved = default
    # colorMovedWS = allow-indentation-change  # <-- can experiment with this, see #144

[color "diff"]
    # Delta should still work correctly, for whatever nonsense you put here!
    old = bold "#aabbcc" brightblack blink
    new = strike ul 22 "#ddeeff" dim

[dandavison]

In order to make the color-moved styles harmonious with your Delta styles, you may want to make the background colors match. To find out the background colors that Delta is using given your current setup, you can use delta --show-config, e.g.

delta --show-config | grep style

Based on that, I'm using something like the following

[color "diff"]
    oldMoved = bold brightmagenta "#ffe0e0"
    oldMovedAlternative = bold brightmagenta "#ffe0e0"
    newMoved = bold brightblue "#d0ffd0"
    newMovedAlternative = bold brightblue "#d0ffd0"

(Although I appreciate that most people will be using a dark background and so the appropriate background colors will be different.)

[navarroaxel]

Hey @dandavison, why in the first part of your screenshot, line added 35, the pub and bool keywords are highlighted?

[dandavison]

Hey @dandavison, why in the first part of your screenshot, line added 35, the pub and bool keywords are highlighted?

That's a good question, I was wondering the same. I've double-checked, and it is not Delta's fault; it's to do with Git's color-moved behavior. If we set diff.colorMoved to zebra, or default, or blocks, then Git outputs that line as a normal removed/added pair, not as a moved pair. But if we set diff.colorMoved=plain, then Git colors it as moved. I haven't fully understood the different Git colorMoved modes yet, does anyone know whether that is all proper and expected behavior for Git?

But in any case, Delta has no ability to second-guess Git here: the rules are that if the colors received from Git are not what is expected for a normal removed/added line then Delta outputs the line raw. But l.35 _is_ colored as expected for a normal added line, so Delta does all the normal Delta processing and rendering for it.

E.g.

HOME=/dev/null git -c 'core.pager=less -R' -c 'diff.colorMoved=default' show c1a49f46f src/config.rs

[navarroaxel]

Oh, I didn't notice! By default, git detects blocks of moved text of at least 20 alphanumeric characters (mode: default, zebra, blocks) but the plain mode picks up any moved line, but «it is not very useful in a review to determine if a block of code was moved without permutation». (quoting the git doc).

So, these highlighted keywords are the expected behavior.

[dandavison]

Thanks @navarroaxel, I see, I didn't read that nearly carefully enough! So if we exclude non-alphanumerics then I guess that line is less than 20 characters.

[dandavison]

This is released now (delta 0.4.0). Thanks everyone for suggesting and discussing this.