protesilaos / ef-themes

Colourful and legible themes for GNU Emacs
GNU General Public License v3.0
318 stars 17 forks source link

Text not legible when cursor is on unused variable in ef-maris-light, ef-elea-light #27

Open stevemolitor opened 1 year ago

stevemolitor commented 1 year ago

I've noticed this issue with ef-maris-light and ef-elea-light, but not with the dark themes or the other light themes I tested. Here var1 is unused, and is correctly highlight in fg-dim (I think):

However when I put my cursor on the unused variable it turns pink and I can no longer read the text:

I'm using clojurescript-mode with lsp-mode. The pink color comes from lsp-face-highlight-textual, which inherits highlight. So maybe highlight and fg-dim (or similar) are incompatible in this themes?

stevemolitor commented 1 year ago

Update: If I customize like this, changing the highlight background from bg-hover to bg-info (a light green color):

EDIT:

(ef-themes-with-colors (custom-set-faces
                          `(lsp-flycheck-error-unnecessary-face ((,c :foreground ,fg-dim)))
                          `(highlight ((,c :background ,bg-info)))
                          ))

Then it looks better:

stevemolitor commented 1 year ago

Actually here's a simpler workaround:

(setq ef-maris-light-palette-overrides '((bg-hover bg-info)))

protesilaos commented 1 year ago

Hello @stevemolitor! This is unexpected because highlight is meant to also affect the foreground colour. Here is what we have in ef-themes.el:

`(highlight ((,c :background ,bg-hover :foreground ,fg-intense)))

In other words, the gray text should not be there.

stevemolitor commented 1 year ago

Hi @protesilaos thanks I see the problem now. I'm using lsp-mode, with the setting to highlight unused variables in a dim color. lsp-flycheck-warning-unnecessary-face applies the gray foreground on unused variables, overriding the highlight foreground:

Face: lsp-flycheck-warning-unnecessary-face (sample) (customize this face)
       ...
       Foreground: gray
       ... everything else is "unspecified"

So I guess I could instead change lsp-flycheck-warning-unnecessary-face to use a different foreground so that it doesn't conflict here. I do like having lsp-mode highlight unused variables in a dimmer color though. 🤔

stevemolitor commented 1 year ago

Or maybe there's a way to have highlight override lsp-flycheck-warning-unnecessary-face? Ideally I'd like lsp-flycheck-warning-unnecessary-face to apply the gray foreground when the cursor is not over the variable, but disregard and use highlight when the cursor is over the variable.

protesilaos commented 1 year ago

Maybe there is a way. Probably some lsp configuration. Can you share with me a minimal setup so I can try to reproduce the problem with the packages you use?

stevemolitor commented 1 year ago

Hi @protesilaos thanks that's very generous of you. Here's a minimal init.el to reproduce the issue:

(require 'package)
(add-to-list 'package-archives '("melpa" . "https://melpa.org/packages/"))

(use-package clojure-mode :ensure t)

(use-package flycheck :ensure t)

(use-package ef-themes :ensure t) 

(use-package lsp-mode
  :ensure t
  :hook ((clojure-mode . lsp))
  :config
  (setq lsp-enable-symbol-highlighting t))

(load-theme 'ef-maris-light :no-confirm)

EDIT: You need to install the clojure-lsp server. Run lsp-install-server and select clojure-lsp.

Then create a test.clj clojure file like this:

(let [unused-var "not-used"])

It will look like this (so far so good):

However if you put your cursor over unused-var it looks like this:

You can't read the variable name now.