Regression on doom-molokai magit vc-added highlights

[viccuad]

What happened?

After updating 13d2377 to b2c0ea0, magit highlights for doom-molokai share the same background and foreground, making the diffs unreadable.

Old, commit, readable no matter if the cursor is inside the diff or not:

New commit, unreadable if the cursor is not inside the diff:

What did you expect to happen?

Have the same behavior as before (no green background for magit highlights). Example: doom-monokai-spectre works as expected.

Steps to reproduce

1. spc g g
2. scroll to a change in the magit status buffer.
3. Fail to read the change.

System Information

• Emacs version: 27.1
• Emacs distro: Doom Emacs
• Operating system: Debian Bullseye
• Installed commit of doom-themes: b2c0ea0
• Which theme are you using: doom-molokai

[ryfow]

I'm seeing the same problem in doom-one, doom-one-light, and doom-vibrant.

[github-actions[bot]]

:warning: This issue has been automatically marked stale because due to 60 days of inactivity. If this issue is still valid, please reply to it or it will be closed in 7 days.

[viccuad]

I can confirm it is still happening.

[hlissner]

Sorry for the tremendously late response. Is this still an issue? At first glance it seems to have already been addressed:

[rmmanseau]

I still have this issue with the doom-molokai theme.

M-x doom/version

GNU Emacs     v27.2            deef5efafb70f4b171265b896505b92b6eef24e6
Doom core     v3.0.0-dev       HEAD -> master 33c5f372 2022-07-09 21:10:40 +0200
Doom modules  v22.07.0-dev     HEAD -> master 33c5f372 2022-07-09 21:10:40 +0200

[rmmanseau]

adding the following face customizations fixes the issue for me in the mean time.

+ '(custom-saved ((t (:background "color-22" :foreground "#b6e63e" :weight bold))))
+ '(custom-state ((t (:background "color-22" :foreground "#b6e63e"))))
+ '(magit-diff-added ((t (:extend t :background "color-22" :foreground "color-114"))))
+ '(magit-diff-added-highlight ((t (:extend t :background "color-22" :foreground "#9ac334" :weight bold))))
+ '(magit-diff-base ((t (:extend t :background "color-58" :foreground "#ca7818"))))
+ '(magit-diff-base-highlight ((t (:extend t :background "color-58" :foreground "#fd971f" :weight bold))))

[rmmanseau]

I recently started playing with 24 bit color & theme customizations & think I understand whats happening a bit better now. see the following snippet from the source:

(def-doom-theme doom-molokai
  "A dark, vibrant theme inspired by Textmate's Monokai."

  ;; name        gui       256       16
  ((bg         '("#1c1e1f" "black"   "black"        ))
   (fg         '("#d6d6d4" "#dfdfdf" "brightwhite"  ))

   ;; These are off-color variants of bg/fg, used primarily for `solaire-mode',
   ;; but can also be useful as a basis for subtle highlights (e.g. for hl-line
   ;; or region), especially when paired with the `doom-darken', `doom-lighten',
   ;; and `doom-blend' helper functions.
   (bg-alt     '("#222323" "black"   "black"        ))
   (fg-alt     '("#556172" "#4d4d4d" "white"        ))

the gui column gets activated if you're using gui (duh) but also if you're using a truecolor terminal (which i was not for a long time). if you use a terminal that doesn't support true color, it uses the 256 column where bg is set to "black". I'm not sure what the theme generation code does in the background but when bg is set to a dark color like "black" the foreground & background colors that get generated for magit-diff-added are near identical.

Is there documentation somewhere that will help me understand how the different faces get generated based on a themes color values? I would love to be able to set bg and fg to "black" (or even nil to get better transparency in my terminal), but as of right now doing so breaks a bunch of faces & I dont know how to work around that.

[rmmanseau]

ah, i found doom-themes-base.el which doesn't rly need much documentation.

  (magit-diff-added             :foreground (doom-darken vc-added 0.2)  :background (doom-blend vc-added bg 0.1) :extend t)
  (magit-diff-added-highlight   :foreground vc-added                    :background (doom-blend vc-added bg 0.2) :weight 'bold :extend t)
  ;; ...
  (magit-diff-removed                :foreground (doom-darken vc-deleted 0.2) :background (doom-blend vc-deleted base3 0.1) :extend t)
  (magit-diff-removed-highlight      :foreground vc-deleted                   :background (doom-blend vc-deleted base3 0.2) :weight 'bold :extend t)

seems that magit-diff-added renders weird because it relies on the bg color, whereas magit-diff-removed uses base3. I would imagine that if they both used base3 then the theme would work even when bg is set to "black"

[rmmanseau]

ive tried setting bg to nil & ive noticed that doom-blend can't deal with that. it also doesn't appear to be able to deal with named colors like "black".

It would be cool if there was a way to set bg to nil and have a fallback variable that gets passed into doom-blend, something like safe-bg which gets set to bg if its a hex string and falls back to bg-fallback if bg is nil.

that way you could decouple the background from all of the faces that get blended with it.

[rmmanseau]

I think this issue should probably be renamed to indicate that a lot of the base theme face generation breaks if you set the themes colors to a color name like black or nil. apologies for the spam, this will be my last comment for now 😅

[all9n]

Hi, I ran into this exact problem. Just want to ask, can we remove the aforementioned(doom-darken vc-added 0.2) in the base theme? First of all there is no guarantee the result will be readable against the background; then if you're using a light theme, "darkening" gives it more contrast...