bug#57639: [PATCH] Add new command 'toggle-theme'

[Lars Ingebrigtsen]

Philip Kaludercic <philipk@posteo.net> writes:

> Ok, that sounds good.  Here is the updated patch:

[...]

> This is actually done a few times by the modus themes and at least once
> by `leuven-dark' (see `leuven-dark-scale-font', tough I don't see why,
> and if the autoloads aren't being generated to begin with the cookie is
> pointless anyway).
> 
> Should this be addressed before the patch is pushed?

Hm...  are these themes also distributed via ELPA or something?  But in
any case, I don't see why you'd have:

;;;###autoload
(defun leuven-dark-scale-font (control default-height)

If you've activated the theme, you've loaded the file, so autoloading a
function like that doesn't seem helpful in any case.

So I think that sounds like it's just a mistake, and the ;;;###autoload
should be removed.  And the same with the commands autoloaded in modus
themes?  But perhaps there's a reason; I've added Prot to the CCs.
Perhaps he can clarify why those autoloads are in modus*.el.

But this looks more problematic:

;;;###autoload
(when (and (boundp 'custom-theme-load-path)
           load-file-name)
  ;; Add theme folder to `custom-theme-load-path' when installing over MELPA.
  (add-to-list 'custom-theme-load-path
               (file-name-as-directory (file-name-directory load-file-name))))

We don't want that in the Emacs loaddefs file, so just adding etc/themes
to our Makefile won't be the right thing to do, and we have to find a
different way to fix this.

Uhm...  Uhm...  I don't immediately see a good way to fix this...