catppuccin / emacs

🍄 Soothing pastel theme for Emacs
MIT License
296 stars 48 forks source link

incorrect colors on keys/variables #189

Open drishal opened 3 months ago

drishal commented 3 months ago

as per the catppuccin docs, the keys/properties are supposed to be blue right? for eg on nix for vscode, we get the blue color: image

however on emacs we get normal white color like this: image

as per catppuccin style guide it should be blue image

another example with json: vscode: image

Emacs: image

infact even the brackets dont seem to be themed correctly

I already have the ts mode enabled for both as well

One thing I am confused with,is the ts-modes somehow causing issues?

drishal commented 3 months ago

so interestingly, after a bit of trying to fiddle with the theme myself it seems as though setting font-lock-variable-name-face :foreground ,ctp-blue I can finally get the keys to be blue, but then it ends up themeing all kinds of variables to blue, not just keys

jtbx commented 2 months ago

Hey thanks for reporting this! The face used for colouring the attributes in nix-ts-mode is font-lock-variable-name-face. This is a tricky decision because if we were to set :foreground ,ctp-blue on font-lock-variable-face it would make all variable definitions blue in all languages. That might work for expression languages like Nix but for other languages it can be very disconcerting seeing that much colour all the time. To be clear this is not an issue with treesitter major modes.

Here's an example of some C code with :foreground ,ctp-blue on font-lock-variable-name-face:

ball

There are two issues here: first, being that the Catppuccin theme does not style property faces (font-lock-property-name-face inherits from font-lock-variable-name-face); second, being the abuse of faces by modes. Faces such as font-lock-property-name-face exist, but in modes such as nix-ts-mode font-lock-variable-name-face is used instead. Modes should use the designated face where possible, and when they don't things can quite easily look oversaturated or have an unintended colour.

Essentially on our side we should patch in support for font-lock-property-name-face and font-lock-property-use-face and modes such as nix-ts-mode should be patched to use these faces where there is too much or not enough colour.

drishal commented 2 months ago

Hey thanks for reporting this! The face used for colouring the attributes in nix-ts-mode is font-lock-variable-name-face. This is a tricky decision because if we were to set :foreground ,ctp-blue on font-lock-variable-face it would make all variable definitions blue in all languages. That might work for expression languages like Nix but for other languages it can be very disconcerting seeing that much colour all the time. To be clear this is not an issue with treesitter major modes.

Here's an example of some C code with :foreground ,ctp-blue on font-lock-variable-name-face:

ball

There are two issues here: first, being that the Catppuccin theme does not style property faces (font-lock-property-name-face inherits from font-lock-variable-name-face); second, being the abuse of faces by modes. Faces such as font-lock-property-name-face exist, but in modes such as nix-ts-mode font-lock-variable-name-face is used instead. Modes should use the designated face where possible, and when they don't things can quite easily look oversaturated or have an unintended colour.

Essentially on our side we should patch in support for font-lock-property-name-face and font-lock-property-use-face and modes such as nix-ts-mode should be patched to use these faces where there is too much or not enough colour.

yes, I also noticed this thing with a lot of other ts modes as well, observed this in both json ts mode and also toml ts mode as well however interestingly if I switch to js-json-mode instead of the treesitter one keys are correctly themed to ctp-blue but in case of toml mode both normal and ts mode are themed incorrectly with ctp-text instead 🤔

drishal commented 2 months ago

also yes I do agree that having :foreground ,ctp-blue is probably a bad idea for other languages

jtbx commented 2 months ago

yes, I also noticed this thing with a lot of other ts modes as well, observed this in both json ts mode and also toml ts mode as well however interestingly if I switch to js-json-mode instead of the treesitter one keys are correctly themed to ctp-blue but in case of toml mode both normal and ts mode are themed incorrectly with ctp-text instead 🤔

I'd have to look into what faces those modes are using, it's especially difficult when different modes use faces for different things