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

Previous Next

Full log

View this message in rfc822 format

From: "Basil L. Contovounesios" <contovob <at> tcd.ie>
To: Protesilaos Stavrou <info <at> protesilaos.com>
Cc: 47845 <at> debbugs.gnu.org
Subject: 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

Previous Next

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