GNU bug report logs - #35695
27.0.50; Faces diff-added and diff-removed look the same in a 256 color terminal

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 35695 in the body.
You can then email your comments to 35695 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: Anders Lindgren <andlind <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: 27.0.50; Faces diff-added and diff-removed look the same in a 256
 color terminal
Date: Sun, 12 May 2019 07:03:41 +0200

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

Hi!

When using Emacs in a terminal that supports 256 colors (such as the
built-in terminal on macOS), the faces diff-added and diff-removed look the
same.

They are defined to have a background color of #eeffee and #ffeeee,
respectively, in light background mode. When a diff is viewed in a
terminal, they are both rendered using light grey background, so it's
impossible to distinguish between them.

In Emac 26, they were defined using #ddffdd and #ffdddd, respectively,
which is rendered fine in the terminal (on both Emacs 26 and 27).

One way to solve this is to add alternative color definitions using the
"min-colors" requirement. Another is to go back to the old values.

     -- Anders Lindgren

In GNU Emacs 27.0.50 (build 1, x86_64-apple-darwin16.7.0, NS appkit-1504.83
Version 10.12.6 (Build 16G29))
 of 2019-05-01 built on mbp.lan
Repository revision: 7dafbe3ab91e838803a84ab388bca03ff985e312
Repository branch: master
System Description:  Mac OS X 10.12.6

Recent messages:
For information about GNU Emacs and the GNU system, type C-h C-a.
delete-backward-char: Text is read-only
Quit
Mark saved where search started
Making completion list...

Configured using:
 'configure --with-gnutls=no'

Configured features:
NOTIFY KQUEUE ACL LIBXML2 ZLIB TOOLKIT_SCROLL_BARS NS THREADS PDUMPER
GMP

Important settings:
  value of $LC_CTYPE: UTF-8
  value of $LANG: en_SE.UTF-8
  locale-coding-system: utf-8-unix

Major mode: Apropos

Minor modes in effect:
  tooltip-mode: t
  global-eldoc-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  tool-bar-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  buffer-read-only: t
  line-number-mode: t
  transient-mark-mode: t

Load-path shadows:
None found.

Features:
(shadow sort mail-extr emacsbug message rmc puny seq byte-opt gv
bytecomp byte-compile cconv dired dired-loaddefs format-spec rfc822 mml
mml-sec password-cache epa derived epg epg-config gnus-util rmail
rmail-loaddefs text-property-search time-date mm-decode mm-bodies
mm-encode mail-parse rfc2231 mailabbrev gmm-utils mailheader sendmail
rfc2047 rfc2045 ietf-drums mm-util mail-prsvr mail-utils misearch
multi-isearch help-mode cl-loaddefs cl-lib apropos smerge-mode
vc-dispatcher vc-svn diff-mode easymenu easy-mmode term/xterm xterm
elec-pair tooltip eldoc electric uniquify ediff-hook vc-hooks
lisp-float-type mwheel term/ns-win ns-win ucs-normalize mule-util
term/common-win tool-bar dnd fontset image regexp-opt fringe
tabulated-list replace newcomment text-mode elisp-mode lisp-mode
prog-mode register page menu-bar rfn-eshadow isearch timer select
scroll-bar mouse jit-lock font-lock syntax facemenu font-core
term/tty-colors frame cl-generic cham georgian utf-8-lang misc-lang
vietnamese tibetan thai tai-viet lao korean japanese eucjp-ms cp51932
hebrew greek romanian slovak czech european ethiopic indian cyrillic
chinese composite charscript charprop case-table epa-hook jka-cmpr-hook
help simple abbrev obarray minibuffer cl-preloaded nadvice loaddefs
button faces cus-face macroexp files text-properties overlay sha1 md5
base64 format env code-pages mule custom widget hashtable-print-readable
backquote threads kqueue cocoa ns multi-tty make-network-process emacs)

Memory information:
((conses 16 56669 7188)
 (symbols 48 6625 1)
 (strings 32 17368 1666)
 (string-bytes 1 565596)
 (vectors 16 7835)
 (vector-slots 8 78808 10720)
 (floats 8 25 439)
 (intervals 56 860 4)
 (buffers 992 14))

[Message part 2 (text/html, inline)]

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Anders Lindgren <andlind <at> gmail.com>
Cc: 35695 <at> debbugs.gnu.org
Subject: Re: bug#35695: 27.0.50;
 Faces diff-added and diff-removed look the same in a 256 color
 terminal
Date: Sun, 12 May 2019 08:39:58 +0300

> From: Anders Lindgren <andlind <at> gmail.com>
> Date: Sun, 12 May 2019 07:03:41 +0200
> 
> In Emac 26, they were defined using #ddffdd and #ffdddd, respectively, which is rendered fine in the terminal
> (on both Emacs 26 and 27).
> 
> One way to solve this is to add alternative color definitions using the "min-colors" requirement. Another is to go
> back to the old values.

I think we should go to the back values when min-colors is 256 or
less.

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: andlind <at> gmail.com
Cc: 35695 <at> debbugs.gnu.org
Subject: Re: bug#35695: 27.0.50;
 Faces diff-added and diff-removed look the same in a 256 color
 terminal
Date: Sun, 12 May 2019 08:58:45 +0300

> Date: Sun, 12 May 2019 08:39:58 +0300
> From: Eli Zaretskii <eliz <at> gnu.org>
> Cc: 35695 <at> debbugs.gnu.org
> 
> I think we should go to the back values when min-colors is 256 or
> less.

I meant:

  I think we should go back to the old values when min-colors is 256
  or less.

Sorry.

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

From: Anders Lindgren <andlind <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 35695 <at> debbugs.gnu.org
Subject: Re: bug#35695: 27.0.50; Faces diff-added and diff-removed look the
 same in a 256 color terminal
Date: Sun, 12 May 2019 20:48:03 +0200

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

Hi!

On Sun, May 12, 2019 at 7:59 AM Eli Zaretskii <eliz <at> gnu.org> wrote:

>   I think we should go back to the old values when min-colors is 256
>   or less.
>

Sounds good to me.

I just gave the following a try. It seems to be working, but I'd like to
double check the details so that I haven't missed a face. (Dark mode isn't
affected, as far as I can see.)

diff --git a/lisp/vc/diff-mode.el b/lisp/vc/diff-mode.el

index a26e9ee..079806d 100644--- a/lisp/vc/diff-mode.el
+++ b/lisp/vc/diff-mode.el@@ -314,8 +314,10 @@ diff-hunk-header
(defface diff-removed
   '((default
      :inherit diff-changed)-    (((class color) (min-colors 88)
(background light))+    (((class color) (min-colors 257) (background
light))      :background "#ffeeee")+    (((class color) (min-colors
88) (background light))+     :background "#ffdddd")     (((class
color) (min-colors 88) (background dark))
      :background "#553333")
     (((class color))@@ -325,8 +327,10 @@ diff-removed (defface diff-added
   '((default
      :inherit diff-changed)-    (((class color) (min-colors 88)
(background light))+    (((class color) (min-colors 257) (background
light))      :background "#eeffee")+    (((class color) (min-colors
88) (background light))+     :background "#ddffdd")     (((class
color) (min-colors 88) (background dark))
      :background "#335533")
     (((class color))@@ -2040,8 +2044,10 @@ diff-refine-changed
(defface diff-refine-removed
   '((default
      :inherit diff-refine-changed)-    (((class color) (min-colors
88) (background light))+    (((class color) (min-colors 257)
(background light))      :background "#ffcccc")+    (((class color)
(min-colors 88) (background light))+     :background "#ffbbbb")
(((class color) (min-colors 88) (background dark))
      :background "#aa2222"))
   "Face used for removed characters shown by `diff-refine-hunk'."


    -- Anders Lindgren

[Message part 2 (text/html, inline)]

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

From: Juri Linkov <juri <at> linkov.net>
To: Anders Lindgren <andlind <at> gmail.com>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 35695 <at> debbugs.gnu.org
Subject: Re: bug#35695: 27.0.50;
 Faces diff-added and diff-removed look the same in a 256 color
 terminal
Date: Sun, 12 May 2019 22:24:48 +0300

>>   I think we should go back to the old values when min-colors is 256
>>   or less.
>
> Sounds good to me.
>
> I just gave the following a try. It seems to be working, but I'd like to
> double check the details so that I haven't missed a face. (Dark mode isn't
> affected, as far as I can see.)

Thanks, diff-refine-added needs old values for 288 colors as well.

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

From: Anders Lindgren <andlind <at> gmail.com>
To: Juri Linkov <juri <at> linkov.net>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 35695 <at> debbugs.gnu.org
Subject: Re: bug#35695: 27.0.50; Faces diff-added and diff-removed look the
 same in a 256 color terminal
Date: Mon, 13 May 2019 22:45:59 +0200

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

Hi!

On Sun, May 12, 2019 at 9:28 PM Juri Linkov <juri <at> linkov.net> wrote:

> Thanks, diff-refine-added needs old values for 288 colors as well.
>

I didn't include it since the output looked good with the new value.
However, for consistency, we should probably add one for it as well.

Should I include a comment describing why we need to treat 256 color
displays differently?

    -- Anders

[Message part 2 (text/html, inline)]

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

From: Juri Linkov <juri <at> linkov.net>
To: Anders Lindgren <andlind <at> gmail.com>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 35695 <at> debbugs.gnu.org
Subject: Re: bug#35695: 27.0.50;
 Faces diff-added and diff-removed look the same in a 256 color
 terminal
Date: Tue, 14 May 2019 23:06:18 +0300

>> Thanks, diff-refine-added needs old values for 288 colors as well.
>
> I didn't include it since the output looked good with the new value.
> However, for consistency, we should probably add one for it as well.
>
> Should I include a comment describing why we need to treat 256 color
> displays differently?

Yes, comments are preferable because we don't have deffaces
with more than 88 min-colors anywhere else.

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

From: Anders Lindgren <andlind <at> gmail.com>
To: Juri Linkov <juri <at> linkov.net>
Cc: 35695-done <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>,
 35695 <at> debbugs.gnu.org
Subject: Re: bug#35695: 27.0.50; Faces diff-added and diff-removed look the
 same in a 256 color terminal
Date: Wed, 15 May 2019 21:10:34 +0200

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

On Tue, May 14, 2019 at 10:36 PM Juri Linkov <juri <at> linkov.net> wrote:

> > Should I include a comment describing why we need to treat 256 color
> > displays differently?
>
> Yes, comments are preferable because we don't have deffaces
> with more than 88 min-colors anywhere else.
>

Thanks for the feedback!

I've pushed a patch fixing this issue.

    -- Anders

[Message part 2 (text/html, inline)]

Previous Next

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