search

jwiegley / use-package

A use-package declaration for simplifying your .emacs
https://jwiegley.github.io/use-package
GNU General Public License v3.0
4.39k stars 259 forks source link

Remove use-package theme from global list of custom-enabled-themes #899

Closed tzz closed 3 years ago

tzz commented 3 years ago
tzz commented 3 years ago

@terlar @lbolla @gusbrs please review. I am not sure about the compile-time evaluation but I think it's correct.

I think this approach (removing 'use-package from custom-enabled-themes) is the best approach to non-invasively resolve the issues you raised, while at the same time avoiding changes and feature requests to other packages and to the Emacs core.

This approach also avoids advising functions, which is always fragile as @gusbrs pointed out.

lbolla commented 3 years ago

@tzz I tried the patch and it seems to be working fine for my use case. I am not an expert elisper though, so I am not sure if manually managing custom-enabled-themes may have other consequences.

kljohann commented 3 years ago

The double deftheme call makes the tests pass but I'd love to know if there's a better way.

Maybe eval-and-compile ("and" instead of "when") does what you want?

tzz commented 3 years ago

Maybe eval-and-compile ("and" instead of "when") does what you want?

Perfect; done.

arminfriedl commented 3 years ago

Incidentally this would also fix a quite annoying behavior of customize-themes with the synthetic theme in place. Usually when only one custom theme is enabled, customize-themes will switch between themes disabling the old one. With the synthetic theme in place however customize-themes will overlay themes (because (> (length custom-enabled-themes) 1)).

Here's the relevant code in cus-theme.el:

(defun customize-themes (&optional buffer)
  [...]
  (make-local-variable 'custom-theme-allow-multiple-selections)
  (and (null custom-theme-allow-multiple-selections)
       (> (length custom-enabled-themes) 1)
       (setq custom-theme-allow-multiple-selections t)

When removing use-package synthetic theme from custom-enabled-themes, custom-theme-allow-multiple-selections would work more in line with how was meant to work.