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

Previous Next

Full log

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.

Previous Next

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