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

[Philip Kaludercic <philipk@posteo.net>]

Mauro Aranda <maurooaranda@gmail.com> writes:

> Philip Kaludercic <philipk@posteo.net> writes:
>
>> Mauro Aranda <maurooaranda@gmail.com> writes:
>>
>>> Hi Philip,
>>>
>>> Philip Kaludercic <philipk@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@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.