motform / stimmung-themes

emacs tuned to inner harmonies
GNU General Public License v3.0
120 stars 12 forks source link

Disable other themes when loading stimmung-themes #10

Closed slotThe closed 2 years ago

slotThe commented 2 years ago

This is something I've noticed today while switching from a different theme to stimmung-themes-light. The commit message hopefully explains the problem:

When the user made their choice after invoking stimmung-themes-toggle, we should first disable all other themes before enabling the selection. The reason for that is that custom-enabled-themes holds a list of themes; i.e., multiple may be active at the same time. This can look quite confusing when mixing e.g. a dark and a light theme, possibly leading to unreadable text.

The file mentions the modus themes as inspiration for the code; I double-checked and modus-themes-toggle does also disable all other themes before loading, so it's probably safe.

Along the way, I (imo) simplified the implementation somewhat, compressing everything down into one function. If you don't agree, I can of course remove that part!

slotThe commented 2 years ago

Whoops, I pulled the latest master and realised that this is already implemented now! Sorry for the noise

motform commented 2 years ago

No worries! I had some issues with counsel faces and implemented the modus behaviour as a failed fix for those, so I'm glad that they found use so soon!

That said, I do like your implementation and seems to have missed/glossed over (mapc #'disable-theme custom-enabled-themes) in the modus implementation, so I added that as well.