Subject: Re: bug#45068: [PATCH] Modus themes 1.2.0

From: Protesilaos Stavrou <[email-masked]>
Date: Fri, 05 Mar 2021 19:50:26 +0200

On 2021-03-05, 17:11 +0000, "Basil L. Contovounesios" <[email-masked]> wrote:

>>>> ++ Ideas and user feedback :: Aaron Jensen, Adam Spiers, Adrian Manea,
>>>> +  Alex Griffin, Alex Peitsinis, Alexey Shmalko, Alok Singh, Anders
>>>> +  Johansson, André Alexandre Gomes, Arif Rezai, Basil L. Contovounesios,
>>>                                                           ^^
>>> Hopefully Org's Texinfo export can one day be taught that this is not
>>> the end of a sentence ;).
>>
>> Ah yes, I recall noticing that!  Can we circumvent it somehow?  Perhaps
>> by omitting the space?
>
> Or the 'L. ' wholesale, but using @: as you've done is also fine ;).

I preferred to keep the name the way you use it, so as to avoid
ambiguity (though I would not mind if your doppelgänger was also hacking
on Elisp!).

>>>> +(deftheme modus-vivendi
>>>> +  "Accessible and customizable light theme (WCAG AAA standard).
>>>                                   ^^^^^
>>>                                   dark
>>
>> Fixed!
>>
>> [ Answer only if it is easy: how do you draw those ^^^ below the text? ]
>
> If the answer is easy, or drawing the circumflexes is easy? ;)
>
> Either way, the answer is boring: I create a new line, add the
> appropriate indentation e.g. using indent-relative, and then add the
> circumflexes manually.  Maybe someone else knows a faster trick.

Ah okay, thanks!

>> On 2021-03-04, 13:53 -0300, Mauro Aranda <[email-masked]> wrote:
>>
>>>> ;;;###autoload
>>>> (when (and (boundp 'custom-theme-load-path) load-file-name)
>>>>   (add-to-list 'custom-theme-load-path
>>>>                (file-name-as-directory (file-name-directory load-file-name))))
>>>
>>> A nit: I think this code should avoid adding the value of
>>> custom-theme-directory or the built-in theme directory name to
>>> custom-theme-load-path , if `custom-theme-directory' (for the former) or
>>> t (for the latter) are already present in custom-theme-load-path.  In
>>> particular, a theme distributed with Emacs should at least check for t,
>>> to avoid a repeated entry.
>>>
>>> I've noticed that the leuven theme has a similar code as well: I think
>>> that is a (really minor) bug.
>>
>> I have removed that form altogether.  It makes sense for packages but
>> here they are safe themes.  Is that okay, or have I misunderstood
>> something?
>
> If you don't mind the (minor) maintenance overhead of removing the form
> in emacs.git, then that's fine.  Alternatively, a more DWIM approach
> could be something like this:
>
>   ;;;###autoload
>   (when load-file-name
>     (let ((dir (file-name-directory load-file-name)))
>       (unless (equal dir (expand-file-name "themes/" data-directory))
>         (add-to-list 'custom-theme-load-path dir))))
>
> I'm not sure if this works on non-GNU/Linux platforms, though.
> Does anyone else know or have a better suggestion?
>
> It could probably use file-in-directory-p instead, but that's quite a
> bit slower than file-name-directory + expand-file-name, at least in
> relative terms.
>
> Let me know what works best for you.

I would prefer to avoid the extra maintenance cost, not because I mind
doing the added work, but it can be a cause for errors.

To me your snippet looks right, though I am using GNU/Linux exclusively.
I just added it to my file (citing you as the author) and updated my
repo to version 1.2.3, which includes every change we have covered in
this thread.

> Following the discussion in:
>
>   https://gitlab.com/protesilaos/modus-themes/-/issues/162
>
> I'm preparing to update emacs.git to Modus version 1.2.2.  I just want
> to clarify a thing here and there first, so stay tuned.

Now version 1.2.3.  I guess you prefer the files over a patch?  I attach
them.

-- 
Protesilaos Stavrou
protesilaos.com