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

[Protesilaos Stavrou]

On 2021-03-01, 23:34 +0000, "Basil L. Contovounesios" <contovob@tcd.ie> wrote:

> Protesilaos Stavrou <info@protesilaos.com> writes:
>
>> +---
>
> I think this function deserves to be documented under
> (info "(elisp) Custom Themes"), but...
>
>> +** New function 'require-theme'.
>> +This function is used to load a theme or library stored in the
>> +'custom-theme-load-path'.  It is intended to work as a substitute for
>> +'require' in those cases where that cannot be used.
>
> ...after trying for some time, I failed to do so coherently and am now
> confused about the function's implementation and purpose (I'm sorry for
> not raising these points sooner).

No worries.  I will try to asnwer and we can always amend things.

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

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.

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

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

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

-- 
Protesilaos Stavrou
protesilaos.com