bug#71213: [PATCH] Use the current face foreground for read-passwd icons

bug#71213: [PATCH] Use the current face foreground for read-passwd icons

[Kévin Le Gouguec]

[-- Attachment #1: Type: text/plain, Size: 919 bytes --]

Tags: patch

Been driving with this one for some time and somehow never got around to
submitting it.

We've done something similar for other icons, e.g. in 2021-11-02 "Use
current face foreground for SVG icons in customize" (11702a6dd7c).  I
think it makes sense for these too, since "#2e3436" (and any shade of
not-strictly-gray-but-close-enough) is liable to have low contrast with
the mode line background depending on the theme (e.g. modus-vivendi).

Let me know what you think.


In GNU Emacs 30.0.50 (build 1, x86_64-pc-linux-gnu, GTK+ Version
 3.24.41, cairo version 1.18.0) of 2024-03-24 built on amdahl30
Repository revision: 0da57fa2789d180ae533b6721d60eac14f76eaee
Repository branch: master
Windowing system distributor 'SUSE LINUX', version 11.0.12401000
System Description: openSUSE Tumbleweed

Configured using:
 'configure --prefix=/home/peniblec/apps/emacs --with-cairo
 --with-sqlite3 --with-xinput2'


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Use-the-current-face-foreground-for-read-passwd-icon.patch --]
[-- Type: text/patch, Size: 4204 bytes --]

From 18a9430e438dbf7894bebed4374f51bce0e040ad Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?K=C3=A9vin=20Le=20Gouguec?= <kevin.legouguec@gmail.com>
Date: Fri, 22 Mar 2024 20:24:46 +0100
Subject: [PATCH] Use the current face foreground for read-passwd icons

* etc/images/conceal.svg:
* etc/images/reveal.svg: Remove 'fill' attribute so that the
current face's background is applied: this helps the icon's SVG
representation remain legible regardless of how the mode-line
is themed.
---
 etc/images/conceal.svg | 2 +-
 etc/images/reveal.svg  | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/etc/images/conceal.svg b/etc/images/conceal.svg
index 172b73ed3d3..65695c0024e 100644
--- a/etc/images/conceal.svg
+++ b/etc/images/conceal.svg
@@ -1,4 +1,4 @@
 <?xml version="1.0" encoding="UTF-8"?>
 <svg height="16px" viewBox="0 0 16 16" width="16px" xmlns="http://www.w3.org/2000/svg">
-    <path d="m 1.53125 0.46875 l -1.0625 1.0625 l 14 14 l 1.0625 -1.0625 l -2.382812 -2.382812 c 1.265624 -1.0625 2.171874 -2.496094 2.589843 -4.097657 c -0.914062 -3.523437 -4.097656 -5.984375 -7.738281 -5.988281 c -1.367188 0.011719 -2.707031 0.371094 -3.894531 1.042969 z m 6.46875 3.53125 c 2.210938 0 4 1.789062 4 4 c -0.003906 0.800781 -0.246094 1.578125 -0.699219 2.238281 l -1.46875 -1.46875 c 0.105469 -0.242187 0.164063 -0.503906 0.167969 -0.769531 c 0 -1.105469 -0.894531 -2 -2 -2 c -0.265625 0.003906 -0.527344 0.0625 -0.769531 0.167969 l -1.46875 -1.46875 c 0.660156 -0.453125 1.4375 -0.695313 2.238281 -0.699219 z m -6.144531 0.917969 c -0.753907 0.898437 -1.296875 1.957031 -1.59375 3.09375 c 0.914062 3.523437 4.097656 5.984375 7.738281 5.988281 c 0.855469 -0.007812 1.703125 -0.152344 2.511719 -0.425781 l -1.667969 -1.667969 c -0.277344 0.058594 -0.5625 0.089844 -0.84375 0.09375 c -2.210938 0 -4 -1.789062 -4 -4 c 0.003906 -0.28125 0.035156 -0.566406 0.09375 -0.84375 z m 0 0" fill="#2e3436"/>
+    <path d="m 1.53125 0.46875 l -1.0625 1.0625 l 14 14 l 1.0625 -1.0625 l -2.382812 -2.382812 c 1.265624 -1.0625 2.171874 -2.496094 2.589843 -4.097657 c -0.914062 -3.523437 -4.097656 -5.984375 -7.738281 -5.988281 c -1.367188 0.011719 -2.707031 0.371094 -3.894531 1.042969 z m 6.46875 3.53125 c 2.210938 0 4 1.789062 4 4 c -0.003906 0.800781 -0.246094 1.578125 -0.699219 2.238281 l -1.46875 -1.46875 c 0.105469 -0.242187 0.164063 -0.503906 0.167969 -0.769531 c 0 -1.105469 -0.894531 -2 -2 -2 c -0.265625 0.003906 -0.527344 0.0625 -0.769531 0.167969 l -1.46875 -1.46875 c 0.660156 -0.453125 1.4375 -0.695313 2.238281 -0.699219 z m -6.144531 0.917969 c -0.753907 0.898437 -1.296875 1.957031 -1.59375 3.09375 c 0.914062 3.523437 4.097656 5.984375 7.738281 5.988281 c 0.855469 -0.007812 1.703125 -0.152344 2.511719 -0.425781 l -1.667969 -1.667969 c -0.277344 0.058594 -0.5625 0.089844 -0.84375 0.09375 c -2.210938 0 -4 -1.789062 -4 -4 c 0.003906 -0.28125 0.035156 -0.566406 0.09375 -0.84375 z m 0 0"/>
 </svg>
diff --git a/etc/images/reveal.svg b/etc/images/reveal.svg
index 41ae3733a53..f950311ac5a 100644
--- a/etc/images/reveal.svg
+++ b/etc/images/reveal.svg
@@ -1,4 +1,4 @@
 <?xml version="1.0" encoding="UTF-8"?>
 <svg height="16px" viewBox="0 0 16 16" width="16px" xmlns="http://www.w3.org/2000/svg">
-    <path d="m 8 2 c -3.648438 0.003906 -6.832031 2.476562 -7.738281 6.007812 c 0.914062 3.527344 4.097656 5.988282 7.738281 5.992188 c 3.648438 -0.003906 6.832031 -2.476562 7.738281 -6.011719 c -0.914062 -3.523437 -4.097656 -5.984375 -7.738281 -5.988281 z m 0 2 c 2.210938 0 4 1.789062 4 4 s -1.789062 4 -4 4 s -4 -1.789062 -4 -4 s 1.789062 -4 4 -4 z m 0 2 c -1.105469 0 -2 0.894531 -2 2 s 0.894531 2 2 2 s 2 -0.894531 2 -2 s -0.894531 -2 -2 -2 z m 0 0" fill="#2e3436"/>
+    <path d="m 8 2 c -3.648438 0.003906 -6.832031 2.476562 -7.738281 6.007812 c 0.914062 3.527344 4.097656 5.988282 7.738281 5.992188 c 3.648438 -0.003906 6.832031 -2.476562 7.738281 -6.011719 c -0.914062 -3.523437 -4.097656 -5.984375 -7.738281 -5.988281 z m 0 2 c 2.210938 0 4 1.789062 4 4 s -1.789062 4 -4 4 s -4 -1.789062 -4 -4 s 1.789062 -4 4 -4 z m 0 2 c -1.105469 0 -2 0.894531 -2 2 s 0.894531 2 2 2 s 2 -0.894531 2 -2 s -0.894531 -2 -2 -2 z m 0 0"/>
 </svg>
-- 
2.45.1

bug#71213: [PATCH] Use the current face foreground for read-passwd icons

[Michael Albinus via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Kévin Le Gouguec <kevin.legouguec@gmail.com> writes:

Hi Kévin,

> We've done something similar for other icons, e.g. in 2021-11-02 "Use
> current face foreground for SVG icons in customize" (11702a6dd7c).  I
> think it makes sense for these too, since "#2e3436" (and any shade of
> not-strictly-gray-but-close-enough) is liable to have low contrast with
> the mode line background depending on the theme (e.g. modus-vivendi).
>
> Let me know what you think.

I have absolutely no knowledge about image formats. I've stolen the two
files from the Gnome repository, that's it. So I won't object any
change.

While we're at it: do the two correspinding *.pbm files also need some
care? And do we need to create also *.xbm files?

Best regards, Michael.

bug#71213: [PATCH] Use the current face foreground for read-passwd icons

[Kévin Le Gouguec]

Michael Albinus via "Bug reports for GNU Emacs, the Swiss army knife of
text editors" <bug-gnu-emacs@gnu.org> writes:

>> We've done something similar for other icons, e.g. in 2021-11-02 "Use
>> current face foreground for SVG icons in customize" (11702a6dd7c).  I
>> think it makes sense for these too, since "#2e3436" (and any shade of
>> not-strictly-gray-but-close-enough) is liable to have low contrast with
>> the mode line background depending on the theme (e.g. modus-vivendi).
>>
>> Let me know what you think.
>
> I have absolutely no knowledge about image formats. I've stolen the two
> files from the Gnome repository, that's it. So I won't object any
> change.

Right, AFAIU we followed the same approach for the icons I mention above
(grabbed the icons somewhere, realized their foreground fared poorly
depending on the background, stripped their 'fill' attribute).

> While we're at it: do the two correspinding *.pbm files also need some
> care?

It's not a format I am familiar with, but empirically,

  (insert-image (create-image "conceal.pbm"))

inserts an icon that automagically changes color as I switch between
light and dark themes.  pbm(5) says the format is monochrome, and (info
"(elisp) Other Image Types") confirms Emacs does the obvious thing by
default (frame foreground for one bits; frame background for zero bits).

>       And do we need to create also *.xbm files?

Not sure if we have a policy or guidelines there; etc/images/README
explains how to convert from xpm > xbm > pbm, but does not provide
judgments on which are needed.  (info "(elisp) Image Formats") says

       Supported image formats (and the required support libraries) include
    PBM and XBM (which do not depend on support libraries and are always
    available),

which makes me tempted to say "if we have lowest-common-denominator
formats like PBM or XBM for our icons, then we're good, the rest is
bonus"?

bug#71213: [PATCH] Use the current face foreground for read-passwd icons

[Michael Albinus via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Kévin Le Gouguec <kevin.legouguec@gmail.com> writes:

Hi Kévin,

>>> We've done something similar for other icons, e.g. in 2021-11-02 "Use
>>> current face foreground for SVG icons in customize" (11702a6dd7c).  I
>>> think it makes sense for these too, since "#2e3436" (and any shade of
>>> not-strictly-gray-but-close-enough) is liable to have low contrast with
>>> the mode line background depending on the theme (e.g. modus-vivendi).
>>>
>>> Let me know what you think.
>>
>> I have absolutely no knowledge about image formats. I've stolen the two
>> files from the Gnome repository, that's it. So I won't object any
>> change.
>
> Right, AFAIU we followed the same approach for the icons I mention above
> (grabbed the icons somewhere, realized their foreground fared poorly
> depending on the background, stripped their 'fill' attribute).

So I would say it is OK to install your patch. Other people will comment
only if they see an unexpected change, so this is likely the best way to
test your proposal :-)

>> While we're at it: do the two correspinding *.pbm files also need some
>> care?
>
> It's not a format I am familiar with, but empirically,
>
>   (insert-image (create-image "conceal.pbm"))
>
> inserts an icon that automagically changes color as I switch between
> light and dark themes.  pbm(5) says the format is monochrome, and (info
> "(elisp) Other Image Types") confirms Emacs does the obvious thing by
> default (frame foreground for one bits; frame background for zero bits).
>
>>       And do we need to create also *.xbm files?
>
> Not sure if we have a policy or guidelines there; etc/images/README
> explains how to convert from xpm > xbm > pbm, but does not provide
> judgments on which are needed.  (info "(elisp) Image Formats") says
>
>        Supported image formats (and the required support libraries) include
>     PBM and XBM (which do not depend on support libraries and are always
>     available),
>
> which makes me tempted to say "if we have lowest-common-denominator
> formats like PBM or XBM for our icons, then we're good, the rest is
> bonus"?

OK.

Best regards, Michael.

bug#71213: [PATCH] Use the current face foreground for read-passwd icons

[Kévin Le Gouguec]

Michael Albinus <michael.albinus@gmx.de> writes:

>>>> We've done something similar for other icons, e.g. in 2021-11-02 "Use
>>>> current face foreground for SVG icons in customize" (11702a6dd7c).  I
>>>> think it makes sense for these too, since "#2e3436" (and any shade of
>>>> not-strictly-gray-but-close-enough) is liable to have low contrast with
>>>> the mode line background depending on the theme (e.g. modus-vivendi).
>>>>
>>>> Let me know what you think.
>>>
>>> I have absolutely no knowledge about image formats. I've stolen the two
>>> files from the Gnome repository, that's it. So I won't object any
>>> change.
>>
>> Right, AFAIU we followed the same approach for the icons I mention above
>> (grabbed the icons somewhere, realized their foreground fared poorly
>> depending on the background, stripped their 'fill' attribute).
>
> So I would say it is OK to install your patch. Other people will comment
> only if they see an unexpected change, so this is likely the best way to
> test your proposal :-)

A'ight 🫡 thanks for weighing in; will install momentarily then, after
adding the bug# to the changelog.

bug#71213: [PATCH] Use the current face foreground for read-passwd icons

[Kévin Le Gouguec]

Kévin Le Gouguec <kevin.legouguec@gmail.com> writes:

> Michael Albinus <michael.albinus@gmx.de> writes:
>
>>>>> We've done something similar for other icons, e.g. in 2021-11-02 "Use
>>>>> current face foreground for SVG icons in customize" (11702a6dd7c).  I
>>>>> think it makes sense for these too, since "#2e3436" (and any shade of
>>>>> not-strictly-gray-but-close-enough) is liable to have low contrast with
>>>>> the mode line background depending on the theme (e.g. modus-vivendi).
>>>>>
>>>>> Let me know what you think.
>>>>
>>>> I have absolutely no knowledge about image formats. I've stolen the two
>>>> files from the Gnome repository, that's it. So I won't object any
>>>> change.
>>>
>>> Right, AFAIU we followed the same approach for the icons I mention above
>>> (grabbed the icons somewhere, realized their foreground fared poorly
>>> depending on the background, stripped their 'fill' attribute).
>>
>> So I would say it is OK to install your patch. Other people will comment
>> only if they see an unexpected change, so this is likely the best way to
>> test your proposal :-)
>
> A'ight 🫡 thanks for weighing in; will install momentarily then, after
> adding the bug# to the changelog.

… and then promptly forget to close the bug 😳 Doing so now.