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

Previous Next

Package: emacs;

Reported by: Protesilaos Stavrou <info <at> protesilaos.com>

Date: Sun, 6 Dec 2020 12:25:02 UTC

Severity: normal

Tags: patch

Fixed in version 28.1

Done: "Basil L. Contovounesios" <contovob <at> tcd.ie>

Bug is archived. No further changes may be made.

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 45068 in the body.
You can then email your comments to 45068 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

View this report as an mbox folder, status mbox, maintainer mbox


Report forwarded to bug-gnu-emacs <at> gnu.org:
bug#45068; Package emacs. (Sun, 06 Dec 2020 12:25:02 GMT) Full text and rfc822 format available.

Acknowledgement sent to Protesilaos Stavrou <info <at> protesilaos.com>:
New bug report received and forwarded. Copy sent to bug-gnu-emacs <at> gnu.org. (Sun, 06 Dec 2020 12:25:02 GMT) Full text and rfc822 format available.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Protesilaos Stavrou <info <at> protesilaos.com>
To: bug-gnu-emacs <at> gnu.org
Subject: [PATCH] 28.0.50; Update Modus themes 1.0.2 (backward-incompatible)
Date: Sun, 06 Dec 2020 14:23:44 +0200
[Message part 1 (text/plain, inline)]
Dear maintainers,

I have made some major changes to the Modus themes and tagged this as
the first major release.  I am opening the present bug report because
this change has implications on how the themes are distributed with
Emacs.

Currently Emacs ships 'modus-operandi-theme' and 'modus-vivendi-theme'
version 0.13.0.  You can find the files in ../etc/themes/.

Those contained duplicate code and their only real difference was in the
color values they used.  However, they were distributed as if they were
standalone items, so all 'defcustom' declarations they had were unique
to each (duplicate code though different names).

Since version 1.0, I derive the two themes from a common source using a
macro and a shared file for data.  This has two main advantages:

+ All 'defcustom' are unified and apply to both themes.  Users no longer
  have to implement ad hoc code to get a unified look between the two
  items.

+ Everything is easier to maintain, as I no longer need to copy from one
  theme to the other, rename accordingly, and check that there are no
  discrepancies between them.

The problem with contributing this new code to Emacs is that it cannot
work if it is placed in ../etc/themes/.  Instead, the files need to be
in a path that handles byte-compilation, like ../lisp/modus-themes/.

1. This implies that commands such as 'M-x load-theme' cannot work
   without first requiring the appropriate files.  Such as:

       (require 'modus-themes)                   ;; common code
       (require 'modus-operandi-theme)           ;; light theme
       (load-theme 'modus-operandi t)
       ...
       (require 'modus-vivendi-theme)            ;; dark theme
       (load-theme 'modus-vivendi t)

2. Because we do not use ../etc/themes/, neither 'modus-operandi' nor
   'modus-vivendi' are marked as safe.  They are considered custom.
   Hence the non-nil NO-CONFIRM argument to 'load-theme'.

3. This also means that 'M-x customize-themes' and similar will not
   present the themes in their list without first requiring the
   aforementioned features.

* * *

Those are the issues I could identify.  As such, the attached patch
removes the old files and places the new ones in ../lisp/modus-themes/.
The themes' manual is updated to cover the new version.  The NEWS entry
is edited accordingly.

I understand you may object to the course of action covered by the
patch.  Please inform me of a better way to address this issue.  My
intent is to contribute version 1.0 of my themes to Emacs (1.0.2 to be
precise) and am prepared to make the requisite changes based on your
feedback.

Thank you for your time!

Protesilaos

-- 
Protesilaos Stavrou
protesilaos.com
[0001-Update-Modus-themes-1.0.2-backward-incompatible.patch (text/x-patch, attachment)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#45068; Package emacs. (Tue, 08 Dec 2020 12:04:01 GMT) Full text and rfc822 format available.

Message #8 received at 45068 <at> debbugs.gnu.org (full text, mbox):

From: "Basil L. Contovounesios" <contovob <at> tcd.ie>
To: Protesilaos Stavrou <info <at> protesilaos.com>
Cc: 45068 <at> debbugs.gnu.org
Subject: Re: bug#45068: [PATCH] 28.0.50; Update Modus themes 1.0.2
 (backward-incompatible)
Date: Tue, 08 Dec 2020 12:03:27 +0000
Protesilaos Stavrou <info <at> protesilaos.com> writes:

> I have made some major changes to the Modus themes and tagged this as
> the first major release.  I am opening the present bug report because
> this change has implications on how the themes are distributed with
> Emacs.

Thanks for your work on these themes!

[...]

> Those are the issues I could identify.  As such, the attached patch
> removes the old files and places the new ones in ../lisp/modus-themes/.
> The themes' manual is updated to cover the new version.  The NEWS entry
> is edited accordingly.
>
> I understand you may object to the course of action covered by the
> patch.  Please inform me of a better way to address this issue.  My
> intent is to contribute version 1.0 of my themes to Emacs (1.0.2 to be
> precise) and am prepared to make the requisite changes based on your
> feedback.

FWIW, I'd sooner make the infrastructure changes needed to make themes
under etc/themes/ more modular and package-like, than start moving them
out of etc/themes/.

Otherwise, I'd expect themes under lisp/ to behave exactly like those
under etc/themes/, which would make the etc/themes/ directory redundant.
(Maybe we need a lisp/themes/ directory?)

-- 
Basil




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#45068; Package emacs. (Wed, 09 Dec 2020 21:59:02 GMT) Full text and rfc822 format available.

Message #11 received at 45068 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Kangas <stefankangas <at> gmail.com>
To: "Basil L. Contovounesios" <contovob <at> tcd.ie>,
 Protesilaos Stavrou <info <at> protesilaos.com>
Cc: 45068 <at> debbugs.gnu.org
Subject: Re: bug#45068: [PATCH] 28.0.50;
 Update Modus themes 1.0.2 (backward-incompatible)
Date: Wed, 9 Dec 2020 15:58:40 -0600
"Basil L. Contovounesios" <contovob <at> tcd.ie> writes:

>> Those are the issues I could identify.  As such, the attached patch
>> removes the old files and places the new ones in ../lisp/modus-themes/.
>> The themes' manual is updated to cover the new version.  The NEWS entry
>> is edited accordingly.
>>
>> I understand you may object to the course of action covered by the
>> patch.  Please inform me of a better way to address this issue.  My
>> intent is to contribute version 1.0 of my themes to Emacs (1.0.2 to be
>> precise) and am prepared to make the requisite changes based on your
>> feedback.
>
> FWIW, I'd sooner make the infrastructure changes needed to make themes
> under etc/themes/ more modular and package-like, than start moving them
> out of etc/themes/.
>
> Otherwise, I'd expect themes under lisp/ to behave exactly like those
> under etc/themes/, which would make the etc/themes/ directory redundant.
> (Maybe we need a lisp/themes/ directory?)

Yes, I think if we start adding a number of these themes under
"lisp/<foo>-theme/" it risks getting messy over time.

Would it make sense to just add a "lisp/themes" directory, as Basil
suggests, containing the new files modus-themes-core.el and
modus-themes.el?

That way, I assume that we can just keep the themes in "etc/themes" for
now, while any theme that needs it can put its support libraries in
"lisp/themes".  Does that make sense?  Would it work without any changes
to load-theme?




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#45068; Package emacs. (Thu, 10 Dec 2020 07:58:01 GMT) Full text and rfc822 format available.

Message #14 received at 45068 <at> debbugs.gnu.org (full text, mbox):

From: Protesilaos Stavrou <info <at> protesilaos.com>
To: Stefan Kangas <stefankangas <at> gmail.com>
Cc: "Basil L. Contovounesios" <contovob <at> tcd.ie>, 45068 <at> debbugs.gnu.org
Subject: Re: bug#45068: [PATCH] 28.0.50; Update Modus themes 1.0.2
 (backward-incompatible)
Date: Thu, 10 Dec 2020 09:57:46 +0200
On 2020-12-09, 15:58 -0600, Stefan Kangas <stefankangas <at> gmail.com> wrote:

> "Basil L. Contovounesios" <contovob <at> tcd.ie> writes:
>
>>> Those are the issues I could identify.  As such, the attached patch
>>> removes the old files and places the new ones in ../lisp/modus-themes/.
>>> The themes' manual is updated to cover the new version.  The NEWS entry
>>> is edited accordingly.
>>>
>>> I understand you may object to the course of action covered by the
>>> patch.  Please inform me of a better way to address this issue.  My
>>> intent is to contribute version 1.0 of my themes to Emacs (1.0.2 to be
>>> precise) and am prepared to make the requisite changes based on your
>>> feedback.
>>
>> FWIW, I'd sooner make the infrastructure changes needed to make themes
>> under etc/themes/ more modular and package-like, than start moving them
>> out of etc/themes/.
>>
>> Otherwise, I'd expect themes under lisp/ to behave exactly like those
>> under etc/themes/, which would make the etc/themes/ directory redundant.
>> (Maybe we need a lisp/themes/ directory?)
>
> Yes, I think if we start adding a number of these themes under
> "lisp/<foo>-theme/" it risks getting messy over time.
>
> Would it make sense to just add a "lisp/themes" directory, as Basil
> suggests, containing the new files modus-themes-core.el and
> modus-themes.el?
>
> That way, I assume that we can just keep the themes in "etc/themes" for
> now, while any theme that needs it can put its support libraries in
> "lisp/themes".  Does that make sense?  Would it work without any changes
> to load-theme?

Thank you Stefan and Basil!

I agree that putting them all under lisp/ is not the right way to go
right now.  Today I will experiment with placing the libraries in
lisp/themes and the derivatives in etc/themes.  This will obfuscate the
source code though, but I will try nonetheless and report back to you
(without prejudice to any other ideas you or someone else may have).

-- 
Protesilaos Stavrou
protesilaos.com




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#45068; Package emacs. (Thu, 10 Dec 2020 08:26:01 GMT) Full text and rfc822 format available.

Message #17 received at 45068 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Kangas <stefankangas <at> gmail.com>
To: Protesilaos Stavrou <info <at> protesilaos.com>
Cc: "Basil L. Contovounesios" <contovob <at> tcd.ie>, 45068 <at> debbugs.gnu.org
Subject: Re: bug#45068: [PATCH] 28.0.50;
 Update Modus themes 1.0.2 (backward-incompatible)
Date: Thu, 10 Dec 2020 02:25:36 -0600
Protesilaos Stavrou <info <at> protesilaos.com> writes:

> I agree that putting them all under lisp/ is not the right way to go
> right now.  Today I will experiment with placing the libraries in
> lisp/themes and the derivatives in etc/themes.  This will obfuscate the
> source code though, but I will try nonetheless and report back to you
> (without prejudice to any other ideas you or someone else may have).

Thanks!  Looking forward to seeing the new version in Emacs.

Could you explain how it would obfuscate the source code?  I'm not sure
I understand that part.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#45068; Package emacs. (Thu, 10 Dec 2020 11:47:02 GMT) Full text and rfc822 format available.

Message #20 received at 45068 <at> debbugs.gnu.org (full text, mbox):

From: Protesilaos Stavrou <info <at> protesilaos.com>
To: Stefan Kangas <stefankangas <at> gmail.com>
Cc: "Basil L. Contovounesios" <contovob <at> tcd.ie>, 45068 <at> debbugs.gnu.org
Subject: Re: bug#45068: [PATCH] 28.0.50; Update Modus themes 1.0.2
 (backward-incompatible)
Date: Thu, 10 Dec 2020 13:46:36 +0200
[Message part 1 (text/plain, inline)]
On 2020-12-10, 02:25 -0600, Stefan Kangas <stefankangas <at> gmail.com> wrote:

> Protesilaos Stavrou <info <at> protesilaos.com> writes:
>
>> I agree that putting them all under lisp/ is not the right way to go
>> right now.  Today I will experiment with placing the libraries in
>> lisp/themes and the derivatives in etc/themes.  This will obfuscate the
>> source code though, but I will try nonetheless and report back to you
>> (without prejudice to any other ideas you or someone else may have).
>
> Thanks!  Looking forward to seeing the new version in Emacs.
>
> Could you explain how it would obfuscate the source code?  I'm not sure
> I understand that part.

I followed your advice in reviewing my patch.  Please find it attached.

In short:

+ The 'modus-operandi-theme' and 'modus-vivendi-theme' are in
  etc/themes.

  - Standard commands like 'load-theme' and 'enable-theme' work as
    intended.

  - Theme-level functions/commands, such as 'modus-themes-toggle' work
    as expected.

+ The other two files, 'modus-themes-core.el' and 'modus-themes.el' are
  placed under lisp/themes.

Note I tried to edit the texi file in light of bug#45141 and I think I
did everything right, but I cannot run that command locally---it is slow
and my hardware's temparature rises considerably.  Perhaps the texi file
needs to be edited further, in which case I am prepared to do whatever
is necessary (and apologies in advance for taking too much out of your
time).

* * *

On the "obfuscation" point I mentioned before: I expressed myself
poorly.  What I thought was that it would be difficult to study the
theme's code, but one upside (I think) of the aforementioned changes is
that M-x find-library now lists the files I placed in lisp/themes.

-- 
Protesilaos Stavrou
protesilaos.com
[0001-Update-Modus-themes-1.0.2-backward-incompatible.patch (text/x-patch, attachment)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#45068; Package emacs. (Fri, 11 Dec 2020 10:00:03 GMT) Full text and rfc822 format available.

Message #23 received at 45068 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: Protesilaos Stavrou <info <at> protesilaos.com>
Cc: "Basil L. Contovounesios" <contovob <at> tcd.ie>, 45068 <at> debbugs.gnu.org,
 Stefan Kangas <stefankangas <at> gmail.com>
Subject: Re: bug#45068: [PATCH] 28.0.50; Update Modus themes 1.0.2
 (backward-incompatible)
Date: Fri, 11 Dec 2020 11:37:48 +0200
> On the "obfuscation" point I mentioned before: I expressed myself
> poorly.  What I thought was that it would be difficult to study the
> theme's code, but one upside (I think) of the aforementioned changes is
> that M-x find-library now lists the files I placed in lisp/themes.

Indeed, it would be difficult to study the theme's code when it's
split between different directories.  Would it be possible to put
everything under lisp/themes?  Later another theme that would help
novices to enable relevant options, among other things could ask
about enabling modus-themes - such theme could be placed under lisp/themes
as well.

> I cannot run that command locally---it is slow
> and my hardware's temparature rises considerably.

Sometimes it helps just to clean the CPU cooler fan.
Such cleaning could save money instead of buying new hardware.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#45068; Package emacs. (Fri, 11 Dec 2020 13:23:02 GMT) Full text and rfc822 format available.

Message #26 received at 45068 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Kangas <stefankangas <at> gmail.com>
To: Juri Linkov <juri <at> linkov.net>, Protesilaos Stavrou <info <at> protesilaos.com>
Cc: "Basil L. Contovounesios" <contovob <at> tcd.ie>, 45068 <at> debbugs.gnu.org
Subject: Re: bug#45068: [PATCH] 28.0.50;
 Update Modus themes 1.0.2 (backward-incompatible)
Date: Fri, 11 Dec 2020 07:21:53 -0600
Juri Linkov <juri <at> linkov.net> writes:

>> On the "obfuscation" point I mentioned before: I expressed myself
>> poorly.  What I thought was that it would be difficult to study the
>> theme's code, but one upside (I think) of the aforementioned changes is
>> that M-x find-library now lists the files I placed in lisp/themes.
>
> Indeed, it would be difficult to study the theme's code when it's
> split between different directories.  Would it be possible to put
> everything under lisp/themes?  Later another theme that would help
> novices to enable relevant options, among other things could ask
> about enabling modus-themes - such theme could be placed under lisp/themes
> as well.

Taking a step back, why are any themes in "etc/themes" to begin with?
Is it just to avoid byte-compiling them?




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#45068; Package emacs. (Fri, 11 Dec 2020 13:53:01 GMT) Full text and rfc822 format available.

Message #29 received at 45068 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Stefan Kangas <stefankangas <at> gmail.com>
Cc: contovob <at> tcd.ie, info <at> protesilaos.com, 45068 <at> debbugs.gnu.org,
 juri <at> linkov.net
Subject: Re: bug#45068: [PATCH] 28.0.50;
 Update Modus themes 1.0.2 (backward-incompatible)
Date: Fri, 11 Dec 2020 15:52:20 +0200
> From: Stefan Kangas <stefankangas <at> gmail.com>
> Date: Fri, 11 Dec 2020 07:21:53 -0600
> Cc: "Basil L. Contovounesios" <contovob <at> tcd.ie>, 45068 <at> debbugs.gnu.org
> 
> Taking a step back, why are any themes in "etc/themes" to begin with?

This question should have been asked 10 years ago, when we added that
place.  Nowadays I think the question is what's so wrong with that
place that would justify moving the files.  If there are no serious
problems, I'd prefer to keep using that directory for themes.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#45068; Package emacs. (Fri, 11 Dec 2020 14:17:02 GMT) Full text and rfc822 format available.

Message #32 received at 45068 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Kangas <stefankangas <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: contovob <at> tcd.ie, info <at> protesilaos.com, 45068 <at> debbugs.gnu.org,
 juri <at> linkov.net
Subject: Re: bug#45068: [PATCH] 28.0.50;
 Update Modus themes 1.0.2 (backward-incompatible)
Date: Fri, 11 Dec 2020 08:16:47 -0600
Eli Zaretskii <eliz <at> gnu.org> writes:

>> Taking a step back, why are any themes in "etc/themes" to begin with?
>
> This question should have been asked 10 years ago, when we added that
> place.

I wasn't paying attention 10 years ago, I'm afraid.  :-)

> Nowadays I think the question is what's so wrong with that
> place that would justify moving the files.  If there are no serious
> problems, I'd prefer to keep using that directory for themes.

Right.  Well, I don't exactly have a strong opinion either way.  But I
do think that we should be consistent: it would be confusing to have
some themes in "etc/themes" and some in "lisp/themes".

If we can't move all themes to "lisp/themes" and we don't want to use
"lisp/themes" just for support files, to my mind the only remaining way
forward is to add some way to byte-compile (perhaps only some?) files in
"etc/themes".  And then make sure they can be loaded, are installed with
Emacs, etc.

Does that sound good, or could we do even better here?

Protesilaos, what do you think?  Does it sound workable for you?




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#45068; Package emacs. (Fri, 11 Dec 2020 14:26:01 GMT) Full text and rfc822 format available.

Message #35 received at 45068 <at> debbugs.gnu.org (full text, mbox):

From: Christopher Dimech <dimech <at> gmx.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: contovob <at> tcd.ie, info <at> protesilaos.com, juri <at> linkov.net,
 Stefan Kangas <stefankangas <at> gmail.com>, 45068 <at> debbugs.gnu.org
Subject: Re: bug#45068: [PATCH] 28.0.50; Update Modus themes 1.0.2
 (backward-incompatible)
Date: Fri, 11 Dec 2020 15:25:45 +0100

> Sent: Friday, December 11, 2020 at 2:52 PM
> From: "Eli Zaretskii" <eliz <at> gnu.org>
> To: "Stefan Kangas" <stefankangas <at> gmail.com>
> Cc: contovob <at> tcd.ie, info <at> protesilaos.com, 45068 <at> debbugs.gnu.org, juri <at> linkov.net
> Subject: bug#45068: [PATCH] 28.0.50; Update Modus themes 1.0.2 (backward-incompatible)
>
> > From: Stefan Kangas <stefankangas <at> gmail.com>
> > Date: Fri, 11 Dec 2020 07:21:53 -0600
> > Cc: "Basil L. Contovounesios" <contovob <at> tcd.ie>, 45068 <at> debbugs.gnu.org
> >
> > Taking a step back, why are any themes in "etc/themes" to begin with?
>
> This question should have been asked 10 years ago, when we added that
> place.  Nowadays I think the question is what's so wrong with that
> place that would justify moving the files.  If there are no serious
> problems, I'd prefer to keep using that directory for themes.

I agree with your evaluation Eli.

---------------------
Christopher Dimech
General Administrator - Naiad Informatics - GNU Project (Geocomputation)
- Geophysical Simulation
- Geological Subsurface Mapping
- Disaster Preparedness and Mitigation
- Natural Resource Exploration and Production
- Free Software Advocacy








Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#45068; Package emacs. (Fri, 11 Dec 2020 14:34:02 GMT) Full text and rfc822 format available.

Message #38 received at 45068 <at> debbugs.gnu.org (full text, mbox):

From: Christopher Dimech <dimech <at> gmx.com>
To: Stefan Kangas <stefankangas <at> gmail.com>, Eli Zaretskii <eliz <at> gnu.org>
Cc: contovob <at> tcd.ie, info <at> protesilaos.com, 45068 <at> debbugs.gnu.org,
 juri <at> linkov.net
Subject: Re: bug#45068: [PATCH] 28.0.50; Update Modus themes 1.0.2
 (backward-incompatible)
Date: Fri, 11 Dec 2020 15:32:55 +0100
> Sent: Friday, December 11, 2020 at 3:16 PM
> From: "Stefan Kangas" <stefankangas <at> gmail.com>
> To: "Eli Zaretskii" <eliz <at> gnu.org>
> Cc: contovob <at> tcd.ie, info <at> protesilaos.com, 45068 <at> debbugs.gnu.org, juri <at> linkov.net
> Subject: bug#45068: [PATCH] 28.0.50; Update Modus themes 1.0.2 (backward-incompatible)
>
> Eli Zaretskii <eliz <at> gnu.org> writes:
>
> >> Taking a step back, why are any themes in "etc/themes" to begin with?
> >
> > This question should have been asked 10 years ago, when we added that
> > place.
>
> I wasn't paying attention 10 years ago, I'm afraid.  :-)
>
> > Nowadays I think the question is what's so wrong with that
> > place that would justify moving the files.  If there are no serious
> > problems, I'd prefer to keep using that directory for themes.
>
> Right.  Well, I don't exactly have a strong opinion either way.  But I
> do think that we should be consistent: it would be confusing to have
> some themes in "etc/themes" and some in "lisp/themes".
>
> If we can't move all themes to "lisp/themes" and we don't want to use
> "lisp/themes" just for support files, to my mind the only remaining way
> forward is to add some way to byte-compile (perhaps only some?) files in
> "etc/themes".  And then make sure they can be loaded, are installed with
> Emacs, etc.
>
> Does that sound good, or could we do even better here?
>
> Protesilaos, what do you think?  Does it sound workable for you?

We can have a subset of the themes in "lisp/themes", the rest in "etc/themes".
I suggest that Protesilaos' Modus-Themes be used as default for emacs (stored
in "lisp/themes") because they are acessible themes for Gnu Emacs that much
thought was spent conforming with WCAG AAA Standard.

Regards
Christopher










Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#45068; Package emacs. (Fri, 11 Dec 2020 14:34:02 GMT) Full text and rfc822 format available.

Message #41 received at 45068 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Stefan Kangas <stefankangas <at> gmail.com>
Cc: contovob <at> tcd.ie, info <at> protesilaos.com, 45068 <at> debbugs.gnu.org,
 juri <at> linkov.net
Subject: Re: bug#45068: [PATCH] 28.0.50;
 Update Modus themes 1.0.2 (backward-incompatible)
Date: Fri, 11 Dec 2020 16:32:42 +0200
> From: Stefan Kangas <stefankangas <at> gmail.com>
> Date: Fri, 11 Dec 2020 08:16:47 -0600
> Cc: juri <at> linkov.net, info <at> protesilaos.com, contovob <at> tcd.ie, 
> 	45068 <at> debbugs.gnu.org
> 
> Right.  Well, I don't exactly have a strong opinion either way.  But I
> do think that we should be consistent: it would be confusing to have
> some themes in "etc/themes" and some in "lisp/themes".

Why not have all of them in etc/themes?

> If we can't move all themes to "lisp/themes" and we don't want to use
> "lisp/themes" just for support files, to my mind the only remaining way
> forward is to add some way to byte-compile (perhaps only some?) files in
> "etc/themes".

What theme-related files need to be byte-compiled, and why?




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#45068; Package emacs. (Fri, 11 Dec 2020 15:16:01 GMT) Full text and rfc822 format available.

Message #44 received at 45068 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Kangas <stefankangas <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: contovob <at> tcd.ie, info <at> protesilaos.com, 45068 <at> debbugs.gnu.org,
 juri <at> linkov.net
Subject: Re: bug#45068: [PATCH] 28.0.50;
 Update Modus themes 1.0.2 (backward-incompatible)
Date: Fri, 11 Dec 2020 09:15:32 -0600
Eli Zaretskii <eliz <at> gnu.org> writes:

>> If we can't move all themes to "lisp/themes" and we don't want to use
>> "lisp/themes" just for support files, to my mind the only remaining way
>> forward is to add some way to byte-compile (perhaps only some?) files in
>> "etc/themes".
>
> What theme-related files need to be byte-compiled, and why?

I believe the issues involved are explained in some detail in the first
message in this bug report.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#45068; Package emacs. (Fri, 11 Dec 2020 15:33:01 GMT) Full text and rfc822 format available.

Message #47 received at 45068 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Stefan Kangas <stefankangas <at> gmail.com>
Cc: contovob <at> tcd.ie, info <at> protesilaos.com, 45068 <at> debbugs.gnu.org,
 juri <at> linkov.net
Subject: Re: bug#45068: [PATCH] 28.0.50;
 Update Modus themes 1.0.2 (backward-incompatible)
Date: Fri, 11 Dec 2020 17:32:20 +0200
> From: Stefan Kangas <stefankangas <at> gmail.com>
> Date: Fri, 11 Dec 2020 09:15:32 -0600
> Cc: juri <at> linkov.net, info <at> protesilaos.com, contovob <at> tcd.ie, 
> 	45068 <at> debbugs.gnu.org
> 
> > What theme-related files need to be byte-compiled, and why?
> 
> I believe the issues involved are explained in some detail in the first
> message in this bug report.

If they are, I'm missing that.  All I see is an assertion that the
files must be byte-compiled:

> The problem with contributing this new code to Emacs is that it cannot
> work if it is placed in ../etc/themes/.  Instead, the files need to be
> in a path that handles byte-compilation, like ../lisp/modus-themes/.

I'm probably missing something, and that is what I'm asking to
elaborate on.

TIA




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#45068; Package emacs. (Fri, 11 Dec 2020 15:43:01 GMT) Full text and rfc822 format available.

Message #50 received at 45068 <at> debbugs.gnu.org (full text, mbox):

From: Christopher Dimech <dimech <at> gmx.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: contovob <at> tcd.ie, info <at> protesilaos.com, juri <at> linkov.net,
 Stefan Kangas <stefankangas <at> gmail.com>, 45068 <at> debbugs.gnu.org
Subject: Re: bug#45068: [PATCH] 28.0.50; Update Modus themes 1.0.2
 (backward-incompatible)
Date: Fri, 11 Dec 2020 16:42:10 +0100
> Sent: Friday, December 11, 2020 at 4:32 PM
> From: "Eli Zaretskii" <eliz <at> gnu.org>
> To: "Stefan Kangas" <stefankangas <at> gmail.com>
> Cc: contovob <at> tcd.ie, info <at> protesilaos.com, 45068 <at> debbugs.gnu.org, juri <at> linkov.net
> Subject: bug#45068: [PATCH] 28.0.50; Update Modus themes 1.0.2 (backward-incompatible)
>
> > From: Stefan Kangas <stefankangas <at> gmail.com>
> > Date: Fri, 11 Dec 2020 09:15:32 -0600
> > Cc: juri <at> linkov.net, info <at> protesilaos.com, contovob <at> tcd.ie,
> > 	45068 <at> debbugs.gnu.org
> >
> > > What theme-related files need to be byte-compiled, and why?
> >
> > I believe the issues involved are explained in some detail in the first
> > message in this bug report.
>
> If they are, I'm missing that.  All I see is an assertion that the
> files must be byte-compiled:
>
> > The problem with contributing this new code to Emacs is that it cannot
> > work if it is placed in ../etc/themes/.  Instead, the files need to be
> > in a path that handles byte-compilation, like ../lisp/modus-themes/.

Then a good idea to use "lisp/themes" for modus-themes :)

> I'm probably missing something, and that is what I'm asking to
> elaborate on.
>
> TIA
>
>
>
>




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#45068; Package emacs. (Fri, 11 Dec 2020 15:54:02 GMT) Full text and rfc822 format available.

Message #53 received at 45068 <at> debbugs.gnu.org (full text, mbox):

From: Protesilaos Stavrou <info <at> protesilaos.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: contovob <at> tcd.ie, 45068 <at> debbugs.gnu.org,
 Stefan Kangas <stefankangas <at> gmail.com>, juri <at> linkov.net
Subject: Re: bug#45068: [PATCH] 28.0.50; Update Modus themes 1.0.2
 (backward-incompatible)
Date: Fri, 11 Dec 2020 17:53:34 +0200
On 2020-12-11, 17:32 +0200, Eli Zaretskii <eliz <at> gnu.org> wrote:

>> From: Stefan Kangas <stefankangas <at> gmail.com>
>> Date: Fri, 11 Dec 2020 09:15:32 -0600
>> Cc: juri <at> linkov.net, info <at> protesilaos.com, contovob <at> tcd.ie, 
>> 	45068 <at> debbugs.gnu.org
>> 
>> > What theme-related files need to be byte-compiled, and why?
>> 
>> I believe the issues involved are explained in some detail in the first
>> message in this bug report.
>
> If they are, I'm missing that.  All I see is an assertion that the
> files must be byte-compiled:
>
>> The problem with contributing this new code to Emacs is that it cannot
>> work if it is placed in ../etc/themes/.  Instead, the files need to be
>> in a path that handles byte-compilation, like ../lisp/modus-themes/.
>
> I'm probably missing something, and that is what I'm asking to
> elaborate on.

There now are four *.el files that make up the project:

+ modus-themes.el contains all defcustom, color palettes as alists,
  helper functions, and the face specs.

+ modus-themes-core.el contains a macro that 'let' binds the
  aforementioned alists around 'custom-theme-set-faces' and
  'custom-theme-set-variables'.

+ modus-{operandi-vivendi}-theme.el contain a 'deftheme' and concomitant
  'provide-theme' referencing each of them and just expand the macro.

  - Each of those files has (require 'modus-themes) and
    (eval-when-compile (require 'modus-themes-core)).  Though I also
    tried without the eval-when-compile part.

When I attempted to place all four files in etc/themes/ and then start a
new 'emacs -Q' session, M-x load-theme RET modus-{operandi,vivendi}
would throw an error:

    Cannot open load file: No such file or directory, modus-themes

Whereas the latest patch I sent[1] does not produce any errors and fixes
the issues I had identified in my original message.

[1]: https://lists.gnu.org/archive/html/bug-gnu-emacs/2020-12/msg00937.html

-- 
Protesilaos Stavrou
protesilaos.com




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#45068; Package emacs. (Fri, 11 Dec 2020 16:06:02 GMT) Full text and rfc822 format available.

Message #56 received at 45068 <at> debbugs.gnu.org (full text, mbox):

From: Protesilaos Stavrou <info <at> protesilaos.com>
To: Stefan Kangas <stefankangas <at> gmail.com>
Cc: contovob <at> tcd.ie, Eli Zaretskii <eliz <at> gnu.org>, 45068 <at> debbugs.gnu.org,
 juri <at> linkov.net
Subject: Re: bug#45068: [PATCH] 28.0.50; Update Modus themes 1.0.2
 (backward-incompatible)
Date: Fri, 11 Dec 2020 18:05:01 +0200
On 2020-12-11, 08:16 -0600, Stefan Kangas <stefankangas <at> gmail.com> wrote:

> Eli Zaretskii <eliz <at> gnu.org> writes:
>
>>> Taking a step back, why are any themes in "etc/themes" to begin with?
>>
>> This question should have been asked 10 years ago, when we added that
>> place.
>
> I wasn't paying attention 10 years ago, I'm afraid.  :-)
>
>> Nowadays I think the question is what's so wrong with that
>> place that would justify moving the files.  If there are no serious
>> problems, I'd prefer to keep using that directory for themes.
>
> Right.  Well, I don't exactly have a strong opinion either way.  But I
> do think that we should be consistent: it would be confusing to have
> some themes in "etc/themes" and some in "lisp/themes".
>
> If we can't move all themes to "lisp/themes" and we don't want to use
> "lisp/themes" just for support files, to my mind the only remaining way
> forward is to add some way to byte-compile (perhaps only some?) files in
> "etc/themes".  And then make sure they can be loaded, are installed with
> Emacs, etc.
>
> Does that sound good, or could we do even better here?
>
> Protesilaos, what do you think?  Does it sound workable for you?

I am fine with whatever is considered best for Emacs and am willing to
make any changes you may consider necessary.

My intent here is to contribute the latest version which satisfies the
number one feature request I kept receiving: let the themes be
customised uniformly.

-- 
Protesilaos Stavrou
protesilaos.com




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#45068; Package emacs. (Fri, 11 Dec 2020 16:32:02 GMT) Full text and rfc822 format available.

Message #59 received at 45068 <at> debbugs.gnu.org (full text, mbox):

From: Christopher Dimech <dimech <at> gmx.com>
To: Stefan Kangas <stefankangas <at> gmail.com>
Cc: contovob <at> tcd.ie, 45068 <at> debbugs.gnu.org,
 Stefan Kangas <stefankangas <at> gmail.com>, juri <at> linkov.net
Subject: bug#45068: [PATCH] 28.0.50; Update Modus themes 1.0.2
 (backward-incompatible)
Date: Fri, 11 Dec 2020 17:31:11 +0100
> Sent: Friday, December 11, 2020 at 5:05 PM
> From: "Protesilaos Stavrou" <info <at> protesilaos.com>
> To: "Stefan Kangas" <stefankangas <at> gmail.com>
> Cc: contovob <at> tcd.ie, 45068 <at> debbugs.gnu.org, juri <at> linkov.net
> Subject: bug#45068: [PATCH] 28.0.50; Update Modus themes 1.0.2 (backward-incompatible)
>
> On 2020-12-11, 08:16 -0600, Stefan Kangas <stefankangas <at> gmail.com> wrote:
>
> > Eli Zaretskii <eliz <at> gnu.org> writes:
> >
> >>> Taking a step back, why are any themes in "etc/themes" to begin with?
> >>
> >> This question should have been asked 10 years ago, when we added that
> >> place.
> >
> > I wasn't paying attention 10 years ago, I'm afraid.  :-)
> >
> >> Nowadays I think the question is what's so wrong with that
> >> place that would justify moving the files.  If there are no serious
> >> problems, I'd prefer to keep using that directory for themes.
> >
> > Right.  Well, I don't exactly have a strong opinion either way.  But I
> > do think that we should be consistent: it would be confusing to have
> > some themes in "etc/themes" and some in "lisp/themes".
> >
> > If we can't move all themes to "lisp/themes" and we don't want to use
> > "lisp/themes" just for support files, to my mind the only remaining way
> > forward is to add some way to byte-compile (perhaps only some?) files in
> > "etc/themes".  And then make sure they can be loaded, are installed with
> > Emacs, etc.
> >
> > Does that sound good, or could we do even better here?

Perhaps you can make a new location for themes, put the required functionality,
then thrash the part that handled  "lisp/themes" and "etc/themes".  That would
then be a long term solution.

> > Protesilaos, what do you think?  Does it sound workable for you?
>
> I am fine with whatever is considered best for Emacs and am willing to
> make any changes you may consider necessary.
>
> My intent here is to contribute the latest version which satisfies the
> number one feature request I kept receiving: let the themes be
> customised uniformly.
>
> --
> Protesilaos Stavrou
> protesilaos.com
>
>
>
>




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#45068; Package emacs. (Fri, 11 Dec 2020 18:40:01 GMT) Full text and rfc822 format available.

Message #62 received at 45068 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Protesilaos Stavrou <info <at> protesilaos.com>
Cc: contovob <at> tcd.ie, 45068 <at> debbugs.gnu.org, stefankangas <at> gmail.com,
 juri <at> linkov.net
Subject: Re: bug#45068: [PATCH] 28.0.50; Update Modus themes 1.0.2
 (backward-incompatible)
Date: Fri, 11 Dec 2020 20:39:20 +0200
> From: Protesilaos Stavrou <info <at> protesilaos.com>
> Cc: Stefan Kangas <stefankangas <at> gmail.com>,  juri <at> linkov.net,
>   contovob <at> tcd.ie,  45068 <at> debbugs.gnu.org
> Date: Fri, 11 Dec 2020 17:53:34 +0200
> 
> >> > What theme-related files need to be byte-compiled, and why?
> >> 
> >> I believe the issues involved are explained in some detail in the first
> >> message in this bug report.
> >
> > If they are, I'm missing that.  All I see is an assertion that the
> > files must be byte-compiled:
> >
> >> The problem with contributing this new code to Emacs is that it cannot
> >> work if it is placed in ../etc/themes/.  Instead, the files need to be
> >> in a path that handles byte-compilation, like ../lisp/modus-themes/.
> >
> > I'm probably missing something, and that is what I'm asking to
> > elaborate on.
> 
> There now are four *.el files that make up the project:
> 
> + modus-themes.el contains all defcustom, color palettes as alists,
>   helper functions, and the face specs.
> 
> + modus-themes-core.el contains a macro that 'let' binds the
>   aforementioned alists around 'custom-theme-set-faces' and
>   'custom-theme-set-variables'.
> 
> + modus-{operandi-vivendi}-theme.el contain a 'deftheme' and concomitant
>   'provide-theme' referencing each of them and just expand the macro.
> 
>   - Each of those files has (require 'modus-themes) and
>     (eval-when-compile (require 'modus-themes-core)).  Though I also
>     tried without the eval-when-compile part.
> 
> When I attempted to place all four files in etc/themes/ and then start a
> new 'emacs -Q' session, M-x load-theme RET modus-{operandi,vivendi}
> would throw an error:
> 
>     Cannot open load file: No such file or directory, modus-themes

So this is not about byte-compiling, it's about being able to load
some of the files that constitute this group of themes?

If so, I see several possibilities:

  . make a single file with all the functionalities
  . invent a new function custom-require that would search the custom
    load-path, and use that instead of 'require'
  . don't consider this a "theme", but a normal Lisp package (since it
    basically violates the conventions for writing a theme)




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#45068; Package emacs. (Fri, 11 Dec 2020 18:57:01 GMT) Full text and rfc822 format available.

Message #65 received at 45068 <at> debbugs.gnu.org (full text, mbox):

From: Christopher Dimech <dimech <at> gmx.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: contovob <at> tcd.ie, Protesilaos Stavrou <info <at> protesilaos.com>,
 juri <at> linkov.net, 45068 <at> debbugs.gnu.org, stefankangas <at> gmail.com
Subject: Re: bug#45068: [PATCH] 28.0.50; Update Modus themes 1.0.2
 (backward-incompatible)
Date: Fri, 11 Dec 2020 19:56:43 +0100
> Sent: Friday, December 11, 2020 at 7:39 PM
> From: "Eli Zaretskii" <eliz <at> gnu.org>
> To: "Protesilaos Stavrou" <info <at> protesilaos.com>
> Cc: contovob <at> tcd.ie, 45068 <at> debbugs.gnu.org, stefankangas <at> gmail.com, juri <at> linkov.net
> Subject: bug#45068: [PATCH] 28.0.50; Update Modus themes 1.0.2 (backward-incompatible)
>
> > From: Protesilaos Stavrou <info <at> protesilaos.com>
> > Cc: Stefan Kangas <stefankangas <at> gmail.com>,  juri <at> linkov.net,
> >   contovob <at> tcd.ie,  45068 <at> debbugs.gnu.org
> > Date: Fri, 11 Dec 2020 17:53:34 +0200
> >
> > >> > What theme-related files need to be byte-compiled, and why?
> > >>
> > >> I believe the issues involved are explained in some detail in the first
> > >> message in this bug report.
> > >
> > > If they are, I'm missing that.  All I see is an assertion that the
> > > files must be byte-compiled:
> > >
> > >> The problem with contributing this new code to Emacs is that it cannot
> > >> work if it is placed in ../etc/themes/.  Instead, the files need to be
> > >> in a path that handles byte-compilation, like ../lisp/modus-themes/.
> > >
> > > I'm probably missing something, and that is what I'm asking to
> > > elaborate on.
> >
> > There now are four *.el files that make up the project:
> >
> > + modus-themes.el contains all defcustom, color palettes as alists,
> >   helper functions, and the face specs.
> >
> > + modus-themes-core.el contains a macro that 'let' binds the
> >   aforementioned alists around 'custom-theme-set-faces' and
> >   'custom-theme-set-variables'.
> >
> > + modus-{operandi-vivendi}-theme.el contain a 'deftheme' and concomitant
> >   'provide-theme' referencing each of them and just expand the macro.
> >
> >   - Each of those files has (require 'modus-themes) and
> >     (eval-when-compile (require 'modus-themes-core)).  Though I also
> >     tried without the eval-when-compile part.
> >
> > When I attempted to place all four files in etc/themes/ and then start a
> > new 'emacs -Q' session, M-x load-theme RET modus-{operandi,vivendi}
> > would throw an error:
> >
> >     Cannot open load file: No such file or directory, modus-themes
>
> So this is not about byte-compiling, it's about being able to load
> some of the files that constitute this group of themes?
>
> If so, I see several possibilities:
>
>   . make a single file with all the functionalities
>   . invent a new function custom-require that would search the custom
>     load-path, and use that instead of 'require'
>   . don't consider this a "theme", but a normal Lisp package (since it
>     basically violates the conventions for writing a theme)

It is still a theme though.  What conventions does it violate, briefly?
Not being a single file?





Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#45068; Package emacs. (Fri, 11 Dec 2020 19:09:02 GMT) Full text and rfc822 format available.

Message #68 received at 45068 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Kangas <stefankangas <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>, Protesilaos Stavrou <info <at> protesilaos.com>
Cc: contovob <at> tcd.ie, 45068 <at> debbugs.gnu.org, juri <at> linkov.net
Subject: Re: bug#45068: [PATCH] 28.0.50;
 Update Modus themes 1.0.2 (backward-incompatible)
Date: Fri, 11 Dec 2020 13:08:19 -0600
Eli Zaretskii <eliz <at> gnu.org> writes:

>   . don't consider this a "theme", but a normal Lisp package (since it
>     basically violates the conventions for writing a theme)

I guess such violations of our old conventions are getting increasingly
common in (third-party) Emacs theme development.  See for example:

    https://github.com/bbatsov/solarized-emacs
    https://github.com/hlissner/emacs-doom-themes

FWIW, I think this is a sign of the health and maturity of Emacs theme
development; code reuse is as useful in themes as in any other Lisp
code.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#45068; Package emacs. (Fri, 11 Dec 2020 20:06:02 GMT) Full text and rfc822 format available.

Message #71 received at 45068 <at> debbugs.gnu.org (full text, mbox):

From: Christopher Dimech <dimech <at> gmx.com>
To: Stefan Kangas <stefankangas <at> gmail.com>
Cc: contovob <at> tcd.ie, Eli Zaretskii <eliz <at> gnu.org>, juri <at> linkov.net,
 Protesilaos Stavrou <info <at> protesilaos.com>, 45068 <at> debbugs.gnu.org
Subject: Re: bug#45068: [PATCH] 28.0.50; Update Modus themes 1.0.2
 (backward-incompatible)
Date: Fri, 11 Dec 2020 21:05:10 +0100
> Sent: Friday, December 11, 2020 at 8:08 PM
> From: "Stefan Kangas" <stefankangas <at> gmail.com>
> To: "Eli Zaretskii" <eliz <at> gnu.org>, "Protesilaos Stavrou" <info <at> protesilaos.com>
> Cc: contovob <at> tcd.ie, 45068 <at> debbugs.gnu.org, juri <at> linkov.net
> Subject: bug#45068: [PATCH] 28.0.50; Update Modus themes 1.0.2 (backward-incompatible)
>
> Eli Zaretskii <eliz <at> gnu.org> writes:
>
> >   . don't consider this a "theme", but a normal Lisp package (since it
> >     basically violates the conventions for writing a theme)
>
> I guess such violations of our old conventions are getting increasingly
> common in (third-party) Emacs theme development.  See for example:
>
>     https://github.com/bbatsov/solarized-emacs
>     https://github.com/hlissner/emacs-doom-themes
>
> FWIW, I think this is a sign of the health and maturity of Emacs theme
> development; code reuse is as useful in themes as in any other Lisp
> code.

I agree with that.  Themes like those of Protesilaos have reached a very
high standard and have recommended modus-themes as a default for emacs.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#45068; Package emacs. (Fri, 11 Dec 2020 20:15:02 GMT) Full text and rfc822 format available.

Message #74 received at 45068 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Stefan Kangas <stefankangas <at> gmail.com>
Cc: contovob <at> tcd.ie, info <at> protesilaos.com, 45068 <at> debbugs.gnu.org,
 juri <at> linkov.net
Subject: Re: bug#45068: [PATCH] 28.0.50;
 Update Modus themes 1.0.2 (backward-incompatible)
Date: Fri, 11 Dec 2020 22:14:06 +0200
> From: Stefan Kangas <stefankangas <at> gmail.com>
> Date: Fri, 11 Dec 2020 13:08:19 -0600
> Cc: juri <at> linkov.net, contovob <at> tcd.ie, 45068 <at> debbugs.gnu.org
> 
> Eli Zaretskii <eliz <at> gnu.org> writes:
> 
> >   . don't consider this a "theme", but a normal Lisp package (since it
> >     basically violates the conventions for writing a theme)
> 
> I guess such violations of our old conventions are getting increasingly
> common in (third-party) Emacs theme development.

Then maybe the best way is to provide that require-theme function I
proposed.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#45068; Package emacs. (Fri, 11 Dec 2020 20:17:01 GMT) Full text and rfc822 format available.

Message #77 received at 45068 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Christopher Dimech <dimech <at> gmx.com>
Cc: contovob <at> tcd.ie, info <at> protesilaos.com, juri <at> linkov.net,
 45068 <at> debbugs.gnu.org, stefankangas <at> gmail.com
Subject: Re: bug#45068: [PATCH] 28.0.50; Update Modus themes 1.0.2
 (backward-incompatible)
Date: Fri, 11 Dec 2020 22:16:08 +0200
> From: Christopher Dimech <dimech <at> gmx.com>
> Cc: Protesilaos Stavrou <info <at> protesilaos.com>, contovob <at> tcd.ie,
>  45068 <at> debbugs.gnu.org, stefankangas <at> gmail.com, juri <at> linkov.net
> Date: Fri, 11 Dec 2020 19:56:43 +0100
> 
> It is still a theme though.  What conventions does it violate, briefly?
> Not being a single file?

Not being a theme, really.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#45068; Package emacs. (Fri, 11 Dec 2020 20:23:01 GMT) Full text and rfc822 format available.

Message #80 received at 45068 <at> debbugs.gnu.org (full text, mbox):

From: Christopher Dimech <dimech <at> gmx.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: contovob <at> tcd.ie, info <at> protesilaos.com, juri <at> linkov.net,
 Stefan Kangas <stefankangas <at> gmail.com>, 45068 <at> debbugs.gnu.org
Subject: Re: bug#45068: [PATCH] 28.0.50; Update Modus themes 1.0.2
 (backward-incompatible)
Date: Fri, 11 Dec 2020 21:21:58 +0100
> Sent: Friday, December 11, 2020 at 9:14 PM
> From: "Eli Zaretskii" <eliz <at> gnu.org>
> To: "Stefan Kangas" <stefankangas <at> gmail.com>
> Cc: contovob <at> tcd.ie, info <at> protesilaos.com, 45068 <at> debbugs.gnu.org, juri <at> linkov.net
> Subject: bug#45068: [PATCH] 28.0.50; Update Modus themes 1.0.2 (backward-incompatible)
>
> > From: Stefan Kangas <stefankangas <at> gmail.com>
> > Date: Fri, 11 Dec 2020 13:08:19 -0600
> > Cc: juri <at> linkov.net, contovob <at> tcd.ie, 45068 <at> debbugs.gnu.org
> >
> > Eli Zaretskii <eliz <at> gnu.org> writes:
> >
> > >   . don't consider this a "theme", but a normal Lisp package (since it
> > >     basically violates the conventions for writing a theme)
> >
> > I guess such violations of our old conventions are getting increasingly
> > common in (third-party) Emacs theme development.
>
> Then maybe the best way is to provide that require-theme function I
> proposed.

I would think so.





Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#45068; Package emacs. (Fri, 11 Dec 2020 20:31:02 GMT) Full text and rfc822 format available.

Message #83 received at 45068 <at> debbugs.gnu.org (full text, mbox):

From: Christopher Dimech <dimech <at> gmx.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: contovob <at> tcd.ie, info <at> protesilaos.com, juri <at> linkov.net,
 45068 <at> debbugs.gnu.org, stefankangas <at> gmail.com
Subject: Re: bug#45068: [PATCH] 28.0.50; Update Modus themes 1.0.2
 (backward-incompatible)
Date: Fri, 11 Dec 2020 21:29:51 +0100
> Sent: Friday, December 11, 2020 at 9:16 PM
> From: "Eli Zaretskii" <eliz <at> gnu.org>
> To: "Christopher Dimech" <dimech <at> gmx.com>
> Cc: info <at> protesilaos.com, contovob <at> tcd.ie, 45068 <at> debbugs.gnu.org, stefankangas <at> gmail.com, juri <at> linkov.net
> Subject: Re: bug#45068: [PATCH] 28.0.50; Update Modus themes 1.0.2 (backward-incompatible)
>
> > From: Christopher Dimech <dimech <at> gmx.com>
> > Cc: Protesilaos Stavrou <info <at> protesilaos.com>, contovob <at> tcd.ie,
> >  45068 <at> debbugs.gnu.org, stefankangas <at> gmail.com, juri <at> linkov.net
> > Date: Fri, 11 Dec 2020 19:56:43 +0100
> >
> > It is still a theme though.  What conventions does it violate, briefly?
> > Not being a single file?
>
> Not being a theme, really.

We should look at what the result is, not the type of implementation.  Still
I agree on some procedure for theme.  The current one seems a bit restrictive
though.  It is true also that the first version was easier to set up though.

I have been using modus-themes as my new standard.  I find utility in it,
because it passes very good metrics for setting a theme.






Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#45068; Package emacs. (Mon, 25 Jan 2021 08:50:02 GMT) Full text and rfc822 format available.

Message #86 received at 45068 <at> debbugs.gnu.org (full text, mbox):

From: Protesilaos Stavrou <info <at> protesilaos.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: contovob <at> tcd.ie, info <at> protesilaos.com, juri <at> linkov.net,
 Stefan Kangas <stefankangas <at> gmail.com>, 45068 <at> debbugs.gnu.org
Subject: Patch for Modus themes 1.1.1? (was: bug#45068: [PATCH] 28.0.50;
 Update Modus themes 1.0.2 (backward-incompatible))
Date: Mon, 25 Jan 2021 10:49:46 +0200
On 2020-12-11, 22:14 +0200, Eli Zaretskii <eliz <at> gnu.org> wrote:

>> From: Stefan Kangas <stefankangas <at> gmail.com>
>> Date: Fri, 11 Dec 2020 13:08:19 -0600
>> Cc: juri <at> linkov.net, contovob <at> tcd.ie, 45068 <at> debbugs.gnu.org
>> 
>> Eli Zaretskii <eliz <at> gnu.org> writes:
>> 
>> >   . don't consider this a "theme", but a normal Lisp package (since it
>> >     basically violates the conventions for writing a theme)
>> 
>> I guess such violations of our old conventions are getting increasingly
>> common in (third-party) Emacs theme development.
>
> Then maybe the best way is to provide that require-theme function I
> proposed.

Hello again!

I have produced a newer version in the meantime.  Though I understand
that a patch would not be able to be merged under the present
conditions.  Should I prepare it regardless?  This would also fix
bug#45141 pertaining to some formatting issues in modus-themes.texi.

To recapitulate for your convenience:

+ The current etc/themes/modus-{operandi,vivendi}-theme.el are old-style
  themes in that they look like the rest of that directory's contents.

  - They exist as standalone files.  They do not require any library.
  - All their face declarations are furnished therein.
  - Their version is 0.13.0.

+ As of version 1.0.0 (and now 1.1.1) of the Modus themes, those two
  files merely expand a macro and declare their respective theme.  They
  thus depend on a shared library: currently that is modus-themes.el.

  - The library unifies the themes' defcustom declarations and, in
    general, streamlines their development (before I would copy lines
    from one to the other to ensure parity).

  - This approach of a library and concomitant macro expansion is not in
    line with the current design of etc/themes/, though as Stefan Kangas
    observed, such deviations from the established norms are becoming
    increasingly common in third-party packages.

A quick-and-dirty workaround would be to place the library in some other
path, such as lisp/modus-themes.el and keep the other two files were
they currently are: etc/themes/modus-{operandi,vivendi}-theme.el

This could, however, make the source code more difficult to understand.
It would also set a bad precedent for any possible future themes, as
this thread revealed.  So Eli Zaretskii proposed a new 'require-theme'
function.

I believe this is how things stand.

For my part, I am willing to do whatever you consider necessary and will
follow your lead.

Thank you for your time and efforts!
Protesilaos or Prot

-- 
Protesilaos Stavrou
protesilaos.com




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#45068; Package emacs. (Mon, 25 Jan 2021 12:52:02 GMT) Full text and rfc822 format available.

Message #89 received at 45068 <at> debbugs.gnu.org (full text, mbox):

From: Christopher Dimech <dimech <at> gmx.com>
To: Protesilaos Stavrou <info <at> protesilaos.com>
Cc: info <at> protesilaos.com, juri <at> linkov.net, contovob <at> tcd.ie,
 Stefan Kangas <stefankangas <at> gmail.com>, Eli Zaretskii <eliz <at> gnu.org>,
 45068 <at> debbugs.gnu.org
Subject: Re: bug#45068: Patch for Modus themes 1.1.1? (was: bug#45068:
 [PATCH] 28.0.50; Update Modus themes 1.0.2 (backward-incompatible))
Date: Mon, 25 Jan 2021 13:51:35 +0100

> Sent: Monday, January 25, 2021 at 8:49 PM
> From: "Protesilaos Stavrou" <info <at> protesilaos.com>
> To: "Eli Zaretskii" <eliz <at> gnu.org>
> Cc: contovob <at> tcd.ie, info <at> protesilaos.com, 45068 <at> debbugs.gnu.org, "Stefan Kangas" <stefankangas <at> gmail.com>, juri <at> linkov.net
> Subject: bug#45068: Patch for Modus themes 1.1.1? (was: bug#45068: [PATCH] 28.0.50; Update Modus themes 1.0.2 (backward-incompatible))
>
> On 2020-12-11, 22:14 +0200, Eli Zaretskii <eliz <at> gnu.org> wrote:
>
> >> From: Stefan Kangas <stefankangas <at> gmail.com>
> >> Date: Fri, 11 Dec 2020 13:08:19 -0600
> >> Cc: juri <at> linkov.net, contovob <at> tcd.ie, 45068 <at> debbugs.gnu.org
> >>
> >> Eli Zaretskii <eliz <at> gnu.org> writes:
> >>
> >> >   . don't consider this a "theme", but a normal Lisp package (since it
> >> >     basically violates the conventions for writing a theme)
> >>
> >> I guess such violations of our old conventions are getting increasingly
> >> common in (third-party) Emacs theme development.
> >
> > Then maybe the best way is to provide that require-theme function I
> > proposed.
>
> Hello again!
>
> I have produced a newer version in the meantime.  Though I understand
> that a patch would not be able to be merged under the present
> conditions.  Should I prepare it regardless?  This would also fix
> bug#45141 pertaining to some formatting issues in modus-themes.texi.
>
> To recapitulate for your convenience:
>
> + The current etc/themes/modus-{operandi,vivendi}-theme.el are old-style
>   themes in that they look like the rest of that directory's contents.
>
>   - They exist as standalone files.  They do not require any library.
>   - All their face declarations are furnished therein.
>   - Their version is 0.13.0.
>
> + As of version 1.0.0 (and now 1.1.1) of the Modus themes, those two
>   files merely expand a macro and declare their respective theme.  They
>   thus depend on a shared library: currently that is modus-themes.el.
>
>   - The library unifies the themes' defcustom declarations and, in
>     general, streamlines their development (before I would copy lines
>     from one to the other to ensure parity).
>
>   - This approach of a library and concomitant macro expansion is not in
>     line with the current design of etc/themes/, though as Stefan Kangas
>     observed, such deviations from the established norms are becoming
>     increasingly common in third-party packages.

The focus should not be on implementation but on functionality.  As things
flourish, people will find various ways to do it, with some being better
because they yield to a different strategy. I suggest a more pragmatic approach.

> A quick-and-dirty workaround would be to place the library in some other
> path, such as lisp/modus-themes.el and keep the other two files were
> they currently are: etc/themes/modus-{operandi,vivendi}-theme.el
>
> This could, however, make the source code more difficult to understand.
> It would also set a bad precedent for any possible future themes, as
> this thread revealed.  So Eli Zaretskii proposed a new 'require-theme'
> function.
>
> I believe this is how things stand.
>
> For my part, I am willing to do whatever you consider necessary and will
> follow your lead.
>
> Thank you for your time and efforts!
> Protesilaos or Prot
>
> --
> Protesilaos Stavrou
> protesilaos.com
>
>
>
>




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#45068; Package emacs. (Mon, 25 Jan 2021 15:35:02 GMT) Full text and rfc822 format available.

Message #92 received at 45068 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Protesilaos Stavrou <info <at> protesilaos.com>
Cc: contovob <at> tcd.ie, info <at> protesilaos.com, juri <at> linkov.net,
 stefankangas <at> gmail.com, 45068 <at> debbugs.gnu.org
Subject: Re: Patch for Modus themes 1.1.1? (was: bug#45068: [PATCH] 28.0.50;
 Update Modus themes 1.0.2 (backward-incompatible))
Date: Mon, 25 Jan 2021 17:34:22 +0200
> From: Protesilaos Stavrou <info <at> protesilaos.com>
> Cc: Stefan Kangas <stefankangas <at> gmail.com>,  contovob <at> tcd.ie,
>   info <at> protesilaos.com,  45068 <at> debbugs.gnu.org,  juri <at> linkov.net
> Date: Mon, 25 Jan 2021 10:49:46 +0200
> 
> A quick-and-dirty workaround would be to place the library in some other
> path, such as lisp/modus-themes.el and keep the other two files were
> they currently are: etc/themes/modus-{operandi,vivendi}-theme.el
> 
> This could, however, make the source code more difficult to understand.
> It would also set a bad precedent for any possible future themes, as
> this thread revealed.  So Eli Zaretskii proposed a new 'require-theme'
> function.
> 
> I believe this is how things stand.
> 
> For my part, I am willing to do whatever you consider necessary and will
> follow your lead.

Thanks.  So how about adding such a require-theme function, and then
using it in your themes?




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#45068; Package emacs. (Thu, 25 Feb 2021 06:11:02 GMT) Full text and rfc822 format available.

Message #95 received at 45068 <at> debbugs.gnu.org (full text, mbox):

From: Protesilaos Stavrou <info <at> protesilaos.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: contovob <at> tcd.ie, 45068 <at> debbugs.gnu.org, stefankangas <at> gmail.com,
 juri <at> linkov.net
Subject: Re: Patch for Modus themes 1.1.1?
Date: Thu, 25 Feb 2021 08:09:53 +0200
On 2021-01-25, 17:34 +0200, Eli Zaretskii <eliz <at> gnu.org> wrote:

>> From: Protesilaos Stavrou <info <at> protesilaos.com>
>> Cc: Stefan Kangas <stefankangas <at> gmail.com>,  contovob <at> tcd.ie,
>>   info <at> protesilaos.com,  45068 <at> debbugs.gnu.org,  juri <at> linkov.net
>> Date: Mon, 25 Jan 2021 10:49:46 +0200
>> 
>> A quick-and-dirty workaround would be to place the library in some other
>> path, such as lisp/modus-themes.el and keep the other two files were
>> they currently are: etc/themes/modus-{operandi,vivendi}-theme.el
>> 
>> This could, however, make the source code more difficult to understand.
>> It would also set a bad precedent for any possible future themes, as
>> this thread revealed.  So Eli Zaretskii proposed a new 'require-theme'
>> function.
>> 
>> I believe this is how things stand.
>> 
>> For my part, I am willing to do whatever you consider necessary and will
>> follow your lead.
>
> Thanks.  So how about adding such a require-theme function, and then
> using it in your themes?

Hello Eli,

Can you please outline what are the requirements and general
functionality of such a 'require-theme' function?  I wish to give it a
try, though I can only do so in Elisp, whereas 'require' is in C...

If there is some source code or documentation I could consult, please
point me to it.

My intent is to address this issue and then share with you the
forthcoming 1.2.0 version of my themes.

Thanks again for your time and efforts!
Protesilaos

-- 
Protesilaos Stavrou
protesilaos.com




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#45068; Package emacs. (Thu, 25 Feb 2021 14:45:01 GMT) Full text and rfc822 format available.

Message #98 received at 45068 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Protesilaos Stavrou <info <at> protesilaos.com>
Cc: contovob <at> tcd.ie, 45068 <at> debbugs.gnu.org, stefankangas <at> gmail.com,
 juri <at> linkov.net
Subject: Re: Patch for Modus themes 1.1.1?
Date: Thu, 25 Feb 2021 16:44:20 +0200
> From: Protesilaos Stavrou <info <at> protesilaos.com>
> Cc: stefankangas <at> gmail.com,  contovob <at> tcd.ie,  45068 <at> debbugs.gnu.org,
>   juri <at> linkov.net
> Date: Thu, 25 Feb 2021 08:09:53 +0200
> 
> Can you please outline what are the requirements and general
> functionality of such a 'require-theme' function?  I wish to give it a
> try, though I can only do so in Elisp, whereas 'require' is in C...

A simple implementation that checks whether a theme is already loaded,
and if not, looks it up in a suitable list of directories and loads
when found.  The main part is to make sure themes are looked up in the
directories where we expect them to be, as opposed to load-path, which
is where 'require' looks for files to load.  A Lisp implementation
should be fine, I think.

Let me know if you need further clarifications.

Thanks.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#45068; Package emacs. (Sat, 27 Feb 2021 02:36:02 GMT) Full text and rfc822 format available.

Message #101 received at 45068 <at> debbugs.gnu.org (full text, mbox):

From: Protesilaos Stavrou <info <at> protesilaos.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 45068 <at> debbugs.gnu.org
Subject: Re: Patch for Modus themes 1.1.1?
Date: Sat, 27 Feb 2021 04:35:49 +0200
On 2021-02-25, 16:44 +0200, Eli Zaretskii <eliz <at> gnu.org> wrote:

>> From: Protesilaos Stavrou <info <at> protesilaos.com>
>> Cc: stefankangas <at> gmail.com,  contovob <at> tcd.ie,  45068 <at> debbugs.gnu.org,
>>   juri <at> linkov.net
>> Date: Thu, 25 Feb 2021 08:09:53 +0200
>> 
>> Can you please outline what are the requirements and general
>> functionality of such a 'require-theme' function?  I wish to give it a
>> try, though I can only do so in Elisp, whereas 'require' is in C...
>
> A simple implementation that checks whether a theme is already loaded,
> and if not, looks it up in a suitable list of directories and loads
> when found.  The main part is to make sure themes are looked up in the
> directories where we expect them to be, as opposed to load-path, which
> is where 'require' looks for files to load.  A Lisp implementation
> should be fine, I think.
>
> Let me know if you need further clarifications.

Hello Eli,

This is what I could come up with.  It is not what you stipulated, as it
only accounts for the default themes' directory instead of checking the
'custom-theme-load-path'.

    ;; To be added to custom.el
    (defun require-theme-base (base)
      "Load BASE theme file.
    BASE is a library that contains forms which are required by a
    theme declared with `deftheme'.  It is located in the same
    directory as the built-in themes."
      (let* ((themes-dir (expand-file-name "themes" data-directory))
             (themes (directory-files themes-dir nil "\\.el\\'"))
             files)
        (dolist (file themes)
          (unless (string-match-p "-theme\\.el\\'" file)
            (push file files)))
        (setq files (mapcar #'file-name-sans-extension files))
        (if (and (member (format "%s" base) files)
                 (not (custom-theme-p base)))
            (unless (featurep base)
              (load-file (expand-file-name (format "%s.el" base) themes-dir)))
          (error "`%s' is not a valid theme basis" base))))

I added that function to custom.el and placed the current version of my
files at etc/themes:

- modus-themes.el (the "base" file)
- modus-operandi-theme.el
- modus-vivendi-theme.el

The latter two are tweaked to use (require-theme-base 'modus-themes)
instead of (require 'modus-themes).

Compiled and tested on emacs -Q.

All theme loading seems to be working as expected.  Though I understand
that the function is not up to standard for a formal patch.

-- 
Protesilaos Stavrou
protesilaos.com




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#45068; Package emacs. (Sat, 27 Feb 2021 08:17:02 GMT) Full text and rfc822 format available.

Message #104 received at 45068 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Protesilaos Stavrou <info <at> protesilaos.com>
Cc: 45068 <at> debbugs.gnu.org
Subject: Re: Patch for Modus themes 1.1.1?
Date: Sat, 27 Feb 2021 10:15:54 +0200
> From: Protesilaos Stavrou <info <at> protesilaos.com>
> Cc: 45068 <at> debbugs.gnu.org
> Date: Sat, 27 Feb 2021 04:35:49 +0200
> 
> > A simple implementation that checks whether a theme is already loaded,
> > and if not, looks it up in a suitable list of directories and loads
> > when found.  The main part is to make sure themes are looked up in the
> > directories where we expect them to be, as opposed to load-path, which
> > is where 'require' looks for files to load.  A Lisp implementation
> > should be fine, I think.
> >
> > Let me know if you need further clarifications.
> 
> Hello Eli,
> 
> This is what I could come up with.  It is not what you stipulated, as it
> only accounts for the default themes' directory instead of checking the
> 'custom-theme-load-path'.

Hmm... I'm surprised.  What I had in mind was a simple use of
locate-file, which already accepts a path argument, so you could pass
custom-theme-load-path to it, and it would do the job.

Maybe I misunderstand or misremember the problem which led us here.
Wasn't the problem that 'load' and 'require' search along load-path
instead of custom-theme-load-path?  IOW, could you show the code you'd
use to load the other components of the theme if you could use 'load'
and 'require'?  My idea was simply to replace

  (require 'foo-themes)

with

  (require-theme 'foo-themes)

Would that solve your original problem, assuming that require-theme
would look for and load foo-themes.el?

Thanks.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#45068; Package emacs. (Sat, 27 Feb 2021 08:54:02 GMT) Full text and rfc822 format available.

Message #107 received at 45068 <at> debbugs.gnu.org (full text, mbox):

From: Protesilaos Stavrou <info <at> protesilaos.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 45068 <at> debbugs.gnu.org
Subject: Re: Patch for Modus themes 1.1.1?
Date: Sat, 27 Feb 2021 10:53:14 +0200
On 2021-02-27, 10:15 +0200, Eli Zaretskii <eliz <at> gnu.org> wrote:

>> From: Protesilaos Stavrou <info <at> protesilaos.com>
>> Cc: 45068 <at> debbugs.gnu.org
>> Date: Sat, 27 Feb 2021 04:35:49 +0200
>> 
>> > A simple implementation that checks whether a theme is already loaded,
>> > and if not, looks it up in a suitable list of directories and loads
>> > when found.  The main part is to make sure themes are looked up in the
>> > directories where we expect them to be, as opposed to load-path, which
>> > is where 'require' looks for files to load.  A Lisp implementation
>> > should be fine, I think.
>> >
>> > Let me know if you need further clarifications.
>> 
>> Hello Eli,
>> 
>> This is what I could come up with.  It is not what you stipulated, as it
>> only accounts for the default themes' directory instead of checking the
>> 'custom-theme-load-path'.
>
> Hmm... I'm surprised.  What I had in mind was a simple use of
> locate-file, which already accepts a path argument, so you could pass
> custom-theme-load-path to it, and it would do the job.

Thank you for the feedback (and sorry for putting you through the
trouble)!  I will try again using that approach.

> Maybe I misunderstand or misremember the problem which led us here.
> Wasn't the problem that 'load' and 'require' search along load-path
> instead of custom-theme-load-path?

Yes, that was the problem.  So the "modus-themes.el" dependency could
not be placed in etc/themes/ as (require 'modus-themes) would not find
it.

> IOW, could you show the code you'd use to load the other components of
> the theme if you could use 'load' and 'require'?  My idea was simply
> to replace
>
>   (require 'foo-themes)
>
> with
>
>   (require-theme 'foo-themes)
>
> Would that solve your original problem, assuming that require-theme
> would look for and load foo-themes.el?

Indeed, with the function I provided all I had to change was 'require'
to 'require-theme-base'.

-- 
Protesilaos Stavrou
protesilaos.com




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#45068; Package emacs. (Sun, 28 Feb 2021 06:31:01 GMT) Full text and rfc822 format available.

Message #110 received at 45068 <at> debbugs.gnu.org (full text, mbox):

From: Protesilaos Stavrou <info <at> protesilaos.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 45068 <at> debbugs.gnu.org
Subject: Re: Patch for Modus themes 1.1.1?
Date: Sun, 28 Feb 2021 08:30:16 +0200
On 2021-02-27, 10:15 +0200, Eli Zaretskii <eliz <at> gnu.org> wrote:

>> From: Protesilaos Stavrou <info <at> protesilaos.com>
>> Cc: 45068 <at> debbugs.gnu.org
>> Date: Sat, 27 Feb 2021 04:35:49 +0200
>> 
>> > A simple implementation that checks whether a theme is already loaded,
>> > and if not, looks it up in a suitable list of directories and loads
>> > when found.  The main part is to make sure themes are looked up in the
>> > directories where we expect them to be, as opposed to load-path, which
>> > is where 'require' looks for files to load.  A Lisp implementation
>> > should be fine, I think.
>> >
>> > Let me know if you need further clarifications.
>> 
>> Hello Eli,
>> 
>> This is what I could come up with.  It is not what you stipulated, as it
>> only accounts for the default themes' directory instead of checking the
>> 'custom-theme-load-path'.
>
> Hmm... I'm surprised.  What I had in mind was a simple use of
> locate-file, which already accepts a path argument, so you could pass
> custom-theme-load-path to it, and it would do the job.
>
> Maybe I misunderstand or misremember the problem which led us here.
> Wasn't the problem that 'load' and 'require' search along load-path
> instead of custom-theme-load-path?  IOW, could you show the code you'd
> use to load the other components of the theme if you could use 'load'
> and 'require'?  My idea was simply to replace
>
>   (require 'foo-themes)
>
> with
>
>   (require-theme 'foo-themes)
>
> Would that solve your original problem, assuming that require-theme
> would look for and load foo-themes.el?
>
> Thanks.

I retried and feel I am now closer to what you have described.  The
following is meant to go in custom.el:

    (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))
                          (t
                           (error "`%s' must be either a symbol or string" theme))))
             (dirs (or custom-dirs theme-dirs))
             (file (locate-file theme-name dirs '(".el" ".elc"))))
        (cond
         ((custom-theme-p theme)
          (load-theme theme t))
         (file
          (load-file file)))))

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)

I am using functionality 1 on a newly compiled Emacs with 'emacs -Q'.  I
placed my three files in etc/themes (modus-{operandi,vivendi}-theme.el
and modus-themes.el) and tried 'M-x load-theme RET modus-operandi' and
the same for modus-vivendi, as well as 'M-x customize-themes'.

-- 
Protesilaos Stavrou
protesilaos.com




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#45068; Package emacs. (Sun, 28 Feb 2021 11:42:02 GMT) Full text and rfc822 format available.

Message #113 received at 45068 <at> debbugs.gnu.org (full text, mbox):

From: Mauro Aranda <maurooaranda <at> gmail.com>
To: Protesilaos Stavrou <info <at> protesilaos.com>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 45068 <at> debbugs.gnu.org
Subject: Re: bug#45068: [PATCH] 28.0.50; Update Modus themes 1.0.2
 (backward-incompatible)
Date: Sun, 28 Feb 2021 08:41:34 -0300
Hi Protesilaos,

Protesilaos Stavrou <info <at> protesilaos.com> 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".




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#45068; Package emacs. (Sun, 28 Feb 2021 12:47:01 GMT) Full text and rfc822 format available.

Message #116 received at 45068 <at> debbugs.gnu.org (full text, mbox):

From: Protesilaos Stavrou <info <at> protesilaos.com>
To: Mauro Aranda <maurooaranda <at> gmail.com>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 45068 <at> debbugs.gnu.org
Subject: Re: bug#45068: [PATCH] 28.0.50; Update Modus themes 1.0.2
 (backward-incompatible)
Date: Sun, 28 Feb 2021 14:45:59 +0200
On 2021-02-28, 08:41 -0300, Mauro Aranda <maurooaranda <at> gmail.com> wrote:

> Hi Protesilaos,

Hello Mauro and thank you for the valuable feedback!

I have written the function anew based on your suggestions.  I also
added a featurep check to ensure that the same file is not loaded
repeatedly.  Furthermore, I streamlined the type of THEME to always be a
symbol.

    (defun require-theme (theme &optional directories)
      "Load THEME stored in `custom-theme-load-path'.

    THEME is a symbol that corresponds to the file name without its file
    type extension.  That is assumed to be either '.el' or '.elc'.

    If THEME names an element of `custom-available-themes', load it asking
    for confirmation if it is not considered safe by `custom-safe-themes'.
    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."
      (cond
       ((member theme (member theme (custom-available-themes)))
        (load-theme theme))
       ((let* ((theme-dirs (custom-theme--load-path))
               (custom-dirs (when (and directories (listp directories))
                              directories))
               (dirs (or custom-dirs theme-dirs))
               (file (locate-file (symbol-name theme) dirs '(".el" ".elc"))))
          (when (and file (not (featurep theme)))
            (load-file file))))))

Please feel welcome to suggest any further changes/refinements.  This is
not my area of expertise, so apologies for taking so long to get it
right.

-- 
Protesilaos Stavrou
protesilaos.com




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#45068; Package emacs. (Mon, 01 Mar 2021 14:39:02 GMT) Full text and rfc822 format available.

Message #119 received at 45068 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Mauro Aranda <maurooaranda <at> gmail.com>
Cc: info <at> protesilaos.com, 45068 <at> debbugs.gnu.org
Subject: Re: bug#45068: [PATCH] 28.0.50; Update Modus themes 1.0.2
 (backward-incompatible)
Date: Mon, 01 Mar 2021 16:38:22 +0200
> From: Mauro Aranda <maurooaranda <at> gmail.com>
> Cc: Eli Zaretskii <eliz <at> gnu.org>,  45068 <at> debbugs.gnu.org
> Date: Sun, 28 Feb 2021 10:33:14 -0300
> 
> The rest looks good to me, but let's wait for the other people involved
> to give their feedback.

It LGTM as well, thanks.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#45068; Package emacs. (Mon, 01 Mar 2021 14:42:01 GMT) Full text and rfc822 format available.

Message #122 received at 45068 <at> debbugs.gnu.org (full text, mbox):

From: Protesilaos Stavrou <info <at> protesilaos.com>
To: Mauro Aranda <maurooaranda <at> gmail.com>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 45068 <at> debbugs.gnu.org
Subject: Re: bug#45068: [PATCH] 28.0.50; Update Modus themes 1.0.2
 (backward-incompatible)
Date: Sun, 28 Feb 2021 15:56:33 +0200
[ resending as it was lost as Lars explained on emacs-devel: "Discarded
  messages to the bug tracker yesterday" ]

On 2021-02-28, 10:33 -0300, Mauro Aranda <maurooaranda <at> gmail.com> wrote:

> I don't understand this double member call.  Was it, by chance, just a
> typo?
>
> I'd use `memq' rather than `member' here, since,
> `custom-available-themes' returns a list of symbols.

Oops, sorry!  The double 'member' was indeed a mistake.  Though using it
was intentional.  Now switched to 'memq'.  Also added 'require' instead
of 'load-file'.

Here is the revised version, just to avoid confusion:

    (defun require-theme (theme &optional directories)
      "Load THEME stored in `custom-theme-load-path'.

    THEME is a symbol that corresponds to the file name without its file
    type extension.  That is assumed to be either '.el' or '.elc'.

    If THEME names an element of `custom-available-themes', load it asking
    for confirmation if it is not considered safe by `custom-safe-themes'.
    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."
      (cond
       ((memq theme (custom-available-themes))
        (load-theme theme))
       ((let* ((theme-dirs (custom-theme--load-path))
               (custom-dirs (when (and directories (listp directories))
                              directories))
               (dirs (or custom-dirs theme-dirs))
               (file (locate-file (symbol-name theme) dirs '(".el" ".elc"))))
          (when (and file (not (featurep theme)))
            (require theme file))))))

>> Please feel welcome to suggest any further changes/refinements.  This is
>> not my area of expertise, so apologies for taking so long to get it
>> right.
>
> No apologies needed.
>
> The rest looks good to me, but let's wait for the other people involved
> to give their feedback.

Yes sure and thanks again!

-- 
Protesilaos Stavrou
protesilaos.com




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#45068; Package emacs. (Mon, 01 Mar 2021 14:43:02 GMT) Full text and rfc822 format available.

Message #125 received at 45068 <at> debbugs.gnu.org (full text, mbox):

From: Protesilaos Stavrou <info <at> protesilaos.com>
To: "Basil L. Contovounesios" <contovob <at> tcd.ie>
Cc: 45068 <at> debbugs.gnu.org, Mauro Aranda <maurooaranda <at> gmail.com>
Subject: Re: bug#45068: [PATCH] 28.0.50; Update Modus themes 1.0.2
 (backward-incompatible)
Date: Sun, 28 Feb 2021 16:13:26 +0200
[ resending because it was lost, as Lars explained on emacs-devel:
  "Discarded messages to the bug tracker yesterday" ]

On 2021-02-28, 13:47 +0000, "Basil L. Contovounesios" <contovob <at> tcd.ie> wrote:

> Thanks, just some nits from me.
>
> Protesilaos Stavrou <info <at> protesilaos.com> writes:
>
>>     With optional DIRECTORIES as a list of filesystem paths, search
>>     for THEME file in those locations instead and load it, if
>>     present."
>
> GNU terminology distinguishes between "file names" and "paths", where
> the latter is a list of directories to search.
> See (info "(standards) GNU Manuals").
>
> So I suggest calling this argument "path" like that of locate-file.
>
>>       (cond
>>        ((member theme (member theme (custom-available-themes)))
>
> Duplicate member, and this can be memq instead, right?
>
>>         (load-theme theme))
>>        ((let* ((theme-dirs (custom-theme--load-path))
>>                (custom-dirs (when (and directories (listp directories))
>>                               directories))
>
> AKA (and (consp directories) directories), but see below.  I don't think
> we need to cater for callers passing atoms instead of lists.
>
>>                (dirs (or custom-dirs theme-dirs))
>
> I think you could just say (or directories (custom-theme--load-path)).
>
>>                (file (locate-file (symbol-name theme) dirs '(".el" ".elc"))))
>>           (when (and file (not (featurep theme)))
>
> Maybe featurep should be checked before calling locate-file?
>
>>             (load-file file))))))

Thank you Basil!

The duplicate 'member' was a mistake, while Mauro also suggested 'memq'.

If I understood your input correctly, the function should become:

    (defun require-theme (theme &optional paths)
      "Load THEME stored in `custom-theme-load-path'.

    THEME is a symbol that corresponds to the file name without its file
    type extension.  That is assumed to be either '.el' or '.elc'.

    If THEME names an element of `custom-available-themes', load it asking
    for confirmation if it is not considered safe by `custom-safe-themes'.
    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 PATHS as a list of filesystem paths, search for THEME
    file in those locations instead and load it, if present."
      (cond
       ((memq theme (custom-available-themes))
        (load-theme theme))
       ((let* ((dirs (or paths (custom-theme--load-path)))
               (file (unless (featurep theme)
                       (locate-file (symbol-name theme) dirs '(".el" ".elc")))))
          (when file
            (require theme file))))))

[ Note that the 'require' call was part of the parallel thread with my
  reply to Mauro's feedback.  Or keep 'load-file' for that? ]

-- 
Protesilaos Stavrou
protesilaos.com




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#45068; Package emacs. (Mon, 01 Mar 2021 14:44:01 GMT) Full text and rfc822 format available.

Message #128 received at 45068 <at> debbugs.gnu.org (full text, mbox):

From: Protesilaos Stavrou <info <at> protesilaos.com>
To: "Basil L. Contovounesios" <contovob <at> tcd.ie>
Cc: 45068 <at> debbugs.gnu.org, Mauro Aranda <maurooaranda <at> gmail.com>
Subject: Re: bug#45068: [PATCH] 28.0.50; Update Modus themes 1.0.2
 (backward-incompatible)
Date: Sun, 28 Feb 2021 17:57:16 +0200
[ resending because it was lost, as Lars explained on emacs-devel:
  "Discarded messages to the bug tracker yesterday" ]

On 2021-02-28, 15:19 +0000, "Basil L. Contovounesios" <contovob <at> tcd.ie> wrote:

> Protesilaos Stavrou <info <at> protesilaos.com> writes:
>
>>     With optional PATHS as a list of filesystem paths, search for THEME
>>     file in those locations instead and load it, if present."
>
> I think PATH is usually written in the singular, as in e.g.:
>
>   If PATH is non-nil, it should be a list of directories to use
>   instead of `custom-theme-load-path' when searching for THEME.
>   PATH should have the same form as `load-path' or `exec-path'.
>
>>        ((let* ((dirs (or paths (custom-theme--load-path)))
>>                (file (unless (featurep theme)
>>                        (locate-file (symbol-name theme) dirs '(".el" ".elc")))))
>>           (when file
>>             (require theme file))))))
>>
>> [ Note that the 'require' call was part of the parallel thread with my
>>   reply to Mauro's feedback.  Or keep 'load-file' for that? ]
>
> Shouldn't 'load' suffice, given we're already checking 'featurep'?
>
> Thanks,

Very well!  This is the updated version:

    (defun require-theme (theme &optional path)
      "Load THEME stored in `custom-theme-load-path'.

    THEME is a symbol that corresponds to the file name without its file
    type extension.  That is assumed to be either '.el' or '.elc'.

    When THEME is an element of `custom-available-themes', load it and ask
    for confirmation if it is not considered safe by `custom-safe-themes'.
    Otherwise load the file indicated by THEME, if present.  In the latter
    case, the file is intended to work as the basis of a theme declared
    with `deftheme'.

    If optional PATH is non-nil, it should be a list of directories
    to search for THEME in, instead of `custom-theme-load-path'.
    PATH should have the same form as `load-path' or `exec-path'."
      (cond
       ((memq theme (custom-available-themes))
        (load-theme theme))
       ((let* ((dirs (or path (custom-theme--load-path)))
               (file (unless (featurep theme)
                       (locate-file (symbol-name theme) dirs '(".el" ".elc")))))
          (when file
            (load-file file))))))

Code aside, if you think the doc string is too verbose or difficult to
read, I will try to simplify it.

-- 
Protesilaos Stavrou
protesilaos.com




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#45068; Package emacs. (Mon, 01 Mar 2021 14:53:02 GMT) Full text and rfc822 format available.

Message #131 received at 45068 <at> debbugs.gnu.org (full text, mbox):

From: Protesilaos Stavrou <info <at> protesilaos.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: "Basil L. Contovounesios" <contovob <at> tcd.ie>, 45068 <at> debbugs.gnu.org,
 Mauro Aranda <maurooaranda <at> gmail.com>
Subject: Re: bug#45068: [PATCH] 28.0.50; Update Modus themes 1.0.2
 (backward-incompatible)
Date: Mon, 01 Mar 2021 16:52:20 +0200
On 2021-03-01, 16:38 +0200, Eli Zaretskii <eliz <at> gnu.org> wrote:

>> From: Mauro Aranda <maurooaranda <at> gmail.com>
>> Cc: Eli Zaretskii <eliz <at> gnu.org>,  45068 <at> debbugs.gnu.org
>> Date: Sun, 28 Feb 2021 10:33:14 -0300
>> 
>> The rest looks good to me, but let's wait for the other people involved
>> to give their feedback.
>
> It LGTM as well, thanks.

Thank you!  Just to add that Basil (now in cc) shared some more
insights.  The current version is this (any mistakes are mine):

    (defun require-theme (theme &optional path)
      "Load THEME stored in `custom-theme-load-path'.

    THEME is a symbol that corresponds to the file name without its file
    type extension.  That is assumed to be either '.el' or '.elc'.

    When THEME is an element of `custom-available-themes', load it and ask
    for confirmation if it is not considered safe by `custom-safe-themes'.
    Otherwise load the file indicated by THEME, if present.  In the latter
    case, the file is intended to work as the basis of a theme declared
    with `deftheme'.

    If optional PATH is non-nil, it should be a list of directories
    to search for THEME in, instead of `custom-theme-load-path'.
    PATH should have the same form as `load-path' or `exec-path'."
      (cond
       ((memq theme (custom-available-themes))
        (load-theme theme))
       ((let* ((dirs (or path (custom-theme--load-path)))
               (file (unless (featurep theme)
                       (locate-file (symbol-name theme) dirs '(".el" ".elc")))))
          (when file
            (load-file file))))))

-- 
Protesilaos Stavrou
protesilaos.com




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#45068; Package emacs. (Mon, 01 Mar 2021 15:17:02 GMT) Full text and rfc822 format available.

Message #134 received at 45068 <at> debbugs.gnu.org (full text, mbox):

From: Mauro Aranda <maurooaranda <at> gmail.com>
To: Protesilaos Stavrou <info <at> protesilaos.com>
Cc: "Basil L. Contovounesios" <contovob <at> tcd.ie>, Eli Zaretskii <eliz <at> gnu.org>,
 45068 <at> debbugs.gnu.org
Subject: Re: bug#45068: [PATCH] 28.0.50; Update Modus themes 1.0.2
 (backward-incompatible)
Date: Mon, 01 Mar 2021 12:16:06 -0300
Protesilaos Stavrou <info <at> protesilaos.com> writes:

> On 2021-03-01, 16:38 +0200, Eli Zaretskii <eliz <at> gnu.org> wrote:
>
>>> From: Mauro Aranda <maurooaranda <at> gmail.com>
>>> Cc: Eli Zaretskii <eliz <at> gnu.org>,  45068 <at> debbugs.gnu.org
>>> Date: Sun, 28 Feb 2021 10:33:14 -0300
>>> 
>>> The rest looks good to me, but let's wait for the other people involved
>>> to give their feedback.
>>
>> It LGTM as well, thanks.
>
> Thank you!  Just to add that Basil (now in cc) shared some more
> insights.  The current version is this (any mistakes are mine):
>
>     (defun require-theme (theme &optional path)
>       "Load THEME stored in `custom-theme-load-path'.
>
>     THEME is a symbol that corresponds to the file name without its file
>     type extension.  That is assumed to be either '.el' or '.elc'.
>
>     When THEME is an element of `custom-available-themes', load it and ask
>     for confirmation if it is not considered safe by `custom-safe-themes'.
>     Otherwise load the file indicated by THEME, if present.  In the latter
>     case, the file is intended to work as the basis of a theme declared
>     with `deftheme'.
>
>     If optional PATH is non-nil, it should be a list of directories
>     to search for THEME in, instead of `custom-theme-load-path'.
>     PATH should have the same form as `load-path' or `exec-path'."
>       (cond
>        ((memq theme (custom-available-themes))
>         (load-theme theme))
>        ((let* ((dirs (or path (custom-theme--load-path)))
>                (file (unless (featurep theme)
>                        (locate-file (symbol-name theme) dirs '(".el" ".elc")))))
>           (when file
>             (load-file file))))))

Since there's agreement about this, could you send it as a patch?




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#45068; Package emacs. (Mon, 01 Mar 2021 15:37:01 GMT) Full text and rfc822 format available.

Message #137 received at 45068 <at> debbugs.gnu.org (full text, mbox):

From: Protesilaos Stavrou <info <at> protesilaos.com>
To: Mauro Aranda <maurooaranda <at> gmail.com>
Cc: "Basil L. Contovounesios" <contovob <at> tcd.ie>, Eli Zaretskii <eliz <at> gnu.org>,
 45068 <at> debbugs.gnu.org
Subject: Re: bug#45068: [PATCH] 28.0.50; Update Modus themes 1.0.2
 (backward-incompatible)
Date: Mon, 01 Mar 2021 17:35:54 +0200
[Message part 1 (text/plain, inline)]
On 2021-03-01, 12:16 -0300, Mauro Aranda <maurooaranda <at> gmail.com> wrote:

> Since there's agreement about this, could you send it as a patch?

Please find it attached.  I did not know how to mention you as
co-authors, so please do so.

As for the original topic of this bug report, I will prepare a new
version for the themes within the next few days (version 1.2.0).  Then I
will update this thread with a new patch.

Thank you for your help and patience!

-- 
Protesilaos Stavrou
protesilaos.com
[0001-Add-require-theme-function.patch (text/x-patch, attachment)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#45068; Package emacs. (Mon, 01 Mar 2021 19:59:03 GMT) Full text and rfc822 format available.

Message #140 received at 45068 <at> debbugs.gnu.org (full text, mbox):

From: "Basil L. Contovounesios" <contovob <at> tcd.ie>
To: Protesilaos Stavrou <info <at> protesilaos.com>
Cc: Eli Zaretskii <eliz <at> gnu.org>, Mauro Aranda <maurooaranda <at> gmail.com>,
 45068 <at> debbugs.gnu.org
Subject: Re: bug#45068: [PATCH] 28.0.50; Update Modus themes 1.0.2
 (backward-incompatible)
Date: Mon, 01 Mar 2021 19:58:37 +0000
Protesilaos Stavrou <info <at> protesilaos.com> writes:

> On 2021-03-01, 12:16 -0300, Mauro Aranda <maurooaranda <at> gmail.com> wrote:
>
>> Since there's agreement about this, could you send it as a patch?
>
> Please find it attached.

Thanks, pushed to master.

Add 'require-theme' function
59e1867a1f 2021-03-01 19:50:02 +0000
https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=59e1867a1f2b6938cdabac8e3f52acc9e61e9e32

> I did not know how to mention you as co-authors, so please do so.

I for one don't care for attribution nor count code review as
co-authoring; hopefully Mauro agrees in this case :).

-- 
Basil




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#45068; Package emacs. (Mon, 01 Mar 2021 20:04:02 GMT) Full text and rfc822 format available.

Message #143 received at 45068 <at> debbugs.gnu.org (full text, mbox):

From: Mauro Aranda <maurooaranda <at> gmail.com>
To: Protesilaos Stavrou <info <at> protesilaos.com>
Cc: "Basil L. Contovounesios" <contovob <at> tcd.ie>, Eli Zaretskii <eliz <at> gnu.org>,
 45068 <at> debbugs.gnu.org
Subject: Re: bug#45068: [PATCH] 28.0.50; Update Modus themes 1.0.2
 (backward-incompatible)
Date: Mon, 01 Mar 2021 17:03:46 -0300
Protesilaos Stavrou <info <at> protesilaos.com> writes:

> On 2021-03-01, 12:16 -0300, Mauro Aranda <maurooaranda <at> gmail.com> wrote:
>
>> Since there's agreement about this, could you send it as a patch?
>
> Please find it attached.  I did not know how to mention you as
> co-authors, so please do so.

Thanks.  I took the liberty of tweaking a little bit the commit message
(but didn't add co-authors -- I hope I didn't hurt anyone's feelings...)
and pushed it to the master branch.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#45068; Package emacs. (Mon, 01 Mar 2021 20:07:02 GMT) Full text and rfc822 format available.

Message #146 received at 45068 <at> debbugs.gnu.org (full text, mbox):

From: Mauro Aranda <maurooaranda <at> gmail.com>
To: "Basil L. Contovounesios" <contovob <at> tcd.ie>
Cc: Protesilaos Stavrou <info <at> protesilaos.com>, Eli Zaretskii <eliz <at> gnu.org>,
 45068 <at> debbugs.gnu.org
Subject: Re: bug#45068: [PATCH] 28.0.50; Update Modus themes 1.0.2
 (backward-incompatible)
Date: Mon, 01 Mar 2021 17:06:05 -0300
"Basil L. Contovounesios" <contovob <at> tcd.ie> writes:

> Protesilaos Stavrou <info <at> protesilaos.com> writes:
>
>> On 2021-03-01, 12:16 -0300, Mauro Aranda <maurooaranda <at> gmail.com> wrote:
>>
>>> Since there's agreement about this, could you send it as a patch?
>>
>> Please find it attached.
>
> Thanks, pushed to master.
>
> Add 'require-theme' function
> 59e1867a1f 2021-03-01 19:50:02 +0000
> https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=59e1867a1f2b6938cdabac8e3f52acc9e61e9e32
>
>> I did not know how to mention you as co-authors, so please do so.
>
> I for one don't care for attribution nor count code review as
> co-authoring; hopefully Mauro agrees in this case :).

Oops, looks like you beat me.  And we agree about the co-author thing ;)




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#45068; Package emacs. (Mon, 01 Mar 2021 23:35:01 GMT) Full text and rfc822 format available.

Message #149 received at 45068 <at> debbugs.gnu.org (full text, mbox):

From: "Basil L. Contovounesios" <contovob <at> tcd.ie>
To: Protesilaos Stavrou <info <at> protesilaos.com>
Cc: Eli Zaretskii <eliz <at> gnu.org>, Mauro Aranda <maurooaranda <at> gmail.com>,
 45068 <at> debbugs.gnu.org
Subject: Re: bug#45068: [PATCH] 28.0.50; Update Modus themes 1.0.2
 (backward-incompatible)
Date: Mon, 01 Mar 2021 23:34:46 +0000
Protesilaos Stavrou <info <at> 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).

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?

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.

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

BTW, do we need to warn anywhere that require-theme may unconditionally
load files from custom-theme-load-path, or somehow protect against this?
And does it matter if require-theme loads .elc files, given that its
purpose is to load supporting non-theme Lisp files?

Thanks,

-- 
Basil




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#45068; Package emacs. (Tue, 02 Mar 2021 05:49:01 GMT) Full text and rfc822 format available.

Message #152 received at 45068 <at> debbugs.gnu.org (full text, mbox):

From: Protesilaos Stavrou <info <at> protesilaos.com>
To: "Basil L. Contovounesios" <contovob <at> tcd.ie>
Cc: Eli Zaretskii <eliz <at> gnu.org>, Mauro Aranda <maurooaranda <at> gmail.com>,
 45068 <at> debbugs.gnu.org
Subject: Re: bug#45068: [PATCH] 28.0.50; Update Modus themes 1.0.2
 (backward-incompatible)
Date: Tue, 02 Mar 2021 07:47:58 +0200
On 2021-03-01, 23:34 +0000, "Basil L. Contovounesios" <contovob <at> tcd.ie> wrote:

> Protesilaos Stavrou <info <at> 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




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#45068; Package emacs. (Tue, 02 Mar 2021 05:52:01 GMT) Full text and rfc822 format available.

Message #155 received at 45068 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Protesilaos Stavrou <info <at> protesilaos.com>
Cc: contovob <at> tcd.ie, 45068 <at> debbugs.gnu.org, maurooaranda <at> gmail.com
Subject: Re: bug#45068: [PATCH] 28.0.50; Update Modus themes 1.0.2
 (backward-incompatible)
Date: Tue, 02 Mar 2021 07:51:31 +0200
> From: Protesilaos Stavrou <info <at> protesilaos.com>
> Cc: Mauro Aranda <maurooaranda <at> gmail.com>,  Eli Zaretskii <eliz <at> gnu.org>,
>   45068 <at> debbugs.gnu.org
> Date: Tue, 02 Mar 2021 07:47:58 +0200
> 
> >   (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.

Won't the above version cause problems if the loaded theme needs to
require some Emacs package?  Binding load-path would get in the way of
that, no?




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#45068; Package emacs. (Tue, 02 Mar 2021 10:34:01 GMT) Full text and rfc822 format available.

Message #158 received at 45068 <at> debbugs.gnu.org (full text, mbox):

From: "Basil L. Contovounesios" <contovob <at> tcd.ie>
To: Protesilaos Stavrou <info <at> protesilaos.com>
Cc: Eli Zaretskii <eliz <at> gnu.org>, Mauro Aranda <maurooaranda <at> gmail.com>,
 45068 <at> debbugs.gnu.org
Subject: Re: bug#45068: [PATCH] 28.0.50; Update Modus themes 1.0.2
 (backward-incompatible)
Date: Tue, 02 Mar 2021 10:32:51 +0000
Protesilaos Stavrou <info <at> protesilaos.com> writes:

> On 2021-03-01, 23:34 +0000, "Basil L. Contovounesios" <contovob <at> 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




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#45068; Package emacs. (Tue, 02 Mar 2021 10:36:02 GMT) Full text and rfc822 format available.

Message #161 received at 45068 <at> debbugs.gnu.org (full text, mbox):

From: "Basil L. Contovounesios" <contovob <at> tcd.ie>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: Protesilaos Stavrou <info <at> protesilaos.com>, maurooaranda <at> gmail.com,
 45068 <at> debbugs.gnu.org
Subject: Re: bug#45068: [PATCH] 28.0.50; Update Modus themes 1.0.2
 (backward-incompatible)
Date: Tue, 02 Mar 2021 10:35:02 +0000
Eli Zaretskii <eliz <at> gnu.org> writes:

>> From: Protesilaos Stavrou <info <at> protesilaos.com>
>> Cc: Mauro Aranda <maurooaranda <at> gmail.com>,  Eli Zaretskii <eliz <at> gnu.org>,
>>   45068 <at> debbugs.gnu.org
>> Date: Tue, 02 Mar 2021 07:47:58 +0200
>> 
>> >   (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.
>
> Won't the above version cause problems if the loaded theme needs to
> require some Emacs package?  Binding load-path would get in the way of
> that, no?

Probably, so locate-file is indeed the way to go.
(The above was mostly an illustration.)

Thanks,

-- 
Basil




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#45068; Package emacs. (Tue, 02 Mar 2021 11:00:02 GMT) Full text and rfc822 format available.

Message #164 received at 45068 <at> debbugs.gnu.org (full text, mbox):

From: Protesilaos Stavrou <info <at> protesilaos.com>
To: "Basil L. Contovounesios" <contovob <at> tcd.ie>
Cc: Eli Zaretskii <eliz <at> gnu.org>, Mauro Aranda <maurooaranda <at> gmail.com>,
 45068 <at> debbugs.gnu.org
Subject: Re: bug#45068: [PATCH] 28.0.50; Update Modus themes 1.0.2
 (backward-incompatible)
Date: Tue, 02 Mar 2021 12:59:19 +0200
[Message part 1 (text/plain, inline)]
On 2021-03-02, 10:32 +0000, "Basil L. Contovounesios" <contovob <at> tcd.ie> wrote:

> I'll suggest a patch soon to help the discussion.
>
> Thanks for bearing with me,

I also attach the files I intend to use.  Those should be placed in
etc/themes.  Note that this is not version 1.2.0 as I might still make
some minor tweaks before tagging a release either tomorrow or the day
after: just for you to make sense of the requirements.

Thank you!

P.S. If/when this issue is closed I will post on emacs-devel outlining
the minor yet important breaking changes from the themes' 0.13.0 to
version >= 1.0.0.

-- 
Protesilaos Stavrou
protesilaos.com
[modus-operandi-theme.el (text/plain, attachment)]
[modus-themes.el (text/plain, attachment)]
[modus-vivendi-theme.el (text/plain, attachment)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#45068; Package emacs. (Tue, 02 Mar 2021 11:05:01 GMT) Full text and rfc822 format available.

Message #167 received at 45068 <at> debbugs.gnu.org (full text, mbox):

From: Mauro Aranda <maurooaranda <at> gmail.com>
To: "Basil L. Contovounesios" <contovob <at> tcd.ie>
Cc: Protesilaos Stavrou <info <at> protesilaos.com>, Eli Zaretskii <eliz <at> gnu.org>,
 45068 <at> debbugs.gnu.org
Subject: Re: bug#45068: [PATCH] 28.0.50; Update Modus themes 1.0.2
 (backward-incompatible)
Date: Tue, 02 Mar 2021 08:03:49 -0300
"Basil L. Contovounesios" <contovob <at> tcd.ie> writes:

> Protesilaos Stavrou <info <at> protesilaos.com> writes:
>
>> On 2021-03-01, 23:34 +0000, "Basil L. Contovounesios" <contovob <at> tcd.ie> wrote:

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

I was the one that raised the question about loading a theme via
require-theme unconditionally (Protesilaos had a NO-CONFIRM non-nil in
one of the early versions of the patch), so if that bit of the patch is
wrong I'm the one to blame.

I'll just say that I raised the question because (usually) theme files
are just settings, so for a user to check the safety it is normally
enough to go through the custom-theme-set-* functions and see what the
theme is setting.  Now the user would be asked to check a require-theme
call for its safety, and since a call to require-theme looks a lot like
require, it might not be obvious that it can load (and enable) any theme
it wants.  And if a theme uses require-theme to do that, it can "hide"
the "unsafe theme" settings, because the first element of
custom-enabled-themes will just be the "safe" theme.

Those were my reasons, feel free to ignore them if you think they make
no sense.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#45068; Package emacs. (Tue, 02 Mar 2021 11:39:02 GMT) Full text and rfc822 format available.

Message #170 received at 45068 <at> debbugs.gnu.org (full text, mbox):

From: "Basil L. Contovounesios" <contovob <at> tcd.ie>
To: Mauro Aranda <maurooaranda <at> gmail.com>
Cc: Protesilaos Stavrou <info <at> protesilaos.com>, Eli Zaretskii <eliz <at> gnu.org>,
 45068 <at> debbugs.gnu.org
Subject: Re: bug#45068: [PATCH] 28.0.50; Update Modus themes 1.0.2
 (backward-incompatible)
Date: Tue, 02 Mar 2021 11:38:34 +0000
Mauro Aranda <maurooaranda <at> gmail.com> writes:

> I'll just say that I raised the question because (usually) theme files
> are just settings, so for a user to check the safety it is normally
> enough to go through the custom-theme-set-* functions and see what the
> theme is setting.  Now the user would be asked to check a require-theme
> call for its safety, and since a call to require-theme looks a lot like
> require, it might not be obvious that it can load (and enable) any theme
> it wants.

I'm suggesting that require-theme never enables any themes.  And to the
eyes of the user, it would be no different than a call to require or
load, which we already don't warn a second time about.

> And if a theme uses require-theme to do that, it can "hide"
> the "unsafe theme" settings, because the first element of
> custom-enabled-themes will just be the "safe" theme.

Sorry, I'm not sure what you mean, could you please elaborate on the
steps involved?

> Those were my reasons, feel free to ignore them if you think they make
> no sense.

I doubt that's the case :).

Thanks,

-- 
Basil




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#45068; Package emacs. (Tue, 02 Mar 2021 11:57:02 GMT) Full text and rfc822 format available.

Message #173 received at 45068 <at> debbugs.gnu.org (full text, mbox):

From: Mauro Aranda <maurooaranda <at> gmail.com>
To: "Basil L. Contovounesios" <contovob <at> tcd.ie>
Cc: info <at> protesilaos.com, eliz <at> gnu.org, 45068 <at> debbugs.gnu.org
Subject: Re: bug#45068: [PATCH] 28.0.50; Update Modus themes 1.0.2
 (backward-incompatible)
Date: Tue, 02 Mar 2021 08:56:22 -0300
"Basil L. Contovounesios" <contovob <at> tcd.ie> writes:

> Mauro Aranda <maurooaranda <at> gmail.com> writes:
>
>> I'll just say that I raised the question because (usually) theme files
>> are just settings, so for a user to check the safety it is normally
>> enough to go through the custom-theme-set-* functions and see what the
>> theme is setting.  Now the user would be asked to check a require-theme
>> call for its safety, and since a call to require-theme looks a lot like
>> require, it might not be obvious that it can load (and enable) any theme
>> it wants.
>
> I'm suggesting that require-theme never enables any themes.  And to the
> eyes of the user, it would be no different than a call to require or
> load, which we already don't warn a second time about.

Then there's no reason for me to worry at all.

>> And if a theme uses require-theme to do that, it can "hide"
>> the "unsafe theme" settings, because the first element of
>> custom-enabled-themes will just be the "safe" theme.
>
> Sorry, I'm not sure what you mean, could you please elaborate on the
> steps involved?

With the require-theme posted that included a (load-theme t) call, I
thought about the following steps:

The user enables super-safe-theme which hasn't been marked as safe.
The theme has a call to require-theme:
(require-theme 'should-be-safe-theme)
and the user thinks it's fine because should-be-safe-theme comes from
the same author as super-safe-theme.  The user doesn't check
should-be-safe-theme.el (shame on him), and is neither prompted because
of the NO-CONFIRM non-nil.

should-be-safe-theme.el has this call:
(require-theme 'not-so-safe-theme)
and the user is again not prompted for this theme.

After all the loading and enabling, custom-enabled-themes looks like
this:
(super-safe should-be-safe not-so-safe)
and it's not entirely obvious which setting came from which theme.

Of course all of this is already possible, but the theme has to put a
(load-theme t) call in front of the user eyes.

But anyway, if you're thinking about a require-theme that never enables
any theme, this is not relevant anymore.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#45068; Package emacs. (Wed, 03 Mar 2021 16:32:02 GMT) Full text and rfc822 format available.

Message #176 received at 45068 <at> debbugs.gnu.org (full text, mbox):

From: "Basil L. Contovounesios" <contovob <at> tcd.ie>
To: Protesilaos Stavrou <info <at> protesilaos.com>
Cc: Eli Zaretskii <eliz <at> gnu.org>, Mauro Aranda <maurooaranda <at> gmail.com>,
 45068 <at> debbugs.gnu.org
Subject: Re: bug#45068: [PATCH] 28.0.50; Update Modus themes 1.0.2
 (backward-incompatible)
Date: Wed, 03 Mar 2021 16:31:28 +0000
[Message part 1 (text/plain, inline)]
"Basil L. Contovounesios" <contovob <at> tcd.ie> writes:

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

How's the attached?

Thanks,

-- 
Basil

[0001-Decouple-require-theme-from-load-theme.patch (text/x-diff, attachment)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#45068; Package emacs. (Wed, 03 Mar 2021 18:07:01 GMT) Full text and rfc822 format available.

Message #179 received at 45068 <at> debbugs.gnu.org (full text, mbox):

From: Protesilaos Stavrou <info <at> protesilaos.com>
To: "Basil L. Contovounesios" <contovob <at> tcd.ie>
Cc: Eli Zaretskii <eliz <at> gnu.org>, Mauro Aranda <maurooaranda <at> gmail.com>,
 45068 <at> debbugs.gnu.org
Subject: Re: bug#45068: [PATCH] 28.0.50; Update Modus themes 1.0.2
 (backward-incompatible)
Date: Wed, 03 Mar 2021 20:06:42 +0200
On 2021-03-03, 16:31 +0000, "Basil L. Contovounesios" <contovob <at> tcd.ie> wrote:

> "Basil L. Contovounesios" <contovob <at> tcd.ie> writes:
>
>> I'll suggest a patch soon to help the discussion.
>
> How's the attached?
>
> Thanks,

I applied it and it works for what I want to do.  The decoupling feels
more appropriate.

The major difference I see between evaluating 'require-theme' and
'require' forms is that the latter errors with a backtrace buffer, while
the former logs a message.

Compare those:

    (require 'give-me-error)
    (require-theme 'give-me-error)

At any rate, tomorrow I will share with you the new version of my themes
and their manual.

Thank you!

-- 
Protesilaos Stavrou
protesilaos.com




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#45068; Package emacs. (Thu, 04 Mar 2021 02:05:02 GMT) Full text and rfc822 format available.

Message #182 received at 45068 <at> debbugs.gnu.org (full text, mbox):

From: "Basil L. Contovounesios" <contovob <at> tcd.ie>
To: Protesilaos Stavrou <info <at> protesilaos.com>
Cc: Eli Zaretskii <eliz <at> gnu.org>, Mauro Aranda <maurooaranda <at> gmail.com>,
 45068 <at> debbugs.gnu.org
Subject: Re: bug#45068: [PATCH] 28.0.50; Update Modus themes 1.0.2
 (backward-incompatible)
Date: Thu, 04 Mar 2021 02:04:41 +0000
Protesilaos Stavrou <info <at> protesilaos.com> writes:

> I applied it and it works for what I want to do.  The decoupling feels
> more appropriate.

Thanks for reviewing it.

> The major difference I see between evaluating 'require-theme' and
> 'require' forms is that the latter errors with a backtrace buffer, while
> the former logs a message.
>
> Compare those:
>
>     (require 'give-me-error)
>     (require-theme 'give-me-error)

Hm, how are you evaluating them?  Here's what I tried:

0. ./src/emacs -Q
1. (require 'give-me-error) C-j

  Debugger entered--Lisp error:
      (file-missing "Cannot open load file"
                    "No such file or directory"
                    "give-me-error")
    require(give-me-error)
    (progn (require 'give-me-error))
    eval((progn (require 'give-me-error)) t)
    elisp--eval-last-sexp(t)
    eval-last-sexp(t)
    eval-print-last-sexp(nil)
    funcall-interactively(eval-print-last-sexp nil)
    call-interactively(eval-print-last-sexp nil nil)
    command-execute(eval-print-last-sexp)

2. q
3. (require-theme 'give-me-error) C-j

  Debugger entered--Lisp error:
      (file-missing "Cannot open load file"
                    "No such file or directory"
                    "give-me-error")
    require(give-me-error)
    require-theme(give-me-error)
    (progn (require-theme 'give-me-error))
    eval((progn (require-theme 'give-me-error)) t)
    elisp--eval-last-sexp(t)
    eval-last-sexp(t)
    eval-print-last-sexp(nil)
    funcall-interactively(eval-print-last-sexp nil)
    call-interactively(eval-print-last-sexp nil nil)
    command-execute(eval-print-last-sexp)

The patch implements require-theme in terms of require,
so even the error symbol and data should be identical.

> At any rate, tomorrow I will share with you the new version of my themes
> and their manual.

I look forward to it.

Thanks,

-- 
Basil




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#45068; Package emacs. (Thu, 04 Mar 2021 04:54:01 GMT) Full text and rfc822 format available.

Message #185 received at 45068 <at> debbugs.gnu.org (full text, mbox):

From: Protesilaos Stavrou <info <at> protesilaos.com>
To: "Basil L. Contovounesios" <contovob <at> tcd.ie>
Cc: Eli Zaretskii <eliz <at> gnu.org>, Mauro Aranda <maurooaranda <at> gmail.com>,
 45068 <at> debbugs.gnu.org
Subject: Re: bug#45068: [PATCH] 28.0.50; Update Modus themes 1.0.2
 (backward-incompatible)
Date: Thu, 04 Mar 2021 06:53:06 +0200
On 2021-03-04, 02:04 +0000, "Basil L. Contovounesios" <contovob <at> tcd.ie> wrote:

> Protesilaos Stavrou <info <at> protesilaos.com> writes:
>
>> I applied it and it works for what I want to do.  The decoupling feels
>> more appropriate.
>
> Thanks for reviewing it.

You are welcome!

>> The major difference I see between evaluating 'require-theme' and
>> 'require' forms is that the latter errors with a backtrace buffer, while
>> the former logs a message.
>>
>> Compare those:
>>
>>     (require 'give-me-error)
>>     (require-theme 'give-me-error)
>
> Hm, how are you evaluating them?  Here's what I tried:
>
> [...]
>
> The patch implements require-theme in terms of require,
> so even the error symbol and data should be identical.

My bad!  I was trying the patch in emacs -Q and then went to my
unpatched Emacs' scratch buffer...

>> At any rate, tomorrow I will share with you the new version of my themes
>> and their manual.
>
> I look forward to it.

Will finalise the changelog entry and update this thread.  Probably
within 8 hours or so.

-- 
Protesilaos Stavrou
protesilaos.com




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#45068; Package emacs. (Thu, 04 Mar 2021 12:34:01 GMT) Full text and rfc822 format available.

Message #188 received at 45068 <at> debbugs.gnu.org (full text, mbox):

From: Mauro Aranda <maurooaranda <at> gmail.com>
To: "Basil L. Contovounesios" <contovob <at> tcd.ie>
Cc: Protesilaos Stavrou <info <at> protesilaos.com>, Eli Zaretskii <eliz <at> gnu.org>,
 45068 <at> debbugs.gnu.org
Subject: Re: bug#45068: [PATCH] 28.0.50; Update Modus themes 1.0.2
 (backward-incompatible)
Date: Thu, 04 Mar 2021 09:32:58 -0300
"Basil L. Contovounesios" <contovob <at> tcd.ie> writes:

> "Basil L. Contovounesios" <contovob <at> tcd.ie> writes:
>
>> I'll suggest a patch soon to help the discussion.
>
> How's the attached?
>
> Thanks,

LGTM, thank you.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#45068; Package emacs. (Thu, 04 Mar 2021 14:55:02 GMT) Full text and rfc822 format available.

Message #191 received at 45068 <at> debbugs.gnu.org (full text, mbox):

From: Protesilaos Stavrou <info <at> protesilaos.com>
To: Mauro Aranda <maurooaranda <at> gmail.com>
Cc: "Basil L. Contovounesios" <contovob <at> tcd.ie>, Eli Zaretskii <eliz <at> gnu.org>,
 45068 <at> debbugs.gnu.org
Subject: [PATCH] Modus themes 1.2.0 (was: bug#45068: [PATCH] 28.0.50; Update
 Modus themes 1.0.2 (backward-incompatible))
Date: Thu, 04 Mar 2021 16:54:15 +0200
[Message part 1 (text/plain, inline)]
On 2021-03-04, 09:32 -0300, Mauro Aranda <maurooaranda <at> gmail.com> wrote:

> "Basil L. Contovounesios" <contovob <at> tcd.ie> writes:
>
>> "Basil L. Contovounesios" <contovob <at> tcd.ie> writes:
>>
>>> I'll suggest a patch soon to help the discussion.
>>
>> How's the attached?
>>
>> Thanks,
>
> LGTM, thank you.

Hello again!

As noted earlier, please find attached the patch that upgrades the
themes to their newest version 1.2.0.  I tested it with Basil's latest
patch for 'require-theme', though it still works with the variant of
that function currently in trunk.

For your convenience, I also attach a note that I intend to post on
emacs-devel: it warns about the minor backward-incompatible refactoring
of some symbols and offers an overview of what is now available.

Thank you once more!
Prot

-- 
Protesilaos Stavrou
protesilaos.com
[0001-Update-Modus-themes-to-their-version-1.2.0.patch (text/x-patch, attachment)]
[20210304_162534--emacs--post-on-emacs-devel-about-update-to-the-modus-themes.txt (text/plain, attachment)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#45068; Package emacs. (Thu, 04 Mar 2021 15:49:02 GMT) Full text and rfc822 format available.

Message #194 received at 45068 <at> debbugs.gnu.org (full text, mbox):

From: "Basil L. Contovounesios" <contovob <at> tcd.ie>
To: Mauro Aranda <maurooaranda <at> gmail.com>
Cc: Protesilaos Stavrou <info <at> protesilaos.com>, Eli Zaretskii <eliz <at> gnu.org>,
 45068 <at> debbugs.gnu.org
Subject: Re: bug#45068: [PATCH] 28.0.50; Update Modus themes 1.0.2
 (backward-incompatible)
Date: Thu, 04 Mar 2021 15:47:55 +0000
Mauro Aranda <maurooaranda <at> gmail.com> writes:

> LGTM, thank you.

Thanks, pushed.

Decouple require-theme from load-theme
8e759d60cc 2021-03-04 15:40:35 +0000
https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=8e759d60cc234d4beb471dbb46f91d8ca3a20066

-- 
Basil




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#45068; Package emacs. (Thu, 04 Mar 2021 16:54:02 GMT) Full text and rfc822 format available.

Message #197 received at 45068 <at> debbugs.gnu.org (full text, mbox):

From: Mauro Aranda <maurooaranda <at> gmail.com>
To: Protesilaos Stavrou <info <at> protesilaos.com>
Cc: "Basil L. Contovounesios" <contovob <at> tcd.ie>, Eli Zaretskii <eliz <at> gnu.org>,
 45068 <at> debbugs.gnu.org
Subject: Re: bug#45068: [PATCH] 28.0.50; Update Modus themes 1.0.2
 (backward-incompatible)
Date: Thu, 04 Mar 2021 13:53:04 -0300
Hi Protesilaos,

Protesilaos Stavrou <info <at> protesilaos.com> writes:

> Hello again!
>
> As noted earlier, please find attached the patch that upgrades the
> themes to their newest version 1.2.0.  I tested it with Basil's latest
> patch for 'require-theme', though it still works with the variant of
> that function currently in trunk.

[...]

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






Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#45068; Package emacs. (Thu, 04 Mar 2021 18:42:01 GMT) Full text and rfc822 format available.

Message #200 received at 45068 <at> debbugs.gnu.org (full text, mbox):

From: "Basil L. Contovounesios" <contovob <at> tcd.ie>
To: Protesilaos Stavrou <info <at> protesilaos.com>
Cc: 45068 <at> debbugs.gnu.org, Mauro Aranda <maurooaranda <at> gmail.com>
Subject: Re: bug#45068: [PATCH] Modus themes 1.2.0
Date: Thu, 04 Mar 2021 18:41:19 +0000
Protesilaos Stavrou <info <at> protesilaos.com> writes:

> As noted earlier, please find attached the patch that upgrades the
> themes to their newest version 1.2.0.  I tested it with Basil's latest
> patch for 'require-theme', though it still works with the variant of
> that function currently in trunk.

Thanks, they're looking good!  Just a couple of nits from me below.

> For your convenience, I also attach a note that I intend to post on
> emacs-devel: it warns about the minor backward-incompatible refactoring
> of some symbols and offers an overview of what is now available.

> +Before you load a theme, it is necessary to require the main library:
>  
>  #+begin_src emacs-lisp
> +(require 'modus-themes)
>  #+end_src

Is this always true?  Because M-x load-theme RET
modus-{operandi,vivendi} RET works fine here with your patch, and users
can set variables even before they're defined - defvar and defcustom
only change the symbol's value if it's unbound.

Maybe this node should clarify which users/setups/use-cases this applies
to?

Similarly with other suggestions below like this one:

     (require 'modus-themes)
     (require 'modus-operandi-theme)
     (require 'modus-vivendi-theme)

which surely won't work with the built-in themes.

> ++ 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 ;).

> +(require 'cl-lib)

In theory this should be wrapped in eval-when-compile, but it doesn't
make a difference for now since built-in themes are not byte-compiled.

> +(deftheme modus-vivendi
> +  "Accessible and customizable light theme (WCAG AAA standard).
                                  ^^^^^
                                  dark

Thanks,

-- 
Basil




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#45068; Package emacs. (Thu, 04 Mar 2021 20:58:02 GMT) Full text and rfc822 format available.

Message #203 received at 45068 <at> debbugs.gnu.org (full text, mbox):

From: Protesilaos Stavrou <info <at> protesilaos.com>
To: "Basil L. Contovounesios" <contovob <at> tcd.ie>
Cc: 45068 <at> debbugs.gnu.org, Mauro Aranda <maurooaranda <at> gmail.com>
Subject: Re: bug#45068: [PATCH] Modus themes 1.2.0
Date: Thu, 04 Mar 2021 22:57:31 +0200
[Message part 1 (text/plain, inline)]
On 2021-03-04, 18:41 +0000, "Basil L. Contovounesios" <contovob <at> tcd.ie> wrote:

> Protesilaos Stavrou <info <at> protesilaos.com> writes:
>
>> As noted earlier, please find attached the patch that upgrades the
>> themes to their newest version 1.2.0.  I tested it with Basil's latest
>> patch for 'require-theme', though it still works with the variant of
>> that function currently in trunk.
>
> Thanks, they're looking good!  Just a couple of nits from me below.

Thank you Basil and Mauro!  The attached patch addresses your feedback.

>> +Before you load a theme, it is necessary to require the main library:
>>  
>>  #+begin_src emacs-lisp
>> +(require 'modus-themes)
>>  #+end_src
>
> Is this always true?  Because M-x load-theme RET
> modus-{operandi,vivendi} RET works fine here with your patch, and users
> can set variables even before they're defined - defvar and defcustom
> only change the symbol's value if it's unbound.
>
> Maybe this node should clarify which users/setups/use-cases this applies
> to?

I updated the language to disambiguate the use-cases.  If you think it
needs further work, I will rewrite it.

> Similarly with other suggestions below like this one:
>
>      (require 'modus-themes)
>      (require 'modus-operandi-theme)
>      (require 'modus-vivendi-theme)
>
> which surely won't work with the built-in themes.

Appended a comment to the first form noting that it is not needed for
the built-in themes.  Replaced the latter two forms with:

(load-theme 'modus-operandi t t)
(load-theme 'modus-vivendi t t)

>> ++ 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?

>> +(require 'cl-lib)
>
> In theory this should be wrapped in eval-when-compile, but it doesn't
> make a difference for now since built-in themes are not byte-compiled.

Wrapped it in eval-when-compile.

>> +(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? ]


On 2021-03-04, 13:53 -0300, Mauro Aranda <maurooaranda <at> gmail.com> 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?

Thanks again!

-- 
Protesilaos Stavrou
protesilaos.com
[0001-Update-Modus-themes-to-their-version-1.2.0.patch (text/x-patch, attachment)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#45068; Package emacs. (Thu, 04 Mar 2021 22:08:02 GMT) Full text and rfc822 format available.

Message #206 received at 45068 <at> debbugs.gnu.org (full text, mbox):

From: Mauro Aranda <maurooaranda <at> gmail.com>
To: Protesilaos Stavrou <info <at> protesilaos.com>
Cc: "Basil L. Contovounesios" <contovob <at> tcd.ie>, 45068 <at> debbugs.gnu.org
Subject: Re: bug#45068: [PATCH] 28.0.50; Update Modus themes 1.0.2
 (backward-incompatible)
Date: Thu, 04 Mar 2021 19:06:57 -0300
Protesilaos Stavrou <info <at> protesilaos.com> writes:

> On 2021-03-04, 13:53 -0300, Mauro Aranda <maurooaranda <at> gmail.com> 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?

Sounds OK to me; for themes that are only distributed with Emacs, it
doesn't seem to be needed.  But if you plan to keep distributing them as
packages via ELPA, then it might make sense to keep it.  I don't know
what's the plan, so I can't say for sure if the form should stay or not.

> Thanks again!

Thanks to you!




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#45068; Package emacs. (Thu, 04 Mar 2021 22:41:01 GMT) Full text and rfc822 format available.

Message #209 received at 45068 <at> debbugs.gnu.org (full text, mbox):

From: Mauro Aranda <maurooaranda <at> gmail.com>
To: Protesilaos Stavrou <info <at> protesilaos.com>
Cc: "Basil L. Contovounesios" <contovob <at> tcd.ie>, 45068 <at> debbugs.gnu.org
Subject: Re: bug#45068: [PATCH] 28.0.50; Update Modus themes 1.0.2
 (backward-incompatible)
Date: Thu, 04 Mar 2021 19:40:24 -0300
Protesilaos Stavrou <info <at> protesilaos.com> writes:

> On 2021-03-04, 18:41 +0000, "Basil L. Contovounesios" <contovob <at> tcd.ie> wrote:
>
>> Protesilaos Stavrou <info <at> protesilaos.com> writes:

>>> ++ 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?

Perhaps hide it from ox-texinfo like this:
@@texinfo:Basil L.@tie{}Contovounesios@@




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#45068; Package emacs. (Fri, 05 Mar 2021 06:08:02 GMT) Full text and rfc822 format available.

Message #212 received at 45068 <at> debbugs.gnu.org (full text, mbox):

From: Protesilaos Stavrou <info <at> protesilaos.com>
To: Mauro Aranda <maurooaranda <at> gmail.com>
Cc: "Basil L. Contovounesios" <contovob <at> tcd.ie>, 45068 <at> debbugs.gnu.org
Subject: Re: bug#45068: [PATCH] 28.0.50; Update Modus themes 1.0.2
 (backward-incompatible)
Date: Fri, 05 Mar 2021 08:07:15 +0200
[Message part 1 (text/plain, inline)]
On 2021-03-04, 19:40 -0300, Mauro Aranda <maurooaranda <at> gmail.com> wrote:

> Protesilaos Stavrou <info <at> protesilaos.com> writes:
>
>> On 2021-03-04, 18:41 +0000, "Basil L. Contovounesios" <contovob <at> tcd.ie> wrote:
>>
>>> Protesilaos Stavrou <info <at> protesilaos.com> writes:
>
>>>> ++ 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?
>
> Perhaps hide it from ox-texinfo like this:
> @@texinfo:Basil L.@tie{}Contovounesios@@

Thank you Mauro!  I first did what you suggested, but felt that it made
the Org version more difficult to read.  So I used "@:" instead.
Described here:

     (info "(texinfo) Not Ending a Sentence")

Please find attached the revised patch.

-- 
Protesilaos Stavrou
protesilaos.com
[0001-Update-Modus-themes-to-their-version-1.2.0.patch (text/x-patch, attachment)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#45068; Package emacs. (Fri, 05 Mar 2021 06:35:01 GMT) Full text and rfc822 format available.

Message #215 received at 45068 <at> debbugs.gnu.org (full text, mbox):

From: Protesilaos Stavrou <info <at> protesilaos.com>
To: Mauro Aranda <maurooaranda <at> gmail.com>
Cc: "Basil L. Contovounesios" <contovob <at> tcd.ie>, 45068 <at> debbugs.gnu.org
Subject: Re: bug#45068: [PATCH] 28.0.50; Update Modus themes 1.0.2
 (backward-incompatible)
Date: Fri, 05 Mar 2021 08:34:10 +0200
On 2021-03-04, 19:06 -0300, Mauro Aranda <maurooaranda <at> gmail.com> wrote:

> Protesilaos Stavrou <info <at> protesilaos.com> writes:
>
>> On 2021-03-04, 13:53 -0300, Mauro Aranda <maurooaranda <at> gmail.com> 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?
>
> Sounds OK to me; for themes that are only distributed with Emacs, it
> doesn't seem to be needed.

That is my impression as well.

> But if you plan to keep distributing them as packages via ELPA, then
> it might make sense to keep it.  I don't know what's the plan, so I
> can't say for sure if the form should stay or not.

My original plan was to update the themes in emacs.git and then figure
out what needs to be done for elpa.git to treat them as ":core" packages
instead of ":external".

So I had this and would have used a similar technique for the
above-quoted code:

    (if (and (>= emacs-major-version 28)
             (functionp 'require-theme))
          (require-theme 'modus-themes)
        (require 'modus-themes))

But that produced a major bug of not loading the desired theme in
certain setups.[1] I suspect it is because 'require' needs to be at the
top level?  Not sure...  Maybe there is some clean way to fix that,
though I would need more time to research and test it; a time frame that
I cannot estimate right now.

[1]: <https://gitlab.com/protesilaos/modus-themes/-/issues/162>.

So I prefer to use files that 100% work in emacs.git and then I will
treat elpa.git separately.  Using all those untested conditional clauses
will give me trouble.  Perhaps the themes in elpa.git should not be
":core" after all?  Keeping them as ":external", though updated to the
newest release, seems like the most reliable path forward.

Ultimately this means more work for me, though I prefer to not have to
deal with packaging-related bugs (notwithstanding the fact that I need
to ask for someone else to push changes for me in Emacs/ELPA and I would
rather not bother them).

>> Thanks again!
>
> Thanks to you!

I appreciate your contributions ("your" singular and plural).  For me
this is all part of a learning process and am happy to be part of a
community that (i) tolerates my errors and (ii) helps me learn through
them without making any discounts on technical requirements.

-- 
Protesilaos Stavrou
protesilaos.com




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#45068; Package emacs. (Fri, 05 Mar 2021 17:12:01 GMT) Full text and rfc822 format available.

Message #218 received at 45068 <at> debbugs.gnu.org (full text, mbox):

From: "Basil L. Contovounesios" <contovob <at> tcd.ie>
To: Protesilaos Stavrou <info <at> protesilaos.com>
Cc: 45068 <at> debbugs.gnu.org, Mauro Aranda <maurooaranda <at> gmail.com>
Subject: Re: bug#45068: [PATCH] Modus themes 1.2.0
Date: Fri, 05 Mar 2021 17:11:34 +0000
Protesilaos Stavrou <info <at> protesilaos.com> writes:

> On 2021-03-04, 18:41 +0000, "Basil L. Contovounesios" <contovob <at> tcd.ie> wrote:
>
>> Protesilaos Stavrou <info <at> protesilaos.com> writes:
>>
> I updated the language to disambiguate the use-cases.  If you think it
> needs further work, I will rewrite it.

I think it's fine now, thanks.

>>> ++ 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 ;).

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

> On 2021-03-04, 13:53 -0300, Mauro Aranda <maurooaranda <at> gmail.com> 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.

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.

Thanks,

-- 
Basil




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#45068; Package emacs. (Fri, 05 Mar 2021 17:13:02 GMT) Full text and rfc822 format available.

Message #221 received at 45068 <at> debbugs.gnu.org (full text, mbox):

From: "Basil L. Contovounesios" <contovob <at> tcd.ie>
To: Protesilaos Stavrou <info <at> protesilaos.com>
Cc: 45068 <at> debbugs.gnu.org, Mauro Aranda <maurooaranda <at> gmail.com>
Subject: Re: bug#45068: [PATCH] 28.0.50; Update Modus themes 1.0.2
 (backward-incompatible)
Date: Fri, 05 Mar 2021 17:11:54 +0000
Protesilaos Stavrou <info <at> protesilaos.com> writes:

> On 2021-03-04, 19:06 -0300, Mauro Aranda <maurooaranda <at> gmail.com> wrote:
>
>> But if you plan to keep distributing them as packages via ELPA, then
>> it might make sense to keep it.  I don't know what's the plan, so I
>> can't say for sure if the form should stay or not.
>
> My original plan was to update the themes in emacs.git and then figure
> out what needs to be done for elpa.git to treat them as ":core" packages
> instead of ":external".
>
> So I had this and would have used a similar technique for the
> above-quoted code:
>
>     (if (and (>= emacs-major-version 28)
>              (functionp 'require-theme))
>           (require-theme 'modus-themes)
>         (require 'modus-themes))
>
> But that produced a major bug of not loading the desired theme in
> certain setups.[1] I suspect it is because 'require' needs to be at the
> top level?  Not sure...  Maybe there is some clean way to fix that,
> though I would need more time to research and test it; a time frame that
> I cannot estimate right now.
>
> [1]: <https://gitlab.com/protesilaos/modus-themes/-/issues/162>.
>
> So I prefer to use files that 100% work in emacs.git and then I will
> treat elpa.git separately.  Using all those untested conditional clauses
> will give me trouble.  Perhaps the themes in elpa.git should not be
> ":core" after all?  Keeping them as ":external", though updated to the
> newest release, seems like the most reliable path forward.
>
> Ultimately this means more work for me, though I prefer to not have to
> deal with packaging-related bugs (notwithstanding the fact that I need
> to ask for someone else to push changes for me in Emacs/ELPA and I would
> rather not bother them).

Ultimately it's your decision as the maintainer, but after addressing
the issue you reference above in modus-themes.git, I no longer see any
significant hurdles to keeping modus-themes.git / elpa.git / emacs.git
in sync, unless I'm missing something?

> I appreciate your contributions ("your" singular and plural).  For me
> this is all part of a learning process and am happy to be part of a
> community that (i) tolerates my errors and (ii) helps me learn through
> them without making any discounts on technical requirements.

That should/does apply to everyone here :).

-- 
Basil




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#45068; Package emacs. (Fri, 05 Mar 2021 17:35:01 GMT) Full text and rfc822 format available.

Message #224 received at 45068 <at> debbugs.gnu.org (full text, mbox):

From: Protesilaos Stavrou <info <at> protesilaos.com>
To: "Basil L. Contovounesios" <contovob <at> tcd.ie>
Cc: 45068 <at> debbugs.gnu.org, Mauro Aranda <maurooaranda <at> gmail.com>
Subject: Re: bug#45068: [PATCH] 28.0.50; Update Modus themes 1.0.2
 (backward-incompatible)
Date: Fri, 05 Mar 2021 19:34:21 +0200
On 2021-03-05, 17:11 +0000, "Basil L. Contovounesios" <contovob <at> tcd.ie> wrote:

> Protesilaos Stavrou <info <at> protesilaos.com> writes:
>
>> On 2021-03-04, 19:06 -0300, Mauro Aranda <maurooaranda <at> gmail.com> wrote:
>>
>>> But if you plan to keep distributing them as packages via ELPA, then
>>> it might make sense to keep it.  I don't know what's the plan, so I
>>> can't say for sure if the form should stay or not.
>>
>> My original plan was to update the themes in emacs.git and then figure
>> out what needs to be done for elpa.git to treat them as ":core" packages
>> instead of ":external".
>>
>> So I had this and would have used a similar technique for the
>> above-quoted code:
>>
>>     (if (and (>= emacs-major-version 28)
>>              (functionp 'require-theme))
>>           (require-theme 'modus-themes)
>>         (require 'modus-themes))
>>
>> But that produced a major bug of not loading the desired theme in
>> certain setups.[1] I suspect it is because 'require' needs to be at the
>> top level?  Not sure...  Maybe there is some clean way to fix that,
>> though I would need more time to research and test it; a time frame that
>> I cannot estimate right now.
>>
>> [1]: <https://gitlab.com/protesilaos/modus-themes/-/issues/162>.
>>
>> So I prefer to use files that 100% work in emacs.git and then I will
>> treat elpa.git separately.  Using all those untested conditional clauses
>> will give me trouble.  Perhaps the themes in elpa.git should not be
>> ":core" after all?  Keeping them as ":external", though updated to the
>> newest release, seems like the most reliable path forward.
>>
>> Ultimately this means more work for me, though I prefer to not have to
>> deal with packaging-related bugs (notwithstanding the fact that I need
>> to ask for someone else to push changes for me in Emacs/ELPA and I would
>> rather not bother them).
>
> Ultimately it's your decision as the maintainer, but after addressing
> the issue you reference above in modus-themes.git, I no longer see any
> significant hurdles to keeping modus-themes.git / elpa.git / emacs.git
> in sync, unless I'm missing something?

In principle I want to keep everything in sync.  I think things should
work now, thanks to your contributions.

I need to prepare the elpa.git patch separately though, as I have not
yet studied exactly what needs to be changed.

>> I appreciate your contributions ("your" singular and plural).  For me
>> this is all part of a learning process and am happy to be part of a
>> community that (i) tolerates my errors and (ii) helps me learn through
>> them without making any discounts on technical requirements.
>
> That should/does apply to everyone here :).

Very well!

-- 
Protesilaos Stavrou
protesilaos.com




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#45068; Package emacs. (Fri, 05 Mar 2021 17:51:02 GMT) Full text and rfc822 format available.

Message #227 received at 45068 <at> debbugs.gnu.org (full text, mbox):

From: Protesilaos Stavrou <info <at> protesilaos.com>
To: "Basil L. Contovounesios" <contovob <at> tcd.ie>
Cc: 45068 <at> debbugs.gnu.org, Mauro Aranda <maurooaranda <at> gmail.com>
Subject: Re: bug#45068: [PATCH] Modus themes 1.2.0
Date: Fri, 05 Mar 2021 19:50:26 +0200
[Message part 1 (text/plain, inline)]
On 2021-03-05, 17:11 +0000, "Basil L. Contovounesios" <contovob <at> tcd.ie> 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 <maurooaranda <at> gmail.com> 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
[modus-operandi-theme.el (text/plain, attachment)]
[modus-themes.el (text/plain, attachment)]
[modus-vivendi-theme.el (text/plain, attachment)]
[modus-themes.org (application/vnd.lotus-organizer, attachment)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#45068; Package emacs. (Fri, 05 Mar 2021 21:12:01 GMT) Full text and rfc822 format available.

Message #230 received at 45068 <at> debbugs.gnu.org (full text, mbox):

From: Gregory Heytings <gregory <at> heytings.org>
To: Protesilaos Stavrou <info <at> protesilaos.com>
Cc: 45068 <at> debbugs.gnu.org
Subject: Re: bug#45068: [PATCH] Modus themes 1.2.0
Date: Fri, 05 Mar 2021 21:11:33 +0000
>>> +(deftheme modus-vivendi
>>> +  "Accessible and customizable light theme (WCAG AAA standard).
>>                                   ^^^^^
>>                                   dark
>
> [ Answer only if it is easy: how do you draw those ^^^ below the text? ]
>

(defun undercaret (&optional arg)
  (interactive "p")
  (let* ((b (region-beginning))
         (e (region-end))
         (c (save-excursion (goto-char b) (current-column))))
    (when (and (region-active-p)
               (= (line-number-at-pos b) (line-number-at-pos e)))
      (forward-line 1)
      (dotimes (_ c) (insert " "))
      (dotimes (_ (- e b)) (insert "^"))
      (insert "\n")
      (when (/= arg 1)
        (dotimes (_ c) (insert " "))
        (insert "\n"))
      (backward-char))))




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#45068; Package emacs. (Fri, 05 Mar 2021 22:01:01 GMT) Full text and rfc822 format available.

Message #233 received at 45068 <at> debbugs.gnu.org (full text, mbox):

From: "Basil L. Contovounesios" <contovob <at> tcd.ie>
To: Protesilaos Stavrou <info <at> protesilaos.com>
Cc: 45068 <at> debbugs.gnu.org, Mauro Aranda <maurooaranda <at> gmail.com>
Subject: Re: bug#45068: [PATCH] Modus themes 1.2.0
Date: Fri, 05 Mar 2021 22:00:15 +0000
Protesilaos Stavrou <info <at> protesilaos.com> writes:

> On 2021-03-05, 17:11 +0000, "Basil L. Contovounesios" <contovob <at> tcd.ie> 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!).

Don't believe what he says, he only wants to discredit me.

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

Thanks, I've now pushed your original 1.2.0 patch and a new 1.2.3 patch
with you as the author to the scratch/update-modus-themes branch.  It
looks fine to me, so unless you see anything amiss I'll merge it with
master soon.

-- 
Basil




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#45068; Package emacs. (Sat, 06 Mar 2021 05:15:02 GMT) Full text and rfc822 format available.

Message #236 received at 45068 <at> debbugs.gnu.org (full text, mbox):

From: Protesilaos Stavrou <info <at> protesilaos.com>
To: "Basil L. Contovounesios" <contovob <at> tcd.ie>
Cc: 45068 <at> debbugs.gnu.org, Mauro Aranda <maurooaranda <at> gmail.com>
Subject: Re: bug#45068: [PATCH] Modus themes 1.2.0
Date: Sat, 06 Mar 2021 07:13:51 +0200
On 2021-03-05, 22:00 +0000, "Basil L. Contovounesios" <contovob <at> tcd.ie> wrote:

>>> 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.
>
> Thanks, I've now pushed your original 1.2.0 patch and a new 1.2.3 patch
> with you as the author to the scratch/update-modus-themes branch.  It
> looks fine to me, so unless you see anything amiss I'll merge it with
> master soon.

Thank you Basil!  Everything seems to be in order.  Please merge it, at
your convenience.

-- 
Protesilaos Stavrou
protesilaos.com




bug marked as fixed in version 28.1, send any further explanations to 45068 <at> debbugs.gnu.org and Protesilaos Stavrou <info <at> protesilaos.com> Request was from "Basil L. Contovounesios" <contovob <at> tcd.ie> to control <at> debbugs.gnu.org. (Sat, 06 Mar 2021 09:30:02 GMT) Full text and rfc822 format available.

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#45068; Package emacs. (Sat, 06 Mar 2021 09:30:02 GMT) Full text and rfc822 format available.

Message #241 received at 45068-done <at> debbugs.gnu.org (full text, mbox):

From: "Basil L. Contovounesios" <contovob <at> tcd.ie>
To: Protesilaos Stavrou <info <at> protesilaos.com>
Cc: 45068-done <at> debbugs.gnu.org, Mauro Aranda <maurooaranda <at> gmail.com>
Subject: Re: bug#45068: [PATCH] Modus themes 1.2.0
Date: Sat, 06 Mar 2021 09:29:47 +0000
close 45068 28.1
quit

Protesilaos Stavrou <info <at> protesilaos.com> writes:

> On 2021-03-05, 22:00 +0000, "Basil L. Contovounesios" <contovob <at> tcd.ie> wrote:
>
>> Thanks, I've now pushed your original 1.2.0 patch and a new 1.2.3 patch
>> with you as the author to the scratch/update-modus-themes branch.  It
>> looks fine to me, so unless you see anything amiss I'll merge it with
>> master soon.
>
> Thank you Basil!  Everything seems to be in order.  Please merge it, at
> your convenience.

Thanks, done.  And with that, I'm closing this report.

Update Modus themes to their version 1.2.0
de602dd7cf 2021-03-06 09:16:00 +0000
https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=de602dd7cf76b001244964aa5bbef4d9e08ea62b

Pull Modus themes version 1.2.3 from upstream
8fb33bae32 2021-03-06 09:16:00 +0000
https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=8fb33bae32e39f597317eb4857447bb0ea1a4de3

-- 
Basil




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#45068; Package emacs. (Sat, 06 Mar 2021 13:25:02 GMT) Full text and rfc822 format available.

Message #244 received at 45068 <at> debbugs.gnu.org (full text, mbox):

From: Gregory Heytings <gregory <at> heytings.org>
To: Protesilaos Stavrou <info <at> protesilaos.com>
Cc: 45068 <at> debbugs.gnu.org
Subject: Re: bug#45068: [PATCH] Modus themes 1.2.0
Date: Sat, 06 Mar 2021 13:24:01 +0000
>>> +(deftheme modus-vivendi
>>> +  "Accessible and customizable light theme (WCAG AAA standard).
>>                                   ^^^^^
>>                                   dark
> 
> [ Answer only if it is easy: how do you draw those ^^^ below the text? ]
>

Here's a more polished version, which takes care of the prefix / 
fill-prefix, and works on multiple lines:

(defun undercaret (&optional arg)
  (interactive "p")
  (let* ((begin (if (region-active-p) (region-beginning) (line-beginning-position)))
         (end (if (region-active-p) (region-end) (line-end-position)))
         (lines (- (line-number-at-pos end) (line-number-at-pos begin) -1))
         (comment (and (/= arg 1) (= lines 1)))
         (final-forward-line -1))
    (goto-char begin)
    (dotimes (i lines)
      (let* ((line-begin (if (zerop i) begin (line-beginning-position)))
             (line-end (if (= (1+ i) lines) end (line-end-position)))
             (begin-column (progn (goto-char line-begin) (current-column)))
             (prefix-begin (line-beginning-position))
             (prefix-end (progn (beginning-of-line-text) (point)))
             (prefix-end-column (progn (goto-char prefix-end) (current-column)))
             (delta (if (< begin-column prefix-end-column) (- prefix-end-column begin-column) 0))
             (prefix-string (buffer-substring-no-properties prefix-begin prefix-end))
             (prefix (if (string-blank-p prefix-string) "" prefix-string))
             (whitespace (make-string (- (+ begin-column delta) (length prefix)) ?\ ))
             (do-under (< delta (- line-end line-begin)))
             (under (if do-under (make-string (- line-end line-begin delta) ?^) ""))
             (under-string (concat prefix whitespace under "\n")))
        (forward-line 1)
        (if do-under (insert under-string) (setq final-forward-line -2))
        (setq end (+ end (length under-string)))
        (when comment (insert prefix whitespace "\n"))))
    (forward-line final-forward-line)
    (goto-char (line-end-position))))




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#45068; Package emacs. (Sat, 06 Mar 2021 15:23:01 GMT) Full text and rfc822 format available.

Message #247 received at 45068 <at> debbugs.gnu.org (full text, mbox):

From: Protesilaos Stavrou <info <at> protesilaos.com>
To: Gregory Heytings <gregory <at> heytings.org>
Cc: 45068 <at> debbugs.gnu.org
Subject: Re: bug#45068: [PATCH] Modus themes 1.2.0
Date: Sat, 06 Mar 2021 17:22:30 +0200
On 2021-03-06, 13:24 +0000, Gregory Heytings <gregory <at> heytings.org> wrote:

>>>> +(deftheme modus-vivendi
>>>> +  "Accessible and customizable light theme (WCAG AAA standard).
>>>                                   ^^^^^
>>>                                   dark
>> [ Answer only if it is easy: how do you draw those ^^^ below the text?
>> ]
>>
>
> Here's a more polished version, which takes care of the prefix /
> fill-prefix, and works on multiple lines:
>
> (defun undercaret (&optional arg)
>        ^^^^^^^^^^

It works.  Thank you!

-- 
Protesilaos Stavrou
protesilaos.com




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#45068; Package emacs. (Sat, 06 Mar 2021 18:26:01 GMT) Full text and rfc822 format available.

Message #250 received at 45068 <at> debbugs.gnu.org (full text, mbox):

From: Gregory Heytings <gregory <at> heytings.org>
To: Protesilaos Stavrou <info <at> protesilaos.com>
Cc: 45068 <at> debbugs.gnu.org
Subject: Re: bug#45068: [PATCH] Modus themes 1.2.0
Date: Sat, 06 Mar 2021 18:25:22 +0000
>
> It works.  Thank you!
>

You are welcome :-)  Actually the previous version had a bug with 
tab-indented lines.  It is fixed now:

(defun undercaret (&optional arg)
  (interactive "p")
  (let* ((begin (if (region-active-p) (region-beginning) (line-beginning-position)))
         (end (if (region-active-p) (region-end) (line-end-position)))
         (lines (- (line-number-at-pos end) (line-number-at-pos begin) -1))
         (comment (and (/= arg 1) (= lines 1)))
         (final-forward-line -1))
    (goto-char begin)
    (dotimes (i lines)
      (let* ((line-begin (if (zerop i) begin (line-beginning-position)))
             (line-end (if (= (1+ i) lines) end (line-end-position)))
             (begin-column (progn (goto-char line-begin) (current-column)))
             (end-column (progn (goto-char line-end) (current-column)))
             (prefix-begin (line-beginning-position))
             (prefix-end (progn (beginning-of-line-text) (point)))
             (prefix-end-column (progn (goto-char prefix-end) (current-column)))
             (delta (if (< begin-column prefix-end-column) (- prefix-end-column begin-column) 0))
             (prefix-string (buffer-substring-no-properties prefix-begin prefix-end))
             (prefix (if (string-match-p "\\` *\\'" prefix-string) "" prefix-string))
             (whitespace (make-string (- (+ begin-column delta) (string-width prefix)) ?\ ))
             (do-under (< delta (- line-end line-begin)))
             (under (if do-under (make-string (- end-column begin-column delta) ?^) ""))
             (under-string (concat prefix whitespace under "\n")))
        (forward-line 1)
        (if do-under (insert under-string) (setq final-forward-line -2))
        (setq end (+ end (length under-string)))
        (when comment (insert prefix whitespace "\n"))))
    (forward-line final-forward-line)
    (goto-char (line-end-position))))

(BTW, sorry for the off-topic noise on your thread on modus-themes, but it 
was you who asked for a solution ;-))




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#45068; Package emacs. (Sat, 06 Mar 2021 18:45:01 GMT) Full text and rfc822 format available.

Message #253 received at 45068 <at> debbugs.gnu.org (full text, mbox):

From: Protesilaos Stavrou <info <at> protesilaos.com>
To: Gregory Heytings <gregory <at> heytings.org>
Cc: 45068 <at> debbugs.gnu.org
Subject: Re: bug#45068: [PATCH] Modus themes 1.2.0
Date: Sat, 06 Mar 2021 20:43:48 +0200
On 2021-03-06, 18:25 +0000, Gregory Heytings <gregory <at> heytings.org> wrote:

>>
>> It works.  Thank you!
>>
>
> You are welcome :-)  Actually the previous version had a bug with
> tab-indented lines.  It is fixed now:
>
> (defun undercaret (&optional arg)
>   (interactive "p")
>   (let* ((begin (if (region-active-p) (region-beginning) (line-beginning-position)))
>          (end (if (region-active-p) (region-end) (line-end-position)))
>          (lines (- (line-number-at-pos end) (line-number-at-pos begin) -1))
>          (comment (and (/= arg 1) (= lines 1)))
>          (final-forward-line -1))
>     (goto-char begin)
>     (dotimes (i lines)
>       (let* ((line-begin (if (zerop i) begin (line-beginning-position)))
>              (line-end (if (= (1+ i) lines) end (line-end-position)))
>              (begin-column (progn (goto-char line-begin) (current-column)))
>              (end-column (progn (goto-char line-end) (current-column)))
>              (prefix-begin (line-beginning-position))
>              (prefix-end (progn (beginning-of-line-text) (point)))
>              (prefix-end-column (progn (goto-char prefix-end) (current-column)))
>              (delta (if (< begin-column prefix-end-column) (- prefix-end-column begin-column) 0))
>              (prefix-string (buffer-substring-no-properties prefix-begin prefix-end))
>              (prefix (if (string-match-p "\\` *\\'" prefix-string) "" prefix-string))
>              (whitespace (make-string (- (+ begin-column delta) (string-width prefix)) ?\ ))
>              (do-under (< delta (- line-end line-begin)))
>              (under (if do-under (make-string (- end-column begin-column delta) ?^) ""))
>              (under-string (concat prefix whitespace under "\n")))
>         (forward-line 1)
>         (if do-under (insert under-string) (setq final-forward-line -2))
>         (setq end (+ end (length under-string)))
>         (when comment (insert prefix whitespace "\n"))))
>     (forward-line final-forward-line)
>     (goto-char (line-end-position))))

I admit not to have tested it thoroughly (not yet, anyway).  Though I do
plan to use your function.  Thanks!

I will eventually include it in my public dotfiles' repo.  Is there a
link I could provide as a reference to your contribution?  Or just cite
the message in this thread?[1]

[1]: <https://debbugs.gnu.org/cgi/bugreport.cgi?bug=45068#250>.

> (BTW, sorry for the off-topic noise on your thread on modus-themes, but
> it was you who asked for a solution ;-))

Yes, I guess this is not the right place to discuss this feature...
Though the bug has been closed successfully.

I had asked because I was under the impression that Gnus or some other
library was implementing it.

-- 
Protesilaos Stavrou
protesilaos.com




bug archived. Request was from Debbugs Internal Request <help-debbugs <at> gnu.org> to internal_control <at> debbugs.gnu.org. (Sun, 04 Apr 2021 11:24:06 GMT) Full text and rfc822 format available.

This bug report was last modified 2 years and 359 days ago.

Previous Next


GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.