bug#45068: [PATCH] 28.0.50; Update Modus themes 1.0.2 (backward-incompatible)

[Basil L. Contovounesios]

Protesilaos Stavrou <info@protesilaos.com> writes:

> On 2021-03-01, 23:34 +0000, "Basil L. Contovounesios" <contovob@tcd.ie> wrote:
>
>> If require-theme is intended as an alternative to require that searches
>> custom-theme-load-path, then why does it sometimes delegate to
>> load-theme and prompt the user about unsafe themes?  This mixes
>> lower-level library functionality with a higher-level user feature, and
>> the only deciding factor between the two is whether foo-theme.el or
>> foo.el exists on custom-theme-load-path, which seems a bit opaque.
>>
>> Wouldn't it be simpler/cleaner if load-theme was used only for the
>> foo-theme.el use case, and require-theme only for the foo.el use case?
>
> My idea was that there could be a future scenario where a derivative
> theme requires a basis and the two are both declared as 'deftheme'.  So
> they would blend together.

So you mean there is a base-theme.el and derivative-theme.el?  In that
case, couldn't derivative-theme.el just call (load-theme 'base) with or
without NO-CONFIRM and NO-ENABLE, depending on its needs?  Or did I
misunderstand what you mean?

> Otherwise yes, it would be simpler to keep things separate.
>
>> IOW, what is the use case for (require-theme 'modus-operandi)?  Why not
>> just call (load-theme 'modus-operandi) instead, with the added
>> flexibility of specifying its optional arguments?  I'm also curious
>> about the use case for require-theme's optional second argument.
>
> In my case (require-theme 'modus-operandi) would not be used.  In the
> file modus-operandi-theme.el I now have (require 'modus-themes).  The
> modus-themes.el is not a 'deftheme' in itself and 'require' cannot find
> it unless it is in the load-path.  Emacs' directory etc/themes is not in
> the load-path, so (require-theme 'modus-themes) is meant to work around
> that constraint.
>
>> Do the Modus themes require the current dual behaviour of require-theme?
>> If not, then could require-theme be as simple as the following, or am I
>> being naive / missing something?
>>
>>   (defun require-theme (feature &optional noerror)
>>     (let ((load-path (custom-theme--load-path)))
>>       (require feature nil noerror)))
>
> No the themes do not need that dual behaviour.  And yes, your version
> works and I am happy with it: all I need is a 'require' that reads from
> etc/themes.

Thanks, in that case, AFAICS, it might indeed be okay to keep load-theme
and require-theme separate (hopefully I'm not missing something
obvious).

> Perhaps I should attach a preliminary patch with the themes so that you
> can inspect it?

Sure, if you think it will help - my questions were just to help me
understand the "requirements" of require-theme ;).

>> BTW, do we need to warn anywhere that require-theme may unconditionally
>> load files from custom-theme-load-path, or somehow protect against this?
>
> That would be consistent with load-theme.

Right, but I'm wondering whether require-theme ought to be consistent in
this regard.

load-theme is a user-level command, and arbitrary themes are considered
risky Lisp, so it has to (conditionally) display the code and ask the
user if they think it looks okay.

require-theme, OTOH, sounds like it's a behind-the-scenes noninteractive
plumbing function to be used by themes themselves, so wouldn't the user
be prompted twice if a theme called require-theme on an element of
custom-available-themes?  IOW, it seems to me like require-theme's
"safety" should already be handled/covered by the theme using it.

>> And does it matter if require-theme loads .elc files, given that its
>> purpose is to load supporting non-theme Lisp files?
>
> For this I am not sure.  Whatever you think is appropriate.

load-theme already prefers the .elc file if the theme is considered safe
by default, so if my suggestions for require-theme are acceptable to
everyone then maybe it would make sense for require-theme to behave like
require in this sense as well.

I'll suggest a patch soon to help the discussion.

Thanks for bearing with me,

-- 
Basil