GNU bug report logs - #47845
28.0.50; [PATCH] Update modus-themes to version 1.3.1

Previous Next

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

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

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: 28.0.50; [PATCH] Update modus-themes to version 1.3.1
Date: Sat, 17 Apr 2021 19:46:28 +0300

[Message part 1 (text/plain, inline)]

Dear maintainers,

I have just published a new tagged release for the modus-themes and
would like to sync the changes with Emacs.  Note that I now have commit
access to emacs.git, though I would prefer if someone could review the
attached patch (is there some other way to conduct such reviews?).

Release notes: <https://protesilaos.com/codelog/2021-04-17-modus-themes-1-3-0/>.

Thank you for your support!

All the best,
Protesilaos or "Prot"

-- 
Protesilaos Stavrou
protesilaos.com

[0001-Update-modus-themes-to-version-1.3.1.patch (text/x-patch, attachment)]

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

From: "Basil L. Contovounesios" <contovob <at> tcd.ie>
To: Protesilaos Stavrou <info <at> protesilaos.com>
Cc: 47845 <at> debbugs.gnu.org
Subject: Re: bug#47845: 28.0.50; [PATCH] Update modus-themes to version 1.3.1
Date: Sat, 17 Apr 2021 22:04:16 +0100

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

> I have just published a new tagged release for the modus-themes and
> would like to sync the changes with Emacs.  Note that I now have commit
> access to emacs.git, though I would prefer if someone could review the
> attached patch

Thanks, LGTM apart from some minor comments below.  (Feel free to
address them in a separate patch.)

> (is there some other way to conduct such reviews?).

Sending patches to bug-gnu-emacs is the preferred way AFAIK.

> +value of =45000= will suffice, contrary to the default =30000=.  Though for
> +the ~accented-moody~ value mentioned above, that should be raised up to
> +=70000=.  Do not set it too high, because it has the adverse effect of
> +always overriding the default colors (which has been carefully designed
                                               ^^^
                                               have

> +to be highly accessible).

> +There are cases where we need to change the buffer-local attributes of a
> +face.  This might be because we have our own minor mode that re-uses a
> +face for a particular purpose, such as a line selection tool that
> +activates ~hl-line-mode~, but we wish to keep distinct it from other
                                                 ^^^^^^^^^^^
                                                 it distinct

> +#+begin_src emacs-lisp
> +(defvar my-rainbow-region-colors
> +  (modus-themes-with-colors
> +    `((red . ,red-subtle-bg)
> +      (green . ,green-subtle-bg)
> +      (yellow . ,yellow-subtle-bg)
> +      (blue . ,blue-subtle-bg)
> +      (magenta . ,magenta-subtle-bg)
> +      (cyan . ,cyan-subtle-bg)))
> +  "Sample list of color values for `my-rainbow-region'.")
> +
> +(defun my-rainbow-region (color)
> +  "Remap buffer-local attribute of `region' using COLOR."
> +  (interactive
> +   (list
> +    (completing-read "Pick a color: "
> +                     (mapcar #'car my-rainbow-region-colors))))

If the cars of my-rainbow-region-colors are symbols, you can pass the
alist to completing-read directly.  Conversely, completing-read isn't
documented as accepting a flat list of symbols.

> +      (dolist (cons palette)
> +	    (setq name (color-saturate-name (cdr cons) percent))
> +	    (setq name (format "%s" name))
> +	    (setq cons `(,(car cons) . ,name))
> +	    (push cons colors))

Texinfo doesn't like tab characters in general, so this indentation (and
any other occurrences of tabs) should be untabified.

> +The idea is to assign that function to a hook that gets called when
> +~pdf-tools~ renders the document: ~pdf-tools-enabled-hook~.  This is enough
> +when you only use one theme.  However it has the downside of setting the
> +background color value only at render time.  In other words, the face
> +remapping function does not get evaluated anew whenever the theme
> +changes, such upon invoking {{{kbd(M-x modus-themes-toggle)}}}.
            ^^^^^
            such as

> +(defun my-pdf-tools-midnight-mode-toggle ()
> +  (when (eq major-mode 'pdf-view-mode)

Nit: maybe (derived-mode-p 'pdf-view-mode) instead?

-- 
Basil

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

From: Protesilaos Stavrou <info <at> protesilaos.com>
To: "Basil L. Contovounesios" <contovob <at> tcd.ie>
Cc: 47845 <at> debbugs.gnu.org
Subject: Re: bug#47845: 28.0.50; [PATCH] Update modus-themes to version 1.3.1
Date: Sun, 18 Apr 2021 06:35:28 +0300

[Message part 1 (text/plain, inline)]

On 2021-04-17, 22:04 +0100, "Basil L. Contovounesios" <contovob <at> tcd.ie> wrote:

> Protesilaos Stavrou <info <at> protesilaos.com> writes:
>
>> I have just published a new tagged release for the modus-themes and
>> would like to sync the changes with Emacs.  Note that I now have commit
>> access to emacs.git, though I would prefer if someone could review the
>> attached patch
>
> Thanks, LGTM apart from some minor comments below.  (Feel free to
> address them in a separate patch.)
>
> [...]

Thank you Basil for taking the time to do this!

I attach the revised patch, which now is for version 1.3.2 (tagged it on
my repo).

-- 
Protesilaos Stavrou
protesilaos.com

[0001-Update-modus-themes-to-version-1.3.2.patch (text/x-patch, attachment)]

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

From: Stefan Kangas <stefan <at> marxist.se>
To: Protesilaos Stavrou <info <at> protesilaos.com>
Cc: "Basil L. Contovounesios" <contovob <at> tcd.ie>, 47845 <at> debbugs.gnu.org
Subject: Re: bug#47845: 28.0.50; [PATCH] Update modus-themes to version 1.3.1
Date: Sun, 18 Apr 2021 11:43:19 -0500

tags 47845 fixed
close 47845 28.1
thanks

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

> On 2021-04-17, 22:04 +0100, "Basil L. Contovounesios" <contovob <at> tcd.ie> wrote:
>
>> Protesilaos Stavrou <info <at> protesilaos.com> writes:
>>
>>> I have just published a new tagged release for the modus-themes and
>>> would like to sync the changes with Emacs.  Note that I now have commit
>>> access to emacs.git, though I would prefer if someone could review the
>>> attached patch
>>
>> Thanks, LGTM apart from some minor comments below.  (Feel free to
>> address them in a separate patch.)
>>
>> [...]
>
> Thank you Basil for taking the time to do this!

Yup, Basil is doing solid work with his reviews.

> I attach the revised patch, which now is for version 1.3.2 (tagged it on
> my repo).

Pushed to master as commit dc9b0dc461.

Thanks for the new version!

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

From: Protesilaos Stavrou <info <at> protesilaos.com>
To: Stefan Kangas <stefan <at> marxist.se>
Cc: "Basil L. Contovounesios" <contovob <at> tcd.ie>, 47845 <at> debbugs.gnu.org
Subject: Re: bug#47845: 28.0.50; [PATCH] Update modus-themes to version 1.3.1
Date: Sun, 18 Apr 2021 20:08:46 +0300

On 2021-04-18, 11:43 -0500, Stefan Kangas <stefan <at> marxist.se> wrote:

>> On 2021-04-17, 22:04 +0100, "Basil L. Contovounesios" <contovob <at> tcd.ie> wrote:
>>
>>> Protesilaos Stavrou <info <at> protesilaos.com> writes:
>>>
>>>> I have just published a new tagged release for the modus-themes and
>>>> would like to sync the changes with Emacs.  Note that I now have commit
>>>> access to emacs.git, though I would prefer if someone could review the
>>>> attached patch
>>>
>>> Thanks, LGTM apart from some minor comments below.  (Feel free to
>>> address them in a separate patch.)
>>>
>>> [...]
>>
>> Thank you Basil for taking the time to do this!
>
> Yup, Basil is doing solid work with his reviews.

Indeed!  Thanks once again!

>> I attach the revised patch, which now is for version 1.3.2 (tagged it on
>> my repo).
>
> Pushed to master as commit dc9b0dc461.
>
> Thanks for the new version!

You are welcome!  On to the next one.

-- 
Protesilaos Stavrou
protesilaos.com

Previous Next

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