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

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 71213 in the body.
You can then email your comments to 71213 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: Kévin Le Gouguec <kevin.legouguec <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: [PATCH] Use the current face foreground for read-passwd icons
Date: Sun, 26 May 2024 19:53:23 +0200

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

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'

[0001-Use-the-current-face-foreground-for-read-passwd-icon.patch (text/patch, attachment)]

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

From: Michael Albinus <michael.albinus <at> gmx.de>
To: Kévin Le Gouguec <kevin.legouguec <at> gmail.com>
Cc: 71213 <at> debbugs.gnu.org
Subject: Re: bug#71213: [PATCH] Use the current face foreground for
 read-passwd icons
Date: Mon, 27 May 2024 09:41:20 +0200

Kévin Le Gouguec <kevin.legouguec <at> 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.

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

From: Kévin Le Gouguec <kevin.legouguec <at> gmail.com>
To: Michael Albinus <michael.albinus <at> gmx.de>
Cc: 71213 <at> debbugs.gnu.org
Subject: Re: bug#71213: [PATCH] Use the current face foreground for
 read-passwd icons
Date: Mon, 27 May 2024 20:11:02 +0200

Michael Albinus via "Bug reports for GNU Emacs, the Swiss army knife of
text editors" <bug-gnu-emacs <at> 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"?

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

From: Michael Albinus <michael.albinus <at> gmx.de>
To: Kévin Le Gouguec <kevin.legouguec <at> gmail.com>
Cc: 71213 <at> debbugs.gnu.org
Subject: Re: bug#71213: [PATCH] Use the current face foreground for
 read-passwd icons
Date: Tue, 28 May 2024 09:34:50 +0200

Kévin Le Gouguec <kevin.legouguec <at> 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.

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

From: Kévin Le Gouguec <kevin.legouguec <at> gmail.com>
To: Michael Albinus <michael.albinus <at> gmx.de>
Cc: 71213 <at> debbugs.gnu.org
Subject: Re: bug#71213: [PATCH] Use the current face foreground for
 read-passwd icons
Date: Tue, 28 May 2024 19:33:46 +0200

Michael Albinus <michael.albinus <at> 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.

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

From: Kévin Le Gouguec <kevin.legouguec <at> gmail.com>
To: Michael Albinus <michael.albinus <at> gmx.de>
Cc: 71213-done <at> debbugs.gnu.org
Subject: Re: bug#71213: [PATCH] Use the current face foreground for
 read-passwd icons
Date: Mon, 03 Jun 2024 19:14:05 +0200

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

> Michael Albinus <michael.albinus <at> 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.

Previous Next

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