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

From: Protesilaos Stavrou <[email-masked]>
Date: Wed, 21 Sep 2022 16:05:54 +0300

> From: Lars Ingebrigtsen <[email-masked]>
> Date: Wed, 21 Sep 2022 13:38:13 +0200
>
> Philip Kaludercic <[email-masked]> 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.

Those autoloads are for the version of the themes distributed via GNU
ELPA.  I am happy to make adaptations though.

-- 
Protesilaos Stavrou
https://protesilaos.com