GNU bug report logs - #57639
[PATCH] Add new command 'toggle-theme'

Previous Next

Package: emacs;

Reported by: Philip Kaludercic <philipk <at> posteo.net>

Date: Wed, 7 Sep 2022 07:20:01 UTC

Severity: wishlist

Tags: patch

Done: Philip Kaludercic <philipk <at> posteo.net>

Bug is archived. No further changes may be made.

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 57639 in the body.
You can then email your comments to 57639 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

View this report as an mbox folder, status mbox, maintainer mbox


Report forwarded to bug-gnu-emacs <at> gnu.org:
bug#57639; Package emacs. (Wed, 07 Sep 2022 07:20:01 GMT) Full text and rfc822 format available.

Acknowledgement sent to Philip Kaludercic <philipk <at> posteo.net>:
New bug report received and forwarded. Copy sent to bug-gnu-emacs <at> gnu.org. (Wed, 07 Sep 2022 07:20:01 GMT) Full text and rfc822 format available.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Philip Kaludercic <philipk <at> posteo.net>
To: bug-gnu-emacs <at> gnu.org
Subject: [PATCH] Add new command 'toggle-theme'
Date: Wed, 07 Sep 2022 07:19:30 +0000
[Message part 1 (text/plain, inline)]
Tags: patch


Find below a patch for a command that a lot of custom themes
re-implement, whenever there exists a light and dark variant.

In GNU Emacs 29.0.50 (build 8, x86_64-pc-linux-gnu, GTK+ Version
 3.24.34, cairo version 1.17.6) of 2022-09-03 built on rhea
Repository revision: 2763a516a048c2cbabb10a5bbe22dc3bbde561f3
Repository branch: master
System Description: Fedora Linux 36 (Workstation Edition)

Configured using:
 'configure --with-pgtk --with-native-compilation --with-imagemagick'

[0001-Add-new-command-toggle-theme.patch (text/patch, attachment)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#57639; Package emacs. (Wed, 07 Sep 2022 08:07:02 GMT) Full text and rfc822 format available.

Message #8 received at 57639 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Kangas <stefankangas <at> gmail.com>
To: Philip Kaludercic <philipk <at> posteo.net>, 57639 <at> debbugs.gnu.org
Subject: Re: bug#57639: [PATCH] Add new command 'toggle-theme'
Date: Wed, 7 Sep 2022 04:06:36 -0400
Philip Kaludercic <philipk <at> posteo.net> writes:

> Find below a patch for a command that a lot of custom themes
> re-implement, whenever there exists a light and dark variant.

Could/should this be generalized to more than two variants?

The gruvbox theme, for example, has these variants:

    gruvbox-dark-hard
    gruvbox-dark-medium
    gruvbox-dark-soft
    gruvbox-light-hard
    gruvbox-light-medium
    gruvbox-light-soft

But I'm not sure what a reasonable user interface would look like.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#57639; Package emacs. (Wed, 07 Sep 2022 08:34:01 GMT) Full text and rfc822 format available.

Message #11 received at 57639 <at> debbugs.gnu.org (full text, mbox):

From: Philip Kaludercic <philipk <at> posteo.net>
To: Stefan Kangas <stefankangas <at> gmail.com>
Cc: 57639 <at> debbugs.gnu.org
Subject: Re: bug#57639: [PATCH] Add new command 'toggle-theme'
Date: Wed, 07 Sep 2022 08:33:08 +0000
Stefan Kangas <stefankangas <at> gmail.com> writes:

> Philip Kaludercic <philipk <at> posteo.net> writes:
>
>> Find below a patch for a command that a lot of custom themes
>> re-implement, whenever there exists a light and dark variant.
>
> Could/should this be generalized to more than two variants?

In principle the current implementation doesn't insist on there only
being two variants.  Each theme defines its dual, and these don't have
to be symmetrical.

> The gruvbox theme, for example, has these variants:
>
>     gruvbox-dark-hard
>     gruvbox-dark-medium
>     gruvbox-dark-soft
>     gruvbox-light-hard
>     gruvbox-light-medium
>     gruvbox-light-soft
>
> But I'm not sure what a reasonable user interface would look like.

E.g. in this case one could do:

(put 'gruvbox-dark-hard 'theme-dual 'gruvbox-dark-medium)
(put 'gruvbox-dark-medium 'theme-dual 'gruvbox-dark-soft)
(put 'gruvbox-dark-soft 'theme-dual 'gruvbox-light-hard)
(put 'gruvbox-light-hard 'theme-dual 'gruvbox-light-medium)
(put 'gruvbox-light-medium 'theme-dual 'gruvbox-light-soft)
(put 'gruvbox-light-soft 'theme-dual 'gruvbox-dark-hard)

But wouldn't just associating gruvbox-dark-hard with gruvbox-light-hard,
etc. be just as good?




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#57639; Package emacs. (Wed, 07 Sep 2022 08:53:02 GMT) Full text and rfc822 format available.

Message #14 received at 57639 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Kangas <stefankangas <at> gmail.com>
To: Philip Kaludercic <philipk <at> posteo.net>
Cc: 57639 <at> debbugs.gnu.org
Subject: Re: bug#57639: [PATCH] Add new command 'toggle-theme'
Date: Wed, 7 Sep 2022 01:51:58 -0700
Philip Kaludercic <philipk <at> posteo.net> writes:

> In principle the current implementation doesn't insist on there only
> being two variants.  Each theme defines its dual, and these don't have
> to be symmetrical.

That sounds good, thanks for clarifying.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#57639; Package emacs. (Wed, 07 Sep 2022 13:04:02 GMT) Full text and rfc822 format available.

Message #17 received at 57639 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Philip Kaludercic <philipk <at> posteo.net>
Cc: 57639 <at> debbugs.gnu.org
Subject: Re: bug#57639: [PATCH] Add new command 'toggle-theme'
Date: Wed, 07 Sep 2022 15:03:23 +0200
Philip Kaludercic <philipk <at> posteo.net> writes:

> Find below a patch for a command that a lot of custom themes
> re-implement, whenever there exists a light and dark variant.

I think it would also be nice if the themes themselves explicitly
defined themselves as being light/dark, really.  That way, when Emacs
finally grows a "react to the OS dark mode settings", Emacs could also
flip to the correct theme (if we have a dual theme).






Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#57639; Package emacs. (Wed, 07 Sep 2022 13:32:02 GMT) Full text and rfc822 format available.

Message #20 received at 57639 <at> debbugs.gnu.org (full text, mbox):

From: Philip Kaludercic <philipk <at> posteo.net>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: 57639 <at> debbugs.gnu.org
Subject: Re: bug#57639: [PATCH] Add new command 'toggle-theme'
Date: Wed, 07 Sep 2022 13:31:18 +0000
Lars Ingebrigtsen <larsi <at> gnus.org> writes:

> Philip Kaludercic <philipk <at> posteo.net> writes:
>
>> Find below a patch for a command that a lot of custom themes
>> re-implement, whenever there exists a light and dark variant.
>
> I think it would also be nice if the themes themselves explicitly
> defined themselves as being light/dark, really.  That way, when Emacs
> finally grows a "react to the OS dark mode settings", Emacs could also
> flip to the correct theme (if we have a dual theme).

Do you think it would be better to less general, and instead of talking
about "dual" themes always assume we want to toggle between a light and
a dark variant?  If not, this sounds like a separate feature (say that
could be handled by a different property `theme-variant').




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#57639; Package emacs. (Thu, 08 Sep 2022 11:47:01 GMT) Full text and rfc822 format available.

Message #23 received at 57639 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Philip Kaludercic <philipk <at> posteo.net>
Cc: 57639 <at> debbugs.gnu.org
Subject: Re: bug#57639: [PATCH] Add new command 'toggle-theme'
Date: Thu, 08 Sep 2022 13:46:15 +0200
Philip Kaludercic <philipk <at> posteo.net> writes:

> Do you think it would be better to less general, and instead of talking
> about "dual" themes always assume we want to toggle between a light and
> a dark variant?  If not, this sounds like a separate feature (say that
> could be handled by a different property `theme-variant').

Well, if we had marking for "theme with variant", then we'd know which
themes are "the same", but with a variant.

So your command could instead be `theme-choose-variant', but since
there's only two variants here, it'd toggle.  If there were more, the
user would be prompted for which variant to use, which would cover
gruvbox, which Stefan mentioned.

And in addition, this tagging could be used for automatic dark/light OS
changes, so it's at least three birds with one stone.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#57639; Package emacs. (Sun, 11 Sep 2022 08:28:01 GMT) Full text and rfc822 format available.

Message #26 received at 57639 <at> debbugs.gnu.org (full text, mbox):

From: Philip Kaludercic <philipk <at> posteo.net>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: 57639 <at> debbugs.gnu.org
Subject: Re: bug#57639: [PATCH] Add new command 'toggle-theme'
Date: Sun, 11 Sep 2022 08:26:53 +0000
Lars Ingebrigtsen <larsi <at> gnus.org> writes:

> Philip Kaludercic <philipk <at> posteo.net> writes:
>
>> Do you think it would be better to less general, and instead of talking
>> about "dual" themes always assume we want to toggle between a light and
>> a dark variant?  If not, this sounds like a separate feature (say that
>> could be handled by a different property `theme-variant').
>
> Well, if we had marking for "theme with variant", then we'd know which
> themes are "the same", but with a variant.
>
> So your command could instead be `theme-choose-variant', but since
> there's only two variants here, it'd toggle.  If there were more, the
> user would be prompted for which variant to use, which would cover
> gruvbox, which Stefan mentioned.

So we are thinking about something like a symbol property
`theme-variants' that each theme symbol may have attached.  This symbol
could point to a list representing a set of alternatives.  Let's say the
set might contain the theme itself (for the sake of convenience), so it
is remq'ed before we check if the set has more than one alternative.  If
it does we use a modified `load-theme'-like prompt, otherwise we toggle.
I think it would also make sense to silently remove non-existent themes
automatically, in case a variant theme is not part of the same package.

I think it would make sense to alias the name, since most themes, if
they do have variants just come with dark and light where "toggle" is
a more intuitive term.

> And in addition, this tagging could be used for automatic dark/light OS
> changes, so it's at least three birds with one stone.

OK, but that is a long-term plan, or do the means already exist for
detecting these kinds of changes?

Also, how to be distinguish between what is light and dark, and what do
we do when a theme has multiple variants?




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#57639; Package emacs. (Sun, 11 Sep 2022 11:13:02 GMT) Full text and rfc822 format available.

Message #29 received at 57639 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Philip Kaludercic <philipk <at> posteo.net>
Cc: 57639 <at> debbugs.gnu.org
Subject: Re: bug#57639: [PATCH] Add new command 'toggle-theme'
Date: Sun, 11 Sep 2022 13:11:51 +0200
Philip Kaludercic <philipk <at> posteo.net> writes:

> So we are thinking about something like a symbol property
> `theme-variants' that each theme symbol may have attached.  This symbol
> could point to a list representing a set of alternatives.  Let's say the
> set might contain the theme itself (for the sake of convenience), so it
> is remq'ed before we check if the set has more than one alternative.  If
> it does we use a modified `load-theme'-like prompt, otherwise we toggle.
> I think it would also make sense to silently remove non-existent themes
> automatically, in case a variant theme is not part of the same package.

I think that sounds correct, but I'm not 100% sure.  😀

>> And in addition, this tagging could be used for automatic dark/light OS
>> changes, so it's at least three birds with one stone.
>
> OK, but that is a long-term plan, or do the means already exist for
> detecting these kinds of changes?

Code exists for both Windows and Macos in the bug tracker somewhere, but
haven't been integrated yet because nobody has sat down to make a
consistent interface across all the platforms.

> Also, how to be distinguish between what is light and dark, and what do
> we do when a theme has multiple variants?

I'm not sure what you mean -- the proposal is to make the themes say
whether they consider themselves to be light or dark.  And I don't think
a theme would have multiple dark variants.





Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#57639; Package emacs. (Sun, 11 Sep 2022 11:23:02 GMT) Full text and rfc822 format available.

Message #32 received at 57639 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Philip Kaludercic <philipk <at> posteo.net>
Cc: 57639 <at> debbugs.gnu.org
Subject: Re: bug#57639: [PATCH] Add new command 'toggle-theme'
Date: Sun, 11 Sep 2022 13:22:40 +0200
Lars Ingebrigtsen <larsi <at> gnus.org> writes:

> I'm not sure what you mean -- the proposal is to make the themes say
> whether they consider themselves to be light or dark.  And I don't think
> a theme would have multiple dark variants.

But now I see that you've pushed `toggle-theme'...  *sigh*




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#57639; Package emacs. (Sun, 11 Sep 2022 11:43:02 GMT) Full text and rfc822 format available.

Message #35 received at 57639 <at> debbugs.gnu.org (full text, mbox):

From: Philip Kaludercic <philipk <at> posteo.net>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: 57639 <at> debbugs.gnu.org
Subject: Re: bug#57639: [PATCH] Add new command 'toggle-theme'
Date: Sun, 11 Sep 2022 11:42:33 +0000
Lars Ingebrigtsen <larsi <at> gnus.org> writes:

> Philip Kaludercic <philipk <at> posteo.net> writes:
>
>> So we are thinking about something like a symbol property
>> `theme-variants' that each theme symbol may have attached.  This symbol
>> could point to a list representing a set of alternatives.  Let's say the
>> set might contain the theme itself (for the sake of convenience), so it
>> is remq'ed before we check if the set has more than one alternative.  If
>> it does we use a modified `load-theme'-like prompt, otherwise we toggle.
>> I think it would also make sense to silently remove non-existent themes
>> automatically, in case a variant theme is not part of the same package.
>
> I think that sounds correct, but I'm not 100% sure.  😀

Ok, I'll implement this then.

>>> And in addition, this tagging could be used for automatic dark/light OS
>>> changes, so it's at least three birds with one stone.
>>
>> OK, but that is a long-term plan, or do the means already exist for
>> detecting these kinds of changes?
>
> Code exists for both Windows and Macos in the bug tracker somewhere, but
> haven't been integrated yet because nobody has sat down to make a
> consistent interface across all the platforms.

What about GNU/Linux desktops?

>> Also, how to be distinguish between what is light and dark, and what do
>> we do when a theme has multiple variants?
>
> I'm not sure what you mean -- the proposal is to make the themes say
> whether they consider themselves to be light or dark.  And I don't think
> a theme would have multiple dark variants.

Stefans Gruvbox example had multiple dark and light variants.  E.g.
Would the new command toggle from `gruvbox-dark-soft' to
`gruvbox-dark-light' or prompt the user to select from all the gruvbox
themes?

Lars Ingebrigtsen <larsi <at> gnus.org> writes:

> Lars Ingebrigtsen <larsi <at> gnus.org> writes:
>
>> I'm not sure what you mean -- the proposal is to make the themes say
>> whether they consider themselves to be light or dark.  And I don't think
>> a theme would have multiple dark variants.
>
> But now I see that you've pushed `toggle-theme'...  *sigh*

I'm very sorry about that, I've reverted those commits, since they were
accidentally pushed!




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#57639; Package emacs. (Sun, 11 Sep 2022 11:55:02 GMT) Full text and rfc822 format available.

Message #38 received at 57639 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Philip Kaludercic <philipk <at> posteo.net>
Cc: 57639 <at> debbugs.gnu.org
Subject: Re: bug#57639: [PATCH] Add new command 'toggle-theme'
Date: Sun, 11 Sep 2022 13:54:01 +0200
Philip Kaludercic <philipk <at> posteo.net> writes:

>> Code exists for both Windows and Macos in the bug tracker somewhere, but
>> haven't been integrated yet because nobody has sat down to make a
>> consistent interface across all the platforms.
>
> What about GNU/Linux desktops?

I thought we already had support for this under Gtk?  But I may be
misremembering.

> Stefans Gruvbox example had multiple dark and light variants.  E.g.
> Would the new command toggle from `gruvbox-dark-soft' to
> `gruvbox-dark-light' or prompt the user to select from all the gruvbox
> themes?

Yes, but if you have the soft gruvbox active, you don't want to switch
to the hard one when changing between dark/light, so there's no ambiguity.

>> But now I see that you've pushed `toggle-theme'...  *sigh*
>
> I'm very sorry about that, I've reverted those commits, since they were
> accidentally pushed!

*phew*  Thanks.  😀




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#57639; Package emacs. (Sun, 11 Sep 2022 18:49:01 GMT) Full text and rfc822 format available.

Message #41 received at 57639 <at> debbugs.gnu.org (full text, mbox):

From: Philip Kaludercic <philipk <at> posteo.net>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: 57639 <at> debbugs.gnu.org
Subject: Re: bug#57639: [PATCH] Add new command 'toggle-theme'
Date: Sun, 11 Sep 2022 18:47:40 +0000
[Message part 1 (text/plain, inline)]
Lars Ingebrigtsen <larsi <at> gnus.org> writes:

> Philip Kaludercic <philipk <at> posteo.net> writes:
>
>>> Code exists for both Windows and Macos in the bug tracker somewhere, but
>>> haven't been integrated yet because nobody has sat down to make a
>>> consistent interface across all the platforms.
>>
>> What about GNU/Linux desktops?
>
> I thought we already had support for this under Gtk?  But I may be
> misremembering.

Am I reading it correctly that `style_changed_cb' in gtkutils.c would be
a place to start looking for this functionality?

>> Stefans Gruvbox example had multiple dark and light variants.  E.g.
>> Would the new command toggle from `gruvbox-dark-soft' to
>> `gruvbox-dark-light' or prompt the user to select from all the gruvbox
>> themes?
>
> Yes, but if you have the soft gruvbox active, you don't want to switch
> to the hard one when changing between dark/light, so there's no ambiguity.

So would gruvbox-soft-dark have all the other themes as variants or only
gruvbox-soft-dark?  

>>> But now I see that you've pushed `toggle-theme'...  *sigh*
>>
>> I'm very sorry about that, I've reverted those commits, since they were
>> accidentally pushed!
>
> *phew*  Thanks.  😀

Here is an updated version:

[0001-Add-new-command-theme-choose-variant.patch (text/x-patch, attachment)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#57639; Package emacs. (Mon, 12 Sep 2022 10:11:01 GMT) Full text and rfc822 format available.

Message #44 received at 57639 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Philip Kaludercic <philipk <at> posteo.net>
Cc: Po Lu <luangruo <at> yahoo.com>, 57639 <at> debbugs.gnu.org
Subject: Re: bug#57639: [PATCH] Add new command 'toggle-theme'
Date: Mon, 12 Sep 2022 12:10:19 +0200
Philip Kaludercic <philipk <at> posteo.net> writes:

>> I thought we already had support for this under Gtk?  But I may be
>> misremembering.
>
> Am I reading it correctly that `style_changed_cb' in gtkutils.c would be
> a place to start looking for this functionality?

Ah, yes, I think that's the one...  Po Lu probably know more about this;
added to the CCs.

> So would gruvbox-soft-dark have all the other themes as variants or only
> gruvbox-soft-dark?  

Well, I was saying that the term "variants" isn't really what we need
here, but tagging themes for features.

So gruvbox-soft-dark would say

  (theme-featues :name 'gruvbox :mode 'dark :softness 'soft)

or...  feature names that make more sense.  😀

Then the `theme-change-feature' if you have a gruvbox theme enabled
would give you three other themes to choose from.

If there's just the dark/light difference, then there's only one other
theme to choose from, and no prompting would be needed.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#57639; Package emacs. (Mon, 12 Sep 2022 11:07:02 GMT) Full text and rfc822 format available.

Message #47 received at 57639 <at> debbugs.gnu.org (full text, mbox):

From: Philip Kaludercic <philipk <at> posteo.net>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: Po Lu <luangruo <at> yahoo.com>, 57639 <at> debbugs.gnu.org
Subject: Re: bug#57639: [PATCH] Add new command 'toggle-theme'
Date: Mon, 12 Sep 2022 11:06:45 +0000
Lars Ingebrigtsen <larsi <at> gnus.org> writes:

> Philip Kaludercic <philipk <at> posteo.net> writes:
>
>>> I thought we already had support for this under Gtk?  But I may be
>>> misremembering.
>>
>> Am I reading it correctly that `style_changed_cb' in gtkutils.c would be
>> a place to start looking for this functionality?
>
> Ah, yes, I think that's the one...  Po Lu probably know more about this;
> added to the CCs.
>
>> So would gruvbox-soft-dark have all the other themes as variants or only
>> gruvbox-soft-dark?  
>
> Well, I was saying that the term "variants" isn't really what we need
> here, but tagging themes for features.
>
> So gruvbox-soft-dark would say
>
>   (theme-featues :name 'gruvbox :mode 'dark :softness 'soft)
>
> or...  feature names that make more sense.  😀
>
> Then the `theme-change-feature' if you have a gruvbox theme enabled
> would give you three other themes to choose from.
>
> If there's just the dark/light difference, then there's only one other
> theme to choose from, and no prompting would be needed.

OK, now I understand what confused me.  It certainly sounds interesting,
I just wonder what "features" there really are, or should it be kept
open-ended (i.e. any theme can specify any number of attributes)?




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#57639; Package emacs. (Mon, 12 Sep 2022 11:13:02 GMT) Full text and rfc822 format available.

Message #50 received at 57639 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Philip Kaludercic <philipk <at> posteo.net>
Cc: Po Lu <luangruo <at> yahoo.com>, 57639 <at> debbugs.gnu.org
Subject: Re: bug#57639: [PATCH] Add new command 'toggle-theme'
Date: Mon, 12 Sep 2022 13:12:27 +0200
Philip Kaludercic <philipk <at> posteo.net> writes:

> OK, now I understand what confused me.  It certainly sounds interesting,
> I just wonder what "features" there really are, or should it be kept
> open-ended (i.e. any theme can specify any number of attributes)?

Yes, it's open-ended.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#57639; Package emacs. (Mon, 12 Sep 2022 13:13:01 GMT) Full text and rfc822 format available.

Message #53 received at 57639 <at> debbugs.gnu.org (full text, mbox):

From: Po Lu <luangruo <at> yahoo.com>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: Philip Kaludercic <philipk <at> posteo.net>, 57639 <at> debbugs.gnu.org
Subject: Re: bug#57639: [PATCH] Add new command 'toggle-theme'
Date: Mon, 12 Sep 2022 21:11:48 +0800
Lars Ingebrigtsen <larsi <at> gnus.org> writes:

> Philip Kaludercic <philipk <at> posteo.net> writes:
>
>>> I thought we already had support for this under Gtk?  But I may be
>>> misremembering.
>>
>> Am I reading it correctly that `style_changed_cb' in gtkutils.c would be
>> a place to start looking for this functionality?
>
> Ah, yes, I think that's the one...  Po Lu probably know more about this;
> added to the CCs.

What exactly is the wanted feature?

If it's automatically switching between dark and light themes, then
style_changed_cb is probably not the right place.  It's mostly vestigial
code from when we used to update the region face along with the GTK
stylesheet.  Instead, TRT used to be to listen for changes to the
`gtk-application-prefer-dark-theme' property of the GtkSettings object.

However, that has been superseeded by a dbus based interface in recent
versions of GNOME.  I do not understand the details, but it seems like
it could be implemented in Lisp:

  https://flatpak.github.io/xdg-desktop-portal/#gdbus-org.freedesktop.portal.Settings




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#57639; Package emacs. (Mon, 12 Sep 2022 14:53:01 GMT) Full text and rfc822 format available.

Message #56 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Michael Albinus <michael.albinus <at> gmx.de>
To: Po Lu via "Bug reports for GNU Emacs, the Swiss army knife of text
 editors" <bug-gnu-emacs <at> gnu.org>
Cc: Po Lu <luangruo <at> yahoo.com>, Lars Ingebrigtsen <larsi <at> gnus.org>,
 Philip Kaludercic <philipk <at> posteo.net>, 57639 <at> debbugs.gnu.org
Subject: Re: bug#57639: [PATCH] Add new command 'toggle-theme'
Date: Mon, 12 Sep 2022 16:51:54 +0200
Po Lu via "Bug reports for GNU Emacs, the Swiss army knife of text
editors" <bug-gnu-emacs <at> gnu.org> writes:

> If it's automatically switching between dark and light themes, then
> style_changed_cb is probably not the right place.  It's mostly vestigial
> code from when we used to update the region face along with the GTK
> stylesheet.  Instead, TRT used to be to listen for changes to the
> `gtk-application-prefer-dark-theme' property of the GtkSettings object.
>
> However, that has been superseeded by a dbus based interface in recent
> versions of GNOME.  I do not understand the details, but it seems like
> it could be implemented in Lisp:
>
>   https://flatpak.github.io/xdg-desktop-portal/#gdbus-org.freedesktop.portal.Settings

Definitely. Example D-Bus code is at <https://github.com/doomemacs/doomemacs/issues/6027>.

And if I understand
<https://github.com/flatpak/xdg-desktop-portal/blob/main/README.md>,
this is not restricted to GTK only.

Best regards, Michael.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#57639; Package emacs. (Mon, 12 Sep 2022 14:53:02 GMT) Full text and rfc822 format available.

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#57639; Package emacs. (Mon, 12 Sep 2022 15:27:01 GMT) Full text and rfc822 format available.

Message #62 received at 57639 <at> debbugs.gnu.org (full text, mbox):

From: Philip Kaludercic <philipk <at> posteo.net>
To: Po Lu <luangruo <at> yahoo.com>
Cc: Lars Ingebrigtsen <larsi <at> gnus.org>, 57639 <at> debbugs.gnu.org
Subject: Re: bug#57639: [PATCH] Add new command 'toggle-theme'
Date: Mon, 12 Sep 2022 15:26:25 +0000
Po Lu <luangruo <at> yahoo.com> writes:

> Lars Ingebrigtsen <larsi <at> gnus.org> writes:
>
>> Philip Kaludercic <philipk <at> posteo.net> writes:
>>
>>>> I thought we already had support for this under Gtk?  But I may be
>>>> misremembering.
>>>
>>> Am I reading it correctly that `style_changed_cb' in gtkutils.c would be
>>> a place to start looking for this functionality?
>>
>> Ah, yes, I think that's the one...  Po Lu probably know more about this;
>> added to the CCs.
>
> What exactly is the wanted feature?
>
> If it's automatically switching between dark and light themes

For now it is just detecting that a switch should be made.

>                                                              , then
> style_changed_cb is probably not the right place.  It's mostly vestigial
> code from when we used to update the region face along with the GTK
> stylesheet.  Instead, TRT used to be to listen for changes to the
> `gtk-application-prefer-dark-theme' property of the GtkSettings object.
>
> However, that has been superseeded by a dbus based interface in recent
> versions of GNOME.  I do not understand the details, but it seems like
> it could be implemented in Lisp:
>
>   https://flatpak.github.io/xdg-desktop-portal/#gdbus-org.freedesktop.portal.Settings

Would this have to be re-implemented for most desktop environments, or
do most of them use DBus to signal these changes?




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#57639; Package emacs. (Tue, 13 Sep 2022 02:27:02 GMT) Full text and rfc822 format available.

Message #65 received at 57639 <at> debbugs.gnu.org (full text, mbox):

From: Po Lu <luangruo <at> yahoo.com>
To: Philip Kaludercic <philipk <at> posteo.net>
Cc: Lars Ingebrigtsen <larsi <at> gnus.org>, 57639 <at> debbugs.gnu.org
Subject: Re: bug#57639: [PATCH] Add new command 'toggle-theme'
Date: Tue, 13 Sep 2022 10:25:45 +0800
Philip Kaludercic <philipk <at> posteo.net> writes:

> For now it is just detecting that a switch should be made.

Right... that's easier than trying to determine if a theme is dark or
light, but is still currently a mess on the X desktop.

> Would this have to be re-implemented for most desktop environments, or
> do most of them use DBus to signal these changes?

Recent releases of KDE and GNOME do.  I know nothing about the others,
but they probably do not provide a way to toggle between "dark" and
"light" themes in the first place.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#57639; Package emacs. (Tue, 13 Sep 2022 11:11:01 GMT) Full text and rfc822 format available.

Message #68 received at 57639 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Po Lu <luangruo <at> yahoo.com>
Cc: Philip Kaludercic <philipk <at> posteo.net>, 57639 <at> debbugs.gnu.org
Subject: Re: bug#57639: [PATCH] Add new command 'toggle-theme'
Date: Tue, 13 Sep 2022 13:10:19 +0200
Po Lu <luangruo <at> yahoo.com> writes:

> However, that has been superseeded by a dbus based interface in recent
> versions of GNOME.  I do not understand the details, but it seems like
> it could be implemented in Lisp:
>
>   https://flatpak.github.io/xdg-desktop-portal/#gdbus-org.freedesktop.portal.Settings

Yeah, doing this via dbus is probably the best solution for GNU/Linux.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#57639; Package emacs. (Tue, 13 Sep 2022 12:10:02 GMT) Full text and rfc822 format available.

Message #71 received at 57639 <at> debbugs.gnu.org (full text, mbox):

From: Po Lu <luangruo <at> yahoo.com>
To: Michael Albinus <michael.albinus <at> gmx.de>
Cc: Lars Ingebrigtsen <larsi <at> gnus.org>, Philip Kaludercic <philipk <at> posteo.net>,
 57639 <at> debbugs.gnu.org
Subject: Re: bug#57639: [PATCH] Add new command 'toggle-theme'
Date: Tue, 13 Sep 2022 20:09:08 +0800
Michael Albinus <michael.albinus <at> gmx.de> writes:

> And if I understand
> <https://github.com/flatpak/xdg-desktop-portal/blob/main/README.md>,
> this is not restricted to GTK only.

Indeed, it is not.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#57639; Package emacs. (Wed, 14 Sep 2022 11:39:02 GMT) Full text and rfc822 format available.

Message #74 received at 57639 <at> debbugs.gnu.org (full text, mbox):

From: Philip Kaludercic <philipk <at> posteo.net>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: 57639 <at> debbugs.gnu.org
Subject: Re: bug#57639: [PATCH] Add new command 'toggle-theme'
Date: Wed, 14 Sep 2022 11:38:11 +0000
Philip Kaludercic <philipk <at> posteo.net> writes:

> Here is an updated version:

Any comments on the updated patch?  Is it OK to push it in the current
state or is something missing?




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#57639; Package emacs. (Wed, 14 Sep 2022 13:04:01 GMT) Full text and rfc822 format available.

Message #77 received at 57639 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Philip Kaludercic <philipk <at> posteo.net>
Cc: 57639 <at> debbugs.gnu.org
Subject: Re: bug#57639: [PATCH] Add new command 'toggle-theme'
Date: Wed, 14 Sep 2022 15:03:28 +0200
Philip Kaludercic <philipk <at> posteo.net> writes:

>> Here is an updated version:
>
> Any comments on the updated patch?  Is it OK to push it in the current
> state or is something missing?

Well, the current state is with "variants" like

+(put 'tango 'theme-variants 'tango-dark) ;see `toggle-theme'

while I wanted to see tagging themes with features instead?




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#57639; Package emacs. (Wed, 14 Sep 2022 14:38:02 GMT) Full text and rfc822 format available.

Message #80 received at 57639 <at> debbugs.gnu.org (full text, mbox):

From: Philip Kaludercic <philipk <at> posteo.net>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: 57639 <at> debbugs.gnu.org
Subject: Re: bug#57639: [PATCH] Add new command 'toggle-theme'
Date: Wed, 14 Sep 2022 14:37:17 +0000
Lars Ingebrigtsen <larsi <at> gnus.org> writes:

> Philip Kaludercic <philipk <at> posteo.net> writes:
>
>>> Here is an updated version:
>>
>> Any comments on the updated patch?  Is it OK to push it in the current
>> state or is something missing?
>
> Well, the current state is with "variants" like
>
> +(put 'tango 'theme-variants 'tango-dark) ;see `toggle-theme'
>
> while I wanted to see tagging themes with features instead?

That was my question, should variants serve as the foundation for
tagging or is tagging an alternative for variants?




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#57639; Package emacs. (Wed, 14 Sep 2022 15:22:02 GMT) Full text and rfc822 format available.

Message #83 received at 57639 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Philip Kaludercic <philipk <at> posteo.net>
Cc: 57639 <at> debbugs.gnu.org
Subject: Re: bug#57639: [PATCH] Add new command 'toggle-theme'
Date: Wed, 14 Sep 2022 17:21:08 +0200
Philip Kaludercic <philipk <at> posteo.net> writes:

> That was my question, should variants serve as the foundation for
> tagging or is tagging an alternative for variants?

I think feature tagging should be the basis here, because then we can
use that tagging in several contexts (which we can't with variants).





Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#57639; Package emacs. (Sat, 17 Sep 2022 18:14:01 GMT) Full text and rfc822 format available.

Message #86 received at 57639 <at> debbugs.gnu.org (full text, mbox):

From: Philip Kaludercic <philipk <at> posteo.net>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: 57639 <at> debbugs.gnu.org
Subject: Re: bug#57639: [PATCH] Add new command 'toggle-theme'
Date: Sat, 17 Sep 2022 18:13:39 +0000
[Message part 1 (text/plain, inline)]
Lars Ingebrigtsen <larsi <at> gnus.org> writes:

> Philip Kaludercic <philipk <at> posteo.net> writes:
>
>> That was my question, should variants serve as the foundation for
>> tagging or is tagging an alternative for variants?
>
> I think feature tagging should be the basis here, because then we can
> use that tagging in several contexts (which we can't with variants).

Ok, I've updated the patch:

[0001-Tag-themes-with-properties.patch (text/x-patch, inline)]
From 0b346c567ffc0bd130b8c4157a9654134055f0e5 Mon Sep 17 00:00:00 2001
From: Philip Kaludercic <philipk <at> posteo.net>
Date: Sat, 17 Sep 2022 20:11:42 +0200
Subject: [PATCH] Tag themes with properties

* doc/emacs/custom.texi (Custom Themes): Document 'theme-choose-variant'.
* doc/lispref/customize.texi (Custom Themes): Document the new
optional argument to 'deftheme'.
* etc/themes/leuven-dark-theme.el (leuven-dark): Add properties.
* etc/themes/leuven-theme.el (leuven): Add properties.
* etc/themes/tango-dark-theme.el (tango-dark): Add properties.
* etc/themes/tango-theme.el (tango): Add properties.
* etc/themes/tsdh-dark-theme.el (tsdh-dark): Add properties.
* etc/themes/tsdh-light-theme.el (tsdh-light): Add properties.
* lisp/custom.el (deftheme): Allow for optional arguments to set the
property list.
(custom-declare-theme): Accept the same optional arguments as 'deftheme'.
(theme-list-variants): Add new function.
(theme-choose-variant): Add new command for switching between members
of a theme family.
(toggle-theme): Add an alias for 'theme-choose-variant'.
---
 doc/emacs/custom.texi           |  8 +++++
 doc/lispref/customize.texi      |  5 +--
 etc/themes/leuven-dark-theme.el |  6 ++--
 etc/themes/leuven-theme.el      |  6 ++--
 etc/themes/tango-dark-theme.el  |  4 ++-
 etc/themes/tango-theme.el       |  4 ++-
 etc/themes/tsdh-dark-theme.el   |  4 ++-
 etc/themes/tsdh-light-theme.el  |  4 ++-
 lisp/custom.el                  | 61 +++++++++++++++++++++++++++++----
 9 files changed, 86 insertions(+), 16 deletions(-)

diff --git a/doc/emacs/custom.texi b/doc/emacs/custom.texi
index ff7ab83190..f86c7e2dd9 100644
--- a/doc/emacs/custom.texi
+++ b/doc/emacs/custom.texi
@@ -667,6 +667,14 @@ Custom Themes
 the @file{*Custom Themes*} buffer; or type @kbd{M-x describe-theme}
 anywhere in Emacs and enter the theme name.
 
+@findex theme-choose-variant
+  Some themes have variants (most often these are light and dark
+pairs).  You can switch between these by typing @kbd{M-x
+theme-choose-variant}.  Note that this only works if only one theme is
+active.  If a theme has only one alternative, it will toggle
+automatically.  If there are more of them, it will query which one to
+use.
+
 @node Creating Custom Themes
 @subsection Creating Custom Themes
 @cindex custom themes, creating
diff --git a/doc/lispref/customize.texi b/doc/lispref/customize.texi
index 6ba35cffff..7a8b62a85b 100644
--- a/doc/lispref/customize.texi
+++ b/doc/lispref/customize.texi
@@ -1428,12 +1428,13 @@ Custom Themes
 be a call to @code{deftheme}, and the last form should be a call to
 @code{provide-theme}.
 
-@defmac deftheme theme &optional doc
+@defmac deftheme theme &optional doc &rest properties
 This macro declares @var{theme} (a symbol) as the name of a Custom
 theme.  The optional argument @var{doc} should be a string describing
 the theme; this is the description shown when the user invokes the
 @code{describe-theme} command or types @kbd{?} in the @samp{*Custom
-Themes*} buffer.
+Themes*} buffer.  The remaining arguments @var{properties} is used
+pass a property list with theme attributes.
 
 Two special theme names are disallowed (using them causes an error):
 @code{user} is a dummy theme that stores the user's direct
diff --git a/etc/themes/leuven-dark-theme.el b/etc/themes/leuven-dark-theme.el
index 0e162c8bab..91432326de 100644
--- a/etc/themes/leuven-dark-theme.el
+++ b/etc/themes/leuven-dark-theme.el
@@ -5,7 +5,7 @@
 ;; Author: Fabrice Niessen <(concat "fniessen" at-sign "pirilampo.org")>
 ;; Contributor: Thibault Polge <(concat "thibault" at-sign "thb.lt")>
 ;; URL: https://github.com/fniessen/emacs-leuven-dark-theme
-;; Version: 20220202.1126
+;; Version: 20220917.1823
 ;; Keywords: color theme
 
 ;; This file is part of GNU Emacs.
@@ -97,7 +97,9 @@ leuven-dark
   "Face colors with a light background.
 Basic, Font Lock, Isearch, Gnus, Message, Org mode, Diff, Ediff,
 Flyspell, Semantic, and Ansi-Color faces are included -- and much
-more...")
+more..."
+  :brightness 'dark
+  :family 'leuven)
 
 (let ((class '((class color) (min-colors 89)))
 
diff --git a/etc/themes/leuven-theme.el b/etc/themes/leuven-theme.el
index d9a8d5391a..264a455518 100644
--- a/etc/themes/leuven-theme.el
+++ b/etc/themes/leuven-theme.el
@@ -4,7 +4,7 @@
 
 ;; Author: Fabrice Niessen <(concat "fniessen" at-sign "pirilampo.org")>
 ;; URL: https://github.com/fniessen/emacs-leuven-theme
-;; Version: 20200513.1928
+;; Version: 20220917.1825
 ;; Keywords: color theme
 
 ;; This file is part of GNU Emacs.
@@ -78,7 +78,9 @@ leuven
   "Face colors with a light background.
 Basic, Font Lock, Isearch, Gnus, Message, Org mode, Diff, Ediff,
 Flyspell, Semantic, and Ansi-Color faces are included -- and much
-more...")
+more..."
+  :brightness 'light
+  :family 'leuven)
 
 (let ((class '((class color) (min-colors 89)))
 
diff --git a/etc/themes/tango-dark-theme.el b/etc/themes/tango-dark-theme.el
index ef00d2ac49..1e342aa946 100644
--- a/etc/themes/tango-dark-theme.el
+++ b/etc/themes/tango-dark-theme.el
@@ -30,7 +30,9 @@
 (deftheme tango-dark
   "Face colors using the Tango palette (dark background).
 Basic, Font Lock, Isearch, Gnus, Message, Ediff, Flyspell,
-Semantic, and Ansi-Color faces are included.")
+Semantic, and Ansi-Color faces are included."
+  :brightness 'dark
+  :family 'tango)
 
 (let ((class '((class color) (min-colors 89)))
       ;; Tango palette colors.
diff --git a/etc/themes/tango-theme.el b/etc/themes/tango-theme.el
index ecbbf03753..8c3d1298e5 100644
--- a/etc/themes/tango-theme.el
+++ b/etc/themes/tango-theme.el
@@ -30,7 +30,9 @@
 (deftheme tango
   "Face colors using the Tango palette (light background).
 Basic, Font Lock, Isearch, Gnus, Message, Ediff, Flyspell,
-Semantic, and Ansi-Color faces are included.")
+Semantic, and Ansi-Color faces are included."
+  :brightness 'light
+  :family 'tango)
 
 (let ((class '((class color) (min-colors 89)))
       ;; Tango palette colors.
diff --git a/etc/themes/tsdh-dark-theme.el b/etc/themes/tsdh-dark-theme.el
index a88ad75520..cfcad356a3 100644
--- a/etc/themes/tsdh-dark-theme.el
+++ b/etc/themes/tsdh-dark-theme.el
@@ -20,7 +20,9 @@
 ;;; Code:
 
 (deftheme tsdh-dark
-  "A dark theme used and created by Tassilo Horn.")
+  "A dark theme used and created by Tassilo Horn."
+  :brightness 'dark
+  :family 'tsdh)
 
 (custom-theme-set-faces
  'tsdh-dark
diff --git a/etc/themes/tsdh-light-theme.el b/etc/themes/tsdh-light-theme.el
index d9d09b702b..295f56a8b9 100644
--- a/etc/themes/tsdh-light-theme.el
+++ b/etc/themes/tsdh-light-theme.el
@@ -21,7 +21,9 @@
 
 (deftheme tsdh-light
   "A light Emacs theme.
-Used and created by Tassilo Horn.")
+Used and created by Tassilo Horn."
+  :brightness 'light
+  :family 'tsdh)
 
 (custom-theme-set-faces
  'tsdh-light
diff --git a/lisp/custom.el b/lisp/custom.el
index 352b5b0e16..2982573b7d 100644
--- a/lisp/custom.el
+++ b/lisp/custom.el
@@ -1152,9 +1152,11 @@ custom--sort-vars-1
 ;;   (provide-theme 'THEME)
 
 
-(defmacro deftheme (theme &optional doc)
+(defmacro deftheme (theme &optional doc &rest properties)
   "Declare THEME to be a Custom theme.
 The optional argument DOC is a doc string describing the theme.
+PROPERTIES are interpreted as a property list that will be stored
+in the `theme-properties' property for THEME.
 
 Any theme `foo' should be defined in a file called `foo-theme.el';
 see `custom-make-theme-feature' for more information."
@@ -1164,18 +1166,25 @@ deftheme
     ;; It is better not to use backquote in this file,
     ;; because that makes a bootstrapping problem
     ;; if you need to recompile all the Lisp files using interpreted code.
-    (list 'custom-declare-theme (list 'quote theme) (list 'quote feature) doc)))
+    (list 'custom-declare-theme (list 'quote theme) (list 'quote feature) doc
+          (cons 'list properties))))
 
-(defun custom-declare-theme (theme feature &optional doc)
+(defun custom-declare-theme (theme feature &optional doc properties)
   "Like `deftheme', but THEME is evaluated as a normal argument.
-FEATURE is the feature this theme provides.  Normally, this is a symbol
-created from THEME by `custom-make-theme-feature'."
+FEATURE is the feature this theme provides.  Normally, this is a
+symbol created from THEME by `custom-make-theme-feature'.  The
+optional argument DOC may contain the documentation for THEME.
+The optional argument PROPERTIES may contain a property list of
+attributes associated with THEME."
   (unless (custom-theme-name-valid-p theme)
     (error "Custom theme cannot be named %S" theme))
   (unless (memq theme custom-known-themes)
     (push theme custom-known-themes))
   (put theme 'theme-feature feature)
-  (when doc (put theme 'theme-documentation doc)))
+  (when doc
+    (put theme 'theme-documentation doc))
+  (when properties
+    (put theme 'theme-properties properties)))
 
 (defun custom-make-theme-feature (theme)
   "Given a symbol THEME, create a new symbol by appending \"-theme\".
@@ -1372,6 +1381,46 @@ load-theme
     (enable-theme theme))
   t)
 
+(defun theme-list-variants (theme &rest list)
+  "Return a list of theme variants for THEME.
+If the optional argument LIST is not given, "
+  (let* ((properties (get theme 'theme-properties))
+         (family (plist-get properties :family)))
+    (seq-filter
+     (lambda (variant)
+       (and (eq (plist-get (get variant 'theme-properties) :family)
+                family)
+            (not (eq variant theme))))
+     (or list (custom-available-themes)))))
+
+(defun theme-choose-variant (&optional no-confirm no-enable)
+  "Toggle the current active theme by enabling its dual pair.
+The current theme will be immediately disabled before the dual
+theme has been enabled.  If THEME is not active an error will be
+raised.  If theme is nil For NO-CONFIRM and NO-ENABLE, see
+`load-theme'."
+  (interactive)
+  (cond
+   ((length= custom-enabled-themes 0)
+    (user-error "No theme is active, cannot toggle"))
+   ((length> custom-enabled-themes 1)
+    (user-error "More than one theme active, cannot unambiguously toggle")))
+  (let* ((theme (car custom-enabled-themes))
+         (family (plist-get (get theme 'theme-properties) :family)))
+    (unless family
+      (error "`%s' is not part of a family" theme))
+    (let* ((variants (theme-list-variants theme))
+           (choice (cond
+                    ((null variants)
+                     (error "`%s' has no variants" theme))
+                    ((length= variants 1)
+                     (car variants))
+                    ((intern (completing-read "Load custom theme: " variants))))))
+      (disable-theme theme)
+      (load-theme choice no-confirm no-enable))))
+
+(defalias 'toggle-theme #'theme-choose-variant)
+
 (defun custom-theme-load-confirm (hash)
   "Query the user about loading a Custom theme that may not be safe.
 The theme should be in the current buffer.  If the user agrees,
-- 
2.37.3

[Message part 3 (text/plain, inline)]
The only issue with this approach is that the properties are set using
`deftheme', and are therefore not visible before the theme has been
loaded.  Do you (or anyone else) have any ideas how this could be
improved?

I guess the dirty way would be to set the symbol property using a manual
`put' call and autoload it.

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#57639; Package emacs. (Sat, 17 Sep 2022 18:33:02 GMT) Full text and rfc822 format available.

Message #89 received at 57639 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Philip Kaludercic <philipk <at> posteo.net>
Cc: larsi <at> gnus.org, 57639 <at> debbugs.gnu.org
Subject: Re: bug#57639: [PATCH] Add new command 'toggle-theme'
Date: Sat, 17 Sep 2022 21:32:42 +0300
> Cc: 57639 <at> debbugs.gnu.org
> From: Philip Kaludercic <philipk <at> posteo.net>
> Date: Sat, 17 Sep 2022 18:13:39 +0000
> 
> +@findex theme-choose-variant
> +  Some themes have variants (most often these are light and dark
> +pairs).  You can switch between these by typing @kbd{M-x
> +theme-choose-variant}.  Note that this only works if only one theme is
> +active.  If a theme has only one alternative, it will toggle
> +automatically.  If there are more of them, it will query which one to
> +use.

This description is confusing, I needed to read it several times
before I understood what you were trying to say.  The main problem is
that the "Note" sentence doesn't belong, and it interrupts the logic
of the description.  Here's my suggestion for saying it more clearly:

  Some themes have variants (most often just two: light and dark).
  You can switch to another variant with @kbd{M-x
  theme-choose-variant}.  If the currently active theme has only one
  other variant, it will be selected; if there are more variants, the
  command will prompt you which one to switch to.

  Note that @code{theme-choose-variant} only works if a single theme
  is active.

(Btw, what happens if more than one theme is active and the user
invokes theme-choose-variant? should this be described?)

> +Themes*} buffer.  The remaining arguments @var{properties} is used
> +pass a property list with theme attributes.                ^^^^^^^

"are used", in plural.

> +  :brightness 'light

Should we use :background-mode instead of :brightness, for consistency
with frame-background-mode?

> +(defun theme-choose-variant (&optional no-confirm no-enable)
> +  "Toggle the current active theme by enabling its dual pair.

"Toggle ... by enabling"?  "Dual pair"?  Can this sentence be
rephrased to be more clear?

> +The current theme will be immediately disabled before the dual
> +theme has been enabled.

Likewise here: "dual theme" doesn't explain itself, and seems to be
inaccurate, given the description in the manual.

>                          If THEME is not active an error will be
> +raised.

Passive tense alert!

>           If theme is nil For NO-CONFIRM and NO-ENABLE, see
> +`load-theme'."

Something's missing here or wrong with the punctuation?

> +   ((length> custom-enabled-themes 1)
> +    (user-error "More than one theme active, cannot unambiguously toggle")))

Wouldn't it be better to prompt for the theme in this case?

> +  (let* ((theme (car custom-enabled-themes))
> +         (family (plist-get (get theme 'theme-properties) :family)))
> +    (unless family
> +      (error "`%s' is not part of a family" theme))

"Family"? this terminology was never mentioned in the manual or the
doc string.  How about

  Theme `%s' does not have any variants

instead?

Thanks.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#57639; Package emacs. (Sat, 17 Sep 2022 21:34:02 GMT) Full text and rfc822 format available.

Message #92 received at 57639 <at> debbugs.gnu.org (full text, mbox):

From: Philip Kaludercic <philipk <at> posteo.net>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: larsi <at> gnus.org, 57639 <at> debbugs.gnu.org
Subject: Re: bug#57639: [PATCH] Add new command 'toggle-theme'
Date: Sat, 17 Sep 2022 21:33:18 +0000
[Message part 1 (text/plain, inline)]
Eli Zaretskii <eliz <at> gnu.org> writes:

>> Cc: 57639 <at> debbugs.gnu.org
>> From: Philip Kaludercic <philipk <at> posteo.net>
>> Date: Sat, 17 Sep 2022 18:13:39 +0000
>> 
>> +@findex theme-choose-variant
>> +  Some themes have variants (most often these are light and dark
>> +pairs).  You can switch between these by typing @kbd{M-x
>> +theme-choose-variant}.  Note that this only works if only one theme is
>> +active.  If a theme has only one alternative, it will toggle
>> +automatically.  If there are more of them, it will query which one to
>> +use.
>
> This description is confusing, I needed to read it several times
> before I understood what you were trying to say.  The main problem is
> that the "Note" sentence doesn't belong, and it interrupts the logic
> of the description.  Here's my suggestion for saying it more clearly:
>
>   Some themes have variants (most often just two: light and dark).
>   You can switch to another variant with @kbd{M-x
>   theme-choose-variant}.  If the currently active theme has only one
>   other variant, it will be selected; if there are more variants, the
>   command will prompt you which one to switch to.
>
>   Note that @code{theme-choose-variant} only works if a single theme
>   is active.

I prefer your phrasing and have adapted it in my next iteration of the
patch below.

> (Btw, what happens if more than one theme is active and the user
> invokes theme-choose-variant? should this be described?)

The current patch doesn't support this, but I've considered that a
custom theme could clarify that it is a colour scheme in the new property
list, and toggling would then not assume that there is only one active
theme but only one active colour scheme.

>> +Themes*} buffer.  The remaining arguments @var{properties} is used
>> +pass a property list with theme attributes.                ^^^^^^^
>
> "are used", in plural.

Fixed.

>> +  :brightness 'light
>
> Should we use :background-mode instead of :brightness, for consistency
> with frame-background-mode?

I chose "brightness" because I was looking at terms used in colour
theory and everyday language[0], and considered describing properties
such as "hue", "saturation", "contrast", etc. but that would probably be
overkill.  Consistency is probably preferable, so I'll also make this change.

[0] https://en.wikipedia.org/wiki/Color_term#In_natural_languages

>> +(defun theme-choose-variant (&optional no-confirm no-enable)
>> +  "Toggle the current active theme by enabling its dual pair.
>
> "Toggle ... by enabling"?  "Dual pair"?  Can this sentence be
> rephrased to be more clear?

>> +The current theme will be immediately disabled before the dual
>> +theme has been enabled.
>
> Likewise here: "dual theme" doesn't explain itself, and seems to be
> inaccurate, given the description in the manual.

You are right, the documentation was outdated.  See below.

>>                          If THEME is not active an error will be
>> +raised.
>
> Passive tense alert!
>
>>           If theme is nil For NO-CONFIRM and NO-ENABLE, see
>> +`load-theme'."
>
> Something's missing here or wrong with the punctuation?

The sentence was cut-off, so I just removed everything from "It" to
"nil".

>> +   ((length> custom-enabled-themes 1)
>> +    (user-error "More than one theme active, cannot unambiguously toggle")))
>
> Wouldn't it be better to prompt for the theme in this case?

That would be another possibility.  I guess it could be combined with
the above proposal.

>> +  (let* ((theme (car custom-enabled-themes))
>> +         (family (plist-get (get theme 'theme-properties) :family)))
>> +    (unless family
>> +      (error "`%s' is not part of a family" theme))
>
> "Family"? this terminology was never mentioned in the manual or the
> doc string.  How about
>
>   Theme `%s' does not have any variants
>
> instead?

Strictly speaking that error message would be wrong at this point,
because we cannot say if a theme has no variants if it is not part of a
family.  This is because variants of a theme are all those that are part
of the same family.  I think it would be better to clarify this in the
documentation.

> Thanks.

[0001-Tag-themes-with-properties.patch (text/x-patch, inline)]
From ae4f1596c479b9703c27b3635cff88f89a0b730a Mon Sep 17 00:00:00 2001
From: Philip Kaludercic <philipk <at> posteo.net>
Date: Sat, 17 Sep 2022 20:11:42 +0200
Subject: [PATCH] Tag themes with properties

* doc/emacs/custom.texi (Custom Themes): Document 'theme-choose-variant'.
* doc/lispref/customize.texi (Custom Themes): Document the new
optional argument to 'deftheme'.
* etc/themes/leuven-dark-theme.el (leuven-dark): Add properties.
* etc/themes/leuven-theme.el (leuven): Add properties.
* etc/themes/tango-dark-theme.el (tango-dark): Add properties.
* etc/themes/tango-theme.el (tango): Add properties.
* etc/themes/tsdh-dark-theme.el (tsdh-dark): Add properties.
* etc/themes/tsdh-light-theme.el (tsdh-light): Add properties.
* lisp/custom.el (deftheme): Allow for optional arguments to set the
property list.
(custom-declare-theme): Accept the same optional arguments as 'deftheme'.
(theme-list-variants): Add new function.
(theme-choose-variant): Add new command for switching between members
of a theme family.
(toggle-theme): Add an alias for 'theme-choose-variant'.
---
 doc/emacs/custom.texi           | 11 ++++++
 doc/lispref/customize.texi      |  5 +--
 etc/themes/leuven-dark-theme.el |  6 ++--
 etc/themes/leuven-theme.el      |  6 ++--
 etc/themes/tango-dark-theme.el  |  4 ++-
 etc/themes/tango-theme.el       |  4 ++-
 etc/themes/tsdh-dark-theme.el   |  4 ++-
 etc/themes/tsdh-light-theme.el  |  4 ++-
 lisp/custom.el                  | 62 +++++++++++++++++++++++++++++----
 9 files changed, 90 insertions(+), 16 deletions(-)

diff --git a/doc/emacs/custom.texi b/doc/emacs/custom.texi
index ff7ab83190..4d5a674564 100644
--- a/doc/emacs/custom.texi
+++ b/doc/emacs/custom.texi
@@ -667,6 +667,17 @@ Custom Themes
 the @file{*Custom Themes*} buffer; or type @kbd{M-x describe-theme}
 anywhere in Emacs and enter the theme name.
 
+@findex theme-choose-variant
+Some themes have variants (most often just two: light and dark).  In
+this case we say the theme is part of a family of themes.  You can
+switch to another variant within a family using @kbd{M-x
+theme-choose-variant}.  If the currently active theme has only one
+other variant, it will be selected; if there are more variants, the
+command will prompt you which one to switch to.
+
+Note that @code{theme-choose-variant} only works if a single theme
+is active.
+
 @node Creating Custom Themes
 @subsection Creating Custom Themes
 @cindex custom themes, creating
diff --git a/doc/lispref/customize.texi b/doc/lispref/customize.texi
index 6ba35cffff..911b6c4d75 100644
--- a/doc/lispref/customize.texi
+++ b/doc/lispref/customize.texi
@@ -1428,12 +1428,13 @@ Custom Themes
 be a call to @code{deftheme}, and the last form should be a call to
 @code{provide-theme}.
 
-@defmac deftheme theme &optional doc
+@defmac deftheme theme &optional doc &rest properties
 This macro declares @var{theme} (a symbol) as the name of a Custom
 theme.  The optional argument @var{doc} should be a string describing
 the theme; this is the description shown when the user invokes the
 @code{describe-theme} command or types @kbd{?} in the @samp{*Custom
-Themes*} buffer.
+Themes*} buffer.  The remaining arguments @var{properties} are used
+pass a property list with theme attributes.
 
 Two special theme names are disallowed (using them causes an error):
 @code{user} is a dummy theme that stores the user's direct
diff --git a/etc/themes/leuven-dark-theme.el b/etc/themes/leuven-dark-theme.el
index 0e162c8bab..42ebd7b2d6 100644
--- a/etc/themes/leuven-dark-theme.el
+++ b/etc/themes/leuven-dark-theme.el
@@ -5,7 +5,7 @@
 ;; Author: Fabrice Niessen <(concat "fniessen" at-sign "pirilampo.org")>
 ;; Contributor: Thibault Polge <(concat "thibault" at-sign "thb.lt")>
 ;; URL: https://github.com/fniessen/emacs-leuven-dark-theme
-;; Version: 20220202.1126
+;; Version: 20220917.2332
 ;; Keywords: color theme
 
 ;; This file is part of GNU Emacs.
@@ -97,7 +97,9 @@ leuven-dark
   "Face colors with a light background.
 Basic, Font Lock, Isearch, Gnus, Message, Org mode, Diff, Ediff,
 Flyspell, Semantic, and Ansi-Color faces are included -- and much
-more...")
+more..."
+  :background-mode 'dark
+  :family 'leuven)
 
 (let ((class '((class color) (min-colors 89)))
 
diff --git a/etc/themes/leuven-theme.el b/etc/themes/leuven-theme.el
index d9a8d5391a..07c34e944c 100644
--- a/etc/themes/leuven-theme.el
+++ b/etc/themes/leuven-theme.el
@@ -4,7 +4,7 @@
 
 ;; Author: Fabrice Niessen <(concat "fniessen" at-sign "pirilampo.org")>
 ;; URL: https://github.com/fniessen/emacs-leuven-theme
-;; Version: 20200513.1928
+;; Version: 20220917.2332
 ;; Keywords: color theme
 
 ;; This file is part of GNU Emacs.
@@ -78,7 +78,9 @@ leuven
   "Face colors with a light background.
 Basic, Font Lock, Isearch, Gnus, Message, Org mode, Diff, Ediff,
 Flyspell, Semantic, and Ansi-Color faces are included -- and much
-more...")
+more..."
+  :background-mode 'light
+  :family 'leuven)
 
 (let ((class '((class color) (min-colors 89)))
 
diff --git a/etc/themes/tango-dark-theme.el b/etc/themes/tango-dark-theme.el
index ef00d2ac49..fb5a1b29eb 100644
--- a/etc/themes/tango-dark-theme.el
+++ b/etc/themes/tango-dark-theme.el
@@ -30,7 +30,9 @@
 (deftheme tango-dark
   "Face colors using the Tango palette (dark background).
 Basic, Font Lock, Isearch, Gnus, Message, Ediff, Flyspell,
-Semantic, and Ansi-Color faces are included.")
+Semantic, and Ansi-Color faces are included."
+  :background-mode 'dark
+  :family 'tango)
 
 (let ((class '((class color) (min-colors 89)))
       ;; Tango palette colors.
diff --git a/etc/themes/tango-theme.el b/etc/themes/tango-theme.el
index ecbbf03753..026718bf38 100644
--- a/etc/themes/tango-theme.el
+++ b/etc/themes/tango-theme.el
@@ -30,7 +30,9 @@
 (deftheme tango
   "Face colors using the Tango palette (light background).
 Basic, Font Lock, Isearch, Gnus, Message, Ediff, Flyspell,
-Semantic, and Ansi-Color faces are included.")
+Semantic, and Ansi-Color faces are included."
+  :background-mode 'light
+  :family 'tango)
 
 (let ((class '((class color) (min-colors 89)))
       ;; Tango palette colors.
diff --git a/etc/themes/tsdh-dark-theme.el b/etc/themes/tsdh-dark-theme.el
index a88ad75520..ddd710a16e 100644
--- a/etc/themes/tsdh-dark-theme.el
+++ b/etc/themes/tsdh-dark-theme.el
@@ -20,7 +20,9 @@
 ;;; Code:
 
 (deftheme tsdh-dark
-  "A dark theme used and created by Tassilo Horn.")
+  "A dark theme used and created by Tassilo Horn."
+  :background-mode 'dark
+  :family 'tsdh)
 
 (custom-theme-set-faces
  'tsdh-dark
diff --git a/etc/themes/tsdh-light-theme.el b/etc/themes/tsdh-light-theme.el
index d9d09b702b..724b081880 100644
--- a/etc/themes/tsdh-light-theme.el
+++ b/etc/themes/tsdh-light-theme.el
@@ -21,7 +21,9 @@
 
 (deftheme tsdh-light
   "A light Emacs theme.
-Used and created by Tassilo Horn.")
+Used and created by Tassilo Horn."
+  :background-mode 'light
+  :family 'tsdh)
 
 (custom-theme-set-faces
  'tsdh-light
diff --git a/lisp/custom.el b/lisp/custom.el
index 352b5b0e16..f04241ca4d 100644
--- a/lisp/custom.el
+++ b/lisp/custom.el
@@ -1152,9 +1152,11 @@ custom--sort-vars-1
 ;;   (provide-theme 'THEME)
 
 
-(defmacro deftheme (theme &optional doc)
+(defmacro deftheme (theme &optional doc &rest properties)
   "Declare THEME to be a Custom theme.
 The optional argument DOC is a doc string describing the theme.
+PROPERTIES are interpreted as a property list that will be stored
+in the `theme-properties' property for THEME.
 
 Any theme `foo' should be defined in a file called `foo-theme.el';
 see `custom-make-theme-feature' for more information."
@@ -1164,18 +1166,25 @@ deftheme
     ;; It is better not to use backquote in this file,
     ;; because that makes a bootstrapping problem
     ;; if you need to recompile all the Lisp files using interpreted code.
-    (list 'custom-declare-theme (list 'quote theme) (list 'quote feature) doc)))
+    (list 'custom-declare-theme (list 'quote theme) (list 'quote feature) doc
+          (cons 'list properties))))
 
-(defun custom-declare-theme (theme feature &optional doc)
+(defun custom-declare-theme (theme feature &optional doc properties)
   "Like `deftheme', but THEME is evaluated as a normal argument.
-FEATURE is the feature this theme provides.  Normally, this is a symbol
-created from THEME by `custom-make-theme-feature'."
+FEATURE is the feature this theme provides.  Normally, this is a
+symbol created from THEME by `custom-make-theme-feature'.  The
+optional argument DOC may contain the documentation for THEME.
+The optional argument PROPERTIES may contain a property list of
+attributes associated with THEME."
   (unless (custom-theme-name-valid-p theme)
     (error "Custom theme cannot be named %S" theme))
   (unless (memq theme custom-known-themes)
     (push theme custom-known-themes))
   (put theme 'theme-feature feature)
-  (when doc (put theme 'theme-documentation doc)))
+  (when doc
+    (put theme 'theme-documentation doc))
+  (when properties
+    (put theme 'theme-properties properties)))
 
 (defun custom-make-theme-feature (theme)
   "Given a symbol THEME, create a new symbol by appending \"-theme\".
@@ -1372,6 +1381,47 @@ load-theme
     (enable-theme theme))
   t)
 
+(defun theme-list-variants (theme &rest list)
+  "Return a list of theme variants for THEME.
+If the optional argument LIST is not given, "
+  (let* ((properties (get theme 'theme-properties))
+         (family (plist-get properties :family)))
+    (seq-filter
+     (lambda (variant)
+       (and (eq (plist-get (get variant 'theme-properties) :family)
+                family)
+            (not (eq variant theme))))
+     (or list (custom-available-themes)))))
+
+(defun theme-choose-variant (&optional no-confirm no-enable)
+  "Prompt to switch from the current theme to a variant.
+Themes only have variants if they are part of a family of themes.
+The current theme will be immediately disabled before variant is
+enabled.  In case the current theme has only one variant, it will
+be toggled without prompting.  For NO-CONFIRM and NO-ENABLE, see
+`load-theme'."
+  (interactive)
+  (cond
+   ((length= custom-enabled-themes 0)
+    (user-error "No theme is active, cannot toggle"))
+   ((length> custom-enabled-themes 1)
+    (user-error "More than one theme active, cannot unambiguously toggle")))
+  (let* ((theme (car custom-enabled-themes))
+         (family (plist-get (get theme 'theme-properties) :family)))
+    (unless family
+      (error "`%s' is not part of a family" theme))
+    (let* ((variants (theme-list-variants theme))
+           (choice (cond
+                    ((null variants)
+                     (error "`%s' has no variants" theme))
+                    ((length= variants 1)
+                     (car variants))
+                    ((intern (completing-read "Load custom theme: " variants))))))
+      (disable-theme theme)
+      (load-theme choice no-confirm no-enable))))
+
+(defalias 'toggle-theme #'theme-choose-variant)
+
 (defun custom-theme-load-confirm (hash)
   "Query the user about loading a Custom theme that may not be safe.
 The theme should be in the current buffer.  If the user agrees,
-- 
2.37.3


Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#57639; Package emacs. (Sun, 18 Sep 2022 06:55:01 GMT) Full text and rfc822 format available.

Message #95 received at 57639 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Philip Kaludercic <philipk <at> posteo.net>
Cc: larsi <at> gnus.org, 57639 <at> debbugs.gnu.org
Subject: Re: bug#57639: [PATCH] Add new command 'toggle-theme'
Date: Sun, 18 Sep 2022 09:53:53 +0300
> From: Philip Kaludercic <philipk <at> posteo.net>
> Cc: larsi <at> gnus.org,  57639 <at> debbugs.gnu.org
> Date: Sat, 17 Sep 2022 21:33:18 +0000
> 
> >> +  (let* ((theme (car custom-enabled-themes))
> >> +         (family (plist-get (get theme 'theme-properties) :family)))
> >> +    (unless family
> >> +      (error "`%s' is not part of a family" theme))
> >
> > "Family"? this terminology was never mentioned in the manual or the
> > doc string.  How about
> >
> >   Theme `%s' does not have any variants
> >
> > instead?
> 
> Strictly speaking that error message would be wrong at this point,
> because we cannot say if a theme has no variants if it is not part of a
> family.  This is because variants of a theme are all those that are part
> of the same family.  I think it would be better to clarify this in the
> documentation.

But the documentation doesn't explain what it means for a theme to be
part of a family.  Specifically, how does one tell, by looking at a
theme, whether it is or isn't part of a family?

An alternative for what I suggested above is to say

  Theme `%s' does not have any known variants

> +(defun theme-choose-variant (&optional no-confirm no-enable)
> +  "Prompt to switch from the current theme to a variant.
                                              ^^^^^^^^^^^^
"to one of its variants" is more clear.

> +Themes only have variants if they are part of a family of themes.

This should explain what it means to be part of a family, otherwise
this sentence is not helpful.

> +The current theme will be immediately disabled before variant is
> +enabled.

The "immediately" part should probably be removed, as it doesn't seem
to convey anything of importance, does it?  Come to think of it, what
exactly does this sentence try to say? isn't it obvious that the
current theme will be disabled and then the variant will be enabled?

>          In case the current theme has only one variant, it will
> +be toggled without prompting.

"toggled" is wrong here.  I suggest

  If the current theme has only one variant, switch to that variant
  without prompting, otherwise prompt for the variant to select.

>                                 For NO-CONFIRM and NO-ENABLE, see
> +`load-theme'."

I suggest to say this the other way around:

   See `load-theme' for the meaning of NO-CONFIRM and NO-ENABLE.

Thanks.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#57639; Package emacs. (Sun, 18 Sep 2022 09:39:01 GMT) Full text and rfc822 format available.

Message #98 received at 57639 <at> debbugs.gnu.org (full text, mbox):

From: Philip Kaludercic <philipk <at> posteo.net>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: larsi <at> gnus.org, 57639 <at> debbugs.gnu.org
Subject: Re: bug#57639: [PATCH] Add new command 'toggle-theme'
Date: Sun, 18 Sep 2022 09:38:02 +0000
[Message part 1 (text/plain, inline)]
Eli Zaretskii <eliz <at> gnu.org> writes:

>> From: Philip Kaludercic <philipk <at> posteo.net>
>> Cc: larsi <at> gnus.org,  57639 <at> debbugs.gnu.org
>> Date: Sat, 17 Sep 2022 21:33:18 +0000
>> 
>> >> +  (let* ((theme (car custom-enabled-themes))
>> >> +         (family (plist-get (get theme 'theme-properties) :family)))
>> >> +    (unless family
>> >> +      (error "`%s' is not part of a family" theme))
>> >
>> > "Family"? this terminology was never mentioned in the manual or the
>> > doc string.  How about
>> >
>> >   Theme `%s' does not have any variants
>> >
>> > instead?
>> 
>> Strictly speaking that error message would be wrong at this point,
>> because we cannot say if a theme has no variants if it is not part of a
>> family.  This is because variants of a theme are all those that are part
>> of the same family.  I think it would be better to clarify this in the
>> documentation.
>
> But the documentation doesn't explain what it means for a theme to be
> part of a family.  Specifically, how does one tell, by looking at a
> theme, whether it is or isn't part of a family?
>
> An alternative for what I suggested above is to say
>
>   Theme `%s' does not have any known variants

I get what you mean... how about

    (error "Theme `%s' is not part of a family of variants" theme)

?

>> +(defun theme-choose-variant (&optional no-confirm no-enable)
>> +  "Prompt to switch from the current theme to a variant.
>                                               ^^^^^^^^^^^^
> "to one of its variants" is more clear.

Done.

>> +Themes only have variants if they are part of a family of themes.
>
> This should explain what it means to be part of a family, otherwise
> this sentence is not helpful.

My intention was for this to be an explanation.  The issue is that
variants and family are mutually recursive concepts:

- A variant of a theme are those which are part of the same family
- A family of themes is the set of all variants of a theme.

Perhaps it is just easier to collapse both concepts into either variant
of family and just "expose" by documenting it.  I've tried doing so
below.

>> +The current theme will be immediately disabled before variant is
>> +enabled.
>
> The "immediately" part should probably be removed, as it doesn't seem
> to convey anything of importance, does it?  Come to think of it, what
> exactly does this sentence try to say? isn't it obvious that the
> current theme will be disabled and then the variant will be enabled?

I guess it isn't necessary, the point was just to emphasise that nothing
happens between disabling the previous theme and enabling the variant.
There was something I had in mind when writing this initially, but I
cannot recall it anymore.

So I've removed it for now.

>>          In case the current theme has only one variant, it will
>> +be toggled without prompting.
>
> "toggled" is wrong here.  I suggest
>
>   If the current theme has only one variant, switch to that variant
>   without prompting, otherwise prompt for the variant to select.

Done.

>>                                 For NO-CONFIRM and NO-ENABLE, see
>> +`load-theme'."
>
> I suggest to say this the other way around:
>
>    See `load-theme' for the meaning of NO-CONFIRM and NO-ENABLE.

Done.

> Thanks.

[0001-Tag-themes-with-properties.patch (text/x-patch, inline)]
From 4aa11070669c4b7c802d54d7dd7dffbd41a1a409 Mon Sep 17 00:00:00 2001
From: Philip Kaludercic <philipk <at> posteo.net>
Date: Sat, 17 Sep 2022 20:11:42 +0200
Subject: [PATCH] Tag themes with properties

* doc/emacs/custom.texi (Custom Themes): Document 'theme-choose-variant'.
* doc/lispref/customize.texi (Custom Themes): Document the new
optional argument to 'deftheme'.
* etc/themes/leuven-dark-theme.el (leuven-dark): Add properties.
* etc/themes/leuven-theme.el (leuven): Add properties.
* etc/themes/tango-dark-theme.el (tango-dark): Add properties.
* etc/themes/tango-theme.el (tango): Add properties.
* etc/themes/tsdh-dark-theme.el (tsdh-dark): Add properties.
* etc/themes/tsdh-light-theme.el (tsdh-light): Add properties.
* lisp/custom.el (deftheme): Allow for optional arguments to set the
property list.
(custom-declare-theme): Accept the same optional arguments as 'deftheme'.
(theme-list-variants): Add new function.
(theme-choose-variant): Add new command for switching between members
of a theme family.
(toggle-theme): Add an alias for 'theme-choose-variant'.
---
 doc/emacs/custom.texi           | 10 ++++++
 doc/lispref/customize.texi      |  5 +--
 etc/themes/leuven-dark-theme.el |  6 ++--
 etc/themes/leuven-theme.el      |  6 ++--
 etc/themes/tango-dark-theme.el  |  4 ++-
 etc/themes/tango-theme.el       |  4 ++-
 etc/themes/tsdh-dark-theme.el   |  4 ++-
 etc/themes/tsdh-light-theme.el  |  4 ++-
 lisp/custom.el                  | 61 +++++++++++++++++++++++++++++----
 9 files changed, 88 insertions(+), 16 deletions(-)

diff --git a/doc/emacs/custom.texi b/doc/emacs/custom.texi
index ff7ab83190..f98527bf9a 100644
--- a/doc/emacs/custom.texi
+++ b/doc/emacs/custom.texi
@@ -667,6 +667,16 @@ Custom Themes
 the @file{*Custom Themes*} buffer; or type @kbd{M-x describe-theme}
 anywhere in Emacs and enter the theme name.
 
+@findex theme-choose-variant
+Some themes have variants (most often just two: light and dark).  You
+can switch to another variant using @kbd{M-x theme-choose-variant}.
+If the currently active theme has only one other variant, it will be
+selected; if there are more variants, the command will prompt you
+which one to switch to.
+
+Note that @code{theme-choose-variant} only works if a single theme
+is active.
+
 @node Creating Custom Themes
 @subsection Creating Custom Themes
 @cindex custom themes, creating
diff --git a/doc/lispref/customize.texi b/doc/lispref/customize.texi
index 6ba35cffff..911b6c4d75 100644
--- a/doc/lispref/customize.texi
+++ b/doc/lispref/customize.texi
@@ -1428,12 +1428,13 @@ Custom Themes
 be a call to @code{deftheme}, and the last form should be a call to
 @code{provide-theme}.
 
-@defmac deftheme theme &optional doc
+@defmac deftheme theme &optional doc &rest properties
 This macro declares @var{theme} (a symbol) as the name of a Custom
 theme.  The optional argument @var{doc} should be a string describing
 the theme; this is the description shown when the user invokes the
 @code{describe-theme} command or types @kbd{?} in the @samp{*Custom
-Themes*} buffer.
+Themes*} buffer.  The remaining arguments @var{properties} are used
+pass a property list with theme attributes.
 
 Two special theme names are disallowed (using them causes an error):
 @code{user} is a dummy theme that stores the user's direct
diff --git a/etc/themes/leuven-dark-theme.el b/etc/themes/leuven-dark-theme.el
index 0e162c8bab..42ebd7b2d6 100644
--- a/etc/themes/leuven-dark-theme.el
+++ b/etc/themes/leuven-dark-theme.el
@@ -5,7 +5,7 @@
 ;; Author: Fabrice Niessen <(concat "fniessen" at-sign "pirilampo.org")>
 ;; Contributor: Thibault Polge <(concat "thibault" at-sign "thb.lt")>
 ;; URL: https://github.com/fniessen/emacs-leuven-dark-theme
-;; Version: 20220202.1126
+;; Version: 20220917.2332
 ;; Keywords: color theme
 
 ;; This file is part of GNU Emacs.
@@ -97,7 +97,9 @@ leuven-dark
   "Face colors with a light background.
 Basic, Font Lock, Isearch, Gnus, Message, Org mode, Diff, Ediff,
 Flyspell, Semantic, and Ansi-Color faces are included -- and much
-more...")
+more..."
+  :background-mode 'dark
+  :family 'leuven)
 
 (let ((class '((class color) (min-colors 89)))
 
diff --git a/etc/themes/leuven-theme.el b/etc/themes/leuven-theme.el
index d9a8d5391a..07c34e944c 100644
--- a/etc/themes/leuven-theme.el
+++ b/etc/themes/leuven-theme.el
@@ -4,7 +4,7 @@
 
 ;; Author: Fabrice Niessen <(concat "fniessen" at-sign "pirilampo.org")>
 ;; URL: https://github.com/fniessen/emacs-leuven-theme
-;; Version: 20200513.1928
+;; Version: 20220917.2332
 ;; Keywords: color theme
 
 ;; This file is part of GNU Emacs.
@@ -78,7 +78,9 @@ leuven
   "Face colors with a light background.
 Basic, Font Lock, Isearch, Gnus, Message, Org mode, Diff, Ediff,
 Flyspell, Semantic, and Ansi-Color faces are included -- and much
-more...")
+more..."
+  :background-mode 'light
+  :family 'leuven)
 
 (let ((class '((class color) (min-colors 89)))
 
diff --git a/etc/themes/tango-dark-theme.el b/etc/themes/tango-dark-theme.el
index ef00d2ac49..fb5a1b29eb 100644
--- a/etc/themes/tango-dark-theme.el
+++ b/etc/themes/tango-dark-theme.el
@@ -30,7 +30,9 @@
 (deftheme tango-dark
   "Face colors using the Tango palette (dark background).
 Basic, Font Lock, Isearch, Gnus, Message, Ediff, Flyspell,
-Semantic, and Ansi-Color faces are included.")
+Semantic, and Ansi-Color faces are included."
+  :background-mode 'dark
+  :family 'tango)
 
 (let ((class '((class color) (min-colors 89)))
       ;; Tango palette colors.
diff --git a/etc/themes/tango-theme.el b/etc/themes/tango-theme.el
index ecbbf03753..026718bf38 100644
--- a/etc/themes/tango-theme.el
+++ b/etc/themes/tango-theme.el
@@ -30,7 +30,9 @@
 (deftheme tango
   "Face colors using the Tango palette (light background).
 Basic, Font Lock, Isearch, Gnus, Message, Ediff, Flyspell,
-Semantic, and Ansi-Color faces are included.")
+Semantic, and Ansi-Color faces are included."
+  :background-mode 'light
+  :family 'tango)
 
 (let ((class '((class color) (min-colors 89)))
       ;; Tango palette colors.
diff --git a/etc/themes/tsdh-dark-theme.el b/etc/themes/tsdh-dark-theme.el
index a88ad75520..ddd710a16e 100644
--- a/etc/themes/tsdh-dark-theme.el
+++ b/etc/themes/tsdh-dark-theme.el
@@ -20,7 +20,9 @@
 ;;; Code:
 
 (deftheme tsdh-dark
-  "A dark theme used and created by Tassilo Horn.")
+  "A dark theme used and created by Tassilo Horn."
+  :background-mode 'dark
+  :family 'tsdh)
 
 (custom-theme-set-faces
  'tsdh-dark
diff --git a/etc/themes/tsdh-light-theme.el b/etc/themes/tsdh-light-theme.el
index d9d09b702b..724b081880 100644
--- a/etc/themes/tsdh-light-theme.el
+++ b/etc/themes/tsdh-light-theme.el
@@ -21,7 +21,9 @@
 
 (deftheme tsdh-light
   "A light Emacs theme.
-Used and created by Tassilo Horn.")
+Used and created by Tassilo Horn."
+  :background-mode 'light
+  :family 'tsdh)
 
 (custom-theme-set-faces
  'tsdh-light
diff --git a/lisp/custom.el b/lisp/custom.el
index 352b5b0e16..5a3d3b95e2 100644
--- a/lisp/custom.el
+++ b/lisp/custom.el
@@ -1152,9 +1152,11 @@ custom--sort-vars-1
 ;;   (provide-theme 'THEME)
 
 
-(defmacro deftheme (theme &optional doc)
+(defmacro deftheme (theme &optional doc &rest properties)
   "Declare THEME to be a Custom theme.
 The optional argument DOC is a doc string describing the theme.
+PROPERTIES are interpreted as a property list that will be stored
+in the `theme-properties' property for THEME.
 
 Any theme `foo' should be defined in a file called `foo-theme.el';
 see `custom-make-theme-feature' for more information."
@@ -1164,18 +1166,25 @@ deftheme
     ;; It is better not to use backquote in this file,
     ;; because that makes a bootstrapping problem
     ;; if you need to recompile all the Lisp files using interpreted code.
-    (list 'custom-declare-theme (list 'quote theme) (list 'quote feature) doc)))
+    (list 'custom-declare-theme (list 'quote theme) (list 'quote feature) doc
+          (cons 'list properties))))
 
-(defun custom-declare-theme (theme feature &optional doc)
+(defun custom-declare-theme (theme feature &optional doc properties)
   "Like `deftheme', but THEME is evaluated as a normal argument.
-FEATURE is the feature this theme provides.  Normally, this is a symbol
-created from THEME by `custom-make-theme-feature'."
+FEATURE is the feature this theme provides.  Normally, this is a
+symbol created from THEME by `custom-make-theme-feature'.  The
+optional argument DOC may contain the documentation for THEME.
+The optional argument PROPERTIES may contain a property list of
+attributes associated with THEME."
   (unless (custom-theme-name-valid-p theme)
     (error "Custom theme cannot be named %S" theme))
   (unless (memq theme custom-known-themes)
     (push theme custom-known-themes))
   (put theme 'theme-feature feature)
-  (when doc (put theme 'theme-documentation doc)))
+  (when doc
+    (put theme 'theme-documentation doc))
+  (when properties
+    (put theme 'theme-properties properties)))
 
 (defun custom-make-theme-feature (theme)
   "Given a symbol THEME, create a new symbol by appending \"-theme\".
@@ -1372,6 +1381,46 @@ load-theme
     (enable-theme theme))
   t)
 
+(defun theme-list-variants (theme &rest list)
+  "Return a list of theme variants for THEME.
+If the optional argument LIST is not given, "
+  (let* ((properties (get theme 'theme-properties))
+         (family (plist-get properties :family)))
+    (seq-filter
+     (lambda (variant)
+       (and (eq (plist-get (get variant 'theme-properties) :family)
+                family)
+            (not (eq variant theme))))
+     (or list (custom-available-themes)))))
+
+(defun theme-choose-variant (&optional no-confirm no-enable)
+  "Prompt to switch from the current theme to one of its a variants.
+The current theme will be disabled before variant is enabled.  If
+the current theme has only one variant, switch to that variant
+without prompting, otherwise prompt for the variant to select.
+See `load-theme' for the meaning of NO-CONFIRM and NO-ENABLE."
+  (interactive)
+  (cond
+   ((length= custom-enabled-themes 0)
+    (user-error "No theme is active, cannot toggle"))
+   ((length> custom-enabled-themes 1)
+    (user-error "More than one theme active, cannot unambiguously toggle")))
+  (let* ((theme (car custom-enabled-themes))
+         (family (plist-get (get theme 'theme-properties) :family)))
+    (unless family
+      (error "Theme `%s' does not have any known variants" theme))
+    (let* ((variants (theme-list-variants theme))
+           (choice (cond
+                    ((null variants)
+                     (error "`%s' has no variants" theme))
+                    ((length= variants 1)
+                     (car variants))
+                    ((intern (completing-read "Load custom theme: " variants))))))
+      (disable-theme theme)
+      (load-theme choice no-confirm no-enable))))
+
+(defalias 'toggle-theme #'theme-choose-variant)
+
 (defun custom-theme-load-confirm (hash)
   "Query the user about loading a Custom theme that may not be safe.
 The theme should be in the current buffer.  If the user agrees,
-- 
2.37.3


Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#57639; Package emacs. (Sun, 18 Sep 2022 10:40:02 GMT) Full text and rfc822 format available.

Message #101 received at 57639 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Philip Kaludercic <philipk <at> posteo.net>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 57639 <at> debbugs.gnu.org
Subject: Re: bug#57639: [PATCH] Add new command 'toggle-theme'
Date: Sun, 18 Sep 2022 12:39:27 +0200
Philip Kaludercic <philipk <at> posteo.net> writes:

> Subject: [PATCH] Tag themes with properties

Thanks; looks good to me.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#57639; Package emacs. (Sun, 18 Sep 2022 11:40:01 GMT) Full text and rfc822 format available.

Message #104 received at 57639 <at> debbugs.gnu.org (full text, mbox):

From: Philip Kaludercic <philipk <at> posteo.net>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 57639 <at> debbugs.gnu.org
Subject: Re: bug#57639: [PATCH] Add new command 'toggle-theme'
Date: Sun, 18 Sep 2022 11:39:39 +0000
[Message part 1 (text/plain, inline)]
Lars Ingebrigtsen <larsi <at> gnus.org> writes:

> Philip Kaludercic <philipk <at> posteo.net> writes:
>
>> Subject: [PATCH] Tag themes with properties
>
> Thanks; looks good to me.

One last additional, I've added a property to all the themes indicating
that they are colour schemes.  This would make it easier to toggle
between dark and light modes if multiple themes are enabled of which
only one is a colour theme:

[0001-Tag-themes-with-properties.patch (text/x-patch, inline)]
From 62c55cc27024348e43ae4591c3d239d705f8ad1b Mon Sep 17 00:00:00 2001
From: Philip Kaludercic <philipk <at> posteo.net>
Date: Sat, 17 Sep 2022 20:11:42 +0200
Subject: [PATCH] Tag themes with properties

* doc/emacs/custom.texi (Custom Themes): Document 'theme-choose-variant'.
* doc/lispref/customize.texi (Custom Themes): Document the new
optional argument to 'deftheme'.
* etc/themes/adwaita-theme.el (adwaita): Add properties.
* etc/themes/deeper-blue-theme.el (deeper-blue): Add properties.
* etc/themes/dichromacy-theme.el (dichromacy): Add properties.
* etc/themes/light-blue-theme.el (light-blue): Add properties.
* etc/themes/manoj-dark-theme.el (manoj-dark): Add properties.
* etc/themes/misterioso-theme.el (misterioso): Add properties.
* etc/themes/tango-dark-theme.el (tango-dark): Add properties.
* etc/themes/tango-theme.el (tango): Add properties.
* etc/themes/tsdh-dark-theme.el (tsdh-dark): Add properties.
* etc/themes/tsdh-light-theme.el (tsdh-light): Add properties.
* etc/themes/wheatgrass-theme.el (wheatgrass): Add properties.
* etc/themes/whiteboard-theme.el (whiteboard): Add properties.
* etc/themes/wombat-theme.el (wombat): Add properties.
* lisp/custom.el (deftheme): Allow for optional arguments to set the
property list.
(custom-declare-theme): Accept the same optional arguments as 'deftheme'.
(theme-list-variants): Add new function.
(theme-choose-variant): Add new command for switching between members
of a theme family.
(toggle-theme): Add an alias for 'theme-choose-variant'.  (Bug#57639)
---
 doc/emacs/custom.texi           | 10 +++++
 doc/lispref/customize.texi      |  5 ++-
 etc/themes/adwaita-theme.el     |  4 +-
 etc/themes/deeper-blue-theme.el |  4 +-
 etc/themes/dichromacy-theme.el  |  4 +-
 etc/themes/leuven-dark-theme.el |  6 ++-
 etc/themes/leuven-theme.el      |  6 ++-
 etc/themes/light-blue-theme.el  |  4 +-
 etc/themes/manoj-dark-theme.el  |  4 +-
 etc/themes/misterioso-theme.el  |  4 +-
 etc/themes/tango-dark-theme.el  |  5 ++-
 etc/themes/tango-theme.el       |  5 ++-
 etc/themes/tsdh-dark-theme.el   |  5 ++-
 etc/themes/tsdh-light-theme.el  |  5 ++-
 etc/themes/wheatgrass-theme.el  |  4 +-
 etc/themes/whiteboard-theme.el  |  4 +-
 etc/themes/wombat-theme.el      |  4 +-
 lisp/custom.el                  | 70 ++++++++++++++++++++++++++++++---
 18 files changed, 128 insertions(+), 25 deletions(-)

diff --git a/doc/emacs/custom.texi b/doc/emacs/custom.texi
index ff7ab83190..f98527bf9a 100644
--- a/doc/emacs/custom.texi
+++ b/doc/emacs/custom.texi
@@ -667,6 +667,16 @@ Custom Themes
 the @file{*Custom Themes*} buffer; or type @kbd{M-x describe-theme}
 anywhere in Emacs and enter the theme name.
 
+@findex theme-choose-variant
+Some themes have variants (most often just two: light and dark).  You
+can switch to another variant using @kbd{M-x theme-choose-variant}.
+If the currently active theme has only one other variant, it will be
+selected; if there are more variants, the command will prompt you
+which one to switch to.
+
+Note that @code{theme-choose-variant} only works if a single theme
+is active.
+
 @node Creating Custom Themes
 @subsection Creating Custom Themes
 @cindex custom themes, creating
diff --git a/doc/lispref/customize.texi b/doc/lispref/customize.texi
index 6ba35cffff..911b6c4d75 100644
--- a/doc/lispref/customize.texi
+++ b/doc/lispref/customize.texi
@@ -1428,12 +1428,13 @@ Custom Themes
 be a call to @code{deftheme}, and the last form should be a call to
 @code{provide-theme}.
 
-@defmac deftheme theme &optional doc
+@defmac deftheme theme &optional doc &rest properties
 This macro declares @var{theme} (a symbol) as the name of a Custom
 theme.  The optional argument @var{doc} should be a string describing
 the theme; this is the description shown when the user invokes the
 @code{describe-theme} command or types @kbd{?} in the @samp{*Custom
-Themes*} buffer.
+Themes*} buffer.  The remaining arguments @var{properties} are used
+pass a property list with theme attributes.
 
 Two special theme names are disallowed (using them causes an error):
 @code{user} is a dummy theme that stores the user's direct
diff --git a/etc/themes/adwaita-theme.el b/etc/themes/adwaita-theme.el
index ba83a0578c..6ac7d8f316 100644
--- a/etc/themes/adwaita-theme.el
+++ b/etc/themes/adwaita-theme.el
@@ -24,7 +24,9 @@
 (deftheme adwaita
   "Face colors similar to the default theme of Gnome 3 (Adwaita).
 The colors are chosen to match Adwaita window decorations and the
-default look of the Gnome 3 desktop.")
+default look of the Gnome 3 desktop."
+  :background-mode 'light
+  :kind 'color-scheme)
 
 (let ((class '((class color) (min-colors 89))))
   (custom-theme-set-faces
diff --git a/etc/themes/deeper-blue-theme.el b/etc/themes/deeper-blue-theme.el
index 8f19147f91..db3b9b5b60 100644
--- a/etc/themes/deeper-blue-theme.el
+++ b/etc/themes/deeper-blue-theme.el
@@ -22,7 +22,9 @@
 ;;; Code:
 
 (deftheme deeper-blue
-  "Face colors using a deep blue background.")
+  "Face colors using a deep blue background."
+  :background-mode 'dark
+  :kind 'color-scheme)
 
 (let ((class '((class color) (min-colors 89))))
   (custom-theme-set-faces
diff --git a/etc/themes/dichromacy-theme.el b/etc/themes/dichromacy-theme.el
index d53c075d92..d2c5983862 100644
--- a/etc/themes/dichromacy-theme.el
+++ b/etc/themes/dichromacy-theme.el
@@ -28,7 +28,9 @@ dichromacy
 differentiated by individuals with protanopia or deuteranopia.
 
 Basic, Font Lock, Isearch, Gnus, Message, Flyspell, and
-Ansi-Color faces are included.")
+Ansi-Color faces are included."
+  :background-mode 'light
+  :kind 'color-scheme)
 
 (let ((class '((class color) (min-colors 89)))
       (orange "#e69f00")
diff --git a/etc/themes/leuven-dark-theme.el b/etc/themes/leuven-dark-theme.el
index 0e162c8bab..42ebd7b2d6 100644
--- a/etc/themes/leuven-dark-theme.el
+++ b/etc/themes/leuven-dark-theme.el
@@ -5,7 +5,7 @@
 ;; Author: Fabrice Niessen <(concat "fniessen" at-sign "pirilampo.org")>
 ;; Contributor: Thibault Polge <(concat "thibault" at-sign "thb.lt")>
 ;; URL: https://github.com/fniessen/emacs-leuven-dark-theme
-;; Version: 20220202.1126
+;; Version: 20220917.2332
 ;; Keywords: color theme
 
 ;; This file is part of GNU Emacs.
@@ -97,7 +97,9 @@ leuven-dark
   "Face colors with a light background.
 Basic, Font Lock, Isearch, Gnus, Message, Org mode, Diff, Ediff,
 Flyspell, Semantic, and Ansi-Color faces are included -- and much
-more...")
+more..."
+  :background-mode 'dark
+  :family 'leuven)
 
 (let ((class '((class color) (min-colors 89)))
 
diff --git a/etc/themes/leuven-theme.el b/etc/themes/leuven-theme.el
index d9a8d5391a..07c34e944c 100644
--- a/etc/themes/leuven-theme.el
+++ b/etc/themes/leuven-theme.el
@@ -4,7 +4,7 @@
 
 ;; Author: Fabrice Niessen <(concat "fniessen" at-sign "pirilampo.org")>
 ;; URL: https://github.com/fniessen/emacs-leuven-theme
-;; Version: 20200513.1928
+;; Version: 20220917.2332
 ;; Keywords: color theme
 
 ;; This file is part of GNU Emacs.
@@ -78,7 +78,9 @@ leuven
   "Face colors with a light background.
 Basic, Font Lock, Isearch, Gnus, Message, Org mode, Diff, Ediff,
 Flyspell, Semantic, and Ansi-Color faces are included -- and much
-more...")
+more..."
+  :background-mode 'light
+  :family 'leuven)
 
 (let ((class '((class color) (min-colors 89)))
 
diff --git a/etc/themes/light-blue-theme.el b/etc/themes/light-blue-theme.el
index eeca46210c..449600d01d 100644
--- a/etc/themes/light-blue-theme.el
+++ b/etc/themes/light-blue-theme.el
@@ -27,7 +27,9 @@
 ;;; Code:
 
 (deftheme light-blue
-  "Face colors utilizing a light blue background.")
+  "Face colors utilizing a light blue background."
+  :background-mode 'light
+  :kind 'color-scheme)
 
 (make-obsolete 'light-blue nil "29.1")
 
diff --git a/etc/themes/manoj-dark-theme.el b/etc/themes/manoj-dark-theme.el
index af5576386c..402aafe49d 100644
--- a/etc/themes/manoj-dark-theme.el
+++ b/etc/themes/manoj-dark-theme.el
@@ -67,7 +67,9 @@
 (deftheme manoj-dark
   "Very high contrast faces with a black background.
 This theme avoids subtle color variations, while avoiding the
-jarring angry fruit salad look to reduce eye fatigue.")
+jarring angry fruit salad look to reduce eye fatigue."
+  :background-mode 'dark
+  :kind 'color-scheme)
 
 (custom-theme-set-faces
  'manoj-dark
diff --git a/etc/themes/misterioso-theme.el b/etc/themes/misterioso-theme.el
index 55186384ad..7e3f0289f1 100644
--- a/etc/themes/misterioso-theme.el
+++ b/etc/themes/misterioso-theme.el
@@ -22,7 +22,9 @@
 ;;; Code:
 
 (deftheme misterioso
-  "Predominantly blue/cyan faces on a dark cyan background.")
+  "Predominantly blue/cyan faces on a dark cyan background."
+  :background-mode 'dark
+  :kind 'color-scheme)
 
 (let ((class '((class color) (min-colors 89))))
 
diff --git a/etc/themes/tango-dark-theme.el b/etc/themes/tango-dark-theme.el
index ef00d2ac49..73a928e445 100644
--- a/etc/themes/tango-dark-theme.el
+++ b/etc/themes/tango-dark-theme.el
@@ -30,7 +30,10 @@
 (deftheme tango-dark
   "Face colors using the Tango palette (dark background).
 Basic, Font Lock, Isearch, Gnus, Message, Ediff, Flyspell,
-Semantic, and Ansi-Color faces are included.")
+Semantic, and Ansi-Color faces are included."
+  :background-mode 'dark
+  :kind 'color-scheme
+  :family 'tango)
 
 (let ((class '((class color) (min-colors 89)))
       ;; Tango palette colors.
diff --git a/etc/themes/tango-theme.el b/etc/themes/tango-theme.el
index ecbbf03753..8b8011bd1f 100644
--- a/etc/themes/tango-theme.el
+++ b/etc/themes/tango-theme.el
@@ -30,7 +30,10 @@
 (deftheme tango
   "Face colors using the Tango palette (light background).
 Basic, Font Lock, Isearch, Gnus, Message, Ediff, Flyspell,
-Semantic, and Ansi-Color faces are included.")
+Semantic, and Ansi-Color faces are included."
+  :background-mode 'light
+  :kind 'color-scheme
+  :family 'tango)
 
 (let ((class '((class color) (min-colors 89)))
       ;; Tango palette colors.
diff --git a/etc/themes/tsdh-dark-theme.el b/etc/themes/tsdh-dark-theme.el
index a88ad75520..2a2507f147 100644
--- a/etc/themes/tsdh-dark-theme.el
+++ b/etc/themes/tsdh-dark-theme.el
@@ -20,7 +20,10 @@
 ;;; Code:
 
 (deftheme tsdh-dark
-  "A dark theme used and created by Tassilo Horn.")
+  "A dark theme used and created by Tassilo Horn."
+  :background-mode 'dark
+  :kind 'color-scheme
+  :family 'tsdh)
 
 (custom-theme-set-faces
  'tsdh-dark
diff --git a/etc/themes/tsdh-light-theme.el b/etc/themes/tsdh-light-theme.el
index d9d09b702b..130b2a33d4 100644
--- a/etc/themes/tsdh-light-theme.el
+++ b/etc/themes/tsdh-light-theme.el
@@ -21,7 +21,10 @@
 
 (deftheme tsdh-light
   "A light Emacs theme.
-Used and created by Tassilo Horn.")
+Used and created by Tassilo Horn."
+  :background-mode 'light
+  :kind 'color-scheme
+  :family 'tsdh)
 
 (custom-theme-set-faces
  'tsdh-light
diff --git a/etc/themes/wheatgrass-theme.el b/etc/themes/wheatgrass-theme.el
index c56c8a2d8a..5b4370351f 100644
--- a/etc/themes/wheatgrass-theme.el
+++ b/etc/themes/wheatgrass-theme.el
@@ -23,7 +23,9 @@ wheatgrass
   "High-contrast green/blue/brown faces on a black background.
 Basic, Font Lock, Isearch, Gnus, and Message faces are included.
 The default face foreground is wheat, with other faces in shades
-of green, brown, and blue.")
+of green, brown, and blue."
+  :background-mode 'dark
+  :kind 'color-scheme)
 
 (let ((class '((class color) (min-colors 89))))
   (custom-theme-set-faces
diff --git a/etc/themes/whiteboard-theme.el b/etc/themes/whiteboard-theme.el
index f21b18b421..676e0e0f70 100644
--- a/etc/themes/whiteboard-theme.el
+++ b/etc/themes/whiteboard-theme.el
@@ -22,7 +22,9 @@
 ;;; Code:
 
 (deftheme whiteboard
-  "Face colors similar to markers on a whiteboard.")
+  "Face colors similar to markers on a whiteboard."
+  :background-mode 'light
+  :kind 'color-scheme)
 
 (let ((class '((class color) (min-colors 89))))
   (custom-theme-set-faces
diff --git a/etc/themes/wombat-theme.el b/etc/themes/wombat-theme.el
index d9fab8ac78..4eef29841b 100644
--- a/etc/themes/wombat-theme.el
+++ b/etc/themes/wombat-theme.el
@@ -25,7 +25,9 @@ wombat
   "Medium-contrast faces with a dark gray background.
 Adapted, with permission, from a Vim color scheme by Lars H. Nielsen.
 Basic, Font Lock, Isearch, Gnus, Message, and Ansi-Color faces
-are included.")
+are included."
+  :background-mode 'dark
+  :kind 'color-scheme)
 
 (let ((class '((class color) (min-colors 89))))
   (custom-theme-set-faces
diff --git a/lisp/custom.el b/lisp/custom.el
index 352b5b0e16..3b36544d9d 100644
--- a/lisp/custom.el
+++ b/lisp/custom.el
@@ -1152,9 +1152,11 @@ custom--sort-vars-1
 ;;   (provide-theme 'THEME)
 
 
-(defmacro deftheme (theme &optional doc)
+(defmacro deftheme (theme &optional doc &rest properties)
   "Declare THEME to be a Custom theme.
 The optional argument DOC is a doc string describing the theme.
+PROPERTIES are interpreted as a property list that will be stored
+in the `theme-properties' property for THEME.
 
 Any theme `foo' should be defined in a file called `foo-theme.el';
 see `custom-make-theme-feature' for more information."
@@ -1164,18 +1166,25 @@ deftheme
     ;; It is better not to use backquote in this file,
     ;; because that makes a bootstrapping problem
     ;; if you need to recompile all the Lisp files using interpreted code.
-    (list 'custom-declare-theme (list 'quote theme) (list 'quote feature) doc)))
+    (list 'custom-declare-theme (list 'quote theme) (list 'quote feature) doc
+          (cons 'list properties))))
 
-(defun custom-declare-theme (theme feature &optional doc)
+(defun custom-declare-theme (theme feature &optional doc properties)
   "Like `deftheme', but THEME is evaluated as a normal argument.
-FEATURE is the feature this theme provides.  Normally, this is a symbol
-created from THEME by `custom-make-theme-feature'."
+FEATURE is the feature this theme provides.  Normally, this is a
+symbol created from THEME by `custom-make-theme-feature'.  The
+optional argument DOC may contain the documentation for THEME.
+The optional argument PROPERTIES may contain a property list of
+attributes associated with THEME."
   (unless (custom-theme-name-valid-p theme)
     (error "Custom theme cannot be named %S" theme))
   (unless (memq theme custom-known-themes)
     (push theme custom-known-themes))
   (put theme 'theme-feature feature)
-  (when doc (put theme 'theme-documentation doc)))
+  (when doc
+    (put theme 'theme-documentation doc))
+  (when properties
+    (put theme 'theme-properties properties)))
 
 (defun custom-make-theme-feature (theme)
   "Given a symbol THEME, create a new symbol by appending \"-theme\".
@@ -1372,6 +1381,55 @@ load-theme
     (enable-theme theme))
   t)
 
+(defun theme-list-variants (theme &rest list)
+  "Return a list of theme variants for THEME.
+If the optional argument LIST is not given, "
+  (let* ((properties (get theme 'theme-properties))
+         (family (plist-get properties :family)))
+    (seq-filter
+     (lambda (variant)
+       (and (eq (plist-get (get variant 'theme-properties) :family)
+                family)
+            (not (eq variant theme))))
+     (or list (custom-available-themes)))))
+
+(defun theme-choose-variant (&optional no-confirm no-enable)
+  "Prompt to switch from the current theme to one of its a variants.
+The current theme will be disabled before variant is enabled.  If
+the current theme has only one variant, switch to that variant
+without prompting, otherwise prompt for the variant to select.
+See `load-theme' for the meaning of NO-CONFIRM and NO-ENABLE."
+  (interactive)
+  (let ((active-color-schemes
+         (seq-filter
+          (lambda (theme)
+            ;; FIXME: As most themes currently do not have a `:kind'
+            ;; tag, it is assumed that a theme is a color scheme by
+            ;; default.  This should be reconsidered in the future.
+            (memq (plist-get (get theme 'theme-properties) :kind)
+                  '(color-scheme nil)))
+          custom-enabled-themes)))
+    (cond
+     ((length= active-color-schemes 0)
+      (user-error "No theme is active, cannot toggle"))
+     ((length> active-color-schemes 1)
+      (user-error "More than one theme active, cannot unambiguously toggle")))
+    (let* ((theme (car active-color-schemes))
+           (family (plist-get (get theme 'theme-properties) :family)))
+      (unless family
+        (error "Theme `%s' does not have any known variants" theme))
+      (let* ((variants (theme-list-variants theme))
+             (choice (cond
+                      ((null variants)
+                       (error "`%s' has no variants" theme))
+                      ((length= variants 1)
+                       (car variants))
+                      ((intern (completing-read "Load custom theme: " variants))))))
+        (disable-theme theme)
+        (load-theme choice no-confirm no-enable)))))
+
+(defalias 'toggle-theme #'theme-choose-variant)
+
 (defun custom-theme-load-confirm (hash)
   "Query the user about loading a Custom theme that may not be safe.
 The theme should be in the current buffer.  If the user agrees,
-- 
2.37.3

[Message part 3 (text/plain, inline)]

Also, the issue I mentioned previously remains.  The properties are only
noticed if the theme file is loaded.  So if you enabled `leuven-dark',
you won't be able to toggle before `leuven' is loaded at least once.

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#57639; Package emacs. (Sun, 18 Sep 2022 12:53:02 GMT) Full text and rfc822 format available.

Message #107 received at 57639 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Philip Kaludercic <philipk <at> posteo.net>
Cc: larsi <at> gnus.org, 57639 <at> debbugs.gnu.org
Subject: Re: bug#57639: [PATCH] Add new command 'toggle-theme'
Date: Sun, 18 Sep 2022 15:52:27 +0300
> From: Philip Kaludercic <philipk <at> posteo.net>
> Cc: larsi <at> gnus.org,  57639 <at> debbugs.gnu.org
> Date: Sun, 18 Sep 2022 09:38:02 +0000
> 
> >> >> +  (let* ((theme (car custom-enabled-themes))
> >> >> +         (family (plist-get (get theme 'theme-properties) :family)))
> >> >> +    (unless family
> >> >> +      (error "`%s' is not part of a family" theme))
> >> >
> >> > "Family"? this terminology was never mentioned in the manual or the
> >> > doc string.  How about
> >> >
> >> >   Theme `%s' does not have any variants
> >> >
> >> > instead?
> >> 
> >> Strictly speaking that error message would be wrong at this point,
> >> because we cannot say if a theme has no variants if it is not part of a
> >> family.  This is because variants of a theme are all those that are part
> >> of the same family.  I think it would be better to clarify this in the
> >> documentation.
> >
> > But the documentation doesn't explain what it means for a theme to be
> > part of a family.  Specifically, how does one tell, by looking at a
> > theme, whether it is or isn't part of a family?
> >
> > An alternative for what I suggested above is to say
> >
> >   Theme `%s' does not have any known variants
> 
> I get what you mean... how about
> 
>     (error "Theme `%s' is not part of a family of variants" theme)
> 
> ?

  Theme `%s' is not part of a family of theme variants

> >> +Themes only have variants if they are part of a family of themes.
> >
> > This should explain what it means to be part of a family, otherwise
> > this sentence is not helpful.
> 
> My intention was for this to be an explanation.  The issue is that
> variants and family are mutually recursive concepts:
> 
> - A variant of a theme are those which are part of the same family
> - A family of themes is the set of all variants of a theme.
> 
> Perhaps it is just easier to collapse both concepts into either variant
> of family and just "expose" by documenting it.  I've tried doing so
> below.

Here's an attempt to explain what is a "family", in case it can still
be of interest:

  A @dfn{family} of themes is a set of similar themes that differ by
  minor aspects, such as face colors that are meant for the light vs
  dark background of the frame.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#57639; Package emacs. (Sun, 18 Sep 2022 12:57:01 GMT) Full text and rfc822 format available.

Message #110 received at 57639 <at> debbugs.gnu.org (full text, mbox):

From: Philip Kaludercic <philipk <at> posteo.net>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: larsi <at> gnus.org, 57639 <at> debbugs.gnu.org
Subject: Re: bug#57639: [PATCH] Add new command 'toggle-theme'
Date: Sun, 18 Sep 2022 12:56:30 +0000
Eli Zaretskii <eliz <at> gnu.org> writes:

>> From: Philip Kaludercic <philipk <at> posteo.net>
>> Cc: larsi <at> gnus.org,  57639 <at> debbugs.gnu.org
>> Date: Sun, 18 Sep 2022 09:38:02 +0000
>> 
>> >> >> +  (let* ((theme (car custom-enabled-themes))
>> >> >> +         (family (plist-get (get theme 'theme-properties) :family)))
>> >> >> +    (unless family
>> >> >> +      (error "`%s' is not part of a family" theme))
>> >> >
>> >> > "Family"? this terminology was never mentioned in the manual or the
>> >> > doc string.  How about
>> >> >
>> >> >   Theme `%s' does not have any variants
>> >> >
>> >> > instead?
>> >> 
>> >> Strictly speaking that error message would be wrong at this point,
>> >> because we cannot say if a theme has no variants if it is not part of a
>> >> family.  This is because variants of a theme are all those that are part
>> >> of the same family.  I think it would be better to clarify this in the
>> >> documentation.
>> >
>> > But the documentation doesn't explain what it means for a theme to be
>> > part of a family.  Specifically, how does one tell, by looking at a
>> > theme, whether it is or isn't part of a family?
>> >
>> > An alternative for what I suggested above is to say
>> >
>> >   Theme `%s' does not have any known variants
>> 
>> I get what you mean... how about
>> 
>>     (error "Theme `%s' is not part of a family of variants" theme)
>> 
>> ?
>
>   Theme `%s' is not part of a family of theme variants
>
>> >> +Themes only have variants if they are part of a family of themes.
>> >
>> > This should explain what it means to be part of a family, otherwise
>> > this sentence is not helpful.
>> 
>> My intention was for this to be an explanation.  The issue is that
>> variants and family are mutually recursive concepts:
>> 
>> - A variant of a theme are those which are part of the same family
>> - A family of themes is the set of all variants of a theme.
>> 
>> Perhaps it is just easier to collapse both concepts into either variant
>> of family and just "expose" by documenting it.  I've tried doing so
>> below.
>
> Here's an attempt to explain what is a "family", in case it can still
> be of interest:
>
>   A @dfn{family} of themes is a set of similar themes that differ by
>   minor aspects, such as face colors that are meant for the light vs
>   dark background of the frame.

I personally think it is better to omit this for now.  Perhaps this can
be mentioned if or when the theme tags are formalised.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#57639; Package emacs. (Mon, 19 Sep 2022 07:59:02 GMT) Full text and rfc822 format available.

Message #113 received at 57639 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Philip Kaludercic <philipk <at> posteo.net>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 57639 <at> debbugs.gnu.org
Subject: Re: bug#57639: [PATCH] Add new command 'toggle-theme'
Date: Mon, 19 Sep 2022 09:58:13 +0200
Philip Kaludercic <philipk <at> posteo.net> writes:

> One last additional, I've added a property to all the themes indicating
> that they are colour schemes.  This would make it easier to toggle
> between dark and light modes if multiple themes are enabled of which
> only one is a colour theme:

Makes sense.  One thing that occurred to me now that I didn't think of
before is:

>  (deftheme adwaita
>    "Face colors similar to the default theme of Gnome 3 (Adwaita).
>  The colors are chosen to match Adwaita window decorations and the
> -default look of the Gnome 3 desktop.")
> +default look of the Gnome 3 desktop."
> +  :background-mode 'light
> +  :kind 'color-scheme)

Is this backwards compatible, and is that something we need to care
about?  I know that some (many?) themes are also distributed outside of
Emacs, and should work under a range of Emacs versions.  I don't think
extending `deftheme' in this way would be backwards compatible, though,
even if it makes sense from a language design point of view.





Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#57639; Package emacs. (Mon, 19 Sep 2022 08:06:02 GMT) Full text and rfc822 format available.

Message #116 received at 57639 <at> debbugs.gnu.org (full text, mbox):

From: Philip Kaludercic <philipk <at> posteo.net>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 57639 <at> debbugs.gnu.org
Subject: Re: bug#57639: [PATCH] Add new command 'toggle-theme'
Date: Mon, 19 Sep 2022 08:05:23 +0000
Lars Ingebrigtsen <larsi <at> gnus.org> writes:

> Philip Kaludercic <philipk <at> posteo.net> writes:
>
>> One last additional, I've added a property to all the themes indicating
>> that they are colour schemes.  This would make it easier to toggle
>> between dark and light modes if multiple themes are enabled of which
>> only one is a colour theme:
>
> Makes sense.  One thing that occurred to me now that I didn't think of
> before is:
>
>>  (deftheme adwaita
>>    "Face colors similar to the default theme of Gnome 3 (Adwaita).
>>  The colors are chosen to match Adwaita window decorations and the
>> -default look of the Gnome 3 desktop.")
>> +default look of the Gnome 3 desktop."
>> +  :background-mode 'light
>> +  :kind 'color-scheme)
>
> Is this backwards compatible, and is that something we need to care
> about?  I know that some (many?) themes are also distributed outside of
> Emacs, and should work under a range of Emacs versions.  I don't think
> extending `deftheme' in this way would be backwards compatible, though,
> even if it makes sense from a language design point of view.

I don't think so, but if a theme were concerned about this, all they
need to do is

(put 'theme-name 'theme-properties '(:background-mode light :kind color-scheme))

As that is all the macro does.  As I said, we could do this too and add
a autoload-cookie before it to solve the visibility issue, but it
doesn't look that nice...  Can (deftheme)s be autoloaded?





Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#57639; Package emacs. (Mon, 19 Sep 2022 08:16:02 GMT) Full text and rfc822 format available.

Message #119 received at 57639 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Philip Kaludercic <philipk <at> posteo.net>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 57639 <at> debbugs.gnu.org
Subject: Re: bug#57639: [PATCH] Add new command 'toggle-theme'
Date: Mon, 19 Sep 2022 10:15:10 +0200
Philip Kaludercic <philipk <at> posteo.net> writes:

> I don't think so, but if a theme were concerned about this, all they
> need to do is
>
> (put 'theme-name 'theme-properties '(:background-mode light :kind color-scheme))
>
> As that is all the macro does.

Ah, then that's fine -- feel free to push when you're ready.

> As I said, we could do this too and add
> a autoload-cookie before it to solve the visibility issue, but it
> doesn't look that nice...  Can (deftheme)s be autoloaded?

Hm...  well, anything can be put into the loaddefs.el file, really, so
yes, but I'm not sure we want to...




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#57639; Package emacs. (Mon, 19 Sep 2022 10:14:01 GMT) Full text and rfc822 format available.

Message #122 received at 57639 <at> debbugs.gnu.org (full text, mbox):

From: Philip Kaludercic <philipk <at> posteo.net>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 57639 <at> debbugs.gnu.org
Subject: Re: bug#57639: [PATCH] Add new command 'toggle-theme'
Date: Mon, 19 Sep 2022 10:13:46 +0000
Lars Ingebrigtsen <larsi <at> gnus.org> writes:

> Philip Kaludercic <philipk <at> posteo.net> writes:
>
>> I don't think so, but if a theme were concerned about this, all they
>> need to do is
>>
>> (put 'theme-name 'theme-properties '(:background-mode light :kind color-scheme))
>>
>> As that is all the macro does.
>
> Ah, then that's fine -- feel free to push when you're ready.
>
>> As I said, we could do this too and add
>> a autoload-cookie before it to solve the visibility issue, but it
>> doesn't look that nice...  Can (deftheme)s be autoloaded?
>
> Hm...  well, anything can be put into the loaddefs.el file, really, so
> yes, but I'm not sure we want to...

I think I'll modify the patch for now and pull out the properties via
`put' calls, and maybe someone will think of a better solution later on.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#57639; Package emacs. (Tue, 20 Sep 2022 03:41:02 GMT) Full text and rfc822 format available.

Message #125 received at 57639 <at> debbugs.gnu.org (full text, mbox):

From: Richard Stallman <rms <at> gnu.org>
To: Philip Kaludercic <philipk <at> posteo.net>
Cc: eliz <at> gnu.org, larsi <at> gnus.org, 57639 <at> debbugs.gnu.org
Subject: Toggling, in general
Date: Mon, 19 Sep 2022 23:40:33 -0400
[[[ To any NSA and FBI agents reading my email: please consider    ]]]
[[[ whether defending the US Constitution against all enemies,     ]]]
[[[ foreign or domestic, requires you to follow Snowden's example. ]]]

Is anyone investigating the idea of a more general UI facility
for toggling various kinds of settings?

This oculd be a prefix key.  Or we might be able to do this more
deeply, and implement toggling various kinds of settings with one Liep
command.

Either way, it would do many jobs and not require users to remember
may different commands.

-- 
Dr Richard Stallman (https://stallman.org)
Chief GNUisance of the GNU Project (https://gnu.org)
Founder, Free Software Foundation (https://fsf.org)
Internet Hall-of-Famer (https://internethalloffame.org)






Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#57639; Package emacs. (Tue, 20 Sep 2022 08:08:02 GMT) Full text and rfc822 format available.

Message #128 received at 57639 <at> debbugs.gnu.org (full text, mbox):

From: Philip Kaludercic <philipk <at> posteo.net>
To: Richard Stallman <rms <at> gnu.org>
Cc: eliz <at> gnu.org, larsi <at> gnus.org, 57639 <at> debbugs.gnu.org
Subject: Re: Toggling, in general
Date: Tue, 20 Sep 2022 08:07:00 +0000
Richard Stallman <rms <at> gnu.org> writes:

> [[[ To any NSA and FBI agents reading my email: please consider    ]]]
> [[[ whether defending the US Constitution against all enemies,     ]]]
> [[[ foreign or domestic, requires you to follow Snowden's example. ]]]
>
> Is anyone investigating the idea of a more general UI facility
> for toggling various kinds of settings?
>
> This oculd be a prefix key.  Or we might be able to do this more
> deeply, and implement toggling various kinds of settings with one Liep
> command.
>
> Either way, it would do many jobs and not require users to remember
> may different commands.

What kind of things are you thinking about?  Things like the menu bar,
tool bar, etc. are already toggled using minor modes.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#57639; Package emacs. (Tue, 20 Sep 2022 12:21:03 GMT) Full text and rfc822 format available.

Message #131 received at 57639 <at> debbugs.gnu.org (full text, mbox):

From: Visuwesh <visuweshm <at> gmail.com>
To: Richard Stallman <rms <at> gnu.org>
Cc: larsi <at> gnus.org, Philip Kaludercic <philipk <at> posteo.net>, eliz <at> gnu.org,
 57639 <at> debbugs.gnu.org
Subject: Re: bug#57639: Toggling, in general
Date: Tue, 20 Sep 2022 17:50:24 +0530
[திங்கள் செப்டம்பர் 19, 2022] Richard Stallman wrote:

> Is anyone investigating the idea of a more general UI facility
> for toggling various kinds of settings?

Custom themes are a perfect candidate for this general facility.  You
can turn on major and minor modes, set variables as a part of a custom
theme.  So these "presets" could be different custom themes and we can
have a command to toggle between said themes.
[ You need not have to worry about custom file getting mangled since
  user options set by a custom theme are not saved [1].  ]

> This oculd be a prefix key.

Though, I'm lost here.  If we are talking about, say, send-mail as the
command to be run, you're saying this prefix key would change the From
address, Signature, etc. right?  Then I think having such a command
would be welcome.

> Or we might be able to do this more deeply, and implement toggling
> various kinds of settings with one Liep command.

See above, if we use custom themes, this should be trivial.

1. Only those from the special 'user' custom theme are saved.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#57639; Package emacs. (Tue, 20 Sep 2022 21:09:01 GMT) Full text and rfc822 format available.

Message #134 received at 57639 <at> debbugs.gnu.org (full text, mbox):

From: Philip Kaludercic <philipk <at> posteo.net>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 57639 <at> debbugs.gnu.org
Subject: Re: bug#57639: [PATCH] Add new command 'toggle-theme'
Date: Tue, 20 Sep 2022 21:08:29 +0000
Philip Kaludercic <philipk <at> posteo.net> writes:

> Lars Ingebrigtsen <larsi <at> gnus.org> writes:
>
>> Philip Kaludercic <philipk <at> posteo.net> writes:
>>
>>> I don't think so, but if a theme were concerned about this, all they
>>> need to do is
>>>
>>> (put 'theme-name 'theme-properties '(:background-mode light :kind color-scheme))
>>>
>>> As that is all the macro does.
>>
>> Ah, then that's fine -- feel free to push when you're ready.
>>
>>> As I said, we could do this too and add
>>> a autoload-cookie before it to solve the visibility issue, but it
>>> doesn't look that nice...  Can (deftheme)s be autoloaded?
>>
>> Hm...  well, anything can be put into the loaddefs.el file, really, so
>> yes, but I'm not sure we want to...
>
> I think I'll modify the patch for now and pull out the properties via
> `put' calls, and maybe someone will think of a better solution later on.

I've just tried this out but it appears that despite re-building Emacs
the autoloaded calls are not registered.  Is this because of some
special handling of themes?




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#57639; Package emacs. (Tue, 20 Sep 2022 21:12:01 GMT) Full text and rfc822 format available.

Message #137 received at 57639 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Philip Kaludercic <philipk <at> posteo.net>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 57639 <at> debbugs.gnu.org
Subject: Re: bug#57639: [PATCH] Add new command 'toggle-theme'
Date: Tue, 20 Sep 2022 23:11:16 +0200
Philip Kaludercic <philipk <at> posteo.net> writes:

> I've just tried this out but it appears that despite re-building Emacs
> the autoloaded calls are not registered.  Is this because of some
> special handling of themes?

Oh -- the themes are in etc/themes, and I don't think they're
included...  Let's see...

Yup:

autoloads: $(lisp)/emacs-lisp/loaddefs-gen.elc gen-lisp
	$(AM_V_GEN)$(emacs) \
            -l $(lisp)/emacs-lisp/loaddefs-gen.elc \
	    -f loaddefs-generate--emacs-batch ${SUBDIRS_ALMOST}

We can add ../etc/themes there after ${SUBDIRS_ALMOST}, though.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#57639; Package emacs. (Tue, 20 Sep 2022 21:36:01 GMT) Full text and rfc822 format available.

Message #140 received at 57639 <at> debbugs.gnu.org (full text, mbox):

From: Philip Kaludercic <philipk <at> posteo.net>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 57639 <at> debbugs.gnu.org
Subject: Re: bug#57639: [PATCH] Add new command 'toggle-theme'
Date: Tue, 20 Sep 2022 21:35:25 +0000
Lars Ingebrigtsen <larsi <at> gnus.org> writes:

> Philip Kaludercic <philipk <at> posteo.net> writes:
>
>> I've just tried this out but it appears that despite re-building Emacs
>> the autoloaded calls are not registered.  Is this because of some
>> special handling of themes?
>
> Oh -- the themes are in etc/themes, and I don't think they're
> included...  Let's see...
>
> Yup:
>
> autoloads: $(lisp)/emacs-lisp/loaddefs-gen.elc gen-lisp
> 	$(AM_V_GEN)$(emacs) \
>             -l $(lisp)/emacs-lisp/loaddefs-gen.elc \
> 	    -f loaddefs-generate--emacs-batch ${SUBDIRS_ALMOST}
>
> We can add ../etc/themes there after ${SUBDIRS_ALMOST}, though.

Would there be a disadvantage to doing so?




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#57639; Package emacs. (Wed, 21 Sep 2022 02:48:02 GMT) Full text and rfc822 format available.

Message #143 received at 57639 <at> debbugs.gnu.org (full text, mbox):

From: Richard Stallman <rms <at> gnu.org>
To: Philip Kaludercic <philipk <at> posteo.net>
Cc: eliz <at> gnu.org, larsi <at> gnus.org, 57639 <at> debbugs.gnu.org
Subject: Re: bug#57639: Toggling, in general
Date: Tue, 20 Sep 2022 22:47:26 -0400
[[[ To any NSA and FBI agents reading my email: please consider    ]]]
[[[ whether defending the US Constitution against all enemies,     ]]]
[[[ foreign or domestic, requires you to follow Snowden's example. ]]]

  > What kind of things are you thinking about?  Things like the menu bar,
  > tool bar, etc. are already toggled using minor modes.

What I have in mind is settings that have many possible values,
and there's a way to select a new setting.  It could be useful
to have a general way to toggle between the last two values
of the setting.

One such thing is the selected buffer.  Right now you toggle that with
C-x b RET, which may be the shortest sequence of keys we can get, but you
see what I mean.

Another is the current input method.  ISTR there is an ad-hoc command to
reselect the previous input method.  This is another thing that a general
toggle could handle, and avoid the ad-hoc additional command.


-- 
Dr Richard Stallman (https://stallman.org)
Chief GNUisance of the GNU Project (https://gnu.org)
Founder, Free Software Foundation (https://fsf.org)
Internet Hall-of-Famer (https://internethalloffame.org)






Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#57639; Package emacs. (Wed, 21 Sep 2022 09:23:01 GMT) Full text and rfc822 format available.

Message #146 received at 57639 <at> debbugs.gnu.org (full text, mbox):

From: Robert Pluim <rpluim <at> gmail.com>
To: Richard Stallman <rms <at> gnu.org>
Cc: larsi <at> gnus.org, Philip Kaludercic <philipk <at> posteo.net>, eliz <at> gnu.org,
 57639 <at> debbugs.gnu.org
Subject: Re: bug#57639: Toggling, in general
Date: Wed, 21 Sep 2022 11:22:20 +0200
>>>>> On Tue, 20 Sep 2022 22:47:26 -0400, Richard Stallman <rms <at> gnu.org> said:

    Richard> Another is the current input method.  ISTR there is an ad-hoc command to
    Richard> reselect the previous input method.  This is another thing that a general
    Richard> toggle could handle, and avoid the ad-hoc additional command.

I donʼt think `toggle-input-method' counts as ad-hoc. The first time
you use it, it follows `default-input-method' if set, otherwise it
prompts for an input method. Subsequent uses turn that input method on
and off (modulo the prefix arg). It *is* the command for selecting an
input-method, not a separate toggling command.

Robert
-- 




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#57639; Package emacs. (Wed, 21 Sep 2022 11:03:02 GMT) Full text and rfc822 format available.

Message #149 received at 57639 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Philip Kaludercic <philipk <at> posteo.net>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 57639 <at> debbugs.gnu.org
Subject: Re: bug#57639: [PATCH] Add new command 'toggle-theme'
Date: Wed, 21 Sep 2022 13:02:29 +0200
Philip Kaludercic <philipk <at> posteo.net> writes:

>> autoloads: $(lisp)/emacs-lisp/loaddefs-gen.elc gen-lisp
>> 	$(AM_V_GEN)$(emacs) \
>>             -l $(lisp)/emacs-lisp/loaddefs-gen.elc \
>> 	    -f loaddefs-generate--emacs-batch ${SUBDIRS_ALMOST}
>>
>> We can add ../etc/themes there after ${SUBDIRS_ALMOST}, though.
>
> Would there be a disadvantage to doing so?

There's no problems when doing

;;;###autoload(put ...)

because those are entered into the loaddefs.el file literally.  There
may be some confusion if people start putting

;;;###autoload
(defun ...)

into the theme files, because the etc/themes directory is not in
`load-path' (so those autoloads will fail).

So it has a slightly inconsistent effect, but I don't think there'll be
a problem in practice, because people don't put functions into the theme
files.  *knocks on wood*





Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#57639; Package emacs. (Wed, 21 Sep 2022 11:31:02 GMT) Full text and rfc822 format available.

Message #152 received at 57639 <at> debbugs.gnu.org (full text, mbox):

From: Philip Kaludercic <philipk <at> posteo.net>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 57639 <at> debbugs.gnu.org
Subject: Re: bug#57639: [PATCH] Add new command 'toggle-theme'
Date: Wed, 21 Sep 2022 11:30:20 +0000
[Message part 1 (text/plain, inline)]
Lars Ingebrigtsen <larsi <at> gnus.org> writes:

> Philip Kaludercic <philipk <at> posteo.net> writes:
>
>>> autoloads: $(lisp)/emacs-lisp/loaddefs-gen.elc gen-lisp
>>> 	$(AM_V_GEN)$(emacs) \
>>>             -l $(lisp)/emacs-lisp/loaddefs-gen.elc \
>>> 	    -f loaddefs-generate--emacs-batch ${SUBDIRS_ALMOST}
>>>
>>> We can add ../etc/themes there after ${SUBDIRS_ALMOST}, though.
>>
>> Would there be a disadvantage to doing so?
>
> There's no problems when doing
>
> ;;;###autoload(put ...)

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

[0001-Tag-themes-with-properties.patch (text/x-patch, inline)]
From 6e6c5a9cf356b5f634ba388f8e2724a1de893297 Mon Sep 17 00:00:00 2001
From: Philip Kaludercic <philipk <at> posteo.net>
Date: Sat, 17 Sep 2022 20:11:42 +0200
Subject: [PATCH] Tag themes with properties

* doc/emacs/custom.texi (Custom Themes): Document 'theme-choose-variant'.
* doc/lispref/customize.texi (Custom Themes): Document the new
optional argument to 'deftheme'.
* etc/themes/adwaita-theme.el (adwaita): Add properties.
* etc/themes/deeper-blue-theme.el (deeper-blue): Add properties.
* etc/themes/dichromacy-theme.el (dichromacy): Add properties.
* etc/themes/light-blue-theme.el (light-blue): Add properties.
* etc/themes/manoj-dark-theme.el (manoj-dark): Add properties.
* etc/themes/misterioso-theme.el (misterioso): Add properties.
* etc/themes/tango-dark-theme.el (tango-dark): Add properties.
* etc/themes/tango-theme.el (tango): Add properties.
* etc/themes/tsdh-dark-theme.el (tsdh-dark): Add properties.
* etc/themes/tsdh-light-theme.el (tsdh-light): Add properties.
* etc/themes/wheatgrass-theme.el (wheatgrass): Add properties.
* etc/themes/whiteboard-theme.el (whiteboard): Add properties.
* etc/themes/wombat-theme.el (wombat): Add properties.
* lisp/custom.el (deftheme): Allow for optional arguments to set the
property list.
(custom-declare-theme): Accept the same optional arguments as 'deftheme'.
(theme-list-variants): Add new function.
(theme-choose-variant): Add new command for switching between members
of a theme family.
(toggle-theme): Add an alias for 'theme-choose-variant'.  (Bug#57639)

This patch adds theme properties twice, once as part of the deftheme
declarations and once by explicitly manipulating the symbol plist.
Ideally only the first case would be necessary, but in that case the
theme properties only become visible after the theme has been loaded
which is (initially) unfortunate if you want to toggle between themes.
---
 doc/emacs/custom.texi           | 10 +++++
 doc/lispref/customize.texi      |  5 ++-
 etc/themes/adwaita-theme.el     |  6 ++-
 etc/themes/deeper-blue-theme.el |  6 ++-
 etc/themes/dichromacy-theme.el  |  6 ++-
 etc/themes/leuven-dark-theme.el |  9 ++++-
 etc/themes/leuven-theme.el      |  9 ++++-
 etc/themes/light-blue-theme.el  |  6 ++-
 etc/themes/manoj-dark-theme.el  |  6 ++-
 etc/themes/misterioso-theme.el  |  6 ++-
 etc/themes/tango-dark-theme.el  |  7 +++-
 etc/themes/tango-theme.el       |  7 +++-
 etc/themes/tsdh-dark-theme.el   |  7 +++-
 etc/themes/tsdh-light-theme.el  |  7 +++-
 etc/themes/wheatgrass-theme.el  |  6 ++-
 etc/themes/whiteboard-theme.el  |  6 ++-
 etc/themes/wombat-theme.el      |  6 ++-
 lisp/custom.el                  | 70 ++++++++++++++++++++++++++++++---
 18 files changed, 160 insertions(+), 25 deletions(-)

diff --git a/doc/emacs/custom.texi b/doc/emacs/custom.texi
index ff7ab83190..f98527bf9a 100644
--- a/doc/emacs/custom.texi
+++ b/doc/emacs/custom.texi
@@ -667,6 +667,16 @@ Custom Themes
 the @file{*Custom Themes*} buffer; or type @kbd{M-x describe-theme}
 anywhere in Emacs and enter the theme name.
 
+@findex theme-choose-variant
+Some themes have variants (most often just two: light and dark).  You
+can switch to another variant using @kbd{M-x theme-choose-variant}.
+If the currently active theme has only one other variant, it will be
+selected; if there are more variants, the command will prompt you
+which one to switch to.
+
+Note that @code{theme-choose-variant} only works if a single theme
+is active.
+
 @node Creating Custom Themes
 @subsection Creating Custom Themes
 @cindex custom themes, creating
diff --git a/doc/lispref/customize.texi b/doc/lispref/customize.texi
index 6ba35cffff..911b6c4d75 100644
--- a/doc/lispref/customize.texi
+++ b/doc/lispref/customize.texi
@@ -1428,12 +1428,13 @@ Custom Themes
 be a call to @code{deftheme}, and the last form should be a call to
 @code{provide-theme}.
 
-@defmac deftheme theme &optional doc
+@defmac deftheme theme &optional doc &rest properties
 This macro declares @var{theme} (a symbol) as the name of a Custom
 theme.  The optional argument @var{doc} should be a string describing
 the theme; this is the description shown when the user invokes the
 @code{describe-theme} command or types @kbd{?} in the @samp{*Custom
-Themes*} buffer.
+Themes*} buffer.  The remaining arguments @var{properties} are used
+pass a property list with theme attributes.
 
 Two special theme names are disallowed (using them causes an error):
 @code{user} is a dummy theme that stores the user's direct
diff --git a/etc/themes/adwaita-theme.el b/etc/themes/adwaita-theme.el
index ba83a0578c..c1d694f5dc 100644
--- a/etc/themes/adwaita-theme.el
+++ b/etc/themes/adwaita-theme.el
@@ -24,7 +24,11 @@
 (deftheme adwaita
   "Face colors similar to the default theme of Gnome 3 (Adwaita).
 The colors are chosen to match Adwaita window decorations and the
-default look of the Gnome 3 desktop.")
+default look of the Gnome 3 desktop."
+  :background-mode 'light
+  :kind 'color-scheme)
+
+;;;###autoload (put 'adwaita 'theme-properties '(:background-mode light :kind color-scheme))
 
 (let ((class '((class color) (min-colors 89))))
   (custom-theme-set-faces
diff --git a/etc/themes/deeper-blue-theme.el b/etc/themes/deeper-blue-theme.el
index 8f19147f91..13abbe0672 100644
--- a/etc/themes/deeper-blue-theme.el
+++ b/etc/themes/deeper-blue-theme.el
@@ -22,7 +22,11 @@
 ;;; Code:
 
 (deftheme deeper-blue
-  "Face colors using a deep blue background.")
+  "Face colors using a deep blue background."
+  :background-mode 'dark
+  :kind 'color-scheme)
+
+;;;###autoload (put 'deeper-blue 'theme-properties '(:background-mode dark :kind color-scheme))
 
 (let ((class '((class color) (min-colors 89))))
   (custom-theme-set-faces
diff --git a/etc/themes/dichromacy-theme.el b/etc/themes/dichromacy-theme.el
index d53c075d92..a25d2c310f 100644
--- a/etc/themes/dichromacy-theme.el
+++ b/etc/themes/dichromacy-theme.el
@@ -28,7 +28,11 @@ dichromacy
 differentiated by individuals with protanopia or deuteranopia.
 
 Basic, Font Lock, Isearch, Gnus, Message, Flyspell, and
-Ansi-Color faces are included.")
+Ansi-Color faces are included."
+  :background-mode 'light
+  :kind 'color-scheme)
+
+;;;###autoload (put 'dichromacy 'theme-properties '(:background-mode light :kind color-scheme))
 
 (let ((class '((class color) (min-colors 89)))
       (orange "#e69f00")
diff --git a/etc/themes/leuven-dark-theme.el b/etc/themes/leuven-dark-theme.el
index 0e162c8bab..0d3e1970ac 100644
--- a/etc/themes/leuven-dark-theme.el
+++ b/etc/themes/leuven-dark-theme.el
@@ -5,7 +5,7 @@
 ;; Author: Fabrice Niessen <(concat "fniessen" at-sign "pirilampo.org")>
 ;; Contributor: Thibault Polge <(concat "thibault" at-sign "thb.lt")>
 ;; URL: https://github.com/fniessen/emacs-leuven-dark-theme
-;; Version: 20220202.1126
+;; Version: 20220921.1327
 ;; Keywords: color theme
 
 ;; This file is part of GNU Emacs.
@@ -97,7 +97,12 @@ leuven-dark
   "Face colors with a light background.
 Basic, Font Lock, Isearch, Gnus, Message, Org mode, Diff, Ediff,
 Flyspell, Semantic, and Ansi-Color faces are included -- and much
-more...")
+more..."
+  :background-mode 'dark
+  :family 'leuven
+  :kind 'color-scheme)
+
+;;;###autoload (put 'leuven-dark 'theme-properties '(:background-mode dark :family leuven :kind color-scheme))
 
 (let ((class '((class color) (min-colors 89)))
 
diff --git a/etc/themes/leuven-theme.el b/etc/themes/leuven-theme.el
index d9a8d5391a..0bbc69aa05 100644
--- a/etc/themes/leuven-theme.el
+++ b/etc/themes/leuven-theme.el
@@ -4,7 +4,7 @@
 
 ;; Author: Fabrice Niessen <(concat "fniessen" at-sign "pirilampo.org")>
 ;; URL: https://github.com/fniessen/emacs-leuven-theme
-;; Version: 20200513.1928
+;; Version: 20220921.1328
 ;; Keywords: color theme
 
 ;; This file is part of GNU Emacs.
@@ -78,7 +78,12 @@ leuven
   "Face colors with a light background.
 Basic, Font Lock, Isearch, Gnus, Message, Org mode, Diff, Ediff,
 Flyspell, Semantic, and Ansi-Color faces are included -- and much
-more...")
+more..."
+  :background-mode 'light
+  :kind 'color-scheme
+  :family 'leuven)
+
+;;;###autoload (put 'leuven 'theme-properties '(:background-mode light :family leuven :kind color-scheme))
 
 (let ((class '((class color) (min-colors 89)))
 
diff --git a/etc/themes/light-blue-theme.el b/etc/themes/light-blue-theme.el
index eeca46210c..ff1fde85a9 100644
--- a/etc/themes/light-blue-theme.el
+++ b/etc/themes/light-blue-theme.el
@@ -27,7 +27,11 @@
 ;;; Code:
 
 (deftheme light-blue
-  "Face colors utilizing a light blue background.")
+  "Face colors utilizing a light blue background."
+  :background-mode 'light
+  :kind 'color-scheme)
+
+;;;###autoload (put 'light-blue 'theme-properties '(:background-mode light :kind color-scheme))
 
 (make-obsolete 'light-blue nil "29.1")
 
diff --git a/etc/themes/manoj-dark-theme.el b/etc/themes/manoj-dark-theme.el
index af5576386c..791ad2f353 100644
--- a/etc/themes/manoj-dark-theme.el
+++ b/etc/themes/manoj-dark-theme.el
@@ -67,7 +67,11 @@
 (deftheme manoj-dark
   "Very high contrast faces with a black background.
 This theme avoids subtle color variations, while avoiding the
-jarring angry fruit salad look to reduce eye fatigue.")
+jarring angry fruit salad look to reduce eye fatigue."
+  :background-mode 'dark
+  :kind 'color-scheme)
+
+;;;###autoload (put 'manoj-dark 'theme-properties '(:background-mode dark :kind color-scheme))
 
 (custom-theme-set-faces
  'manoj-dark
diff --git a/etc/themes/misterioso-theme.el b/etc/themes/misterioso-theme.el
index 55186384ad..e7e5dac3dc 100644
--- a/etc/themes/misterioso-theme.el
+++ b/etc/themes/misterioso-theme.el
@@ -22,7 +22,11 @@
 ;;; Code:
 
 (deftheme misterioso
-  "Predominantly blue/cyan faces on a dark cyan background.")
+  "Predominantly blue/cyan faces on a dark cyan background."
+  :background-mode 'dark
+  :kind 'color-scheme)
+
+;;;###autoload (put 'misterioso 'theme-properties '(:background-mode dark :kind color-scheme))
 
 (let ((class '((class color) (min-colors 89))))
 
diff --git a/etc/themes/tango-dark-theme.el b/etc/themes/tango-dark-theme.el
index ef00d2ac49..f7d13c5bd5 100644
--- a/etc/themes/tango-dark-theme.el
+++ b/etc/themes/tango-dark-theme.el
@@ -30,7 +30,12 @@
 (deftheme tango-dark
   "Face colors using the Tango palette (dark background).
 Basic, Font Lock, Isearch, Gnus, Message, Ediff, Flyspell,
-Semantic, and Ansi-Color faces are included.")
+Semantic, and Ansi-Color faces are included."
+  :background-mode 'dark
+  :kind 'color-scheme
+  :family 'tango)
+
+;;;###autoload (put 'tango-dark 'theme-properties '(:background-mode dark :kind color-scheme :family tango))
 
 (let ((class '((class color) (min-colors 89)))
       ;; Tango palette colors.
diff --git a/etc/themes/tango-theme.el b/etc/themes/tango-theme.el
index ecbbf03753..8df3f50ded 100644
--- a/etc/themes/tango-theme.el
+++ b/etc/themes/tango-theme.el
@@ -30,7 +30,12 @@
 (deftheme tango
   "Face colors using the Tango palette (light background).
 Basic, Font Lock, Isearch, Gnus, Message, Ediff, Flyspell,
-Semantic, and Ansi-Color faces are included.")
+Semantic, and Ansi-Color faces are included."
+  :background-mode 'light
+  :kind 'color-scheme
+  :family 'tango)
+
+;;;###autoload (put 'tango 'theme-properties '(:background-mode light :kind color-scheme :family tango))
 
 (let ((class '((class color) (min-colors 89)))
       ;; Tango palette colors.
diff --git a/etc/themes/tsdh-dark-theme.el b/etc/themes/tsdh-dark-theme.el
index a88ad75520..afb915dcce 100644
--- a/etc/themes/tsdh-dark-theme.el
+++ b/etc/themes/tsdh-dark-theme.el
@@ -20,7 +20,12 @@
 ;;; Code:
 
 (deftheme tsdh-dark
-  "A dark theme used and created by Tassilo Horn.")
+  "A dark theme used and created by Tassilo Horn."
+  :background-mode 'dark
+  :kind 'color-scheme
+  :family 'tsdh)
+
+;;;###autoload (put 'tsdh-dark 'theme-properties '(:background-mode dark :kind color-scheme :family tsdh))
 
 (custom-theme-set-faces
  'tsdh-dark
diff --git a/etc/themes/tsdh-light-theme.el b/etc/themes/tsdh-light-theme.el
index d9d09b702b..7fad6c337a 100644
--- a/etc/themes/tsdh-light-theme.el
+++ b/etc/themes/tsdh-light-theme.el
@@ -21,7 +21,12 @@
 
 (deftheme tsdh-light
   "A light Emacs theme.
-Used and created by Tassilo Horn.")
+Used and created by Tassilo Horn."
+  :background-mode 'light
+  :kind 'color-scheme
+  :family 'tsdh)
+
+;;;###autoload (put 'tsdh-light 'theme-properties '(:background-mode light :kind color-scheme :family tsdh))
 
 (custom-theme-set-faces
  'tsdh-light
diff --git a/etc/themes/wheatgrass-theme.el b/etc/themes/wheatgrass-theme.el
index c56c8a2d8a..81aa68cb34 100644
--- a/etc/themes/wheatgrass-theme.el
+++ b/etc/themes/wheatgrass-theme.el
@@ -23,7 +23,11 @@ wheatgrass
   "High-contrast green/blue/brown faces on a black background.
 Basic, Font Lock, Isearch, Gnus, and Message faces are included.
 The default face foreground is wheat, with other faces in shades
-of green, brown, and blue.")
+of green, brown, and blue."
+  :background-mode 'dark
+  :kind 'color-scheme)
+
+;;;###autoload (put 'wheatgrass 'theme-properties '(:background-mode dark :kind color-scheme))
 
 (let ((class '((class color) (min-colors 89))))
   (custom-theme-set-faces
diff --git a/etc/themes/whiteboard-theme.el b/etc/themes/whiteboard-theme.el
index f21b18b421..7b92510049 100644
--- a/etc/themes/whiteboard-theme.el
+++ b/etc/themes/whiteboard-theme.el
@@ -22,7 +22,11 @@
 ;;; Code:
 
 (deftheme whiteboard
-  "Face colors similar to markers on a whiteboard.")
+  "Face colors similar to markers on a whiteboard."
+  :background-mode 'light
+  :kind 'color-scheme)
+
+;;;###autoload (put 'whiteboard 'theme-properties '(:background-mode light :kind color-scheme))
 
 (let ((class '((class color) (min-colors 89))))
   (custom-theme-set-faces
diff --git a/etc/themes/wombat-theme.el b/etc/themes/wombat-theme.el
index d9fab8ac78..2d0669f632 100644
--- a/etc/themes/wombat-theme.el
+++ b/etc/themes/wombat-theme.el
@@ -25,7 +25,11 @@ wombat
   "Medium-contrast faces with a dark gray background.
 Adapted, with permission, from a Vim color scheme by Lars H. Nielsen.
 Basic, Font Lock, Isearch, Gnus, Message, and Ansi-Color faces
-are included.")
+are included."
+  :background-mode 'dark
+  :kind 'color-scheme)
+
+;;;###autoload (put 'wombat 'theme-properties '(:background-mode dark :kind color-scheme))
 
 (let ((class '((class color) (min-colors 89))))
   (custom-theme-set-faces
diff --git a/lisp/custom.el b/lisp/custom.el
index 352b5b0e16..93ea80ef43 100644
--- a/lisp/custom.el
+++ b/lisp/custom.el
@@ -1152,9 +1152,11 @@ custom--sort-vars-1
 ;;   (provide-theme 'THEME)
 
 
-(defmacro deftheme (theme &optional doc)
+(defmacro deftheme (theme &optional doc &rest properties)
   "Declare THEME to be a Custom theme.
 The optional argument DOC is a doc string describing the theme.
+PROPERTIES are interpreted as a property list that will be stored
+in the `theme-properties' property for THEME.
 
 Any theme `foo' should be defined in a file called `foo-theme.el';
 see `custom-make-theme-feature' for more information."
@@ -1164,18 +1166,25 @@ deftheme
     ;; It is better not to use backquote in this file,
     ;; because that makes a bootstrapping problem
     ;; if you need to recompile all the Lisp files using interpreted code.
-    (list 'custom-declare-theme (list 'quote theme) (list 'quote feature) doc)))
+    (list 'custom-declare-theme (list 'quote theme) (list 'quote feature) doc
+          (cons 'list properties))))
 
-(defun custom-declare-theme (theme feature &optional doc)
+(defun custom-declare-theme (theme feature &optional doc properties)
   "Like `deftheme', but THEME is evaluated as a normal argument.
-FEATURE is the feature this theme provides.  Normally, this is a symbol
-created from THEME by `custom-make-theme-feature'."
+FEATURE is the feature this theme provides.  Normally, this is a
+symbol created from THEME by `custom-make-theme-feature'.  The
+optional argument DOC may contain the documentation for THEME.
+The optional argument PROPERTIES may contain a property list of
+attributes associated with THEME."
   (unless (custom-theme-name-valid-p theme)
     (error "Custom theme cannot be named %S" theme))
   (unless (memq theme custom-known-themes)
     (push theme custom-known-themes))
   (put theme 'theme-feature feature)
-  (when doc (put theme 'theme-documentation doc)))
+  (when doc
+    (put theme 'theme-documentation doc))
+  (when properties
+    (put theme 'theme-properties properties)))
 
 (defun custom-make-theme-feature (theme)
   "Given a symbol THEME, create a new symbol by appending \"-theme\".
@@ -1372,6 +1381,55 @@ load-theme
     (enable-theme theme))
   t)
 
+(defun theme-list-variants (theme)
+  "Return a list of theme variants for THEME."
+  (let* ((properties (get theme 'theme-properties))
+         (family (plist-get properties :family)))
+    (when family
+      (seq-filter
+       (lambda (variant)
+         (and (eq (plist-get (get variant 'theme-properties) :family)
+                  family)
+              (not (eq variant theme))))
+       (custom-available-themes)))))
+
+(defun theme-choose-variant (&optional no-confirm no-enable)
+  "Prompt to switch from the current theme to one of its a variants.
+The current theme will be disabled before variant is enabled.  If
+the current theme has only one variant, switch to that variant
+without prompting, otherwise prompt for the variant to select.
+See `load-theme' for the meaning of NO-CONFIRM and NO-ENABLE."
+  (interactive)
+  (let ((active-color-schemes
+         (seq-filter
+          (lambda (theme)
+            ;; FIXME: As most themes currently do not have a `:kind'
+            ;; tag, it is assumed that a theme is a color scheme by
+            ;; default.  This should be reconsidered in the future.
+            (memq (plist-get (get theme 'theme-properties) :kind)
+                  '(color-scheme nil)))
+          custom-enabled-themes)))
+    (cond
+     ((length= active-color-schemes 0)
+      (user-error "No theme is active, cannot toggle"))
+     ((length> active-color-schemes 1)
+      (user-error "More than one theme active, cannot unambiguously toggle")))
+    (let* ((theme (car active-color-schemes))
+           (family (plist-get (get theme 'theme-properties) :family)))
+      (unless family
+        (error "Theme `%s' does not have any known variants" theme))
+      (let* ((variants (theme-list-variants theme))
+             (choice (cond
+                      ((null variants)
+                       (error "`%s' has no variants" theme))
+                      ((length= variants 1)
+                       (car variants))
+                      ((intern (completing-read "Load custom theme: " variants))))))
+        (disable-theme theme)
+        (load-theme choice no-confirm no-enable)))))
+
+(defalias 'toggle-theme #'theme-choose-variant)
+
 (defun custom-theme-load-confirm (hash)
   "Query the user about loading a Custom theme that may not be safe.
 The theme should be in the current buffer.  If the user agrees,
-- 
2.37.3

[Message part 3 (text/plain, inline)]
> because those are entered into the loaddefs.el file literally.  There
> may be some confusion if people start putting
>
> ;;;###autoload
> (defun ...)
>
> into the theme files, because the etc/themes directory is not in
> `load-path' (so those autoloads will fail).
>
> So it has a slightly inconsistent effect, but I don't think there'll be
> a problem in practice, because people don't put functions into the theme
> files.  *knocks on wood*

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?

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#57639; Package emacs. (Wed, 21 Sep 2022 11:39:02 GMT) Full text and rfc822 format available.

Message #155 received at 57639 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Philip Kaludercic <philipk <at> posteo.net>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 57639 <at> debbugs.gnu.org,
 Protesilaos Stavrou <info <at> protesilaos.com>
Subject: Re: bug#57639: [PATCH] Add new command 'toggle-theme'
Date: Wed, 21 Sep 2022 13:38:13 +0200
Philip Kaludercic <philipk <at> 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...




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#57639; Package emacs. (Wed, 21 Sep 2022 11:47:01 GMT) Full text and rfc822 format available.

Message #158 received at 57639 <at> debbugs.gnu.org (full text, mbox):

From: Philip Kaludercic <philipk <at> posteo.net>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 57639 <at> debbugs.gnu.org,
 Protesilaos Stavrou <info <at> protesilaos.com>
Subject: Re: bug#57639: [PATCH] Add new command 'toggle-theme'
Date: Wed, 21 Sep 2022 11:46:06 +0000
Lars Ingebrigtsen <larsi <at> gnus.org> writes:

> Philip Kaludercic <philipk <at> 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:

Modus-themes is distributed via GNU ELPA, and Leuven via MELPA.  I
didn't modify Modus-themes because I know it is sync'ed back regularly,
but now that I think about it I don't know if I ought to have changed
those either...

> ;;;###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...

There probably is not automatic way to resolve this issue, but perhaps
it might be enough to remind all the external theme maintainers to
remove these snippets before updating the files in emacs.git.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#57639; Package emacs. (Wed, 21 Sep 2022 12:07:02 GMT) Full text and rfc822 format available.

Message #161 received at 57639 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Philip Kaludercic <philipk <at> posteo.net>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 57639 <at> debbugs.gnu.org,
 Protesilaos Stavrou <info <at> protesilaos.com>
Subject: Re: bug#57639: [PATCH] Add new command 'toggle-theme'
Date: Wed, 21 Sep 2022 14:06:32 +0200
Philip Kaludercic <philipk <at> posteo.net> writes:

> There probably is not automatic way to resolve this issue, but perhaps
> it might be enough to remind all the external theme maintainers to
> remove these snippets before updating the files in emacs.git.

That would be inconvenient to have to have different versions in Emacs
and on ELPA(s).

As a gross hack, we could add a new file in etc/themes that has these
`put' statements and then load that in Emacs.  But...  that's kinda
gross, so if somebody has a better idea, that's be nice.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#57639; Package emacs. (Wed, 21 Sep 2022 12:28:02 GMT) Full text and rfc822 format available.

Message #164 received at 57639 <at> debbugs.gnu.org (full text, mbox):

From: Philip Kaludercic <philipk <at> posteo.net>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 57639 <at> debbugs.gnu.org,
 Protesilaos Stavrou <info <at> protesilaos.com>
Subject: Re: bug#57639: [PATCH] Add new command 'toggle-theme'
Date: Wed, 21 Sep 2022 12:26:56 +0000
Lars Ingebrigtsen <larsi <at> gnus.org> writes:

> Philip Kaludercic <philipk <at> posteo.net> writes:
>
>> There probably is not automatic way to resolve this issue, but perhaps
>> it might be enough to remind all the external theme maintainers to
>> remove these snippets before updating the files in emacs.git.
>
> That would be inconvenient to have to have different versions in Emacs
> and on ELPA(s).

Functionally they wouldn't differ from one another, the only difference
would be that the core packages wouldn't include the code necessary to
bootstrap a theme as a package.

> As a gross hack, we could add a new file in etc/themes that has these
> `put' statements and then load that in Emacs.  But...  that's kinda
> gross, so if somebody has a better idea, that's be nice.

If there were a way to generate "autoloaded" code as a side effect of
byte compilation or something, then deftheme would take care of that.
But I don't know enough about the bootstrapping process to say how
feasible that is or not.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#57639; Package emacs. (Wed, 21 Sep 2022 13:07:01 GMT) Full text and rfc822 format available.

Message #167 received at 57639 <at> debbugs.gnu.org (full text, mbox):

From: Protesilaos Stavrou <info <at> protesilaos.com>
To: Lars Ingebrigtsen <larsi <at> gnus.org>, Philip Kaludercic <philipk <at> posteo.net>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 57639 <at> debbugs.gnu.org
Subject: Re: bug#57639: [PATCH] Add new command 'toggle-theme'
Date: Wed, 21 Sep 2022 16:05:54 +0300
> From: Lars Ingebrigtsen <larsi <at> gnus.org>
> Date: Wed, 21 Sep 2022 13:38:13 +0200
>
> Philip Kaludercic <philipk <at> 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.

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




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#57639; Package emacs. (Wed, 21 Sep 2022 13:09:02 GMT) Full text and rfc822 format available.

Message #170 received at 57639 <at> debbugs.gnu.org (full text, mbox):

From: Protesilaos Stavrou <info <at> protesilaos.com>
To: Philip Kaludercic <philipk <at> posteo.net>, Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 57639 <at> debbugs.gnu.org
Subject: Re: bug#57639: [PATCH] Add new command 'toggle-theme'
Date: Wed, 21 Sep 2022 16:08:23 +0300
> From: Philip Kaludercic <philipk <at> posteo.net>
> Date: Wed, 21 Sep 2022 11:46:06 +0000

> [... 19 lines elided]

> Modus-themes is distributed via GNU ELPA, and Leuven via MELPA.  I
> didn't modify Modus-themes because I know it is sync'ed back regularly,
> but now that I think about it I don't know if I ought to have changed
> those either...

I am fine either way.  Do whatever you think is better.  For the
completeness of your patch, it makes more sense to include them as well.
Then I can copy the relevant parts into my Git repo (or you prepare a
patch).

At any rate, I am following this discussion to know what changes I need
to make.

-- 
Protesilaos Stavrou
https://protesilaos.com




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#57639; Package emacs. (Wed, 21 Sep 2022 13:15:02 GMT) Full text and rfc822 format available.

Message #173 received at 57639 <at> debbugs.gnu.org (full text, mbox):

From: Philip Kaludercic <philipk <at> posteo.net>
To: Protesilaos Stavrou <info <at> protesilaos.com>
Cc: Lars Ingebrigtsen <larsi <at> gnus.org>, Eli Zaretskii <eliz <at> gnu.org>,
 57639 <at> debbugs.gnu.org
Subject: Re: bug#57639: [PATCH] Add new command 'toggle-theme'
Date: Wed, 21 Sep 2022 13:13:50 +0000
Protesilaos Stavrou <info <at> protesilaos.com> writes:

>> From: Philip Kaludercic <philipk <at> posteo.net>
>> Date: Wed, 21 Sep 2022 11:46:06 +0000
>
>> [... 19 lines elided]
>
>> Modus-themes is distributed via GNU ELPA, and Leuven via MELPA.  I
>> didn't modify Modus-themes because I know it is sync'ed back regularly,
>> but now that I think about it I don't know if I ought to have changed
>> those either...
>
> I am fine either way.  Do whatever you think is better.  For the
> completeness of your patch, it makes more sense to include them as well.
> Then I can copy the relevant parts into my Git repo (or you prepare a
> patch).

FWIW I didn't change your themes for now, just Leuven.  But if you are
fine with it, I'd change those too and then send you a patch.

> At any rate, I am following this discussion to know what changes I need
> to make.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#57639; Package emacs. (Wed, 21 Sep 2022 13:22:02 GMT) Full text and rfc822 format available.

Message #176 received at 57639 <at> debbugs.gnu.org (full text, mbox):

From: Protesilaos Stavrou <info <at> protesilaos.com>
To: Philip Kaludercic <philipk <at> posteo.net>
Cc: Lars Ingebrigtsen <larsi <at> gnus.org>, Eli Zaretskii <eliz <at> gnu.org>,
 57639 <at> debbugs.gnu.org
Subject: Re: bug#57639: [PATCH] Add new command 'toggle-theme'
Date: Wed, 21 Sep 2022 16:21:27 +0300
> From: Philip Kaludercic <philipk <at> posteo.net>
> Date: Wed, 21 Sep 2022 13:13:50 +0000
>
> Protesilaos Stavrou <info <at> protesilaos.com> writes:
>
>>> From: Philip Kaludercic <philipk <at> posteo.net>
>>> Date: Wed, 21 Sep 2022 11:46:06 +0000
>>
>>> [... 19 lines elided]
>>
>>> Modus-themes is distributed via GNU ELPA, and Leuven via MELPA.  I
>>> didn't modify Modus-themes because I know it is sync'ed back regularly,
>>> but now that I think about it I don't know if I ought to have changed
>>> those either...
>>
>> I am fine either way.  Do whatever you think is better.  For the
>> completeness of your patch, it makes more sense to include them as well.
>> Then I can copy the relevant parts into my Git repo (or you prepare a
>> patch).
>
> FWIW I didn't change your themes for now, just Leuven.  But if you are
> fine with it, I'd change those too and then send you a patch.

Yes, all good.  Thank you!

-- 
Protesilaos Stavrou
https://protesilaos.com




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#57639; Package emacs. (Sun, 09 Oct 2022 12:59:02 GMT) Full text and rfc822 format available.

Message #179 received at 57639 <at> debbugs.gnu.org (full text, mbox):

From: Philip Kaludercic <philipk <at> posteo.net>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 57639 <at> debbugs.gnu.org,
 Protesilaos Stavrou <info <at> protesilaos.com>
Subject: Re: bug#57639: [PATCH] Add new command 'toggle-theme'
Date: Sun, 09 Oct 2022 12:58:04 +0000
Philip Kaludercic <philipk <at> posteo.net> writes:

> Lars Ingebrigtsen <larsi <at> gnus.org> writes:
>
>> Philip Kaludercic <philipk <at> posteo.net> writes:
>>
>>> There probably is not automatic way to resolve this issue, but perhaps
>>> it might be enough to remind all the external theme maintainers to
>>> remove these snippets before updating the files in emacs.git.
>>
>> That would be inconvenient to have to have different versions in Emacs
>> and on ELPA(s).
>
> Functionally they wouldn't differ from one another, the only difference
> would be that the core packages wouldn't include the code necessary to
> bootstrap a theme as a package.
>
>> As a gross hack, we could add a new file in etc/themes that has these
>> `put' statements and then load that in Emacs.  But...  that's kinda
>> gross, so if somebody has a better idea, that's be nice.
>
> If there were a way to generate "autoloaded" code as a side effect of
> byte compilation or something, then deftheme would take care of that.
> But I don't know enough about the bootstrapping process to say how
> feasible that is or not.

Ping?




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#57639; Package emacs. (Sun, 09 Oct 2022 14:28:02 GMT) Full text and rfc822 format available.

Message #182 received at 57639 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Philip Kaludercic <philipk <at> posteo.net>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 57639 <at> debbugs.gnu.org,
 Protesilaos Stavrou <info <at> protesilaos.com>
Subject: Re: bug#57639: [PATCH] Add new command 'toggle-theme'
Date: Sun, 09 Oct 2022 16:26:59 +0200
Philip Kaludercic <philipk <at> posteo.net> writes:

>>> As a gross hack, we could add a new file in etc/themes that has these
>>> `put' statements and then load that in Emacs.  But...  that's kinda
>>> gross, so if somebody has a better idea, that's be nice.
>>
>> If there were a way to generate "autoloaded" code as a side effect of
>> byte compilation or something, then deftheme would take care of that.
>> But I don't know enough about the bootstrapping process to say how
>> feasible that is or not.
>
> Ping?

Sorry; I'm not sure what the question is any more?





Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#57639; Package emacs. (Sun, 09 Oct 2022 15:14:01 GMT) Full text and rfc822 format available.

Message #185 received at 57639 <at> debbugs.gnu.org (full text, mbox):

From: Philip Kaludercic <philipk <at> posteo.net>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 57639 <at> debbugs.gnu.org,
 Protesilaos Stavrou <info <at> protesilaos.com>
Subject: Re: bug#57639: [PATCH] Add new command 'toggle-theme'
Date: Sun, 09 Oct 2022 15:13:03 +0000
Lars Ingebrigtsen <larsi <at> gnus.org> writes:

> Philip Kaludercic <philipk <at> posteo.net> writes:
>
>>>> As a gross hack, we could add a new file in etc/themes that has these
>>>> `put' statements and then load that in Emacs.  But...  that's kinda
>>>> gross, so if somebody has a better idea, that's be nice.
>>>
>>> If there were a way to generate "autoloaded" code as a side effect of
>>> byte compilation or something, then deftheme would take care of that.
>>> But I don't know enough about the bootstrapping process to say how
>>> feasible that is or not.
>>
>> Ping?
>
> Sorry; I'm not sure what the question is any more?

The last issue preventing this patch from being merged is that
expressions marked for auto-loading are not gathered.  I believe one
option was adding etc/themes to the list of scraped directories, but
that would require comments like

--8<---------------cut here---------------start------------->8---
;;;###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))))
--8<---------------cut here---------------end--------------->8---

from leuven-theme.el to be removed.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#57639; Package emacs. (Sun, 09 Oct 2022 20:33:02 GMT) Full text and rfc822 format available.

Message #188 received at 57639 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Kangas <stefankangas <at> gmail.com>
To: Philip Kaludercic <philipk <at> posteo.net>, Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 57639 <at> debbugs.gnu.org,
 Protesilaos Stavrou <info <at> protesilaos.com>
Subject: Re: bug#57639: [PATCH] Add new command 'toggle-theme'
Date: Sun, 9 Oct 2022 22:32:49 +0200
Philip Kaludercic <philipk <at> posteo.net> writes:

> --8<---------------cut here---------------start------------->8---
> ;;;###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))))
> --8<---------------cut here---------------end--------------->8---
>
> from leuven-theme.el to be removed.

Why would the above be necessary for leuven-theme.el in particular?

I've never had a problem loading themes installed from MELPA, so I don't
understand why it should need any special treatment.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#57639; Package emacs. (Sun, 09 Oct 2022 21:04:01 GMT) Full text and rfc822 format available.

Message #191 received at 57639 <at> debbugs.gnu.org (full text, mbox):

From: Philip Kaludercic <philipk <at> posteo.net>
To: Stefan Kangas <stefankangas <at> gmail.com>
Cc: Fabrice Niessen <fniessen <at> pirilampo.org>,
 Lars Ingebrigtsen <larsi <at> gnus.org>, Eli Zaretskii <eliz <at> gnu.org>,
 Protesilaos Stavrou <info <at> protesilaos.com>, 57639 <at> debbugs.gnu.org
Subject: Re: bug#57639: [PATCH] Add new command 'toggle-theme'
Date: Sun, 09 Oct 2022 21:02:59 +0000
Stefan Kangas <stefankangas <at> gmail.com> writes:

> Philip Kaludercic <philipk <at> posteo.net> writes:
>
>> --8<---------------cut here---------------start------------->8---
>> ;;;###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))))
>> --8<---------------cut here---------------end--------------->8---
>>
>> from leuven-theme.el to be removed.
>
> Why would the above be necessary for leuven-theme.el in particular?
>
> I've never had a problem loading themes installed from MELPA, so I don't
> understand why it should need any special treatment.

It probably isn't necessary at all and it could be removed.  My guess is
that it used to be necessary for [some reason] but [something] changed
at [some point], but the snippet was kept anyway.

I have CC'ed the maintainer Fabrice to find out if my guess is accurate.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#57639; Package emacs. (Sun, 09 Oct 2022 21:44:01 GMT) Full text and rfc822 format available.

Message #194 received at 57639 <at> debbugs.gnu.org (full text, mbox):

From: Philip Kaludercic <philipk <at> posteo.net>
To: Stefan Kangas <stefankangas <at> gmail.com>
Cc: Lars Ingebrigtsen <larsi <at> gnus.org>, Eli Zaretskii <eliz <at> gnu.org>,
 Protesilaos Stavrou <info <at> protesilaos.com>, 57639 <at> debbugs.gnu.org
Subject: Re: bug#57639: [PATCH] Add new command 'toggle-theme'
Date: Sun, 09 Oct 2022 21:43:13 +0000
Philip Kaludercic <philipk <at> posteo.net> writes:

> I have CC'ed the maintainer Fabrice to find out if my guess is accurate.

It appears the email address mentioned in the package header doesn't
actually exist.  I'll contact him directly tomorrow to sort this out.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#57639; Package emacs. (Mon, 10 Oct 2022 00:58:02 GMT) Full text and rfc822 format available.

Message #197 received at 57639 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Kangas <stefankangas <at> gmail.com>
To: Philip Kaludercic <philipk <at> posteo.net>
Cc: Lars Ingebrigtsen <larsi <at> gnus.org>, Eli Zaretskii <eliz <at> gnu.org>,
 Protesilaos Stavrou <info <at> protesilaos.com>, 57639 <at> debbugs.gnu.org
Subject: Re: bug#57639: [PATCH] Add new command 'toggle-theme'
Date: Mon, 10 Oct 2022 02:56:50 +0200
Philip Kaludercic <philipk <at> posteo.net> writes:

> Philip Kaludercic <philipk <at> posteo.net> writes:
>
>> I have CC'ed the maintainer Fabrice to find out if my guess is accurate.
>
> It appears the email address mentioned in the package header doesn't
> actually exist.  I'll contact him directly tomorrow to sort this out.

I've managed to reach Fabrice through the leuven-themes bug tracker, so
I'd start there.

It would be good if we had a working email address in the library
headers though.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#57639; Package emacs. (Mon, 10 Oct 2022 01:18:02 GMT) Full text and rfc822 format available.

Message #200 received at 57639 <at> debbugs.gnu.org (full text, mbox):

From: Protesilaos Stavrou <info <at> protesilaos.com>
To: Stefan Kangas <stefankangas <at> gmail.com>, Philip Kaludercic
 <philipk <at> posteo.net>, Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 57639 <at> debbugs.gnu.org
Subject: Re: bug#57639: [PATCH] Add new command 'toggle-theme'
Date: Mon, 10 Oct 2022 04:17:38 +0300
> From: Stefan Kangas <stefankangas <at> gmail.com>
> Date: Sun,  9 Oct 2022 22:32:49 +0200
>
> Philip Kaludercic <philipk <at> posteo.net> writes:
>
>> --8<---------------cut here---------------start------------->8---
>> ;;;###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))))
>> --8<---------------cut here---------------end--------------->8---
>>
>> from leuven-theme.el to be removed.
>
> Why would the above be necessary for leuven-theme.el in particular?
>
> I've never had a problem loading themes installed from MELPA, so I don't
> understand why it should need any special treatment.

Note that the modus-themes.el have this:

    ;;;###autoload
    (when load-file-name
      (let ((dir (file-name-directory load-file-name)))
        (unless (equal dir (expand-file-name "themes/" data-directory))
          (add-to-list 'custom-theme-load-path dir))))

It is (was?) necessary to make the theme available when installing with
package.el (GNU ELPA).

-- 
Protesilaos Stavrou
https://protesilaos.com




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#57639; Package emacs. (Mon, 10 Oct 2022 08:15:02 GMT) Full text and rfc822 format available.

Message #203 received at 57639 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Philip Kaludercic <philipk <at> posteo.net>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 57639 <at> debbugs.gnu.org,
 Protesilaos Stavrou <info <at> protesilaos.com>
Subject: Re: bug#57639: [PATCH] Add new command 'toggle-theme'
Date: Mon, 10 Oct 2022 10:14:41 +0200
Philip Kaludercic <philipk <at> posteo.net> writes:

> The last issue preventing this patch from being merged is that
> expressions marked for auto-loading are not gathered.  I believe one
> option was adding etc/themes to the list of scraped directories, but
> that would require comments like
>
> ;;;###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))))
>
> from leuven-theme.el to be removed.

Ah, right.

Well, we could come up with a special rule for etc/themes.  For
instance, we could use

;;;###theme-autoload (put ...)

for these things, and 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).





Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#57639; Package emacs. (Mon, 10 Oct 2022 08:17:02 GMT) Full text and rfc822 format available.

Message #206 received at 57639 <at> debbugs.gnu.org (full text, mbox):

From: Philip Kaludercic <philipk <at> posteo.net>
To: Protesilaos Stavrou <info <at> protesilaos.com>
Cc: Lars Ingebrigtsen <larsi <at> gnus.org>, Eli Zaretskii <eliz <at> gnu.org>,
 Stefan Kangas <stefankangas <at> gmail.com>, 57639 <at> debbugs.gnu.org
Subject: Re: bug#57639: [PATCH] Add new command 'toggle-theme'
Date: Mon, 10 Oct 2022 08:16:06 +0000
Protesilaos Stavrou <info <at> protesilaos.com> writes:

>> From: Stefan Kangas <stefankangas <at> gmail.com>
>> Date: Sun,  9 Oct 2022 22:32:49 +0200
>>
>> Philip Kaludercic <philipk <at> posteo.net> writes:
>>
>>> --8<---------------cut here---------------start------------->8---
>>> ;;;###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))))
>>> --8<---------------cut here---------------end--------------->8---
>>>
>>> from leuven-theme.el to be removed.
>>
>> Why would the above be necessary for leuven-theme.el in particular?
>>
>> I've never had a problem loading themes installed from MELPA, so I don't
>> understand why it should need any special treatment.
>
> Note that the modus-themes.el have this:
>
>     ;;;###autoload
>     (when load-file-name
>       (let ((dir (file-name-directory load-file-name)))
>         (unless (equal dir (expand-file-name "themes/" data-directory))
>           (add-to-list 'custom-theme-load-path dir))))
>
> It is (was?) necessary to make the theme available when installing with
> package.el (GNU ELPA).

I have just checked a few themes from MELPA, and they all include this
snippet.  And it appears rightfully, as package.el doesn't do any theme
handling.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#57639; Package emacs. (Mon, 10 Oct 2022 11:03:02 GMT) Full text and rfc822 format available.

Message #209 received at 57639 <at> debbugs.gnu.org (full text, mbox):

From: Philip Kaludercic <philipk <at> posteo.net>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 57639 <at> debbugs.gnu.org,
 Protesilaos Stavrou <info <at> protesilaos.com>
Subject: Re: bug#57639: [PATCH] Add new command 'toggle-theme'
Date: Mon, 10 Oct 2022 11:02:37 +0000
[Message part 1 (text/plain, inline)]
Lars Ingebrigtsen <larsi <at> gnus.org> writes:

> Philip Kaludercic <philipk <at> posteo.net> writes:
>
>> The last issue preventing this patch from being merged is that
>> expressions marked for auto-loading are not gathered.  I believe one
>> option was adding etc/themes to the list of scraped directories, but
>> that would require comments like
>>
>> ;;;###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))))
>>
>> from leuven-theme.el to be removed.
>
> Ah, right.
>
> Well, we could come up with a special rule for etc/themes.  For
> instance, we could use
>
> ;;;###theme-autoload (put ...)
>
> for these things, and 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 am not sure how that is done exactly, but if we take the
";;;###theme-autoload" approach it should be able to have a handler to
convert autoloaded (defcustom ...) forms in to (put ...) ones:

[0001-Tag-themes-with-properties.patch (text/x-patch, inline)]
From 36505800b5a0c8d7b4af5ceb54483cfdfdbf6925 Mon Sep 17 00:00:00 2001
From: Philip Kaludercic <philipk <at> posteo.net>
Date: Sat, 17 Sep 2022 20:11:42 +0200
Subject: [PATCH] Tag themes with properties

* doc/emacs/custom.texi (Custom Themes): Document 'theme-choose-variant'.
* doc/lispref/customize.texi (Custom Themes): Document the new
optional argument to 'deftheme'.
* etc/themes/adwaita-theme.el (adwaita): Add properties.
* etc/themes/deeper-blue-theme.el (deeper-blue): Add properties.
* etc/themes/dichromacy-theme.el (dichromacy): Add properties.
* etc/themes/light-blue-theme.el (light-blue): Add properties.
* etc/themes/manoj-dark-theme.el (manoj-dark): Add properties.
* etc/themes/misterioso-theme.el (misterioso): Add properties.
* etc/themes/tango-dark-theme.el (tango-dark): Add properties.
* etc/themes/tango-theme.el (tango): Add properties.
* etc/themes/tsdh-dark-theme.el (tsdh-dark): Add properties.
* etc/themes/tsdh-light-theme.el (tsdh-light): Add properties.
* etc/themes/wheatgrass-theme.el (wheatgrass): Add properties.
* etc/themes/whiteboard-theme.el (whiteboard): Add properties.
* etc/themes/wombat-theme.el (wombat): Add properties.
* lisp/custom.el (deftheme): Allow for optional arguments to set the
property list.
(custom-declare-theme): Accept the same optional arguments as 'deftheme'.
(theme-list-variants): Add new function.
(theme-choose-variant): Add new command for switching between members
of a theme family.
(toggle-theme): Add an alias for 'theme-choose-variant'.  (Bug#57639)
* lisp/emacs-lisp/loaddefs-gen.el (loaddefs-generate--make-autoload):
Handle 'defcustom's by extracting the properties.
---
 doc/emacs/custom.texi           | 10 +++++
 doc/lispref/customize.texi      |  5 ++-
 etc/themes/adwaita-theme.el     |  5 ++-
 etc/themes/deeper-blue-theme.el |  5 ++-
 etc/themes/dichromacy-theme.el  |  5 ++-
 etc/themes/leuven-dark-theme.el |  8 +++-
 etc/themes/leuven-theme.el      |  8 +++-
 etc/themes/light-blue-theme.el  |  5 ++-
 etc/themes/manoj-dark-theme.el  |  5 ++-
 etc/themes/misterioso-theme.el  |  5 ++-
 etc/themes/tango-dark-theme.el  |  7 +++-
 etc/themes/tango-theme.el       |  6 ++-
 etc/themes/tsdh-dark-theme.el   |  6 ++-
 etc/themes/tsdh-light-theme.el  |  6 ++-
 etc/themes/wheatgrass-theme.el  |  5 ++-
 etc/themes/whiteboard-theme.el  |  5 ++-
 etc/themes/wombat-theme.el      |  5 ++-
 lisp/custom.el                  | 70 ++++++++++++++++++++++++++++++---
 lisp/emacs-lisp/loaddefs-gen.el |  6 +++
 19 files changed, 152 insertions(+), 25 deletions(-)

diff --git a/doc/emacs/custom.texi b/doc/emacs/custom.texi
index ff7ab83190..f98527bf9a 100644
--- a/doc/emacs/custom.texi
+++ b/doc/emacs/custom.texi
@@ -667,6 +667,16 @@ Custom Themes
 the @file{*Custom Themes*} buffer; or type @kbd{M-x describe-theme}
 anywhere in Emacs and enter the theme name.
 
+@findex theme-choose-variant
+Some themes have variants (most often just two: light and dark).  You
+can switch to another variant using @kbd{M-x theme-choose-variant}.
+If the currently active theme has only one other variant, it will be
+selected; if there are more variants, the command will prompt you
+which one to switch to.
+
+Note that @code{theme-choose-variant} only works if a single theme
+is active.
+
 @node Creating Custom Themes
 @subsection Creating Custom Themes
 @cindex custom themes, creating
diff --git a/doc/lispref/customize.texi b/doc/lispref/customize.texi
index 6ba35cffff..911b6c4d75 100644
--- a/doc/lispref/customize.texi
+++ b/doc/lispref/customize.texi
@@ -1428,12 +1428,13 @@ Custom Themes
 be a call to @code{deftheme}, and the last form should be a call to
 @code{provide-theme}.
 
-@defmac deftheme theme &optional doc
+@defmac deftheme theme &optional doc &rest properties
 This macro declares @var{theme} (a symbol) as the name of a Custom
 theme.  The optional argument @var{doc} should be a string describing
 the theme; this is the description shown when the user invokes the
 @code{describe-theme} command or types @kbd{?} in the @samp{*Custom
-Themes*} buffer.
+Themes*} buffer.  The remaining arguments @var{properties} are used
+pass a property list with theme attributes.
 
 Two special theme names are disallowed (using them causes an error):
 @code{user} is a dummy theme that stores the user's direct
diff --git a/etc/themes/adwaita-theme.el b/etc/themes/adwaita-theme.el
index ba83a0578c..6ad8405559 100644
--- a/etc/themes/adwaita-theme.el
+++ b/etc/themes/adwaita-theme.el
@@ -21,10 +21,13 @@
 
 ;;; Code:
 
+;;;###theme-autoload
 (deftheme adwaita
   "Face colors similar to the default theme of Gnome 3 (Adwaita).
 The colors are chosen to match Adwaita window decorations and the
-default look of the Gnome 3 desktop.")
+default look of the Gnome 3 desktop."
+  :background-mode 'light
+  :kind 'color-scheme)
 
 (let ((class '((class color) (min-colors 89))))
   (custom-theme-set-faces
diff --git a/etc/themes/deeper-blue-theme.el b/etc/themes/deeper-blue-theme.el
index 8f19147f91..48ed9ba061 100644
--- a/etc/themes/deeper-blue-theme.el
+++ b/etc/themes/deeper-blue-theme.el
@@ -21,8 +21,11 @@
 
 ;;; Code:
 
+;;;###theme-autoload
 (deftheme deeper-blue
-  "Face colors using a deep blue background.")
+  "Face colors using a deep blue background."
+  :background-mode 'dark
+  :kind 'color-scheme)
 
 (let ((class '((class color) (min-colors 89))))
   (custom-theme-set-faces
diff --git a/etc/themes/dichromacy-theme.el b/etc/themes/dichromacy-theme.el
index d53c075d92..fe44d520cc 100644
--- a/etc/themes/dichromacy-theme.el
+++ b/etc/themes/dichromacy-theme.el
@@ -21,6 +21,7 @@
 
 ;;; Code:
 
+;;;###theme-autoload
 (deftheme dichromacy
   "Face colors suitable for red/green color-blind users.
 The color palette is from B. Wong, Nature Methods 8, 441 (2011).
@@ -28,7 +29,9 @@ dichromacy
 differentiated by individuals with protanopia or deuteranopia.
 
 Basic, Font Lock, Isearch, Gnus, Message, Flyspell, and
-Ansi-Color faces are included.")
+Ansi-Color faces are included."
+  :background-mode 'light
+  :kind 'color-scheme)
 
 (let ((class '((class color) (min-colors 89)))
       (orange "#e69f00")
diff --git a/etc/themes/leuven-dark-theme.el b/etc/themes/leuven-dark-theme.el
index 0e162c8bab..08978a2668 100644
--- a/etc/themes/leuven-dark-theme.el
+++ b/etc/themes/leuven-dark-theme.el
@@ -5,7 +5,7 @@
 ;; Author: Fabrice Niessen <(concat "fniessen" at-sign "pirilampo.org")>
 ;; Contributor: Thibault Polge <(concat "thibault" at-sign "thb.lt")>
 ;; URL: https://github.com/fniessen/emacs-leuven-dark-theme
-;; Version: 20220202.1126
+;; Version: 20221010.1208
 ;; Keywords: color theme
 
 ;; This file is part of GNU Emacs.
@@ -93,11 +93,15 @@ leuven-dark-scale-font
 
 ;;; Theme Faces.
 
+;;;###theme-autoload
 (deftheme leuven-dark
   "Face colors with a light background.
 Basic, Font Lock, Isearch, Gnus, Message, Org mode, Diff, Ediff,
 Flyspell, Semantic, and Ansi-Color faces are included -- and much
-more...")
+more..."
+  :background-mode 'dark
+  :family 'leuven
+  :kind 'color-scheme)
 
 (let ((class '((class color) (min-colors 89)))
 
diff --git a/etc/themes/leuven-theme.el b/etc/themes/leuven-theme.el
index d9a8d5391a..e712a79adf 100644
--- a/etc/themes/leuven-theme.el
+++ b/etc/themes/leuven-theme.el
@@ -4,7 +4,7 @@
 
 ;; Author: Fabrice Niessen <(concat "fniessen" at-sign "pirilampo.org")>
 ;; URL: https://github.com/fniessen/emacs-leuven-theme
-;; Version: 20200513.1928
+;; Version: 20221010.1209
 ;; Keywords: color theme
 
 ;; This file is part of GNU Emacs.
@@ -74,11 +74,15 @@ leuven-scale-font
 
 ;;; Theme Faces.
 
+;;;###theme-autoload
 (deftheme leuven
   "Face colors with a light background.
 Basic, Font Lock, Isearch, Gnus, Message, Org mode, Diff, Ediff,
 Flyspell, Semantic, and Ansi-Color faces are included -- and much
-more...")
+more..."
+  :background-mode 'light
+  :kind 'color-scheme
+  :family 'leuven)
 
 (let ((class '((class color) (min-colors 89)))
 
diff --git a/etc/themes/light-blue-theme.el b/etc/themes/light-blue-theme.el
index eeca46210c..808fcbfeb2 100644
--- a/etc/themes/light-blue-theme.el
+++ b/etc/themes/light-blue-theme.el
@@ -26,8 +26,11 @@
 
 ;;; Code:
 
+;;;###theme-autoload
 (deftheme light-blue
-  "Face colors utilizing a light blue background.")
+  "Face colors utilizing a light blue background."
+  :background-mode 'light
+  :kind 'color-scheme)
 
 (make-obsolete 'light-blue nil "29.1")
 
diff --git a/etc/themes/manoj-dark-theme.el b/etc/themes/manoj-dark-theme.el
index af5576386c..f9aaa97c25 100644
--- a/etc/themes/manoj-dark-theme.el
+++ b/etc/themes/manoj-dark-theme.el
@@ -64,10 +64,13 @@
 
 ;;; Code:
 
+;;;###theme-autoload
 (deftheme manoj-dark
   "Very high contrast faces with a black background.
 This theme avoids subtle color variations, while avoiding the
-jarring angry fruit salad look to reduce eye fatigue.")
+jarring angry fruit salad look to reduce eye fatigue."
+  :background-mode 'dark
+  :kind 'color-scheme)
 
 (custom-theme-set-faces
  'manoj-dark
diff --git a/etc/themes/misterioso-theme.el b/etc/themes/misterioso-theme.el
index 55186384ad..3fd6cdb5af 100644
--- a/etc/themes/misterioso-theme.el
+++ b/etc/themes/misterioso-theme.el
@@ -21,8 +21,11 @@
 
 ;;; Code:
 
+;;;###theme-autoload
 (deftheme misterioso
-  "Predominantly blue/cyan faces on a dark cyan background.")
+  "Predominantly blue/cyan faces on a dark cyan background."
+  :background-mode 'dark
+  :kind 'color-scheme)
 
 (let ((class '((class color) (min-colors 89))))
 
diff --git a/etc/themes/tango-dark-theme.el b/etc/themes/tango-dark-theme.el
index ef00d2ac49..85995e4e99 100644
--- a/etc/themes/tango-dark-theme.el
+++ b/etc/themes/tango-dark-theme.el
@@ -27,10 +27,15 @@
 
 ;;; Code:
 
+;;;###theme-autoload
 (deftheme tango-dark
   "Face colors using the Tango palette (dark background).
 Basic, Font Lock, Isearch, Gnus, Message, Ediff, Flyspell,
-Semantic, and Ansi-Color faces are included.")
+Semantic, and Ansi-Color faces are included."
+  :background-mode 'dark
+  :kind 'color-scheme
+  :family 'tango)
+
 
 (let ((class '((class color) (min-colors 89)))
       ;; Tango palette colors.
diff --git a/etc/themes/tango-theme.el b/etc/themes/tango-theme.el
index ecbbf03753..2ac1b42294 100644
--- a/etc/themes/tango-theme.el
+++ b/etc/themes/tango-theme.el
@@ -27,10 +27,14 @@
 
 ;;; Code:
 
+;;;###theme-autoload
 (deftheme tango
   "Face colors using the Tango palette (light background).
 Basic, Font Lock, Isearch, Gnus, Message, Ediff, Flyspell,
-Semantic, and Ansi-Color faces are included.")
+Semantic, and Ansi-Color faces are included."
+  :background-mode 'light
+  :kind 'color-scheme
+  :family 'tango)
 
 (let ((class '((class color) (min-colors 89)))
       ;; Tango palette colors.
diff --git a/etc/themes/tsdh-dark-theme.el b/etc/themes/tsdh-dark-theme.el
index a88ad75520..6b1e865e42 100644
--- a/etc/themes/tsdh-dark-theme.el
+++ b/etc/themes/tsdh-dark-theme.el
@@ -19,8 +19,12 @@
 
 ;;; Code:
 
+;;;###theme-autoload
 (deftheme tsdh-dark
-  "A dark theme used and created by Tassilo Horn.")
+  "A dark theme used and created by Tassilo Horn."
+  :background-mode 'dark
+  :kind 'color-scheme
+  :family 'tsdh)
 
 (custom-theme-set-faces
  'tsdh-dark
diff --git a/etc/themes/tsdh-light-theme.el b/etc/themes/tsdh-light-theme.el
index d9d09b702b..ac964d66d6 100644
--- a/etc/themes/tsdh-light-theme.el
+++ b/etc/themes/tsdh-light-theme.el
@@ -19,9 +19,13 @@
 
 ;;; Code:
 
+;;;###theme-autoload
 (deftheme tsdh-light
   "A light Emacs theme.
-Used and created by Tassilo Horn.")
+Used and created by Tassilo Horn."
+  :background-mode 'light
+  :kind 'color-scheme
+  :family 'tsdh)
 
 (custom-theme-set-faces
  'tsdh-light
diff --git a/etc/themes/wheatgrass-theme.el b/etc/themes/wheatgrass-theme.el
index c56c8a2d8a..20e7bbbac2 100644
--- a/etc/themes/wheatgrass-theme.el
+++ b/etc/themes/wheatgrass-theme.el
@@ -19,11 +19,14 @@
 
 ;;; Code:
 
+;;;###theme-autoload
 (deftheme wheatgrass
   "High-contrast green/blue/brown faces on a black background.
 Basic, Font Lock, Isearch, Gnus, and Message faces are included.
 The default face foreground is wheat, with other faces in shades
-of green, brown, and blue.")
+of green, brown, and blue."
+  :background-mode 'dark
+  :kind 'color-scheme)
 
 (let ((class '((class color) (min-colors 89))))
   (custom-theme-set-faces
diff --git a/etc/themes/whiteboard-theme.el b/etc/themes/whiteboard-theme.el
index f21b18b421..2f86234b32 100644
--- a/etc/themes/whiteboard-theme.el
+++ b/etc/themes/whiteboard-theme.el
@@ -21,8 +21,11 @@
 
 ;;; Code:
 
+;;;###theme-autoload
 (deftheme whiteboard
-  "Face colors similar to markers on a whiteboard.")
+  "Face colors similar to markers on a whiteboard."
+  :background-mode 'light
+  :kind 'color-scheme)
 
 (let ((class '((class color) (min-colors 89))))
   (custom-theme-set-faces
diff --git a/etc/themes/wombat-theme.el b/etc/themes/wombat-theme.el
index d9fab8ac78..9bb026ead1 100644
--- a/etc/themes/wombat-theme.el
+++ b/etc/themes/wombat-theme.el
@@ -21,11 +21,14 @@
 
 ;;; Code:
 
+;;;###theme-autoload
 (deftheme wombat
   "Medium-contrast faces with a dark gray background.
 Adapted, with permission, from a Vim color scheme by Lars H. Nielsen.
 Basic, Font Lock, Isearch, Gnus, Message, and Ansi-Color faces
-are included.")
+are included."
+  :background-mode 'dark
+  :kind 'color-scheme)
 
 (let ((class '((class color) (min-colors 89))))
   (custom-theme-set-faces
diff --git a/lisp/custom.el b/lisp/custom.el
index 604b1a3ff4..5aecbe6fe5 100644
--- a/lisp/custom.el
+++ b/lisp/custom.el
@@ -1152,9 +1152,11 @@ custom--sort-vars-1
 ;;   (provide-theme 'THEME)
 
 
-(defmacro deftheme (theme &optional doc)
+(defmacro deftheme (theme &optional doc &rest properties)
   "Declare THEME to be a Custom theme.
 The optional argument DOC is a doc string describing the theme.
+PROPERTIES are interpreted as a property list that will be stored
+in the `theme-properties' property for THEME.
 
 Any theme `foo' should be defined in a file called `foo-theme.el';
 see `custom-make-theme-feature' for more information."
@@ -1164,18 +1166,25 @@ deftheme
     ;; It is better not to use backquote in this file,
     ;; because that makes a bootstrapping problem
     ;; if you need to recompile all the Lisp files using interpreted code.
-    (list 'custom-declare-theme (list 'quote theme) (list 'quote feature) doc)))
+    (list 'custom-declare-theme (list 'quote theme) (list 'quote feature) doc
+          (cons 'list properties))))
 
-(defun custom-declare-theme (theme feature &optional doc)
+(defun custom-declare-theme (theme feature &optional doc properties)
   "Like `deftheme', but THEME is evaluated as a normal argument.
-FEATURE is the feature this theme provides.  Normally, this is a symbol
-created from THEME by `custom-make-theme-feature'."
+FEATURE is the feature this theme provides.  Normally, this is a
+symbol created from THEME by `custom-make-theme-feature'.  The
+optional argument DOC may contain the documentation for THEME.
+The optional argument PROPERTIES may contain a property list of
+attributes associated with THEME."
   (unless (custom-theme-name-valid-p theme)
     (error "Custom theme cannot be named %S" theme))
   (unless (memq theme custom-known-themes)
     (push theme custom-known-themes))
   (put theme 'theme-feature feature)
-  (when doc (put theme 'theme-documentation doc)))
+  (when doc
+    (put theme 'theme-documentation doc))
+  (when properties
+    (put theme 'theme-properties properties)))
 
 (defun custom-make-theme-feature (theme)
   "Given a symbol THEME, create a new symbol by appending \"-theme\".
@@ -1372,6 +1381,55 @@ load-theme
     (enable-theme theme))
   t)
 
+(defun theme-list-variants (theme &rest list)
+  "Return a list of theme variants for THEME.
+If the optional argument LIST is not given, "
+  (let* ((properties (get theme 'theme-properties))
+         (family (plist-get properties :family)))
+    (seq-filter
+     (lambda (variant)
+       (and (eq (plist-get (get variant 'theme-properties) :family)
+                family)
+            (not (eq variant theme))))
+     (or list (custom-available-themes)))))
+
+(defun theme-choose-variant (&optional no-confirm no-enable)
+  "Prompt to switch from the current theme to one of its a variants.
+The current theme will be disabled before variant is enabled.  If
+the current theme has only one variant, switch to that variant
+without prompting, otherwise prompt for the variant to select.
+See `load-theme' for the meaning of NO-CONFIRM and NO-ENABLE."
+  (interactive)
+  (let ((active-color-schemes
+         (seq-filter
+          (lambda (theme)
+            ;; FIXME: As most themes currently do not have a `:kind'
+            ;; tag, it is assumed that a theme is a color scheme by
+            ;; default.  This should be reconsidered in the future.
+            (memq (plist-get (get theme 'theme-properties) :kind)
+                  '(color-scheme nil)))
+          custom-enabled-themes)))
+    (cond
+     ((length= active-color-schemes 0)
+      (user-error "No theme is active, cannot toggle"))
+     ((length> active-color-schemes 1)
+      (user-error "More than one theme active, cannot unambiguously toggle")))
+    (let* ((theme (car active-color-schemes))
+           (family (plist-get (get theme 'theme-properties) :family)))
+      (unless family
+        (error "Theme `%s' does not have any known variants" theme))
+      (let* ((variants (theme-list-variants theme))
+             (choice (cond
+                      ((null variants)
+                       (error "`%s' has no variants" theme))
+                      ((length= variants 1)
+                       (car variants))
+                      ((intern (completing-read "Load custom theme: " variants))))))
+        (disable-theme theme)
+        (load-theme choice no-confirm no-enable)))))
+
+(defalias 'toggle-theme #'theme-choose-variant)
+
 (defun custom-theme-load-confirm (hash)
   "Query the user about loading a Custom theme that may not be safe.
 The theme should be in the current buffer.  If the user agrees,
diff --git a/lisp/emacs-lisp/loaddefs-gen.el b/lisp/emacs-lisp/loaddefs-gen.el
index 964d23c770..e57024aee5 100644
--- a/lisp/emacs-lisp/loaddefs-gen.el
+++ b/lisp/emacs-lisp/loaddefs-gen.el
@@ -283,6 +283,12 @@ loaddefs-generate--make-autoload
            ,@(when-let ((safe (plist-get props :safe)))
                `((put ',varname 'safe-local-variable ,safe))))))
 
+     ;; Extract theme properties
+     ((eq car 'deftheme)
+      (let* ((name (car-safe (cdr-safe form)))
+	     (props (nthcdr 3 form)))
+	`(put ',name 'theme-properties (list ,@props))))
+
      ((eq car 'defgroup)
       ;; In Emacs this is normally handled separately by cus-dep.el, but for
       ;; third party packages, it can be convenient to explicitly autoload
-- 
2.37.3


Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#57639; Package emacs. (Tue, 11 Oct 2022 00:26:02 GMT) Full text and rfc822 format available.

Message #212 received at 57639 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Philip Kaludercic <philipk <at> posteo.net>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 57639 <at> debbugs.gnu.org,
 Protesilaos Stavrou <info <at> protesilaos.com>
Subject: Re: bug#57639: [PATCH] Add new command 'toggle-theme'
Date: Tue, 11 Oct 2022 02:25:06 +0200
Philip Kaludercic <philipk <at> posteo.net> writes:

> I am not sure how that is done exactly, but if we take the
> ";;;###theme-autoload" approach it should be able to have a handler to
> convert autoloaded (defcustom ...) forms in to (put ...) ones:

Yes.  But as noted before, since many themes are distributed via ELPA
(too), we can't really change the definition of deftheme.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#57639; Package emacs. (Tue, 11 Oct 2022 09:08:02 GMT) Full text and rfc822 format available.

Message #215 received at 57639 <at> debbugs.gnu.org (full text, mbox):

From: Philip Kaludercic <philipk <at> posteo.net>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 57639 <at> debbugs.gnu.org,
 Protesilaos Stavrou <info <at> protesilaos.com>
Subject: Re: bug#57639: [PATCH] Add new command 'toggle-theme'
Date: Tue, 11 Oct 2022 09:06:54 +0000
Lars Ingebrigtsen <larsi <at> gnus.org> writes:

> Philip Kaludercic <philipk <at> posteo.net> writes:
>
>> I am not sure how that is done exactly, but if we take the
>> ";;;###theme-autoload" approach it should be able to have a handler to
>> convert autoloaded (defcustom ...) forms in to (put ...) ones:
>
> Yes.  But as noted before, since many themes are distributed via ELPA
> (too), we can't really change the definition of deftheme.

The change to `deftheme' is only a feature of convenience, the
backwards-compatibile way to set the attributes remains using `put'.  In
fact, that `put'-expression is exactly what `deftheme' expands to.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#57639; Package emacs. (Tue, 11 Oct 2022 19:45:01 GMT) Full text and rfc822 format available.

Message #218 received at 57639 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Philip Kaludercic <philipk <at> posteo.net>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 57639 <at> debbugs.gnu.org,
 Protesilaos Stavrou <info <at> protesilaos.com>
Subject: Re: bug#57639: [PATCH] Add new command 'toggle-theme'
Date: Tue, 11 Oct 2022 21:43:51 +0200
Philip Kaludercic <philipk <at> posteo.net> writes:

> The change to `deftheme' is only a feature of convenience, the
> backwards-compatibile way to set the attributes remains using `put'.  In
> fact, that `put'-expression is exactly what `deftheme' expands to.

But your patch added altered the deftheme calls (and thereby making the
themes non-backward-compatible) -- did you send the wrong patch?




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#57639; Package emacs. (Tue, 11 Oct 2022 19:52:01 GMT) Full text and rfc822 format available.

Message #221 received at 57639 <at> debbugs.gnu.org (full text, mbox):

From: Philip Kaludercic <philipk <at> posteo.net>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 57639 <at> debbugs.gnu.org,
 Protesilaos Stavrou <info <at> protesilaos.com>
Subject: Re: bug#57639: [PATCH] Add new command 'toggle-theme'
Date: Tue, 11 Oct 2022 19:51:48 +0000
Lars Ingebrigtsen <larsi <at> gnus.org> writes:

> Philip Kaludercic <philipk <at> posteo.net> writes:
>
>> The change to `deftheme' is only a feature of convenience, the
>> backwards-compatibile way to set the attributes remains using `put'.  In
>> fact, that `put'-expression is exactly what `deftheme' expands to.
>
> But your patch added altered the deftheme calls (and thereby making the
> themes non-backward-compatible) -- did you send the wrong patch?

No, but these themes are built-in, and to my knowledge not distributed
on ELPA (or at least changes in emacs.git are not exported to ELPA.  The
Modus Themes are manually synchronised), so it shouldn't make any
difference.  If so, I'll revert the changes where necessary.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#57639; Package emacs. (Tue, 11 Oct 2022 20:05:01 GMT) Full text and rfc822 format available.

Message #224 received at 57639 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Philip Kaludercic <philipk <at> posteo.net>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 57639 <at> debbugs.gnu.org,
 Protesilaos Stavrou <info <at> protesilaos.com>
Subject: Re: bug#57639: [PATCH] Add new command 'toggle-theme'
Date: Tue, 11 Oct 2022 22:04:15 +0200
Philip Kaludercic <philipk <at> posteo.net> writes:

> No, but these themes are built-in, and to my knowledge not distributed
> on ELPA (or at least changes in emacs.git are not exported to ELPA.  The
> Modus Themes are manually synchronised), so it shouldn't make any
> difference.  If so, I'll revert the changes where necessary.

Ah, OK, then that's fine.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#57639; Package emacs. (Tue, 11 Oct 2022 20:10:01 GMT) Full text and rfc822 format available.

Message #227 received at 57639 <at> debbugs.gnu.org (full text, mbox):

From: Philip Kaludercic <philipk <at> posteo.net>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 57639 <at> debbugs.gnu.org,
 Protesilaos Stavrou <info <at> protesilaos.com>
Subject: Re: bug#57639: [PATCH] Add new command 'toggle-theme'
Date: Tue, 11 Oct 2022 20:09:42 +0000
Lars Ingebrigtsen <larsi <at> gnus.org> writes:

> Philip Kaludercic <philipk <at> posteo.net> writes:
>
>> No, but these themes are built-in, and to my knowledge not distributed
>> on ELPA (or at least changes in emacs.git are not exported to ELPA.  The
>> Modus Themes are manually synchronised), so it shouldn't make any
>> difference.  If so, I'll revert the changes where necessary.
>
> Ah, OK, then that's fine.

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).




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#57639; Package emacs. (Tue, 11 Oct 2022 20:50:02 GMT) Full text and rfc822 format available.

Message #230 received at 57639 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Philip Kaludercic <philipk <at> posteo.net>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 57639 <at> debbugs.gnu.org,
 Protesilaos Stavrou <info <at> protesilaos.com>
Subject: Re: bug#57639: [PATCH] Add new command 'toggle-theme'
Date: Tue, 11 Oct 2022 22:49:01 +0200
Philip Kaludercic <philipk <at> posteo.net> 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.

  




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#57639; Package emacs. (Thu, 13 Oct 2022 08:51:01 GMT) Full text and rfc822 format available.

Message #233 received at 57639 <at> debbugs.gnu.org (full text, mbox):

From: Philip Kaludercic <philipk <at> posteo.net>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 57639 <at> debbugs.gnu.org,
 Protesilaos Stavrou <info <at> protesilaos.com>
Subject: Re: bug#57639: [PATCH] Add new command 'toggle-theme'
Date: Thu, 13 Oct 2022 08:50:34 +0000
[Message part 1 (text/plain, inline)]
Lars Ingebrigtsen <larsi <at> gnus.org> writes:

> Philip Kaludercic <philipk <at> posteo.net> 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:

[0001-Tag-themes-with-properties.patch (text/x-patch, attachment)]
[0001-Handle-theme-autoload-comments-in-etc-themes.patch (text/x-patch, attachment)]
[Message part 4 (text/plain, inline)]
Note that this includes changes to both the Leuven and Modus themes.

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#57639; Package emacs. (Thu, 13 Oct 2022 09:11:02 GMT) Full text and rfc822 format available.

Message #236 received at 57639 <at> debbugs.gnu.org (full text, mbox):

From: Philip Kaludercic <philipk <at> posteo.net>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 57639 <at> debbugs.gnu.org,
 Protesilaos Stavrou <info <at> protesilaos.com>
Subject: Re: bug#57639: [PATCH] Add new command 'toggle-theme'
Date: Thu, 13 Oct 2022 09:10:00 +0000
> From a9bc4ba6b07aef118f05022486f9fc10a0e41e64 Mon Sep 17 00:00:00 2001
> From: Philip Kaludercic <philipk <at> posteo.net>
> Date: Thu, 13 Oct 2022 10:43:36 +0200
> Subject: [PATCH] Handle ;;;###theme-autoload comments in etc/themes
>
> * lisp/emacs-lisp/loaddefs-gen.el (loaddefs-generate--emacs-batch):
> Extract the autoloads and have them loaded along with loaddefs.el.
> ---
>  lisp/emacs-lisp/loaddefs-gen.el | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/lisp/emacs-lisp/loaddefs-gen.el b/lisp/emacs-lisp/loaddefs-gen.el
> index e57024aee5..39eec87f72 100644
> --- a/lisp/emacs-lisp/loaddefs-gen.el
> +++ b/lisp/emacs-lisp/loaddefs-gen.el
> @@ -736,7 +736,15 @@ loaddefs-generate--emacs-batch
>       ;; updated.
>       (file-newer-than-file-p
>        (expand-file-name "emacs-lisp/loaddefs-gen.el" lisp-directory)
> -      output-file))))
> +      output-file)))
> +  (let ((lisp-mode-autoload-regexp
> +           "^;;;###\\(\\(noexist\\)-\\)?\\(theme-autoload\\)")
> +          (output-file ))

I have just noticed that this variable was not used and have removed it
locally.

> +      (loaddefs-generate
> +       (expand-file-name "../etc/themes/" lisp-directory)
> +       (expand-file-name "theme-loaddefs.el" lisp-directory))))
> +
> +;;;###autoload (load "theme-loaddefs.el")
>  
>  (provide 'loaddefs-gen)




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#57639; Package emacs. (Thu, 13 Oct 2022 10:35:02 GMT) Full text and rfc822 format available.

Message #239 received at 57639 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Philip Kaludercic <philipk <at> posteo.net>
Cc: larsi <at> gnus.org, 57639 <at> debbugs.gnu.org, info <at> protesilaos.com
Subject: Re: bug#57639: [PATCH] Add new command 'toggle-theme'
Date: Thu, 13 Oct 2022 13:34:25 +0300
> From: Philip Kaludercic <philipk <at> posteo.net>
> Cc: Eli Zaretskii <eliz <at> gnu.org>,  57639 <at> debbugs.gnu.org,  Protesilaos
>  Stavrou <info <at> protesilaos.com>
> Date: Thu, 13 Oct 2022 08:50:34 +0000
> 
> Lars Ingebrigtsen <larsi <at> gnus.org> writes:
> 
> > Philip Kaludercic <philipk <at> posteo.net> 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?

Also, this needs a NEWS entry.

Thanks.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#57639; Package emacs. (Thu, 13 Oct 2022 10:37:01 GMT) Full text and rfc822 format available.

Message #242 received at 57639 <at> debbugs.gnu.org (full text, mbox):

From: Philip Kaludercic <philipk <at> posteo.net>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: larsi <at> gnus.org, 57639 <at> debbugs.gnu.org, info <at> protesilaos.com
Subject: Re: bug#57639: [PATCH] Add new command 'toggle-theme'
Date: Thu, 13 Oct 2022 10:35:51 +0000
Eli Zaretskii <eliz <at> gnu.org> writes:

>> From: Philip Kaludercic <philipk <at> posteo.net>
>> Cc: Eli Zaretskii <eliz <at> gnu.org>,  57639 <at> debbugs.gnu.org,  Protesilaos
>>  Stavrou <info <at> protesilaos.com>
>> Date: Thu, 13 Oct 2022 08:50:34 +0000
>> 
>> Lars Ingebrigtsen <larsi <at> gnus.org> writes:
>> 
>> > Philip Kaludercic <philipk <at> posteo.net> 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.




Severity set to 'wishlist' from 'normal' Request was from Stefan Kangas <stefankangas <at> gmail.com> to control <at> debbugs.gnu.org. (Thu, 13 Oct 2022 13:49:03 GMT) Full text and rfc822 format available.

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#57639; Package emacs. (Thu, 13 Oct 2022 14:05:01 GMT) Full text and rfc822 format available.

Message #247 received at 57639 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Kangas <stefankangas <at> gmail.com>
To: Philip Kaludercic <philipk <at> posteo.net>, Eli Zaretskii <eliz <at> gnu.org>
Cc: larsi <at> gnus.org, 57639 <at> debbugs.gnu.org, info <at> protesilaos.com
Subject: Re: bug#57639: [PATCH] Add new command 'toggle-theme'
Date: Thu, 13 Oct 2022 16:03:54 +0200
Philip Kaludercic <philipk <at> posteo.net> writes:

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

If that means we can get rid of the boilerplate that themes have to add
now, I think it would be a good thing.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#57639; Package emacs. (Thu, 13 Oct 2022 14:09:01 GMT) Full text and rfc822 format available.

Message #250 received at 57639 <at> debbugs.gnu.org (full text, mbox):

From: Philip Kaludercic <philipk <at> posteo.net>
To: Stefan Kangas <stefankangas <at> gmail.com>
Cc: Eli Zaretskii <eliz <at> gnu.org>, larsi <at> gnus.org, info <at> protesilaos.com,
 57639 <at> debbugs.gnu.org
Subject: Re: bug#57639: [PATCH] Add new command 'toggle-theme'
Date: Thu, 13 Oct 2022 14:07:56 +0000
Stefan Kangas <stefankangas <at> gmail.com> writes:

> Philip Kaludercic <philipk <at> posteo.net> writes:
>
>> Nothing for now, but I could add support for package.el.
>
> If that means we can get rid of the boilerplate that themes have to add
> now, I think it would be a good thing.

The fundamental issue that necessitates boilerplate is that packages are
not added to the theme load path, and adding a new cookie type doesn't
change that.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#57639; Package emacs. (Thu, 13 Oct 2022 19:13:02 GMT) Full text and rfc822 format available.

Message #253 received at 57639 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Philip Kaludercic <philipk <at> posteo.net>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 57639 <at> debbugs.gnu.org,
 Protesilaos Stavrou <info <at> protesilaos.com>
Subject: Re: bug#57639: [PATCH] Add new command 'toggle-theme'
Date: Thu, 13 Oct 2022 21:12:00 +0200
Philip Kaludercic <philipk <at> posteo.net> writes:

> OK, it looks like this worked.  Just to recap, these are the proposed
> changes:

Looks good to me (but I haven't actually tried the code).




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#57639; Package emacs. (Thu, 13 Oct 2022 20:23:02 GMT) Full text and rfc822 format available.

Message #256 received at 57639 <at> debbugs.gnu.org (full text, mbox):

From: Philip Kaludercic <philipk <at> posteo.net>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 57639 <at> debbugs.gnu.org,
 Protesilaos Stavrou <info <at> protesilaos.com>
Subject: Re: bug#57639: [PATCH] Add new command 'toggle-theme'
Date: Thu, 13 Oct 2022 20:21:53 +0000
Lars Ingebrigtsen <larsi <at> gnus.org> writes:

> Philip Kaludercic <philipk <at> posteo.net> writes:
>
>> OK, it looks like this worked.  Just to recap, these are the proposed
>> changes:
>
> Looks good to me (but I haven't actually tried the code).

I will be testing it for a day or two, and then push it.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#57639; Package emacs. (Thu, 13 Oct 2022 20:48:01 GMT) Full text and rfc822 format available.

Message #259 received at 57639 <at> debbugs.gnu.org (full text, mbox):

From: Mauro Aranda <maurooaranda <at> gmail.com>
To: Philip Kaludercic <philipk <at> posteo.net>
Cc: Lars Ingebrigtsen <larsi <at> gnus.org>, Eli Zaretskii <eliz <at> gnu.org>,
 Protesilaos Stavrou <info <at> protesilaos.com>, 57639 <at> debbugs.gnu.org
Subject: Re: bug#57639: [PATCH] Add new command 'toggle-theme'
Date: Thu, 13 Oct 2022 17:46:55 -0300
Hi Philip,

Philip Kaludercic <philipk <at> posteo.net> writes:

> +Themes*} buffer.  The remaining arguments @var{properties} are used
> +pass a property list with theme attributes.

I think this added sentence is not clear.

Also, no documentation for these special properties for toggling themes?

> +(defun theme-list-variants (theme &rest list)
> +  "Return a list of theme variants for THEME.
> +If the optional argument LIST is not given, "

This docstring is incomplete.

> +  (let* ((properties (get theme 'theme-properties))
> +         (family (plist-get properties :family)))
> +    (seq-filter
> +     (lambda (variant)
> +       (and (eq (plist-get (get variant 'theme-properties) :family)
> +                family)
> +            (not (eq variant theme))))
> +     (or list (custom-available-themes)))))
> +
> +(defun theme-choose-variant (&optional no-confirm no-enable)
> +  "Prompt to switch from the current theme to one of its a variants.

I'd say: "Command to switch..."

> +  (let ((active-color-schemes
> +         (seq-filter
> +          (lambda (theme)
> +            ;; FIXME: As most themes currently do not have a `:kind'
> +            ;; tag, it is assumed that a theme is a color scheme by
> +            ;; default.  This should be reconsidered in the future.
> +            (memq (plist-get (get theme 'theme-properties) :kind)
> +                  '(color-scheme nil)))

I think that theme writers who care about this functionality will add
:kind and :family to the themes, and those who don't won't bother with
that.  So I don't really see the point in supporting (:kind nil).

> +          custom-enabled-themes)))
> +    (cond
> +     ((length= active-color-schemes 0)
> +      (user-error "No theme is active, cannot toggle"))

This message will be confusing when there are themes whose :kind is not
color-scheme...

> +     ((length> active-color-schemes 1)
> +      (user-error "More than one theme active, cannot unambiguously 
toggle")))
> +    (let* ((theme (car active-color-schemes))
> +           (family (plist-get (get theme 'theme-properties) :family)))
> +      (unless family
> +        (error "Theme `%s' does not have any known variants" theme))

This will pretty much always error with themes that don't really care
about toggling (see above).  Could you tell more about what is the
benefit of supporting (:kind nil)?

> --- a/lisp/emacs-lisp/loaddefs-gen.el
> +++ b/lisp/emacs-lisp/loaddefs-gen.el
> @@ -283,6 +283,12 @@ loaddefs-generate--make-autoload
>             ,@(when-let ((safe (plist-get props :safe)))
>                 `((put ',varname 'safe-local-variable ,safe))))))
>
> +     ;; Extract theme properties

Full stop missing.

> +     ((eq car 'deftheme)
> +      (let* ((name (car-safe (cdr-safe form)))
> +         (props (nthcdr 3 form)))
> +    `(put ',name 'theme-properties (list ,@props))))

In the Autoload section of the Elisp Manual, we have this:
"The forms which are not copied verbatim are the following:..."

Shouldn't deftheme be added to that list as well?





Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#57639; Package emacs. (Thu, 13 Oct 2022 22:20:01 GMT) Full text and rfc822 format available.

Message #262 received at 57639 <at> debbugs.gnu.org (full text, mbox):

From: Philip Kaludercic <philipk <at> posteo.net>
To: Mauro Aranda <maurooaranda <at> gmail.com>
Cc: Lars Ingebrigtsen <larsi <at> gnus.org>, Eli Zaretskii <eliz <at> gnu.org>,
 Protesilaos Stavrou <info <at> protesilaos.com>, 57639 <at> debbugs.gnu.org
Subject: Re: bug#57639: [PATCH] Add new command 'toggle-theme'
Date: Thu, 13 Oct 2022 22:19:20 +0000
Mauro Aranda <maurooaranda <at> gmail.com> writes:

> Hi Philip,
>
> Philip Kaludercic <philipk <at> posteo.net> writes:
>
>> +Themes*} buffer.  The remaining arguments @var{properties} are used
>> +pass a property list with theme attributes.
>
> I think this added sentence is not clear.
>
> Also, no documentation for these special properties for toggling themes?

Currently no.

>> +(defun theme-list-variants (theme &rest list)
>> +  "Return a list of theme variants for THEME.
>> +If the optional argument LIST is not given, "
>
> This docstring is incomplete.

Fixed, thanks.

>> +  (let* ((properties (get theme 'theme-properties))
>> +         (family (plist-get properties :family)))
>> +    (seq-filter
>> +     (lambda (variant)
>> +       (and (eq (plist-get (get variant 'theme-properties) :family)
>> +                family)
>> +            (not (eq variant theme))))
>> +     (or list (custom-available-themes)))))
>> +
>> +(defun theme-choose-variant (&optional no-confirm no-enable)
>> +  "Prompt to switch from the current theme to one of its a variants.
>
> I'd say: "Command to switch..."

Do you think it is necessary to point out that it is a command?

>> +  (let ((active-color-schemes
>> +         (seq-filter
>> +          (lambda (theme)
>> +            ;; FIXME: As most themes currently do not have a `:kind'
>> +            ;; tag, it is assumed that a theme is a color scheme by
>> +            ;; default.  This should be reconsidered in the future.
>> +            (memq (plist-get (get theme 'theme-properties) :kind)
>> +                  '(color-scheme nil)))
>
> I think that theme writers who care about this functionality will add
> :kind and :family to the themes, and those who don't won't bother with
> that.  So I don't really see the point in supporting (:kind nil).

:kind nil will probably not occur in practice, it is just that
`plist-get' will return nil if no :kind is specified.

>> +          custom-enabled-themes)))
>> +    (cond
>> +     ((length= active-color-schemes 0)
>> +      (user-error "No theme is active, cannot toggle"))
>
> This message will be confusing when there are themes whose :kind is not
> color-scheme...

How come?  Or do you think that we should explicitly clarify that
`theme-choose-variant' is just for color-schemes?

>> +     ((length> active-color-schemes 1)
>> +      (user-error "More than one theme active, cannot unambiguously
>   toggle")))
>> +    (let* ((theme (car active-color-schemes))
>> +           (family (plist-get (get theme 'theme-properties) :family)))
>> +      (unless family
>> +        (error "Theme `%s' does not have any known variants" theme))
>
> This will pretty much always error with themes that don't really care
> about toggling (see above).  Could you tell more about what is the
> benefit of supporting (:kind nil)?

I guess you are right in saying that nobody will set :family without
setting :kind... But that won't change anything here, because what you
describe is intended (a theme that has no variants, cannot be toggled.)

>> --- a/lisp/emacs-lisp/loaddefs-gen.el
>> +++ b/lisp/emacs-lisp/loaddefs-gen.el
>> @@ -283,6 +283,12 @@ loaddefs-generate--make-autoload
>>             ,@(when-let ((safe (plist-get props :safe)))
>>                 `((put ',varname 'safe-local-variable ,safe))))))
>>
>> +     ;; Extract theme properties
>
> Full stop missing.

Noted.

>> +     ((eq car 'deftheme)
>> +      (let* ((name (car-safe (cdr-safe form)))
>> +         (props (nthcdr 3 form)))
>> +    `(put ',name 'theme-properties (list ,@props))))
>
> In the Autoload section of the Elisp Manual, we have this:
> "The forms which are not copied verbatim are the following:..."
>
> Shouldn't deftheme be added to that list as well?

Good point, will do.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#57639; Package emacs. (Thu, 13 Oct 2022 22:54:02 GMT) Full text and rfc822 format available.

Message #265 received at 57639 <at> debbugs.gnu.org (full text, mbox):

From: Mauro Aranda <maurooaranda <at> gmail.com>
To: Philip Kaludercic <philipk <at> posteo.net>
Cc: Lars Ingebrigtsen <larsi <at> gnus.org>, Eli Zaretskii <eliz <at> gnu.org>,
 Protesilaos Stavrou <info <at> protesilaos.com>, 57639 <at> debbugs.gnu.org
Subject: Re: bug#57639: [PATCH] Add new command 'toggle-theme'
Date: Thu, 13 Oct 2022 19:53:19 -0300
Philip Kaludercic <philipk <at> posteo.net> writes:

> Mauro Aranda <maurooaranda <at> gmail.com> writes:
>
>> Hi Philip,
>>
>> Philip Kaludercic <philipk <at> posteo.net> writes:
>>
>>> +Themes*} buffer.  The remaining arguments @var{properties} are used
>>> +pass a property list with theme attributes.
>>
>> I think this added sentence is not clear.
>>
>> Also, no documentation for these special properties for toggling themes?
>
> Currently no.

I hope there will be.

>>> +(defun theme-choose-variant (&optional no-confirm no-enable)
>>> +  "Prompt to switch from the current theme to one of its a variants.
>>
>> I'd say: "Command to switch..."
>
> Do you think it is necessary to point out that it is a command?

OK, maybe not.  But why start with "Prompt to"? It is likely that it
will not prompt.  Why not get rid of it?

>>> +  (let ((active-color-schemes
>>> +         (seq-filter
>>> +          (lambda (theme)
>>> +            ;; FIXME: As most themes currently do not have a `:kind'
>>> +            ;; tag, it is assumed that a theme is a color scheme by
>>> +            ;; default.  This should be reconsidered in the future.
>>> +            (memq (plist-get (get theme 'theme-properties) :kind)
>>> +                  '(color-scheme nil)))
>>
>> I think that theme writers who care about this functionality will add
>> :kind and :family to the themes, and those who don't won't bother with
>> that.  So I don't really see the point in supporting (:kind nil).
>
> :kind nil will probably not occur in practice, it is just that
> `plist-get' will return nil if no :kind is specified.

I know that.  I didn't say a theme will pass :kind nil, I was referring
to the FIXME.  But let me try to be clearer: I don't see the point in
special handling the absense of :kind.

>>> +          custom-enabled-themes)))
>>> +    (cond
>>> +     ((length= active-color-schemes 0)
>>> +      (user-error "No theme is active, cannot toggle"))
>>
>> This message will be confusing when there are themes whose :kind is not
>> color-scheme...
>
> How come?  Or do you think that we should explicitly clarify that
> `theme-choose-variant' is just for color-schemes?

If you're filtering by :kind, and if there are themes that in the future
specify another :kind value, then you'll be saying that there's no theme
active but that won't be correct.

<Personal experience alert>
I use themes for setting variables too, as a
way to manage my config and change it quickly by {en|dis}abling them.
If I use a theme that supports this kind of toggling (like the modus
themes), I'll have to specify a :kind to my personal themes so that I
can toggle with toggle-theme.

So, there is already a chance that another :kind values will show up,
and the message might become confusing.
<End of alert>

>>> +     ((length> active-color-schemes 1)
>>> +      (user-error "More than one theme active, cannot unambiguously
>>   toggle")))
>>> +    (let* ((theme (car active-color-schemes))
>>> +           (family (plist-get (get theme 'theme-properties) :family)))
>>> +      (unless family
>>> +        (error "Theme `%s' does not have any known variants" theme))
>>
>> This will pretty much always error with themes that don't really care
>> about toggling (see above).  Could you tell more about what is the
>> benefit of supporting (:kind nil)?
>
> I guess you are right in saying that nobody will set :family without
> setting :kind... But that won't change anything here, because what you
> describe is intended (a theme that has no variants, cannot be toggled.)

See above.  I meant to discuss the FIXME and special handling the
absense of :kind.  I'd say it's not needed at all.  But of course,
erroring out here is OK.





Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#57639; Package emacs. (Fri, 14 Oct 2022 06:12:01 GMT) Full text and rfc822 format available.

Message #268 received at 57639 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Philip Kaludercic <philipk <at> posteo.net>
Cc: larsi <at> gnus.org, maurooaranda <at> gmail.com, info <at> protesilaos.com,
 57639 <at> debbugs.gnu.org
Subject: Re: bug#57639: [PATCH] Add new command 'toggle-theme'
Date: Fri, 14 Oct 2022 09:11:30 +0300
> From: Philip Kaludercic <philipk <at> posteo.net>
> Cc: Lars Ingebrigtsen <larsi <at> gnus.org>,  Eli Zaretskii <eliz <at> gnu.org>,
>   57639 <at> debbugs.gnu.org,  Protesilaos Stavrou <info <at> protesilaos.com>
> Date: Thu, 13 Oct 2022 22:19:20 +0000
> 
> Mauro Aranda <maurooaranda <at> gmail.com> writes:
> 
> >> +(defun theme-choose-variant (&optional no-confirm no-enable)
> >> +  "Prompt to switch from the current theme to one of its a variants.
> >
> > I'd say: "Command to switch..."
> 
> Do you think it is necessary to point out that it is a command?

I don't, FWIW.  The doc string should describe what the command does.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#57639; Package emacs. (Fri, 14 Oct 2022 07:30:02 GMT) Full text and rfc822 format available.

Message #271 received at 57639 <at> debbugs.gnu.org (full text, mbox):

From: Philip Kaludercic <philipk <at> posteo.net>
To: Mauro Aranda <maurooaranda <at> gmail.com>
Cc: Lars Ingebrigtsen <larsi <at> gnus.org>, Eli Zaretskii <eliz <at> gnu.org>,
 Protesilaos Stavrou <info <at> protesilaos.com>, 57639 <at> debbugs.gnu.org
Subject: Re: bug#57639: [PATCH] Add new command 'toggle-theme'
Date: Fri, 14 Oct 2022 07:28:51 +0000
Mauro Aranda <maurooaranda <at> gmail.com> writes:

> Philip Kaludercic <philipk <at> posteo.net> writes:
>
>> Mauro Aranda <maurooaranda <at> gmail.com> writes:
>>
>>> Hi Philip,
>>>
>>> Philip Kaludercic <philipk <at> posteo.net> writes:
>>>
>>>> +Themes*} buffer.  The remaining arguments @var{properties} are used
>>>> +pass a property list with theme attributes.
>>>
>>> I think this added sentence is not clear.
>>>
>>> Also, no documentation for these special properties for toggling themes?
>>
>> Currently no.
>
> I hope there will be.

I had to check the thread again, and the most that was said on the topic
was in <87leqo978k.fsf <at> gnus.org>.  So the idea would be to mention
:family, :kind and :background-mode and state that anything else is
undefined/shouldn't be used in case we decide to add another property in
the future?

We should also specify what valid values are.  :family is just a symbol,
:background-mode is either 'light or 'dark, but what about :kind?

>>>> +(defun theme-choose-variant (&optional no-confirm no-enable)
>>>> +  "Prompt to switch from the current theme to one of its a variants.
>>>
>>> I'd say: "Command to switch..."
>>
>> Do you think it is necessary to point out that it is a command?
>
> OK, maybe not.  But why start with "Prompt to"? It is likely that it
> will not prompt.  Why not get rid of it?

Good point, so why not just

     Switch from the current theme to one of its a variants.

>>>> +  (let ((active-color-schemes
>>>> +         (seq-filter
>>>> +          (lambda (theme)
>>>> +            ;; FIXME: As most themes currently do not have a `:kind'
>>>> +            ;; tag, it is assumed that a theme is a color scheme by
>>>> +            ;; default.  This should be reconsidered in the future.
>>>> +            (memq (plist-get (get theme 'theme-properties) :kind)
>>>> +                  '(color-scheme nil)))
>>>
>>> I think that theme writers who care about this functionality will add
>>> :kind and :family to the themes, and those who don't won't bother with
>>> that.  So I don't really see the point in supporting (:kind nil).
>>
>> :kind nil will probably not occur in practice, it is just that
>> `plist-get' will return nil if no :kind is specified.
>
> I know that.  I didn't say a theme will pass :kind nil, I was referring
> to the FIXME.  But let me try to be clearer: I don't see the point in
> special handling the absense of :kind.

On reflection, I agree and will remove this.

>>>> +          custom-enabled-themes)))
>>>> +    (cond
>>>> +     ((length= active-color-schemes 0)
>>>> +      (user-error "No theme is active, cannot toggle"))
>>>
>>> This message will be confusing when there are themes whose :kind is not
>>> color-scheme...
>>
>> How come?  Or do you think that we should explicitly clarify that
>> `theme-choose-variant' is just for color-schemes?
>
> If you're filtering by :kind, and if there are themes that in the future
> specify another :kind value, then you'll be saying that there's no theme
> active but that won't be correct.
>
> <Personal experience alert>
> I use themes for setting variables too, as a
> way to manage my config and change it quickly by {en|dis}abling them.
> If I use a theme that supports this kind of toggling (like the modus
> themes), I'll have to specify a :kind to my personal themes so that I
> can toggle with toggle-theme.
>
> So, there is already a chance that another :kind values will show up,
> and the message might become confusing.
> <End of alert>
>
>>>> +     ((length> active-color-schemes 1)
>>>> +      (user-error "More than one theme active, cannot unambiguously
>>>   toggle")))
>>>> +    (let* ((theme (car active-color-schemes))
>>>> +           (family (plist-get (get theme 'theme-properties) :family)))
>>>> +      (unless family
>>>> +        (error "Theme `%s' does not have any known variants" theme))
>>>
>>> This will pretty much always error with themes that don't really care
>>> about toggling (see above).  Could you tell more about what is the
>>> benefit of supporting (:kind nil)?
>>
>> I guess you are right in saying that nobody will set :family without
>> setting :kind... But that won't change anything here, because what you
>> describe is intended (a theme that has no variants, cannot be toggled.)
>
> See above.  I meant to discuss the FIXME and special handling the
> absense of :kind.  I'd say it's not needed at all.  But of course,
> erroring out here is OK.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#57639; Package emacs. (Fri, 14 Oct 2022 11:23:02 GMT) Full text and rfc822 format available.

Message #274 received at 57639 <at> debbugs.gnu.org (full text, mbox):

From: Mauro Aranda <maurooaranda <at> gmail.com>
To: Philip Kaludercic <philipk <at> posteo.net>
Cc: Lars Ingebrigtsen <larsi <at> gnus.org>, Eli Zaretskii <eliz <at> gnu.org>,
 Protesilaos Stavrou <info <at> protesilaos.com>, 57639 <at> debbugs.gnu.org
Subject: Re: bug#57639: [PATCH] Add new command 'toggle-theme'
Date: Fri, 14 Oct 2022 08:22:20 -0300
Philip Kaludercic <philipk <at> posteo.net> writes:

> Mauro Aranda <maurooaranda <at> gmail.com> writes:

>>>> Also, no documentation for these special properties for toggling 
themes?
>>>
>>> Currently no.
>>
>> I hope there will be.
>
> I had to check the thread again, and the most that was said on the topic
> was in <87leqo978k.fsf <at> gnus.org>.  So the idea would be to mention
> :family, :kind and :background-mode and state that anything else is
> undefined/shouldn't be used in case we decide to add another property in
> the future?
>
> We should also specify what valid values are.  :family is just a symbol,
> :background-mode is either 'light or 'dark, but what about :kind?

I'm not really good at this, but something like:

The following properties are supported:
':family': The value should be a symbol, the name of the family that the
theme is part of.  [And insert here the explanation of family that Eli
gave]
':kind': The value should be a symbol.  If a theme is enabled and this
property has the value color-scheme, then the theme-choose-variant
command will look for other available themes that belong to the same
family in order to switch the themes.


I didn't include background-mode because the code does nothing with that
property, AFAICT.

>>>>> +(defun theme-choose-variant (&optional no-confirm no-enable)
>>>>> +  "Prompt to switch from the current theme to one of its a variants.
>>>>
>>>> I'd say: "Command to switch..."
>>>
>>> Do you think it is necessary to point out that it is a command?
>>
>> OK, maybe not.  But why start with "Prompt to"? It is likely that it
>> will not prompt.  Why not get rid of it?
>
> Good point, so why not just
>
>      Switch from the current theme to one of its a variants.

I think that's better, yes.  I think the "a" between "its" and
"variants" is a typo.





Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#57639; Package emacs. (Fri, 14 Oct 2022 15:22:01 GMT) Full text and rfc822 format available.

Message #277 received at 57639 <at> debbugs.gnu.org (full text, mbox):

From: Philip Kaludercic <philipk <at> posteo.net>
To: Mauro Aranda <maurooaranda <at> gmail.com>
Cc: Lars Ingebrigtsen <larsi <at> gnus.org>, Eli Zaretskii <eliz <at> gnu.org>,
 Protesilaos Stavrou <info <at> protesilaos.com>, 57639 <at> debbugs.gnu.org
Subject: Re: bug#57639: [PATCH] Add new command 'toggle-theme'
Date: Fri, 14 Oct 2022 15:21:30 +0000
Mauro Aranda <maurooaranda <at> gmail.com> writes:

> Philip Kaludercic <philipk <at> posteo.net> writes:
>
>> Mauro Aranda <maurooaranda <at> gmail.com> writes:
>
>>>>> Also, no documentation for these special properties for toggling
>      themes?
>>>>
>>>> Currently no.
>>>
>>> I hope there will be.
>>
>> I had to check the thread again, and the most that was said on the topic
>> was in <87leqo978k.fsf <at> gnus.org>.  So the idea would be to mention
>> :family, :kind and :background-mode and state that anything else is
>> undefined/shouldn't be used in case we decide to add another property in
>> the future?
>>
>> We should also specify what valid values are.  :family is just a symbol,
>> :background-mode is either 'light or 'dark, but what about :kind?
>
> I'm not really good at this, but something like:
>
> The following properties are supported:
> ':family': The value should be a symbol, the name of the family that the
> theme is part of.  [And insert here the explanation of family that Eli
> gave]
> ':kind': The value should be a symbol.  If a theme is enabled and this
> property has the value color-scheme, then the theme-choose-variant
> command will look for other available themes that belong to the same
> family in order to switch the themes.

Sounds good, I'll try to work this in.

> I didn't include background-mode because the code does nothing with that
> property, AFAICT.

True, but the long term idea is for Emacs to be able to use this
property when the system switches between dark and light mode.

>>>>>> +(defun theme-choose-variant (&optional no-confirm no-enable)
>>>>>> +  "Prompt to switch from the current theme to one of its a variants.
>>>>>
>>>>> I'd say: "Command to switch..."
>>>>
>>>> Do you think it is necessary to point out that it is a command?
>>>
>>> OK, maybe not.  But why start with "Prompt to"? It is likely that it
>>> will not prompt.  Why not get rid of it?
>>
>> Good point, so why not just
>>
>>      Switch from the current theme to one of its a variants.
>
> I think that's better, yes.  I think the "a" between "its" and
> "variants" is a typo.

Of course, thanks for catching that!




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#57639; Package emacs. (Fri, 14 Oct 2022 18:22:02 GMT) Full text and rfc822 format available.

Message #280 received at 57639 <at> debbugs.gnu.org (full text, mbox):

From: Philip Kaludercic <philipk <at> posteo.net>
To: Mauro Aranda <maurooaranda <at> gmail.com>
Cc: Lars Ingebrigtsen <larsi <at> gnus.org>, Eli Zaretskii <eliz <at> gnu.org>,
 Protesilaos Stavrou <info <at> protesilaos.com>, 57639 <at> debbugs.gnu.org
Subject: Re: bug#57639: [PATCH] Add new command 'toggle-theme'
Date: Fri, 14 Oct 2022 18:20:50 +0000
[Message part 1 (text/plain, inline)]
Mauro Aranda <maurooaranda <at> gmail.com> writes:

> Philip Kaludercic <philipk <at> posteo.net> writes:
>
>> Mauro Aranda <maurooaranda <at> gmail.com> writes:
>
>>>>> Also, no documentation for these special properties for toggling
>      themes?
>>>>
>>>> Currently no.
>>>
>>> I hope there will be.
>>
>> I had to check the thread again, and the most that was said on the topic
>> was in <87leqo978k.fsf <at> gnus.org>.  So the idea would be to mention
>> :family, :kind and :background-mode and state that anything else is
>> undefined/shouldn't be used in case we decide to add another property in
>> the future?
>>
>> We should also specify what valid values are.  :family is just a symbol,
>> :background-mode is either 'light or 'dark, but what about :kind?
>
> I'm not really good at this, but something like:
>
> The following properties are supported:
> ':family': The value should be a symbol, the name of the family that the
> theme is part of.  [And insert here the explanation of family that Eli
> gave]
> ':kind': The value should be a symbol.  If a theme is enabled and this
> property has the value color-scheme, then the theme-choose-variant
> command will look for other available themes that belong to the same
> family in order to switch the themes.
>
>
> I didn't include background-mode because the code does nothing with that
> property, AFAICT.
>
>>>>>> +(defun theme-choose-variant (&optional no-confirm no-enable)
>>>>>> +  "Prompt to switch from the current theme to one of its a variants.
>>>>>
>>>>> I'd say: "Command to switch..."
>>>>
>>>> Do you think it is necessary to point out that it is a command?
>>>
>>> OK, maybe not.  But why start with "Prompt to"? It is likely that it
>>> will not prompt.  Why not get rid of it?
>>
>> Good point, so why not just
>>
>>      Switch from the current theme to one of its a variants.
>
> I think that's better, yes.  I think the "a" between "its" and
> "variants" is a typo.

Here is the updated patch:

[0001-Tag-themes-with-properties.patch (text/x-patch, attachment)]

Reply sent to Philip Kaludercic <philipk <at> posteo.net>:
You have taken responsibility. (Sat, 15 Oct 2022 15:25:04 GMT) Full text and rfc822 format available.

Notification sent to Philip Kaludercic <philipk <at> posteo.net>:
bug acknowledged by developer. (Sat, 15 Oct 2022 15:25:04 GMT) Full text and rfc822 format available.

Message #285 received at 57639-done <at> debbugs.gnu.org (full text, mbox):

From: Philip Kaludercic <philipk <at> posteo.net>
To: Mauro Aranda <maurooaranda <at> gmail.com>
Cc: Lars Ingebrigtsen <larsi <at> gnus.org>, Eli Zaretskii <eliz <at> gnu.org>,
 Protesilaos Stavrou <info <at> protesilaos.com>, 57639-done <at> debbugs.gnu.org
Subject: Re: bug#57639: [PATCH] Add new command 'toggle-theme'
Date: Sat, 15 Oct 2022 15:24:45 +0000
Philip Kaludercic <philipk <at> posteo.net> writes:

> Here is the updated patch:

As there have been no objections I've pushed the changes.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#57639; Package emacs. (Wed, 19 Oct 2022 02:44:02 GMT) Full text and rfc822 format available.

Message #288 received at 57639 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: Philip Kaludercic <philipk <at> posteo.net>, larsi <at> gnus.org,
 info <at> protesilaos.com, 57639 <at> debbugs.gnu.org
Subject: Re: bug#57639: [PATCH] Add new command 'toggle-theme'
Date: Tue, 18 Oct 2022 22:43:14 -0400
>> > I think you'd basically put something like the following into
>> > `loaddefs-generate--emacs-batch' --
>> >
>> > (let ((lisp-mode-autoload-regexp
>> >         "^;;;###\\(\\(noexist\\)-\\)?\\(theme-autoload\\)"))

Hmm... `lisp-mode-autoload-regexp` is defined as a `defconst`.  If we're
going to set it to some other value, we should change its definition to
a `defvar`, no?

Also, this code should come with a comment explaining why we're doing
this silly dance (it took me a while to go from that code to here).
[ I don't understand the "noexist" thingy, BTW.  Is that intended to be
  a regexp that will never match?  Should it use `regexp-unmatchable`
  then?  Or why not just "^;;;###\\(?3:theme-autoload\\)"?  ]

>> >   (loaddefs-generate
>> >     (expand-file-name "../etc/themes/" lisp-directory)
>> >     (expand-file-name "theme-loaddefs.el" lisp-directory)))

Note that if one of those ;;;###theme-autoload cookies is placed in
front of a function definition, it will still result in a broken:

    (autoload 'FUNCTIONNAME "etc/themes/FILENAME" ...)

which fails because "etc/themes/FILENAME" isn't found in `load-path`.

>> > 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?
>
> Also, this needs a NEWS entry.

I see this got the following entry:

    ** Themes have special autoload cookies.
    All build-in themes are scraped for ;;;###theme-autoload cookies that
    are loaded along with the regular auto-loaded code.

but I can't see any good reason why Emacs users should care about that.
It seems like a purely internal hack (and given the restrictions on
what can be autoloaded this way, we probably shouldn't advertise it too
loudly, unless accompanied with appropriate warnings).


        Stefan





Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#57639; Package emacs. (Wed, 19 Oct 2022 02:47:02 GMT) Full text and rfc822 format available.

Message #291 received at 57639 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Philip Kaludercic <philipk <at> posteo.net>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 57639 <at> debbugs.gnu.org,
 Stefan Kangas <stefan <at> marxist.se>, larsi <at> gnus.org, info <at> protesilaos.com
Subject: Re: bug#57639: [PATCH] Add new command 'toggle-theme'
Date: Tue, 18 Oct 2022 22:46:09 -0400
> The fundamental issue that necessitates boilerplate is that packages are
> not added to the theme load path, and adding a new cookie type doesn't
> change that.

We could arrange for `autoload/loaddefs-gen` to recognize `deftheme` and
add the directory containing the file containing this `deftheme` to the
`custom-theme-load-path`.


        Stefan





Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#57639; Package emacs. (Wed, 19 Oct 2022 07:17:02 GMT) Full text and rfc822 format available.

Message #294 received at 57639 <at> debbugs.gnu.org (full text, mbox):

From: Philip Kaludercic <philipk <at> posteo.net>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 57639 <at> debbugs.gnu.org,
 Stefan Kangas <stefan <at> marxist.se>, larsi <at> gnus.org, info <at> protesilaos.com
Subject: Re: bug#57639: [PATCH] Add new command 'toggle-theme'
Date: Wed, 19 Oct 2022 07:16:13 +0000
Stefan Monnier <monnier <at> iro.umontreal.ca> writes:

>> The fundamental issue that necessitates boilerplate is that packages are
>> not added to the theme load path, and adding a new cookie type doesn't
>> change that.
>
> We could arrange for `autoload/loaddefs-gen` to recognize `deftheme` and
> add the directory containing the file containing this `deftheme` to the
> `custom-theme-load-path`.

For packages installed using package.el or in general?  If this is
possible I think this would be a nice idea.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#57639; Package emacs. (Wed, 19 Oct 2022 07:21:02 GMT) Full text and rfc822 format available.

Message #297 received at 57639 <at> debbugs.gnu.org (full text, mbox):

From: Philip Kaludercic <philipk <at> posteo.net>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: Eli Zaretskii <eliz <at> gnu.org>, larsi <at> gnus.org, info <at> protesilaos.com,
 57639 <at> debbugs.gnu.org
Subject: Re: bug#57639: [PATCH] Add new command 'toggle-theme'
Date: Wed, 19 Oct 2022 07:20:52 +0000
Stefan Monnier <monnier <at> iro.umontreal.ca> writes:

>>> > I think you'd basically put something like the following into
>>> > `loaddefs-generate--emacs-batch' --
>>> >
>>> > (let ((lisp-mode-autoload-regexp
>>> >         "^;;;###\\(\\(noexist\\)-\\)?\\(theme-autoload\\)"))
>
> Hmm... `lisp-mode-autoload-regexp` is defined as a `defconst`.  If we're
> going to set it to some other value, we should change its definition to
> a `defvar`, no?
>
> Also, this code should come with a comment explaining why we're doing
> this silly dance (it took me a while to go from that code to here).
> [ I don't understand the "noexist" thingy, BTW.  Is that intended to be
>   a regexp that will never match?  Should it use `regexp-unmatchable`
>   then?  Or why not just "^;;;###\\(?3:theme-autoload\\)"?  ]
>
>>> >   (loaddefs-generate
>>> >     (expand-file-name "../etc/themes/" lisp-directory)
>>> >     (expand-file-name "theme-loaddefs.el" lisp-directory)))
>
> Note that if one of those ;;;###theme-autoload cookies is placed in
> front of a function definition, it will still result in a broken:
>
>     (autoload 'FUNCTIONNAME "etc/themes/FILENAME" ...)
>
> which fails because "etc/themes/FILENAME" isn't found in `load-path`.

Oh, that sounds bad!

>>> > 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?
>>
>> Also, this needs a NEWS entry.
>
> I see this got the following entry:
>
>     ** Themes have special autoload cookies.
>     All build-in themes are scraped for ;;;###theme-autoload cookies that
>     are loaded along with the regular auto-loaded code.
>
> but I can't see any good reason why Emacs users should care about that.
> It seems like a purely internal hack (and given the restrictions on
> what can be autoloaded this way, we probably shouldn't advertise it too
> loudly, unless accompanied with appropriate warnings).

Hmm, I believe I agree in retrospect that this doesn't really help anyone.

>
>         Stefan




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#57639; Package emacs. (Wed, 19 Oct 2022 11:01:02 GMT) Full text and rfc822 format available.

Message #300 received at 57639 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: philipk <at> posteo.net, larsi <at> gnus.org, info <at> protesilaos.com,
 57639 <at> debbugs.gnu.org
Subject: Re: bug#57639: [PATCH] Add new command 'toggle-theme'
Date: Wed, 19 Oct 2022 13:59:33 +0300
> From: Stefan Monnier <monnier <at> iro.umontreal.ca>
> Cc: Philip Kaludercic <philipk <at> posteo.net>,  larsi <at> gnus.org,
>   57639 <at> debbugs.gnu.org,  info <at> protesilaos.com
> Date: Tue, 18 Oct 2022 22:43:14 -0400
> 
>     ** Themes have special autoload cookies.
>     All build-in themes are scraped for ;;;###theme-autoload cookies that
>     are loaded along with the regular auto-loaded code.
> 
> but I can't see any good reason why Emacs users should care about that.
> It seems like a purely internal hack (and given the restrictions on
> what can be autoloaded this way, we probably shouldn't advertise it too
> loudly, unless accompanied with appropriate warnings).

NEWS is not just for users, it is also for developers, in particular
developers of themes.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#57639; Package emacs. (Wed, 19 Oct 2022 12:23:01 GMT) Full text and rfc822 format available.

Message #303 received at 57639 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Philip Kaludercic <philipk <at> posteo.net>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 57639 <at> debbugs.gnu.org,
 Stefan Kangas <stefan <at> marxist.se>, larsi <at> gnus.org, info <at> protesilaos.com
Subject: Re: bug#57639: [PATCH] Add new command 'toggle-theme'
Date: Wed, 19 Oct 2022 08:21:54 -0400
Philip Kaludercic [2022-10-19 07:16:13] wrote:
> Stefan Monnier <monnier <at> iro.umontreal.ca> writes:
>>> The fundamental issue that necessitates boilerplate is that packages are
>>> not added to the theme load path, and adding a new cookie type doesn't
>>> change that.
>> We could arrange for `autoload/loaddefs-gen` to recognize `deftheme` and
>> add the directory containing the file containing this `deftheme` to the
>> `custom-theme-load-path`.
> For packages installed using package.el or in general?

In general.  For those few built-in themes, it would mean we'd
repeatedly add the same dir to `custom-theme-load-path`, which is
inefficient, but this inefficiency should be completely harmless (it's
paid for during the preload).

> If this is possible I think this would be a nice idea.

It should be easy to do.


        Stefan





Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#57639; Package emacs. (Wed, 19 Oct 2022 12:26:01 GMT) Full text and rfc822 format available.

Message #306 received at 57639 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: philipk <at> posteo.net, larsi <at> gnus.org, info <at> protesilaos.com,
 57639 <at> debbugs.gnu.org
Subject: Re: bug#57639: [PATCH] Add new command 'toggle-theme'
Date: Wed, 19 Oct 2022 08:25:26 -0400
Eli Zaretskii [2022-10-19 13:59:33] wrote:
>> From: Stefan Monnier <monnier <at> iro.umontreal.ca>
>> Cc: Philip Kaludercic <philipk <at> posteo.net>,  larsi <at> gnus.org,
>>   57639 <at> debbugs.gnu.org,  info <at> protesilaos.com
>> Date: Tue, 18 Oct 2022 22:43:14 -0400
>> 
>>     ** Themes have special autoload cookies.
>>     All build-in themes are scraped for ;;;###theme-autoload cookies that
>>     are loaded along with the regular auto-loaded code.
>> 
>> but I can't see any good reason why Emacs users should care about that.
>> It seems like a purely internal hack (and given the restrictions on
>> what can be autoloaded this way, we probably shouldn't advertise it too
>> loudly, unless accompanied with appropriate warnings).
>
> NEWS is not just for users, it is also for developers, in particular
> developers of themes.

But the above is only potentially useful for those themes that are
built-in.  That's a mighty small audience, if you ask me, and IIUC that
audience was already part of the discussion that introduced this change,
so they won't learn it from etc/NEWS :-(


        Stefan





Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#57639; Package emacs. (Thu, 20 Oct 2022 16:41:01 GMT) Full text and rfc822 format available.

Message #309 received at 57639 <at> debbugs.gnu.org (full text, mbox):

From: Philip Kaludercic <philipk <at> posteo.net>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 57639 <at> debbugs.gnu.org,
 Stefan Kangas <stefan <at> marxist.se>, larsi <at> gnus.org, info <at> protesilaos.com
Subject: Re: bug#57639: [PATCH] Add new command 'toggle-theme'
Date: Thu, 20 Oct 2022 16:40:18 +0000
Stefan Monnier <monnier <at> iro.umontreal.ca> writes:

> Philip Kaludercic [2022-10-19 07:16:13] wrote:
>> Stefan Monnier <monnier <at> iro.umontreal.ca> writes:
>>>> The fundamental issue that necessitates boilerplate is that packages are
>>>> not added to the theme load path, and adding a new cookie type doesn't
>>>> change that.
>>> We could arrange for `autoload/loaddefs-gen` to recognize `deftheme` and
>>> add the directory containing the file containing this `deftheme` to the
>>> `custom-theme-load-path`.
>> For packages installed using package.el or in general?
>
> In general.  For those few built-in themes, it would mean we'd
> repeatedly add the same dir to `custom-theme-load-path`, which is
> inefficient, but this inefficiency should be completely harmless (it's
> paid for during the preload).
>
>> If this is possible I think this would be a nice idea.
>
> It should be easy to do.

Sadly I am stumped on how this could be done, as I have little
experience with the bootstrapping process.  Could you give me a few pointers?




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#57639; Package emacs. (Thu, 20 Oct 2022 17:05:01 GMT) Full text and rfc822 format available.

Message #312 received at 57639 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Philip Kaludercic <philipk <at> posteo.net>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 57639 <at> debbugs.gnu.org,
 Stefan Kangas <stefan <at> marxist.se>, larsi <at> gnus.org, info <at> protesilaos.com
Subject: Re: bug#57639: [PATCH] Add new command 'toggle-theme'
Date: Thu, 20 Oct 2022 13:04:28 -0400
>>>> We could arrange for `autoload/loaddefs-gen` to recognize `deftheme` and
>>>> add the directory containing the file containing this `deftheme` to the
>>>> `custom-theme-load-path`.
>>> If this is possible I think this would be a nice idea.
>> It should be easy to do.
> Sadly I am stumped on how this could be done, as I have little
> experience with the bootstrapping process.  Could you give me a few pointers?

I was thinking of something like the patch below.


        Stefan


diff --git a/lisp/emacs-lisp/loaddefs-gen.el b/lisp/emacs-lisp/loaddefs-gen.el
index ecc5f7e47bd..a07d0e9ad61 100644
--- a/lisp/emacs-lisp/loaddefs-gen.el
+++ b/lisp/emacs-lisp/loaddefs-gen.el
@@ -286,8 +286,16 @@ loaddefs-generate--make-autoload
      ;; Extract theme properties.
      ((eq car 'deftheme)
       (let* ((name (car-safe (cdr-safe form)))
-	     (props (nthcdr 3 form)))
-	`(put ',name 'theme-properties (list ,@props))))
+	     (props (nthcdr 3 form))
+	     (dir (file-name-directory file)))
+	`(progn
+	   (add-to-list 'custom-theme-load-path
+	                ,(if dir 
+	                     `(expand-file-name ,dir
+	                                        (file-name-directory
+	                                         (macroexp-file-name)))
+	                   `(file-name-directory (macroexp-file-name))))
+	   (put ',name 'theme-properties (list ,@props)))))
 
      ((eq car 'defgroup)
       ;; In Emacs this is normally handled separately by cus-dep.el, but for





Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#57639; Package emacs. (Thu, 20 Oct 2022 17:43:02 GMT) Full text and rfc822 format available.

Message #315 received at 57639 <at> debbugs.gnu.org (full text, mbox):

From: Philip Kaludercic <philipk <at> posteo.net>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 57639 <at> debbugs.gnu.org,
 Stefan Kangas <stefan <at> marxist.se>, larsi <at> gnus.org, info <at> protesilaos.com
Subject: Re: bug#57639: [PATCH] Add new command 'toggle-theme'
Date: Thu, 20 Oct 2022 17:42:52 +0000
Stefan Monnier <monnier <at> iro.umontreal.ca> writes:

>>>>> We could arrange for `autoload/loaddefs-gen` to recognize `deftheme` and
>>>>> add the directory containing the file containing this `deftheme` to the
>>>>> `custom-theme-load-path`.
>>>> If this is possible I think this would be a nice idea.
>>> It should be easy to do.
>> Sadly I am stumped on how this could be done, as I have little
>> experience with the bootstrapping process.  Could you give me a few pointers?
>
> I was thinking of something like the patch below.
>
>
>         Stefan
>
>
> diff --git a/lisp/emacs-lisp/loaddefs-gen.el b/lisp/emacs-lisp/loaddefs-gen.el
> index ecc5f7e47bd..a07d0e9ad61 100644
> --- a/lisp/emacs-lisp/loaddefs-gen.el
> +++ b/lisp/emacs-lisp/loaddefs-gen.el
> @@ -286,8 +286,16 @@ loaddefs-generate--make-autoload
>       ;; Extract theme properties.
>       ((eq car 'deftheme)
>        (let* ((name (car-safe (cdr-safe form)))
> -	     (props (nthcdr 3 form)))
> -	`(put ',name 'theme-properties (list ,@props))))
> +	     (props (nthcdr 3 form))
> +	     (dir (file-name-directory file)))
> +	`(progn
> +	   (add-to-list 'custom-theme-load-path
> +	                ,(if dir 
> +	                     `(expand-file-name ,dir
> +	                                        (file-name-directory
> +	                                         (macroexp-file-name)))
> +	                   `(file-name-directory (macroexp-file-name))))
> +	   (put ',name 'theme-properties (list ,@props)))))
>  
>       ((eq car 'defgroup)
>        ;; In Emacs this is normally handled separately by cus-dep.el, but for

Ah, I see.  Initially my assumption was that you were talking about the
automatic scraping of unannotated (deftheme ...) forms.  I'll try this
patch out, see how it works and report back.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#57639; Package emacs. (Thu, 20 Oct 2022 20:02:02 GMT) Full text and rfc822 format available.

Message #318 received at 57639 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Philip Kaludercic <philipk <at> posteo.net>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 57639 <at> debbugs.gnu.org,
 Stefan Kangas <stefan <at> marxist.se>, larsi <at> gnus.org, info <at> protesilaos.com
Subject: Re: bug#57639: [PATCH] Add new command 'toggle-theme'
Date: Thu, 20 Oct 2022 16:01:08 -0400
> Ah, I see.  Initially my assumption was that you were talking about the
> automatic scraping of unannotated (deftheme ...) forms.  I'll try this
> patch out, see how it works and report back.

Ah, I guess we could do that as well, but it's more work and it doesn't
seem super important.


        Stefan





bug archived. Request was from Debbugs Internal Request <help-debbugs <at> gnu.org> to internal_control <at> debbugs.gnu.org. (Fri, 18 Nov 2022 12:24:05 GMT) Full text and rfc822 format available.

This bug report was last modified 1 year and 159 days ago.

Previous Next


GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.