bug#6693: 24.0.50; font-lock-(builtin|doc) faces are *way* too close

bug#6693: 24.0.50; font-lock-(builtin|doc) faces are *way* too close

[Drew Adams]

emacs -Q

In GNU Emacs 24.0.50.1 (i386-mingw-nt5.1.2600)
 of 2010-07-19 on 3249CTO
Windowing system distributor `Microsoft Corp.', version 5.1.2600
configured using `configure --with-gcc (4.4) --no-opt --cflags -Ic:/xpm/include'
 
Look at any `defcustom'.  The doc string and the keywords (e.g. :type)
appear to have the same face.  In fact, they are only very slightly
different.
 
This is a regression wrt 23.1 (and prior).  Please revert this mistake.
There is no reason for these two faces to be so close.
 
Previously, `font-lock-builtin-face' had an Orchid foreground.  That was
*much* better than having two faces that are essentially
indistinguishable.

bug#6693: 24.0.50; font-lock-(builtin|doc) faces are *way* too close

[Lars Magne Ingebrigtsen]

"Drew Adams" <drew.adams@oracle.com> writes:

> Look at any `defcustom'.  The doc string and the keywords (e.g. :type)
> appear to have the same face.  In fact, they are only very slightly
> different.
>
> This is a regression wrt 23.1 (and prior).  Please revert this mistake.
> There is no reason for these two faces to be so close.
>
> Previously, `font-lock-builtin-face' had an Orchid foreground.  That was
> *much* better than having two faces that are essentially
> indistinguishable.

This is the definition of `font-lock-builtin-face'.  Which of these
cases is causing the confusion for you?

(defface font-lock-builtin-face
  '((((class grayscale) (background light)) (:foreground "LightGray" :weight bold))
    (((class grayscale) (background dark)) (:foreground "DimGray" :weight bold))
    (((class color) (min-colors 88) (background light)) (:foreground "MediumOrchid4"))
    (((class color) (min-colors 88) (background dark)) (:foreground "LightSteelBlue"))
    (((class color) (min-colors 16) (background light)) (:foreground "Orchid"))
    (((class color) (min-colors 16) (background dark)) (:foreground "LightSteelBlue"))
    (((class color) (min-colors 8)) (:foreground "blue" :weight bold))
    (t (:weight bold)))


-- 
(domestic pets only, the antidote for overdose, milk.)
  bloggy blog http://lars.ingebrigtsen.no/

bug#6693: 24.0.50; font-lock-(builtin|doc) faces are *way* too close

[Drew Adams]

> This is the definition of `font-lock-builtin-face'.  Which of these
> cases is causing the confusion for you?
> 
> (defface font-lock-builtin-face
>   '((((class grayscale) (background light)) (:foreground 
> "LightGray" :weight bold))
>     (((class grayscale) (background dark)) (:foreground 
> "DimGray" :weight bold))
>     (((class color) (min-colors 88) (background light)) 
> (:foreground "MediumOrchid4"))
>     (((class color) (min-colors 88) (background dark)) 
> (:foreground "LightSteelBlue"))
>     (((class color) (min-colors 16) (background light)) 
> (:foreground "Orchid"))
>     (((class color) (min-colors 16) (background dark)) 
> (:foreground "LightSteelBlue"))
>     (((class color) (min-colors 8)) (:foreground "blue" :weight bold))
>     (t (:weight bold)))

I guess it would be this one, since that is what emacs -Q gives on my machine, I
assume (light background, many colors):

(((class color) (min-colors 88) (background light)) 
  (:foreground "MediumOrchid4"))

As I said, previously the value was "Orchid", and that was better.

bug#6693: 24.0.50; font-lock-(builtin|doc) faces are *way* too close

[Lars Magne Ingebrigtsen]

"Drew Adams" <drew.adams@oracle.com> writes:

> I guess it would be this one, since that is what emacs -Q gives on my machine, I
> assume (light background, many colors):
>
> (((class color) (min-colors 88) (background light)) 
>   (:foreground "MediumOrchid4"))
>
> As I said, previously the value was "Orchid", and that was better.

The commit message for the change from Orchid to MediumOrchid4 is:

=== modified file 'lisp/ChangeLog'
*** lisp/ChangeLog	2009-08-01 16:21:45 +0000
--- lisp/ChangeLog	2009-08-02 14:20:02 +0000
***************
*** 1,3 ****
--- 1,9 ----
+ 2009-08-02  Chong Yidong  <cyd@stupidchicken.com>
+ 
+ 	* font-lock.el (font-lock-string-face, font-lock-builtin-face)
+ 	(font-lock-variable-name-face, font-lock-constant-face): Darken
+ 	the colors for light backgrounds.
+ 

So presumably some people had difficulty reading Orchid on light
backgrounds?

Perhaps a different colour could be used in your use case that would
still be legible, but separate the doc string from the keywords.  Do you
have a suggestion?

-- 
(domestic pets only, the antidote for overdose, milk.)
  bloggy blog http://lars.ingebrigtsen.no/

bug#6693: 24.0.50; font-lock-(builtin|doc) faces are *way* too close

[Drew Adams]

> The commit message for the change from Orchid to MediumOrchid4 is:
> 
> + 2009-08-02  Chong Yidong  <cyd@stupidchicken.com>
> ... Darken the colors for light backgrounds.
> 
> So presumably some people had difficulty reading Orchid on light
> backgrounds?

Yes, I'm familiar with that commit.  I disagree that it represents an
improvement, which is why I filed the bug report.

> Perhaps a different colour could be used in your use case that would
> still be legible, but separate the doc string from the keywords.
> Do you have a suggestion?

I suggest "Orchid", as before.  But I would probably be OK with any other
solution that distinguishes these two faces better without introducing other
such clashes.

And this is _not_ about my use case.  This is about the Emacs default, -Q, case,
as expressed in the bug report, which starts with "emacs -Q".  Personally, I use
a very different color scheme - this has nothing to do with my use case.

Just _look_ at the colors, using emacs -Q.  Do the same in an older release
also, to compare.  You should be able to see the problem clearly.

bug#6693: 24.0.50; font-lock-(builtin|doc) faces are *way* too close

[Lars Magne Ingebrigtsen]

"Drew Adams" <drew.adams@oracle.com> writes:

> Just _look_ at the colors, using emacs -Q.  Do the same in an older
> release also, to compare.  You should be able to see the problem
> clearly.

I always use a black background, so I don't see this bug personally.  

-- 
(domestic pets only, the antidote for overdose, milk.)
  bloggy blog http://lars.ingebrigtsen.no/

bug#6693: 24.0.50; font-lock-(builtin|doc) faces are *way* too close

[Drew Adams]

> > Just _look_ at the colors, using emacs -Q.  Do the same in an older
> > release also, to compare.  You should be able to see the problem
> > clearly.
> 
> I always use a black background, so I don't see this bug personally.

The bug is not about personal use.  It is about emacs -Q. 

On my platform, emacs -Q uses a white background.  From the emacs -Q
`frame-parameters':

 (background-mode . light)
 (background-color . "white")

bug#6693: 24.0.50; font-lock-(builtin|doc) faces are *way* too close

[David De La Harpe Golden]

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

On 03/07/11 15:34, Drew Adams wrote:

> Just _look_ at the colors, using emacs -Q.  Do the same in an older release
> also, to compare.  You should be able to see the problem clearly.
>
>

Computer systems vary quite a bit in their color reproduction, and 
different people can have different color vision, and not just in a 
"complete red/green colorblindness" fashion either [1].

I'd say we absolutely should try to keep the emacs out-of-box color 
scheme colorblind-safe, so some people having problems _is_ a reason to 
change the scheme.  It's just an assumption that everyone is seeing what 
you see may be faulty. font-lock-doc (#8b2252 or so) definitely still 
looks pretty different to font-lock-builtin (#7a378b or so) on my 
system. Are the colors actually used on your system even coming out as 
similar hex values to those?

See also samples in old emacs-devel thread [2]

[1] http://en.wikipedia.org/wiki/Color_blindness#Anomalous_trichromacy

[2]
http://lists.gnu.org/archive/html/emacs-devel/2009-07/msg01582.html
http://lists.gnu.org/archive/html/emacs-devel/2009-07/msg01588.html

[-- Attachment #2: emacs_builtin_vs_doc.png --]
[-- Type: image/png, Size: 27260 bytes --]

bug#6693: 24.0.50; font-lock-(builtin|doc) faces are *way* too close

[Chong Yidong]

David De La Harpe Golden <david@harpegolden.net> writes:

> I'd say we absolutely should try to keep the emacs out-of-box color
> scheme colorblind-safe

That is hard to get right; a color-blind safe palette inherently
conflicts with the goal of making face colors as distinguishable as
possible for the non-color-blind.  Emacs 24 has a custom theme
(dichromacy) that implements a palette specifically optimized for
dichromatic color blindness.

As for font-lock-builtin-face, changing it back to Orchid is no good;
that shade is far too light, at least on the LCD monitors I've used
Emacs on.  I wouldn't mind changing it to something completely different
though, e.g. midnight blue.  We have too may purplish colors in the
default palette anyway.

bug#6693: 24.0.50; font-lock-(builtin|doc) faces are *way* too close

[Drew Adams]

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

> font-lock-doc (#8b2252 or so) definitely still 
> looks pretty different to font-lock-builtin (#7a378b or so) on my 
> system.

See attached.  I'd say that three of the faces shown are quite close in
appearance: 

`font-lock-builtin-face' (":type", ":group")
`font-lock-doc-face'     (the doc strings)
`font-lock-keyword-face' ("defcustom")

But especially the first two of these are close.

The point was also that previously these faces were better distinguished -
`font-lock-builtin-face' was `Orchid', not `MediumOrchid4'.

> Are the colors actually used on your system even 
> coming out as similar hex values to those?

Those same hex values are used on my system.  Those are the hex values
corresponding to the two named colors used: VioletRed4 and MediumOrchid4.

[-- Attachment #2: throw-emacs-default-colors-too-close.gif --]
[-- Type: image/gif, Size: 8776 bytes --]

bug#6693: 24.0.50; font-lock-(builtin|doc) faces are *way* too close

[Drew Adams]

> As for font-lock-builtin-face, changing it back to Orchid is no good;
> that shade is far too light, at least on the LCD monitors I've used
> Emacs on.

FWIW, I disagree (on the LCD monitors I've used).

> I wouldn't mind changing it to something completely different
> though, e.g. midnight blue.  We have too may purplish colors in the
> default palette anyway.

A priori I have nothing against changing it to something completely different.
Dunno about midnight blue.

I mainly see font-lock with Emacs-Lisp code (in my use), and ther is nothing
similar to midnight blue there.  On the other hand, midnight blue is not too
different from black (the default foreground).  `list-faces-display' shows
nothing super close to midnight blue (and nothing close at all in the font-lock
faces).  But it looks like the medium blue of `minibuffer-prompt-face' is
already quite dark and not too easily distinguished from black.  My guess is
that midnight blue would be too dark (too close to black).

I suggest that you do your best to pick another color, if you disagree with
`Orchid', and that people try it for a while to see (discuss).

This is obviously not the most important bug.  But if we can better separate
these two faces that would be good.

Note that (at least in Emacs-Lisp code) this face is used for coloring only
keywords: `:type', `:group' etc.  It is not for coloring large spans of text
that someone needs to read.  The point in coloring the keywords is just to draw
attention - they do not need to be super-readable (as does a doc string, for
instance).  That's another reason I don't think a face like `Orchid' was a bad
choice.

bug#6693: 24.0.50; font-lock-(builtin|doc) faces are *way* too close

[Chong Yidong]

"Drew Adams" <drew.adams@oracle.com> writes:

> I mainly see font-lock with Emacs-Lisp code (in my use), and ther is
> nothing similar to midnight blue there.  On the other hand, midnight
> blue is not too different from black (the default foreground).

It is maybe a bit too dark.  So maybe something like royal blue is
preferable.

> Note that (at least in Emacs-Lisp code) this face is used for coloring
> only keywords: `:type', `:group' etc.  It is not for coloring large
> spans of text that someone needs to read.

There is no guarantee that this is always true, particularly since many
faces inherit from font lock faces.  So it is important for all the font
lock faces to be easily distinguished from the background.

bug#6693: 24.0.50; font-lock-(builtin|doc) faces are *way* too close

[David De La Harpe Golden]

On 04/07/11 06:02, Drew Adams wrote:
> `font-lock-builtin-face' (":type", ":group")
> `font-lock-doc-face'     (the doc strings)
> `font-lock-keyword-face' ("defcustom")
>
> But especially the first two of these are close.

Hmm. Looking at your screenshot reminded me: but then what about 
font-lock-comment-face (Firebrick)? Do you also see it as a bit too 
close to font-lock-doc-face (VioletRed4), but from the other direction 
(on the hue wheel)?

bug#6693: 24.0.50; font-lock-(builtin|doc) faces are *way* too close

[Drew Adams]

> > `font-lock-builtin-face' (":type", ":group")
> > `font-lock-doc-face'     (the doc strings)
> > `font-lock-keyword-face' ("defcustom")
> >
> > But especially the first two of these are close.
> 
> Hmm. Looking at your screenshot reminded me: but then what about 
> font-lock-comment-face (Firebrick)? Do you also see it as a bit too 
> close to font-lock-doc-face (VioletRed4), but from the other 
> direction (on the hue wheel)?

All three of these are close: 

font-lock-variable-name-face
font-lock-doc-face
font-lock-comment-face

However, I personally am not bothered by those similarities.

Again, personally I see font-lock mostly with Emacs-Lisp code, so I might not be
the best one to ask.

There is almost never any problem confusing a comment with a string (in Emacs
Lisp).  I suppose that a string and a comment on the same line might be
confusable, but in practice I don't think there is a problem here (worth trying
to fix, possibly messing up other things).

Similarly, I don't see a significant problem from the similarity between
`font-lock-variable-name-face' and the others, because of context.

Yidong makes the argument (essentially) that we should not take context into
consideration, since faces can inherit from the font-lock faces.  IMHO, that is
rather a problem with inheriting faces (esp. inheriting willy nilly), but we've
been through that discussion before and I know that I will not be able to
convince you (pl.) about that.

Wrt `font-lock-builtin-face', I say go for it: make some change and see what
happens.  My guess is that medium blue would be better than royal blue, but do
what you think is best.

I'm pretty much done here.  I mainly wanted to draw your attention to the
problem, reporting the color similarity for built-in and doc (and my personal
preference for the previous situation (Orchid)).  Fix the problem as you see
fit, and we'll see then whether anyone has a better approach etc.

I should probably have said that a second problem I have with the MediumViolet4
choice for the builtin face is that it does not stand out from black - IOW, it
is not sufficiently noticeable, for keywords.  That's no doubt another reason
why I preferred Orchid, and a reason why my argument about differing contexts
doesn't mitigate the problem here.

In sum (for the second problem): builtin doesn't stand out enough, and I don't
(personally) care whether it is "too light" (e.g. Orchid).  What's important for
something like keywords is that they stand out.

bug#6693: 24.0.50; font-lock-(builtin|doc) faces are *way* too close

[Stefan Monnier]

> I'd say we absolutely should try to keep the emacs out-of-box color scheme
> colorblind-safe, so some people having problems _is_ a reason to change the
> scheme.

But there are 2 issues:
1- make sure that default faces have a foreground that combines well
   with its background.  I.e. each individual char should be legible
   even for colorblind users.
2- try to use visually different appearances for different font-lock faces.

The first issue is very important (e.g. a requirement to enable
font-lock by default).  The second is very secondary in comparison.


        Stefan "and note that I carefully talk about the appearance of
                the faces, and not their color: most of my faces use the
                default foreground and background color and are
                distinguished by other attributes"

bug#6693: 24.0.50; font-lock-(builtin|doc) faces are *way* too close

[Lars Magne Ingebrigtsen]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

> 1- make sure that default faces have a foreground that combines well
>    with its background.  I.e. each individual char should be legible
>    even for colorblind users.

Yes, that's important.  But I don't think we have much of a problem in
that area at the moment?  Putting red text on a green background (or
vice versa) isn't very nice even for non-colourblind people.  :-)

> 2- try to use visually different appearances for different font-lock faces.
>
> The first issue is very important (e.g. a requirement to enable
> font-lock by default).  The second is very secondary in comparison.

In this specific instance, I think changing the orchid-ish colour to
MediumBlue (or similar) would be a good change, so I've now committed
that.

-- 
(domestic pets only, the antidote for overdose, milk.)
  bloggy blog http://lars.ingebrigtsen.no/

bug#6693: 24.0.50; font-lock-(builtin|doc) faces are *way* too close

[Chong Yidong]

Lars Magne Ingebrigtsen <larsi@gnus.org> writes:

> In this specific instance, I think changing the orchid-ish colour to
> MediumBlue (or similar) would be a good change, so I've now committed
> that.

You ended up changing the color definition on 16-color displays, which
was probably not your intention.  Shortly before, I had already changed
the color on high-color displays to dark slate blue, so I went ahead and
reverted your change.

bug#6693: 24.0.50; font-lock-(builtin|doc) faces are *way* too close

[Lars Magne Ingebrigtsen]

Chong Yidong <cyd@stupidchicken.com> writes:

> You ended up changing the color definition on 16-color displays, which
> was probably not your intention.  Shortly before, I had already changed
> the color on high-color displays to dark slate blue, so I went ahead and
> reverted your change.

Oops; sorry.

-- 
(domestic pets only, the antidote for overdose, milk.)
  bloggy blog http://lars.ingebrigtsen.no/