extend the background color to end-of-visual line

[junyi-hou]

thanks for the great tool!

It is possible to have magit-delta to extand the background color to the end of (visual) line, instead of stopping at "\n"?

[dandavison]

Hi @junyi-hou, thanks for this. I'm pretty sure it's possible! Below are some implementation notes for me or anyone else who wants to take this on:

This didn't fall naturally out of the initial implementation because (a) the terminal emulator instructions delta uses to fill right obviously are not honored by magit, and (b) delta also cannot pad with spaces because those spaces would yield an invalid patch thus breaking magit's assumptions. So I think it will have to be done by adding an additional colored overlay in emacs-lisp, with the same background color as was sent by delta.

[junyi-hou]

Thanks for the reply! I don't think I have time now but I will definitely dig into this later when I have more free time.

[aaronjensen]

Faces can now have the :extend property: https://www.gnu.org/software/emacs/manual/html_node/elisp/Face-Attributes.html

That may be useful here. That said, I tried adding it to the overlay and it didn't help.

Interestingly enough, my magit-diff-added-highlight face has :extend t, and it works but for some reason when magit-delta is enabled, it can't be seen.

Edit: ah, magit-faces-to-override overrides that face. So, one path to this would be to remove those entries from face-remapping-alist and set its background color to match.

[aaronjensen]

For anyone interested, this works for me assuming the GitHub theme (adjust the colors for any other theme):

(with-eval-after-load 'magit-delta
    (set-face-attribute 'magit-diff-added-highlight nil
              :background "#d0ffd0")
    (set-face-attribute 'magit-diff-added nil
              :background "#d0ffd0")
    (set-face-attribute 'magit-diff-removed-highlight nil
              :background "#ffe0e0")
    (set-face-attribute 'magit-diff-removed nil
              :background "#ffe0e0"))

(add-hook 'magit-delta-mode-hook
            (lambda ()
              (setq face-remapping-alist
                    (seq-difference face-remapping-alist
                                    '((magit-diff-removed . default)
                                      (magit-diff-removed-highlight . default)
                                      (magit-diff-added . default)
                                      (magit-diff-added-highlight . default))))))

[dandavison]

@aaronjensen awesome work! Magic:

So, make this the new default with the old behavior optional? Do you feel like making a PR? (I think people will largely appreciate the breaking change and the old behaviour was only like it was because I didn't know how to do this.)

(Sorry for not replying sooner, I saw your comment the other day and have been wanting to find time to try it out.)

[aaronjensen]

It's probably not the best way to do it. I don't know if overlays can extend... they couldn't from my testing. The problem is you'd have to get the background color from the theme and be sure to set it, otherwise it'll look funny. The other change (removing the entries from face-remapping-alist) would be easy. Any suggestions on getting the right colors?

[dandavison]

OK, for the colors does delta --show-config help?

[dandavison]

We can also add code to delta that emits a more sensible format such as emacs lisp or JSON.

[aaronjensen]

Yeah, that could definitely help. I don't think I'll be able to put something together, but a parseable show-config would help someone most likely.

[dandavison]

OK cool, I'll look into it based on the work you've done. Thanks very much!