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

From: Mauro Aranda <[email-masked]>
Date: Sun, 28 Feb 2021 08:41:34 -0300

Hi Protesilaos,

Protesilaos Stavrou <[email-masked]> writes:

>     (defun require-theme (theme &optional directories)
>       "Load THEME stored in `custom-theme-load-path'.
>
>     THEME is a symbol or string that corresponds to the file name without
>     its file type extension.  That is assumed to be either '.el' or '.elc'.
>
>     If THEME names a valid theme, load and enable it.  Otherwise load the
>     file, if present.  In the latter case, the file is intended to work as
>     the basis of a theme declared with `deftheme'.
>
>     With optional DIRECTORIES as a list of filesystem paths, search
>     for THEME file in those locations instead and load it, if present."
>       (let* ((theme-dirs (custom-theme--load-path))
>              (custom-dirs (when (and directories (listp directories))
>                             directories))
>              (theme-name (cond
>                           ((stringp theme)
>                            theme)
>                           ((symbolp theme)
>                            (format "%s" theme))

I think it's better to use `symbol-name' when you're sure it's a symbol.

>         (cond
>          ((custom-theme-p theme)
>           (load-theme theme t))

Some comments about this clause:
The docstring says a valid theme, but this checks for a "known" theme.
I think the docstring should clarify that.

Also, in this clause you don't need all the early work for locating the
file.

More important: doesn't this make it possible to load a non-safe theme
(as controlled by `custom-safe-themes') behind the user's back?

> This works in two ways:
>
> 1. To load a theme's dependency:
>
>    (require-theme 'modus-themes)
>
> 2. To load and enable a theme:
>
>    (require-theme 'modus-operandi)

IIUC, (2) works only after the theme is "known".