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

From: Philip Kaludercic <[email-masked]>
Date: Thu, 13 Oct 2022 10:35:51 +0000

Eli Zaretskii <[email-masked]> writes:

>> From: Philip Kaludercic <[email-masked]>
>> Cc: Eli Zaretskii <[email-masked]>,  [email-masked],  Protesilaos
>>  Stavrou <[email-masked]>
>> Date: Thu, 13 Oct 2022 08:50:34 +0000
>> 
>> Lars Ingebrigtsen <[email-masked]> writes:
>> 
>> > Philip Kaludercic <[email-masked]> writes:
>> >
>> >> Great, can you point me to the places I would have to make changes for
>> >> this to work:
>> >>
>> >>         add a function to loaddefs-gen that only fetched those for the
>> >>         Emacs build (it'd almost be trivial -- we just bind
>> >>         lisp-mode-autoload-regexp to ";;;###theme-autoload" and then
>> >>         point the scraping function at etc/themes).
>> >
>> > I think you'd basically put something like the following into
>> > `loaddefs-generate--emacs-batch' --
>> >
>> > (let ((lisp-mode-autoload-regexp
>> >         "^;;;###\\(\\(noexist\\)-\\)?\\(theme-autoload\\)"))
>> >   (loaddefs-generate
>> >     (expand-file-name "../etc/themes/" lisp-directory)
>> >     (expand-file-name "theme-loaddefs.el" lisp-directory)))
>> >
>> > Then you have to teach `loaddefs-generate--make-autoload' to generate
>> > the correct forms as a result of the `deftheme' -- which should be some
>> > `put's.
>> 
>> OK, it looks like this worked.  Just to recap, these are the proposed
>> changes:
>
> What, if anything, does this mean for themes that aren't bundled with
> Emacs?

Nothing for now, but I could add support for package.el.

> Also, this needs a NEWS entry.

Yes, thanks for the reminder.

> Thanks.