bug#9353: 24.0.50; default grep file name face is uglier now

bug#9353: 24.0.50; default grep file name face is uglier now

[Drew Adams]

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

See attached images. The old face inherited from `font-lock-type-face',
and the appearace was OK (though not wonderful).
 
The new face inherits from a face called `success'. It is uhhhhgggly.
 
For one thing, it should not be bold.  We should almost never use a bold
face as the default face for something, because boldness does not work
well on some platforms and for some (many) faces.
 
I applaud not inheriting from a font-lock face: there is no logical
connection to a grepped file.  But there is also no logical connection
to "success" here.
 
Anyway, whatever you inherit from (but why must you always inherit?),
the default appearance should not be ugly and difficult to read.  That
just forces more users to customize it or live with the difficulty.

In GNU Emacs 24.0.50.1 (i386-mingw-nt5.1.2600)
 of 2011-08-22 on 3249CTO
Windowing system distributor `Microsoft Corp.', version 5.1.2600
configured using `configure --with-gcc (4.5) --no-opt'
 

[-- Attachment #2: throw-grep-file-old.png --]
[-- Type: image/png, Size: 7298 bytes --]

[-- Attachment #3: throw-grep-file-new.png --]
[-- Type: image/png, Size: 10047 bytes --]

bug#9353: 24.0.50; default grep file name face is uglier now

[Juri Linkov]

> See attached images. The old face inherited from `font-lock-type-face',
> and the appearace was OK (though not wonderful).
> The new face inherits from a face called `success'. It is uhhhhgggly.

FYI, old is new and new is old.  I copied the default value of `success'
from the old face definition of `compilation-info' that existed in Emacs 23
and older.  OTOH, colors of `font-lock-type-face' for grep are new
(6 months in the repository).

> For one thing, it should not be bold.  We should almost never use a bold
> face as the default face for something, because boldness does not work
> well on some platforms and for some (many) faces.

I agree, e.g. with my 10px fonts, bold is unreadable.
So I reset the bold attribute from all used faces.
Moreover, I customized this face to "ForestGreen",
i.e. to the color you are suggesting now.

So I agree it's better than the current face definition.
The only reason why I took the old value is because I thought
it's safer to use the old face no one complained about.

> I applaud not inheriting from a font-lock face: there is no logical
> connection to a grepped file.  But there is also no logical connection
> to "success" here.

I think if we'll copy the face definition from `font-lock-type-face'
to `success', the former should not inherit from the latter,
because they are not logically related.

bug#9353: 24.0.50; default grep file name face is uglier now

[Drew Adams]

Juri, sounds like we agree on all of this.  Thx - please close the bug if you've
already taken care of it.

bug#9353: 24.0.50; default grep file name face is uglier now

[Juri Linkov]

> Juri, sounds like we agree on all of this.  Thx - please close the bug
> if you've already taken care of it.

Sorry, I have no intention (inclination, incentive) to remove the bold
attribute from the default value of this face, because there are many
other faces that use bold, and bold serves its purpose here (with default
large fonts).

Maybe, we should provide a special custom theme without bold attributes
for non-bold users?

OTOH, changing its color from "Green3" to "ForestGreen" looks like
a good change that makes it more readable (with default large fonts).

bug#9353: 24.0.50; default grep file name face is uglier now

[Stefan Monnier]

> OTOH, changing its color from "Green3" to "ForestGreen" looks like
> a good change that makes it more readable (with default large fonts).

I don't mind, so feel free to make this change.


        Stefan

bug#9353: 24.0.50; default grep file name face is uglier now

[Juri Linkov]

>> OTOH, changing its color from "Green3" to "ForestGreen" looks like
>> a good change that makes it more readable (with default large fonts).
>
> I don't mind, so feel free to make this change.

I've changed the color without removing boldness, and tested that
objectively it looks good with other faces in etc/compilation.txt
and in the compilation/grep mode-line with default large fonts.

bug#9353: 24.0.50; default grep file name face is uglier now

[Stefan Monnier]

> For one thing, it should not be bold.  We should almost never use a bold
> face as the default face for something, because boldness does not work
> well on some platforms and for some (many) faces.

I tend to agree, tho in this case part of the intention of grep-mode is
to make this part of the text less important since you don't need to
visit the file by hand but can just click on the link instead.

At least I think that's the only reason why noone complained about those
faces until now, even though they've been in use at least since Emacs-23.1.

> But there is also no logical connection to "success" here.

I don't know what's the connection either.  Maybe it's a "match success".

> Anyway, whatever you inherit from (but why must you always inherit?),

It's good to provide structure within the space of faces, so inheritance
is an important tool that should be used whenever possible, but of
course, we need to be careful to only use it when it really makes sense.
AFAICT the `success' face was created pretty much for this, so maybe the
problem is not the inheritance link but the name of the `success' face.

> the default appearance should not be ugly and difficult to read.  That
> just forces more users to customize it or live with the difficulty.

The default font I use is readable in bold, because giving up on such
a useful tool would be too bad (remember that I prefer to use the same
color everywhere and distinguish things based on bold/italics/font), but
that's just me.

I'm not opposed to tweaks for this face, tho a bright bold green seems
to work well in M-x compile (at least for my setup), so please consider
the impact on M-x compile when making those tweaks.


        Stefan

bug#9353: 24.0.50; default grep file name face is uglier now

[Juri Linkov]

> I don't know what's the connection either.  Maybe it's a "match success".

Yes, in the context of compilation messages, it means a "successful match".

> The default font I use is readable in bold, because giving up on such
> a useful tool would be too bad (remember that I prefer to use the same
> color everywhere and distinguish things based on bold/italics/font), but
> that's just me.

IIUC, the only Drew's objection is the face's color, not boldness,
because he uses a dim background.  So we could change the color to
a darker green, that will also look better on the default light background.

bug#9353: 24.0.50; default grep file name face is uglier now

[Drew Adams]

> > The default font I use is readable in bold, because giving 
> > up on such a useful tool would be too bad (remember that I
> > prefer to use the same color everywhere and distinguish
> > things based on bold/italics/font), but
> > that's just me.
> 
> IIUC, the only Drew's objection is the face's color, not boldness,
> because he uses a dim background.  So we could change the color to
> a darker green, that will also look better on the default 
> light background.

No, actually I did & still do object to bold being part of the default
definition.  I said:

 For one thing, it should not be bold.  We should almost never
 use a bold face as the default face for something, because
  boldness does not work well on some platforms and for some
 (many) faces.

(And it is not about appearance against my background - I was just too lazy to
make screenshots with Emacs -Q.)

I won't argue that the bug should not be closed if you refuse to get rid of the
bold part, but I think bold is a mistake for a default face - anywhere in Emacs.

And yes, I know that we have other default faces that also use bold.  That is a
mistake too, IMHO.  At least on Windows, even with the default emacs -Q font, I
think bold is not very legible.

I understand Stefan's personal choice to privilege bold, italic etc. instead of
colors.  But that's suitable as a personal customization, IMO, not as the
default appearance.

To me, the bold fonts I see are so bad that it reminds me of pre-anti-aliasing
Emacs (Emacs 21).  I remember some people arguing that the anti-aliased
appearance was worse and that we should keep the Emacs 21 appearance.  To each
his own...

No, I don't expect to convince you about this.  Thanks for changing the color,
anyway.