Delta: 🐛 Dimmed-zebra doesn't show whether line is removed vs. added

[michaelblyons]

When an unchanged line is moved with diff.colorMoved = dimmed-zebra, it is impossible to tell which place the line came from and which place it moved to. Perhaps the foreground color could be left as Git has it, but the background color assigned by delta?

Here's my raw diff:

$ git --no-pager diff
diff --git a/data/transit/route/bus.json b/data/transit/route/bus.json
index 0e40048b..7b05cfc3 100644
--- a/data/transit/route/bus.json
+++ b/data/transit/route/bus.json
@@ -5411,8 +5411,8 @@
     {
       "displayName": "Lothian Buses",
       "id": "lothianbuses-8e0970",
-      "matchNames": ["lothian city buses"],
       "locationSet": {"include": ["gb"]},
+      "matchNames": ["lothian city buses"],
       "tags": {
         "network": "Lothian Buses",
         "network:wikidata": "Q6684760",
@@ -14523,4 +14523,4 @@
       }
     }
   ]
-}
\ No newline at end of file
+}

The problem is also visible in #280, although there it is mitigated by enabling line numbers. I can also set delta.inspectRawLines = false, but then I lose the dimming that lets me know those exact lines appear elsewhere in the diff.

[dandavison]

Thanks for this @michaelblyons. To give a concrete example I've created branch 371-dimmed-zebra in this repo, which contains commit 8406b1996daa176ca677ac33b18f071b766c87a5, which I think exhibits the problem you're describing.

I don't think I've fully understood git's colorMoved features. Could you start by explaining what vanilla git is doing here? It appears to have just removed the red/green color and left it at the default terminal foreground color (so no dimming, let alone zebras).

[michaelblyons]

Mine is dimming. Maybe your console colors don't have a dim auto?

In case it is helpful, here are all my Git config sections that I imagine might be relevant:

[core]
    autocrlf = input
    pager = less --tabs=4 -RFXS
    # pager = delta
    editor = vim
    whitespace = tabwidth=4
[pager]
    # Only commented items
[interactive]
    diffFilter = delta --color-only
[delta]
    # line-numbers = true
    # inspect-raw-lines = false
    syntax-theme = Dracula
[diff]
    colorMoved = dimmed-zebra
    colorMovedWs = ignore-space-change

[dandavison]

Mine is dimming.

OK, thanks. I'll have to look into why mine isn't!

Perhaps the foreground color could be left as Git has it, but the background color assigned by delta?

Hm, the tricky bit is that delta would have to detect the dimmed color and infer that it should apply a background color. This would depart from the current implementation of colorMoved, i.e. (from the README):

If you have activated this feature in Git, then Delta will automatically detect such differently-styled lines, and display them unchanged, i.e. with the raw colors it receives from Git.

The implementation of color-moved support was quite delicate (involving parsing and inspecting ANSI codes in raw git input), and I admit I am a little hesitant to complicate that further. On the other hand, delta does have quite robust ANSI escape sequence parsing now, using the vte parser from the alacritty project.

Can I check one thing: have you already considered and rejected keep-plus-minus-markers?

[michaelblyons]

Can I check one thing: have you already considered and rejected keep-plus-minus-markers?

I had not tried that! Unfortunately, now that I _have,_ it doesn't seem to be keeping them for the dimmed lines...? It _does_ keep the plus/minus for other lines. 😢

If the kpmm option worked, I'd be fine with that approach.

Also, it's not the end of the world that it's broken for now. I'll just use inspect-raw-lines = false or enable line numbers for the time being. I miss the zebra-dimming, but I'll cope. 😉

[dandavison]

Unfortunately, now that I have, it doesn't seem to be keeping them for the dimmed lines...?

Hm, yes! I just discovered that too :/ That's a bug. If you happen to be in a position to compile from source easily then I've pushed a fix to this branch. I'll probably merge it to master shortly; just need to look into test coverage.

[dandavison]

That's fixed in master (but unreleased).

If the kpmm option worked, I'd be fine with that approach.

Great. I'm going to close this ticket then for now, since I don't want to imply that it's a current development goal to change the inspect-raw-lines logic that currently implements colorMoved support.