M-x toggle-theme' née theme-choose-variant' ignores modus hooks.

[11fdriver]

The built-in function toggle-theme does not run hooks in modus-themes-after-load-theme-hook. It's probably not a big enough issue to be put into the modus package, but it did cause me a touch of confusion earlier today.

Perhaps a quick note could be put in the documentation? I advised theme-choose-variant to get what I wanted:

(defun maybe-run-modus-themes-after-load-theme-hooks ()
  (when (modus-themes--current-theme))
    (run-hooks 'modus-themes-after-load-theme-hook)))

(advice-add 'theme-choose-variant :after #'maybe-run-modus-themes-after-load-theme-hooks)

Cheers for all your work on the modus themes; I'm always deeply impressed when using them!

[protesilaos]

From: "11F. Driver" ***@***.***> Date: Sat, 13 Jul 2024 16:17:11 -0700 The built-in function `toggle-theme` does not run hooks in `modus-themes-after-load-theme-hook`. It's probably not a big enough issue to be put into the modus package, but it did cause me a touch of confusion earlier today.

Maybe you can try the 'enable-theme-functions' hook instead? It will run in that situation. Though it can cause issues if you are switching between a Modus and non-Modus theme. I have not tested that scenario.

…

-- Protesilaos Stavrou https://protesilaos.com

[11fdriver]

Ah, an abnormal hook! (C-h v's words, not mine). It passes the current theme, so I've modified my stuff. Seeing as this is just a hook, would it make sense to add this into modus? It synchronises the behaviour of modus' own toggling commands and the built-in commands.

(defun maybe-run-modus-theme-hooks (theme)
  (when (memq theme '(modus-vivendi modus-operandi))
    (run-hooks 'modus-themes-after-load-theme-hook)))
(add-hook 'enable-theme-functions #'maybe-run-modus-theme-hooks)

[protesilaos]

From: "11F. Driver" ***@***.***> Date: Wed, 17 Jul 2024 16:15:59 -0700 Ah, an abnormal hook! (`C-h v`'s words, not mine). It passes the current theme, so I've modified my stuff. Seeing as this is just a hook, would it make sense to add this into modus? It synchronises the behaviour of modus' own toggling commands and the built-in commands. ```emacs-lisp (defun maybe-run-modus-theme-hooks (theme) (when (memq theme '(modus-vivendi modus-operandi)) (run-hooks 'modus-themes-after-load-theme-hook))) (add-hook 'enable-theme-functions #'maybe-run-modus-theme-hooks) ```

This approach is good, though I am not sure we should have it in the themes. At which point would it be activated? And, more importantly, when do we remove this hook? There is 'disable-theme-functions', of course, but how do we handle it?

…

-- Protesilaos Stavrou https://protesilaos.com