search

protesilaos / modus-themes

Highly accessible themes for GNU Emacs, conforming with the highest standard for colour contrast between background and foreground values (WCAG AAA).
https://protesilaos.com/emacs/modus-themes
GNU General Public License v3.0
526 stars 29 forks source link

Update use-package configurations #67

Closed zoliky closed 1 year ago

zoliky commented 1 year ago

:bind defers the loading of the package (see https://github.com/jwiegley/use-package#modes-and-interpreters) preventing load-theme to load the theme (even if load-theme is listed before :bind). The solution is to use :init.

:ensure nil tells Emacs to configure the built-in package and do not look for it in the online repositories.

Also, I use :custom instead of :config because C-h v categorizes all those variables in the "customization" category. Variables listed in the :custom don't require setq.

I tested the configurations before commiting.

protesilaos commented 1 year ago

Hello @zoliky!

My concern with your PR is that it adds cognitive load to the things I need to remember about use-package. For example, I had forgotten that :bind does its own magic behind the scenes. The :custom is also doing its own thing by writing to the custom-file, which is a common source of misconfigurations. The :init may put us in uncharted territory with the special require-theme function.

As such, I think it is better to use use-package as a very thin wrapper around the example setup with only the :config keyword. Like this:

;;; For the built-in themes which cannot use `require'.
(use-package emacs
  :config
  (require-theme 'modus-themes) ; `require-theme' is ONLY for the built-in Modus themes

  ;; Add all your customizations prior to loading the themes
  (setq modus-themes-italic-constructs t
        modus-themes-bold-constructs nil)

  ;; Maybe define some palette overrides, such as by using our presets
  (setq modus-themes-common-palette-overrides
        modus-themes-preset-overrides-intense)

  ;; Load the theme of your choice.
  (load-theme 'modus-operandi)

  (define-key global-map (kbd "<f5>") #'modus-themes-toggle))

;;; For packaged versions which must use `require'.
(use-package modus-themes
  :ensure t
  :config
  ;; Add all your customizations prior to loading the themes
  (setq modus-themes-italic-constructs t
        modus-themes-bold-constructs nil)

  ;; Maybe define some palette overrides, such as by using our presets
  (setq modus-themes-common-palette-overrides
        modus-themes-preset-overrides-intense)

  ;; Load the theme of your choice.
  (load-theme 'modus-operandi)

  (define-key global-map (kbd "<f5>") #'modus-themes-toggle))

What do you think?

zoliky commented 1 year ago

That's fine.