About the :distant-foreground face attribute

About the :distant-foreground face attribute

[Chong Yidong]

What is the purpose of this face attribute, newly introduced for 24.4?
It seems to be unused in Emacs itself.  Is there a concrete example of
this being needed in external packages or themes?

First of all, the name :distant-foreground is not intuitive.  What does
"distant" mean in this context?

Also, this feature has one ugly consequence.  Previously, the `default'
face must have all its face attributes specified, but now its
:distant-foreground face is unspecified.

Besides that, the implementation seems rather incomplete.  The Customize
interface, `modify-face', `face-spec-reset-face', and other parts of
Emacs haven't been updated for the existence of this face attribute.
It's unclear what functions like `face-foreground' should now do if
there's a :distant-foreground.

This all sounds like an invitation for more bugs.  In my opinion, this
feature is a bad idea.

Re: About the :distant-foreground face attribute

[Jan Djärv]

Hello.

7 jan 2014 kl. 13:55 skrev Chong Yidong <cyd@gnu.org>:

> What is the purpose of this face attribute, newly introduced for 24.4?

Too lazy to read documentation?

"`:distant-foreground'
     Alternative foreground color, a string.  This is like `:foreground'
     but the color is only used as a foreground when the background
     color is near to the foreground that would have been used.  This
     is useful for example when marking text (i.e. the region face).
     If the text has a foreground that is visible with the region face,
     that foreground is used.  If the foreground is near the region
     face background, `:distant-foreground' is used instead so the text
     is readable.
"

> It seems to be unused in Emacs itself.  Is there a concrete example of
> this being needed in external packages or themes?

Too lazy too read the code?

faces.el:

(defface region
  '((((class color) (min-colors 88) (background dark))
     :background "blue3")
    (((class color) (min-colors 88) (background light) (type gtk))
     :distant-foreground "gtk_selection_fg_color"
     :background "gtk_selection_bg_color")
    (((class color) (min-colors 88) (background light) (type ns))
     :distant-foreground "ns_selection_fg_color"
     :background "ns_selection_bg_color")
    (((class color) (min-colors 88) (background light))
     :background "lightgoldenrod2")
    (((class color) (min-colors 16) (background dark))
     :background "blue3")
    (((class color) (min-colors 16) (background light))
     :background "lightgoldenrod2")
    (((class color) (min-colors 8))
     :background "blue" :foreground "white")
    (((type tty) (class mono))
     :inverse-video t)
    (t :background "gray"))
  "Basic face for highlighting the region."
  :version "21.1"
  :group 'basic-faces)

> 
> First of all, the name :distant-foreground is not intuitive.  What does
> "distant" mean in this context?
> 

The same as in any other context, far removed from something else, i.e. the background.

> Also, this feature has one ugly consequence.  Previously, the `default'
> face must have all its face attributes specified, but now its
> :distant-foreground face is unspecified.

Default face does not need to have its font specified, so this is not new.

> 
> Besides that, the implementation seems rather incomplete.  The Customize
> interface, `modify-face', `face-spec-reset-face', and other parts of
> Emacs haven't been updated for the existence of this face attribute.

That is on purpose.  It is supposed to be automatically calculated.

> It's unclear what functions like `face-foreground' should now do if
> there's a :distant-foreground.
> 

No it is not.

> This all sounds like an invitation for more bugs.  In my opinion, this
> feature is a bad idea.

All new features invite new bugs, so are you saying we should never add new features?
Your opinion is not very interesting, but if you have core for an alternative approach that would be interesting.

	Jan D.

RE: About the :distant-foreground face attribute

[Drew Adams]

> > What is the purpose of this face attribute, newly introduced for
> > 24.4?
> 
> Too lazy to read documentation?
> 
> "`:distant-foreground'
>      Alternative foreground color, a string.  This is like
>      `:foreground'
>      but the color is only used as a foreground when the background
>      color is near to the foreground that would have been used.
>      This is useful for example when marking text (i.e. the region
>      face).
>      If the text has a foreground that is visible with the region
>      face, that foreground is used.  If the foreground is near the
>      region face background, `:distant-foreground' is used instead
>      so the text is readable."

I see.  And just how does a user override this automatic "cleverness"?
How can a user really make her preference for the face take effect?

This kind of DWIMity should always allow users to take control back,
and preferably easily.  What you think might be best for all users
all the time might not be what some user thinks is best for herself.

And what about all of the existing code that tests :foreground or
otherwise expects it to reflect the actual foreground used?  Is that
code broken now, because Emacs substitutes a :distant-foreground
color behind its back?  Must all such code now change to test first
one and then the other?

When was this design OK'd (and why)?

> > First of all, the name :distant-foreground is not intuitive.  What
> > does "distant" mean in this context?
> 
> The same as in any other context, far removed from something else,
> i.e. the background.

I agree with Yidong about the name.  This is apparently a fallback,
alternative color, when (you) think the color specified by the
user or program is inappropriate.

> > Besides that, the implementation seems rather incomplete.  The
> > Customize interface, `modify-face', `face-spec-reset-face', and
> > other parts of Emacs haven't been updated for the existence of
> > this face attribute.
> 
> That is on purpose.  It is supposed to be automatically calculated.

So what?  All the more reason to bring it out into the open, so
users can know about it (and try to find some way to work around it,
if they like).

> > It's unclear what functions like `face-foreground' should now do
> > if there's a :distant-foreground.
> 
> No it is not.

Yes it is.  See above.  Search the distributed code and the Internet
for uses of "foreground" in Emacs Lisp code.  How much of that code
now needs to be modified to accommodate this gratuitous change.

Was there a real problem, reported by a real user, that this change
attempts to fix?  Or is this just someone's clever brainchild for
making Emacs smarter?

> > This all sounds like an invitation for more bugs.  In my opinion,
> > this feature is a bad idea.

+1

> All new features invite new bugs, so are you saying we should never
> add new features?

All new features should be proposed and discussed, before being cast
into the product.  That's my humble opinion.

> Your opinion is not very interesting, but if you have core for an
> alternative approach that would be interesting.

Why is any approach needed?  What is the (real) problem that needs
solving?

Re: About the :distant-foreground face attribute

[Eli Zaretskii]

> Date: Tue, 7 Jan 2014 09:28:05 -0800 (PST)
> From: Drew Adams <drew.adams@oracle.com>
> Cc: emacs-devel <emacs-devel@gnu.org>
> 
> All new features should be proposed and discussed, before being cast
> into the product.

This one was.  It solved a bug.

> Why is any approach needed?  What is the (real) problem that needs
> solving?

See http://debbugs.gnu.org/cgi/bugreport.cgi?bug=15668.

Re: About the :distant-foreground face attribute

[Joel Mccracken]

It seems like "contrast" could be used to explain the difference in a more intuitive way. 

On Jan 7, 2014, at 12:28 PM, Drew Adams <drew.adams@oracle.com> wrote:

>>> What is the purpose of this face attribute, newly introduced for
>>> 24.4?
>> 
>> Too lazy to read documentation?
>> 
>> "`:distant-foreground'
>>     Alternative foreground color, a string.  This is like
>>     `:foreground'
>>     but the color is only used as a foreground when the background
>>     color is near to the foreground that would have been used.
>>     This is useful for example when marking text (i.e. the region
>>     face).
>>     If the text has a foreground that is visible with the region
>>     face, that foreground is used.  If the foreground is near the
>>     region face background, `:distant-foreground' is used instead
>>     so the text is readable."
> 
> I see.  And just how does a user override this automatic "cleverness"?
> How can a user really make her preference for the face take effect?
> 
> This kind of DWIMity should always allow users to take control back,
> and preferably easily.  What you think might be best for all users
> all the time might not be what some user thinks is best for herself.
> 
> And what about all of the existing code that tests :foreground or
> otherwise expects it to reflect the actual foreground used?  Is that
> code broken now, because Emacs substitutes a :distant-foreground
> color behind its back?  Must all such code now change to test first
> one and then the other?
> 
> When was this design OK'd (and why)?
> 
>>> First of all, the name :distant-foreground is not intuitive.  What
>>> does "distant" mean in this context?
>> 
>> The same as in any other context, far removed from something else,
>> i.e. the background.
> 
> I agree with Yidong about the name.  This is apparently a fallback,
> alternative color, when (you) think the color specified by the
> user or program is inappropriate.
> 
>>> Besides that, the implementation seems rather incomplete.  The
>>> Customize interface, `modify-face', `face-spec-reset-face', and
>>> other parts of Emacs haven't been updated for the existence of
>>> this face attribute.
>> 
>> That is on purpose.  It is supposed to be automatically calculated.
> 
> So what?  All the more reason to bring it out into the open, so
> users can know about it (and try to find some way to work around it,
> if they like).
> 
>>> It's unclear what functions like `face-foreground' should now do
>>> if there's a :distant-foreground.
>> 
>> No it is not.
> 
> Yes it is.  See above.  Search the distributed code and the Internet
> for uses of "foreground" in Emacs Lisp code.  How much of that code
> now needs to be modified to accommodate this gratuitous change.
> 
> Was there a real problem, reported by a real user, that this change
> attempts to fix?  Or is this just someone's clever brainchild for
> making Emacs smarter?
> 
>>> This all sounds like an invitation for more bugs.  In my opinion,
>>> this feature is a bad idea.
> 
> +1
> 
>> All new features invite new bugs, so are you saying we should never
>> add new features?
> 
> All new features should be proposed and discussed, before being cast
> into the product.  That's my humble opinion.
> 
>> Your opinion is not very interesting, but if you have core for an
>> alternative approach that would be interesting.
> 
> Why is any approach needed?  What is the (real) problem that needs
> solving?
> 

RE: About the :distant-foreground face attribute

[Drew Adams]

> > All new features should be proposed and discussed, before being
> > cast into the product.  That's my humble opinion.
> 
> This one was.  It solved a bug.

The new feature was not proposed on emacs-devel and discussed.
It was dreamt up in passing, in one short bug thread, and discussed
there by only two developers.

> > Why is any approach needed?  What is the (real) problem that needs
> > solving?
> 
> See http://debbugs.gnu.org/cgi/bugreport.cgi?bug=15668.

So the real problem was to have a reasonable background color for
face `region' by default.  On some platforms a bad color choice
was (and apparently still is) being used as the default.

That problem does not require this particular "fix".  Even that
bug thread shows that this was thought to be a nice-to-have new
feature that could also take care of this bug.  It is not needed,
to fix the bug.

  "It would be nice if one could define a face with a foreground
   color to be used when foreground and background otherwise are
   to[o] similar."

And from that dream onward to this one (still unimplemented,
I hope):

>>> It would be nice if...
>>
>> Sounds like a simple enough extension to defface.
>
> Or maybe every face should do that by default.

IMO, the implementation, and probably the feature itself, is not
TRT.

Note, BTW, that even the OP had to back you off the first
enthusiastic fix, so that he could still customize normally and
theme designers would have a simpler time of it.

> I envisioned that querying the face for the foreground would
> automatically give the "fallback"...

At least that was thought of.  But is that the case for all code
that "queries" a face :foreground or modifies it?  Is all such
code somehow automatically adjusted now so that it always DTRT?
I don't see how that would be the case.

It seems that for one tiny piece of the distributed Emacs code
that dream behavior was implemented.  The problem is all other
code that tries to use the :foreground, which attribute might
not even be respected (unused, because of a DWIM substitution).

Such code thinks that it is getting or changing the foreground
color by dealing with just :foreground.  But it is wrong,
because a clever DWIM feature went behind its back.  To DTRT,
it will now have to jump through extra hoops.

Re: About the :distant-foreground face attribute

[Eli Zaretskii]

> Date: Tue, 7 Jan 2014 10:44:26 -0800 (PST)
> From: Drew Adams <drew.adams@oracle.com>
> Cc: jan.h.d@swipnet.se, cyd@gnu.org, emacs-devel@gnu.org
> 
> > > All new features should be proposed and discussed, before being
> > > cast into the product.  That's my humble opinion.
> > 
> > This one was.  It solved a bug.
> 
> The new feature was not proposed on emacs-devel and discussed.

emacs-devel is not the only place where Emacs development is
discussed.  The bug list and the bug reports are another.

> IMO, the implementation, and probably the feature itself, is not
> TRT.

Sorry, I disagree.  I think it's exactly right.

RE: About the :distant-foreground face attribute

[Drew Adams]

> > IMO, the implementation, and probably the feature itself, is not
> > TRT.
> 
> Sorry, I disagree.  I think it's exactly right.

And what do you say about code that expects that a face's
`foreground' attribute actually reflects the face's current
foreground appearance?  Silence on that question, so far.

When examining attribute `foreground' tells you nothing about
the foreground, and changing the attribute value turns silently
into a no-op, I'd say that something basic has been broken.

Re: About the :distant-foreground face attribute

[Eli Zaretskii]

> Date: Tue, 7 Jan 2014 11:06:15 -0800 (PST)
> From: Drew Adams <drew.adams@oracle.com>
> Cc: jan.h.d@swipnet.se, cyd@gnu.org, emacs-devel@gnu.org
> 
> > > IMO, the implementation, and probably the feature itself, is not
> > > TRT.
> > 
> > Sorry, I disagree.  I think it's exactly right.
> 
> And what do you say about code that expects that a face's
> `foreground' attribute actually reflects the face's current
> foreground appearance?

I don't understand the issue, but regardless, the fact that bugs exist
(if they exist) doesn't mean the design and the general implementation
are flawed.

Re: About the :distant-foreground face attribute

[Chong Yidong]

Jan Djärv <jan.h.d@swipnet.se> writes:

> Too lazy to read documentation?
>
> "`:distant-foreground'
>      Alternative foreground color, a string.  This is like `:foreground'
>      but the color is only used as a foreground when the background
>      color is near to the foreground that would have been used.  This
>      is useful for example when marking text (i.e. the region face).
>      If the text has a foreground that is visible with the region face,
>      that foreground is used.  If the foreground is near the region
>      face background, `:distant-foreground' is used instead so the text
>      is readable.

I read that, but the explanation was pretty darn cryptic.  But OK: from
your message, I gather that this feature was introduced to deal with the
problem which setting the `region' face from system (GTK or NS)
settings.

But is there any other even vaguely envisioned usage?  Because, as
explained before, there are better ways to deal with this problem than
something so intrusive as defining a new face attribute.

>> First of all, the name :distant-foreground is not intuitive.  What does
>> "distant" mean in this context?
>> 
>
> The same as in any other context, far removed from something else,
> i.e. the background.

Things that are "distant" are in the BACKGROUND, so "distant foreground"
sounds tautological.  The fact that there isn't a good name for this
face attribute is an indicator that it's not well-conceived.

The feature does not fit well with the design of the rest of the
face-handling code.  We already have a mechanism for checking to see
when to use a particular face: the DISPLAY element in a face spec.  The
:distant-foreground face attribute, by its very existence, is redundant
with what the DISPLAY element was meant to do.  This adds extra
complexity to the design, for no good reason.

If you need to check for a background, the right thing would be to
extend the DISPLAY feature to do what you need.  For example, a DISPLAY
element of `(background dark)' is used to test for a dark background.
Maybe what you want is to be able to specify a color in place of `dark',
which would mean a background close to that color.

>> Also, this feature has one ugly consequence.  Previously, the `default'
>> face must have all its face attributes specified, but now its
>> :distant-foreground face is unspecified.
>
> Default face does not need to have its font specified, so this is not
> new.

M-: (face-attribute 'default :font) RET
  => #<font-object "-unknown-DejaVu Sans Mono-normal-normal-normal-*-13-*-*-*-m-0-iso10646-1">

M-: (face-attribute 'default :distant-foreground) RET
  => unspecified

Re: About the :distant-foreground face attribute

[Jan Djärv]

> If you need to check for a background, the right thing would be to
> extend the DISPLAY feature to do what you need.  For example, a DISPLAY
> element of `(background dark)' is used to test for a dark background.
> Maybe what you want is to be able to specify a color in place of `dark',
> which would mean a background close to that color.

Patches welcome.

	Jan D.

Re: About the :distant-foreground face attribute

[Darren Hoo]

I originally reported bug 15668

Just FYI, the `font-lock on region' thing is still not available in these
bundled themes: wombat, wheatgrass, misterioso.

Re: About the :distant-foreground face attribute

[Eli Zaretskii]

> From: Chong Yidong <cyd@gnu.org>
> Date: Wed, 08 Jan 2014 05:57:41 +0800
> Cc: emacs-devel <emacs-devel@gnu.org>
> 
> The feature does not fit well with the design of the rest of the
> face-handling code.  We already have a mechanism for checking to see
> when to use a particular face: the DISPLAY element in a face spec.  The
> :distant-foreground face attribute, by its very existence, is redundant
> with what the DISPLAY element was meant to do.  This adds extra
> complexity to the design, for no good reason.
> 
> If you need to check for a background, the right thing would be to
> extend the DISPLAY feature to do what you need.  For example, a DISPLAY
> element of `(background dark)' is used to test for a dark background.
> Maybe what you want is to be able to specify a color in place of `dark',
> which would mean a background close to that color.

I don't understand this criticism.  How is this attribute different
from min-colors?

Re: About the :distant-foreground face attribute

[Chong Yidong]

Eli Zaretskii <eliz@gnu.org> writes:

>> The feature does not fit well with the design of the rest of the
>> face-handling code.  We already have a mechanism for checking to see
>> when to use a particular face: the DISPLAY element in a face spec.  The
>> :distant-foreground face attribute, by its very existence, is redundant
>> with what the DISPLAY element was meant to do.  This adds extra
>> complexity to the design, for no good reason.
>
> I don't understand this criticism.  How is this attribute different
> from min-colors?

The min-colors feature doesn't involve adding an extra face attribute.
The analogy would be if there was a :low-color-foreground face attribute
which would override :foreground on low-color displays.  That would be
ugly, as I hope you agree.

OK, after poking around a bit I understand the problem better.  You want
to be free to set face background colors without worrying about making
text illegible (which can be difficult to figure out ahead of time,
because of face inheritance etc).  So here's a proposal:

Change the feature so it applies to all faces, but in a configurable
way.  Introduce a new Lisp variable, `face-minimum-contrast', which
specifies the minimum allowed contrast between the background and
foreground of any face (or nil, which means to disable the feature).  If
the contrast of a face is lower than specified, the foreground color is
adjusted (say, by changing its V component) to conform to the minimum
contrast.

This would avoid having to introduce a :distant-foreground attribute for
all faces, only to use that attribute for just one face (`region') and
for one special purpose (to cope with the GTK selection color).  It
would handle the generic class of problems involving text becoming
illegible, such as due to bad themes.

Re: About the :distant-foreground face attribute

[Jan Djärv]

Hi.

8 jan 2014 kl. 06:24 skrev Chong Yidong <cyd@gnu.org>:

> Change the feature so it applies to all faces, but in a configurable
> way.  Introduce a new Lisp variable, `face-minimum-contrast', which
> specifies the minimum allowed contrast between the background and
> foreground of any face (or nil, which means to disable the feature).  If
> the contrast of a face is lower than specified, the foreground color is
> adjusted (say, by changing its V component) to conform to the minimum
> contrast.
> 

On Gtk and NS the region color to use is in that case specified by the system and should not be generated by modifying a color component.  How do you specify that?
Also, it isn't the contrast between the background and the foreground of a face, but the contrast between the background and foreground to be rendered, which comes from different faces in the case at hand.

> This would avoid having to introduce a :distant-foreground attribute for
> all faces, only to use that attribute for just one face (`region') and
> for one special purpose (to cope with the GTK selection color).  It
> would handle the generic class of problems involving text becoming
> illegible, such as due to bad themes.

distant-foreground can be used on any face, it just isn't.

	Jan D.

Re: About the :distant-foreground face attribute

[Chong Yidong]

Jan Djärv <jan.h.d@swipnet.se> writes:

> On Gtk and NS the region color to use is in that case specified by the
> system and should not be generated by modifying a color component.
> How do you specify that?

The reason this feature was introduced was so that Emacs could get away
with not using the specified *_foreground_color, and use the underlying
face color instead.  It is inconsistent to suddenly want to start
honoring it.

> Also, it isn't the contrast between the background and the foreground
> of a face, but the contrast between the background and foreground to
> be rendered, which comes from different faces in the case at hand.

Yep.

>> This would avoid having to introduce a :distant-foreground attribute for
>> all faces, only to use that attribute for just one face (`region') and
>> for one special purpose (to cope with the GTK selection color).  It
>> would handle the generic class of problems involving text becoming
>> illegible, such as due to bad themes.
>
> distant-foreground can be used on any face, it just isn't.

Yep, not sure what your point is.

Re: About the :distant-foreground face attribute

[Jan Djärv]

8 jan 2014 kl. 10:52 skrev Chong Yidong <cyd@gnu.org>:

> Jan Djärv <jan.h.d@swipnet.se> writes:
> 
>> On Gtk and NS the region color to use is in that case specified by the
>> system and should not be generated by modifying a color component.
>> How do you specify that?
> 
> The reason this feature was introduced was so that Emacs could get away
> with not using the specified *_foreground_color, and use the underlying
> face color instead.  It is inconsistent to suddenly want to start
> honoring it.

Honoring what?  Your statement makes no sense.  I'm talking about the fact that your proposal does not do what the current implementation does.
In your proposal, the foreground to use (distant foreground) would be calculated by Emacs.
In the current implementation, the foreground to use is specified by the system (Gtk+/NS).
How do you specify that in your proposal?

	Jan D.

Re: About the :distant-foreground face attribute

[Stefan Monnier]

> way.  Introduce a new Lisp variable, `face-minimum-contrast', which
> specifies the minimum allowed contrast between the background and
> foreground of any face (or nil, which means to disable the feature).

Some packages use a face with foreground=background so as to hide the
text (while still taking up space).  IOW this "make sure there's enough
contrast" should not apply to all faces.


        Stefan

Re: About the :distant-foreground face attribute

[Chong Yidong]

Jan Djärv <jan.h.d@swipnet.se> writes:

> Honoring what?  Your statement makes no sense.  I'm talking about the
> fact that your proposal does not do what the current implementation
> does.  In your proposal, the foreground to use (distant foreground)
> would be calculated by Emacs.  In the current implementation, the
> foreground to use is specified by the system (Gtk+/NS).  How do you
> specify that in your proposal?

Why is the current behavior TRT?  You are already choosing to ignore
gtk_selection_fg_color under some (arbitrarily hard-coded)
circumstances.

So if you care about obeying gtk_selection_fg_color, why not just set it
to :foreground?  When gtk_selection_bg_color is in use, we might as well
use gtk_selection_fg_color with it.  Users who want a region background
color that works properly with font lock might as well disable the GTK
selection color stuff (it's unfortunate that it's the default, but oh
well).

Re: About the :distant-foreground face attribute

[Jan Djärv]

Hi.

8 jan 2014 kl. 15:49 skrev Chong Yidong <seewhydee@gmail.com>:

> So if you care about obeying gtk_selection_fg_color, why not just set it
> to :foreground?  When gtk_selection_bg_color is in use, we might as well
> use gtk_selection_fg_color with it.  Users who want a region background
> color that works properly with font lock might as well disable the GTK
> selection color stuff (it's unfortunate that it's the default, but oh
> well).


You obviously don't understand the issue, so I'll try to be very clear.

1) Font lock uses faces with specified fore- and background.
2) When text is marked with the mouse, the region face is applied on (overrides) the font lock face.
3) If the region face blindly uses the foreground from the region face (as per your suggestion), for example gtk_selection_fg_color, font lock is lost.  That is what bug 15668 is about.
4) On the other hand, if we always ignore the region foreground color, and use the font lock foregrpund color bug 15668 would be solved.  However, if the font lock foreground and the region background is similar, text is not readable.  So in that case, distant-foreground is used.
5) When making a theme, I suspect one wishes to specify all colors, not use some arbitrary generated color, which most certainly don't match the theme.  So we need a way to specify that color, hence distant-foreground.

Your proposals:
1) Use the selection foreground color when the selection background is used
=> Bug 15668.

2) Disable GTK selection color stuff
This might lead to unreadable text as the region bacground may be close to the foreground.

There is no "region background color that works properly with font lock" for all font lock faces, because the colors in font lock faces are unknown, esp. with themes.

If you can get your scheme to let theme designers specify all colors without adding a new face parameter, via the display property somehow, that could be something.  Not that it would make anything simpler or a better design, it just sounds more involved to me.  Using generated colors is right out IMHO, it makes themes impossible to fully specify.

	Jan D.

RE: About the :distant-foreground face attribute

[Drew Adams]

The foreground attribute for a face should always reflect the actual
appearance of that face (in isolation - this is not about the effects
of combining faces etc.).  Anything different from that is asking for
trouble.

IIUC, this change was made because on some platforms the default
color for the `region' face background is inappropriate, given
the default color for its foreground.

To me, that sounds like the kind of thing that Eli recently
declared to be not-an-Emacs-problem.  But so be it - if Emacs can
reasonably work around that problem, why not?

If Emacs wants to make an attempt to compensate for that bad
defaulting behavior, then I would think that the right approach
would be to automatically tweak the color of the `region' face's
foreground attribute itself.  And preferably only for those
situations where the problem actually arises.

Adding a variable, as Yidong suggested, just spreads the
pollution of this misguided fix across all faces.  And it still
does not sync the attribute value with the appearance.  What the
attribute says you should get is not what you get.  The color is
being changed behind the back of the face spec.

If the face attribute value does not reflect the face appearance
for an attribute, code that examines that attribute, e.g., to base
its behavior on what the face's foreground is, will likely not DTRT.
And code that modifies the face's foreground attribute will likewise
likely not do what it is expected to do.  Code becomes less
transparent and dependable.

Foreground attribute and actual foreground should be and remain
one-to-one - no surprises (again, not counting face mergings etc.).

Can't you please find a way to keep the foreground attribute in
sync with the actual foreground color you need in this scenario?
Can't you perform whatever machinations are needed to change the
foreground attribute itself so that you get the visual effect
needed?

OK, so changing the attribute overrides whatever setting the user
might fix for the attribute.  But that would be done only when
the problem arises.  And it should be done only with the user's
permission.

IOW, I agree with Stefan (almost mentioned it myself) that users
and Lisp code should be able, for whatever reason, to specify
that the foreground and background for a given face are the
exact same color.  IOW, any automatic overriding of what the
face definition specifies should be optional, under user control.

And that user control should be *per face*.  One should not be
obliged to choose either preventing the overriding or allowing it
for all faces.  The choice should be a function of the particular
face.  Now *that* could be done using a new face attribute, if you
want.  (Or a function.)

The important thing is to preserve the correspondence between
each face's current appearance and its current attribute values.

RE: About the :distant-foreground face attribute

[Drew Adams]

> 1) Font lock uses faces with specified fore- and background.
> 2) When text is marked with the mouse, the region face is applied on
>    (overrides) the font lock face.

As it should.

> 3) If the region face blindly uses the foreground from the region
>    face (as per your suggestion), for example gtk_selection_fg_color,
>    font lock is lost.  That is what bug 15668 is about.

Font lock is not "lost".  Font-lock highlighting is covered by the
region highlighting.  And that is what should happen.  This "bug"
should not have been "fixed", IMHO.

This is what text selection highlighting is all about.  A user needs
to be able to see which text is highlighted - each selected char.

And you should not assume that font-locking affects only foregrounds.
Are you going to make the same "fix" for backgrounds also, so that
selecting text lets font-locked (or otherwise highlighted) backgrounds
show through the region highlighting?  And if font locking highlights
both the foreground and background of a character?  Bingo - you cannot
see which text you have selected.

This kind of change seems so misguided.  Hard to believe we've
arrived here.

Re: About the :distant-foreground face attribute

[Eli Zaretskii]

> From: Chong Yidong <cyd@gnu.org>
> Cc: jan.h.d@swipnet.se,  emacs-devel@gnu.org
> Date: Wed, 08 Jan 2014 13:24:17 +0800
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >> The feature does not fit well with the design of the rest of the
> >> face-handling code.  We already have a mechanism for checking to see
> >> when to use a particular face: the DISPLAY element in a face spec.  The
> >> :distant-foreground face attribute, by its very existence, is redundant
> >> with what the DISPLAY element was meant to do.  This adds extra
> >> complexity to the design, for no good reason.
> >
> > I don't understand this criticism.  How is this attribute different
> > from min-colors?
> 
> The min-colors feature doesn't involve adding an extra face attribute.

This is an implementation detail, while your criticism above seems to
be about the design.

> The analogy would be if there was a :low-color-foreground face attribute
> which would override :foreground on low-color displays.  That would be
> ugly, as I hope you agree.

I'm not sure I see the ugliness, please elaborate.

> Change the feature so it applies to all faces, but in a configurable
> way.  Introduce a new Lisp variable, `face-minimum-contrast', which
> specifies the minimum allowed contrast between the background and
> foreground of any face (or nil, which means to disable the feature).  If
> the contrast of a face is lower than specified, the foreground color is
> adjusted (say, by changing its V component) to conform to the minimum
> contrast.

I don't think it will be a good idea to have a single global setting,
because this would preclude a Lisp program from defining a face that
will always use the specified foreground, no matter what.  E.g., I
could imagine an application that would like to make the text
invisible by specifying the same fore- and background colors.  While
probably rare, such use cases are entirely legitimate, I think.

So I think we need this to be specified on the level of individual
faces.

Re: About the :distant-foreground face attribute

[Chong Yidong]

Eli Zaretskii <eliz@gnu.org> writes:

>> The analogy would be if there was a :low-color-foreground face attribute
>> which would override :foreground on low-color displays.  That would be
>> ugly, as I hope you agree.
>
> I'm not sure I see the ugliness, please elaborate.

One face attribute should govern one aspect of how the face is
displayed.  In some cases, it may be hard to avoid having multiple
attributes with overlapping effects, but the results are not pretty.
For example, the interactions between :family, :foundry and :font have
been a source of annoyance over the years.  Introducing another such
situation should, in my view, be avoided as far as possible.

An example of a face attribute that handles things right is the :height
attribute.  An absolute height is given by an integer, while a relative
height can be specified by a float.  Allowing both in a single attribute
is good, because absolute height and the relative height are just
different ways to specify height.  We don't have a :height and a
separate :relative-height attribute.

The DISPLAY spec is another example of avoiding adding extra face
attributes---it allows different sets of face attributes for different
display conditions.

If I cannot convince anyone that there is a problem here, then forget
it.  I don't have time to keep arguing, so I'll just stop working on
this stuff.

Re: About the :distant-foreground face attribute

[Stefan Monnier]

> been a source of annoyance over the years.  Introducing another such
> situation should, in my view, be avoided as far as possible.

That's a good point.  Basically, what you're saying is that instead
of :distant-foreground we could have a :foreground that can be of the
form (NORMAL-FOREGROUND . ALTERNATE-FOREGROUND), where
ALTERNATE-FOREGROUND is used when NORMAL-FOREGROUND would lead to a lack
of contrast.


        Stefan

Re: About the :distant-foreground face attribute

[Eli Zaretskii]

> From: Chong Yidong <cyd@gnu.org>
> Cc: jan.h.d@swipnet.se,  emacs-devel@gnu.org
> Date: Fri, 10 Jan 2014 00:15:00 +0800
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >> The analogy would be if there was a :low-color-foreground face attribute
> >> which would override :foreground on low-color displays.  That would be
> >> ugly, as I hope you agree.
> >
> > I'm not sure I see the ugliness, please elaborate.
> 
> One face attribute should govern one aspect of how the face is
> displayed.  In some cases, it may be hard to avoid having multiple
> attributes with overlapping effects, but the results are not pretty.
> For example, the interactions between :family, :foundry and :font have
> been a source of annoyance over the years.  Introducing another such
> situation should, in my view, be avoided as far as possible.
> 
> An example of a face attribute that handles things right is the :height
> attribute.  An absolute height is given by an integer, while a relative
> height can be specified by a float.  Allowing both in a single attribute
> is good, because absolute height and the relative height are just
> different ways to specify height.  We don't have a :height and a
> separate :relative-height attribute.

If you are arguing for a change in the syntax of :foreground such that
it could convey the information about both the "normal" color, and the
alternate color to be used when the background is too similar to that
"normal" color, then I agree it will probably be a nicer and cleaner
feature.  But that means :foreground will no longer be a simple
string, but some more complex data structure, e.g. a list.

> If I cannot convince anyone that there is a problem here, then forget
> it.

Don't give up just yet ;-)

The solution should be able to cope with the need to dynamically
decide which color is used as a foreground, based on the current
background.  It also needs to support the possibility that a face will
want to force use of a specific fixed foreground color, regardless of
the background.  (Jan, did I miss some additional requirements?)  If
you can propose a cleaner solution that satisfies these requirements,
please do, and let's discuss that.

RE: About the :distant-foreground face attribute

[Drew Adams]

> > been a source of annoyance over the years.  Introducing another
> > such situation should, in my view, be avoided as far as possible.
> 
> That's a good point.  Basically, what you're saying is that instead
> of :distant-foreground we could have a :foreground that can be of
> the form (NORMAL-FOREGROUND . ALTERNATE-FOREGROUND), where
> ALTERNATE-FOREGROUND is used when NORMAL-FOREGROUND would lead to a
> lack of contrast.

Please don't.  That too would break code that expects :foreground to
be as it is now.

Re: About the :distant-foreground face attribute

[David Engster]

Eli Zaretskii writes:
> The solution should be able to cope with the need to dynamically
> decide which color is used as a foreground, based on the current
> background.  It also needs to support the possibility that a face will
> want to force use of a specific fixed foreground color, regardless of
> the background.  (Jan, did I miss some additional requirements?)  If
> you can propose a cleaner solution that satisfies these requirements,
> please do, and let's discuss that.

I still wonder why it is necessary in the first place.

From my experience, editors usually simply drop font-lock when you mark
text (at least Eclipse does that, and in gvim it seems to be
configurable), and I think that is a good default. When you mark a
region, you want to apply some kind of operation on it, after which the
region will be gone and font lock is restored.

If you really really want font-lock on a marked region, then you will
have to choose a region background which will play well with your color
theme. Introducing a new face attribute for such a small annoyance looks
like overkill to me.

However, I also don't feel very strong about this issue.

-David

Re: About the :distant-foreground face attribute

[Lars Magne Ingebrigtsen]

David Engster <deng@randomsample.de> writes:

> I still wonder why it is necessary in the first place.

color.el has functions for calculating "different enough" colours.  I
think using that (or something like that) for computing new foreground
colours when the background changes would be a better idea...

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

Re: About the :distant-foreground face attribute

[Josh]

On Thu, Jan 9, 2014 at 9:05 AM, Eli Zaretskii <eliz@gnu.org> wrote:
>> From: Chong Yidong <cyd@gnu.org>
>> Cc: jan.h.d@swipnet.se,  emacs-devel@gnu.org
>> Date: Fri, 10 Jan 2014 00:15:00 +0800
>>
>> Eli Zaretskii <eliz@gnu.org> writes:
>>
>> >> The analogy would be if there was a :low-color-foreground face attribute
>> >> which would override :foreground on low-color displays.  That would be
>> >> ugly, as I hope you agree.
>> >
>> > I'm not sure I see the ugliness, please elaborate.
>>
>> One face attribute should govern one aspect of how the face is
>> displayed.  In some cases, it may be hard to avoid having multiple
>> attributes with overlapping effects, but the results are not pretty.
>> For example, the interactions between :family, :foundry and :font have
>> been a source of annoyance over the years.  Introducing another such
>> situation should, in my view, be avoided as far as possible.
>>
>> An example of a face attribute that handles things right is the :height
>> attribute.  An absolute height is given by an integer, while a relative
>> height can be specified by a float.  Allowing both in a single attribute
>> is good, because absolute height and the relative height are just
>> different ways to specify height.  We don't have a :height and a
>> separate :relative-height attribute.
>
> If you are arguing for a change in the syntax of :foreground such that
> it could convey the information about both the "normal" color, and the
> alternate color to be used when the background is too similar to that
> "normal" color, then I agree it will probably be a nicer and cleaner
> feature.  But that means :foreground will no longer be a simple
> string, but some more complex data structure, e.g. a list.
>
>> If I cannot convince anyone that there is a problem here, then forget
>> it.
>
> Don't give up just yet ;-)
>
> The solution should be able to cope with the need to dynamically
> decide which color is used as a foreground, based on the current
> background.  It also needs to support the possibility that a face will
> want to force use of a specific fixed foreground color, regardless of
> the background.  (Jan, did I miss some additional requirements?)  If
> you can propose a cleaner solution that satisfies these requirements,
> please do, and let's discuss that.

Instead of explicitly specifying alternative foreground colors,
whether by introducing a new face attribute or extending the
current :foreground attribute, perhaps we could introduce a new
`minimum-color-distance' user preference that would be used
something like this:

  (defun choose-foreground-color (fg bg minimum-color-distance)
    (if (or (> (color-distance fg bg) minimum-color-distance)
            ;; ^^^ foreground and background are sufficiently different
            (= fg bg))
            ;; ^^^ special case for text invisibility via fg==bg
        fg ;; preferred foreground color is ok; use it
      ;; otherwise, compute new foreground color -- not necessarily
      ;; via logxor but somehow guaranteed to be distant from bg
      (cl-mapcar 'logxor fg bg)))

Though such computed colors are likely to be ugly, clash with color
themes, etc. this approach is less invasive than the others being
discussed and I think it accomplishes the primary goal of ensuring
that bad combinations of foreground and background colors cannot
render text illegible.

Josh

Re: About the :distant-foreground face attribute

[Eli Zaretskii]

> Date: Thu, 9 Jan 2014 09:07:43 -0800 (PST)
> From: Drew Adams <drew.adams@oracle.com>
> Cc: Eli Zaretskii <eliz@gnu.org>, jan.h.d@swipnet.se, emacs-devel@gnu.org
> 
> > > been a source of annoyance over the years.  Introducing another
> > > such situation should, in my view, be avoided as far as possible.
> > 
> > That's a good point.  Basically, what you're saying is that instead
> > of :distant-foreground we could have a :foreground that can be of
> > the form (NORMAL-FOREGROUND . ALTERNATE-FOREGROUND), where
> > ALTERNATE-FOREGROUND is used when NORMAL-FOREGROUND would lead to a
> > lack of contrast.
> 
> Please don't.  That too would break code that expects :foreground to
> be as it is now.

Why do you assume that the previous form will not be accepted?  Of
course, it will be.

Re: About the :distant-foreground face attribute

[Jan D.]

Eli Zaretskii skrev 2014-01-09 18:05:
> The solution should be able to cope with the need to dynamically 
> decide which color is used as a foreground, based on the current 
> background. It also needs to support the possibility that a face will 
> want to force use of a specific fixed foreground color, regardless of 
> the background. (Jan, did I miss some additional requirements?) If you 
> can propose a cleaner solution that satisfies these requirements, 
> please do, and let's discuss that. 

If you have :foreground as a list and can specify the normal foreground 
as nil, that is equivalent to :distant-foreground.

     Jan D.

Re: About the :distant-foreground face attribute

[Jan D.]

Hello.

Lars Magne Ingebrigtsen skrev 2014-01-09 18:27:
> David Engster <deng@randomsample.de> writes:
>
>> I still wonder why it is necessary in the first place.
> color.el has functions for calculating "different enough" colours.  I
> think using that (or something like that) for computing new foreground
> colours when the background changes would be a better idea...

Generated colors don't usually go well with custom themes.  For the 
default theme it might be OK, but if you are defining a new theme you 
should be able to specify all colors.

     Jan D.

Re: About the :distant-foreground face attribute

[Eli Zaretskii]

> From: Josh <josh@foxtail.org>
> Date: Thu, 9 Jan 2014 09:39:43 -0800
> Cc: Chong Yidong <cyd@gnu.org>, Jan Djärv <jan.h.d@swipnet.se>, 
> 	emacs-devel <emacs-devel@gnu.org>
> 
> Instead of explicitly specifying alternative foreground colors,
> whether by introducing a new face attribute or extending the
> current :foreground attribute, perhaps we could introduce a new
> `minimum-color-distance' user preference that would be used
> something like this:

First, such preferences should be per face, not global, for the
reasons already explained in this thread.

And second, this is not necessarily a user-level issue.  It could be a
requirement that text in a certain face be legible, come what may.  In
that case, we would like to give the Lisp programmer a way to put this
requirement into the face definition.

Re: About the :distant-foreground face attribute

[Chong Yidong]

Eli Zaretskii <eliz@gnu.org> writes:

>> Please don't.  That too would break code that expects :foreground to
>> be as it is now.
>
> Why do you assume that the previous form will not be accepted?  Of
> course, it will be.

What Drew is worrying about, I think, is that third-party code, or old
versions of Emacs, will barf when they come across the new :foreground
form in user customizations or themes.

I don't know whether this would be a major problem in practice.
Personally, I agree with David Engster that

  If you really really want font-lock on a marked region, then you will
  have to choose a region background which will play well with your
  color theme.  Introducing a new face attribute for such a small
  annoyance looks like overkill to me.

But IF people feel really strongly about having this feature, doing it
by adding a new :foreground type seems like the least bad option, from a
code cleanliness perspective.

Re: About the :distant-foreground face attribute

[Darren Hoo]

David Engster <deng@randomsample.de> writes:

> Eli Zaretskii writes:
>> The solution should be able to cope with the need to dynamically
>> decide which color is used as a foreground, based on the current
>> background.  It also needs to support the possibility that a face will
>> want to force use of a specific fixed foreground color, regardless of
>> the background.  (Jan, did I miss some additional requirements?)  If
>> you can propose a cleaner solution that satisfies these requirements,
>> please do, and let's discuss that.
>
> I still wonder why it is necessary in the first place.
>
> From my experience, editors usually simply drop font-lock when you mark
> text (at least Eclipse does that, and in gvim it seems to be
> configurable), and I think that is a good default. When you mark a
> region, you want to apply some kind of operation on it, after which the
> region will be gone and font lock is restored.
>

That's quite contrary to my experience. Even for old emacs like emacs22
font-lock is kept on marked text with transient-mark-mode enabled.
As to other tools like Eclipse, Netbeans,Xcode selected text does not
lose syntax highlight either. 

Especially for Eclipse I remembered that when I used it about a decade
ago copying selected code from Eclipse to other WYSIWYG applications
like MS PowerPoint the syntax highlight is also copied ie, rich formatted.

> If you really really want font-lock on a marked region, then you will
> have to choose a region background which will play well with your color
> theme. Introducing a new face attribute for such a small annoyance looks
> like overkill to me.

I must say here that my experience with color theme is way much better
and much less flaws than before it's integerated thanks to the work of
all the developers especially cyd.

I hope the feature be kept without introducing too much maitainabitly
burden, with code both easier for user to understand and theme writers
to customize.   


> However, I also don't feel very strong about this issue.

I do. I find myself get an uneasy feeling when font-lock is lost on
marked text.

RE: About the :distant-foreground face attribute

[Drew Adams]

> > > > been a source of annoyance over the years.  Introducing
> > > > another such situation should, in my view, be avoided as
> > > > far as possible.
> > >
> > > That's a good point.  Basically, what you're saying is that
> > > instead of :distant-foreground we could have a :foreground that
> > > can be of the form (NORMAL-FOREGROUND . ALTERNATE-FOREGROUND),
> > > where ALTERNATE-FOREGROUND is used when NORMAL-FOREGROUND would
> > > lead to a lack of contrast.
> >
> > Please don't.  That too would break code that expects :foreground
> > to be as it is now.
> 
> Why do you assume that the previous form will not be accepted?  Of
> course, it will be.

Accepted by what?  Are you thinking of `defface'?  Why would you assume
that I assume that?  I'm sure you will make `defface' etc. work.  I'm
sure that you will make all distributed `emacs -Q' code work, for this.

That's not the point.  It's about being accepted by, recognized by, and
useful for other, existing code.  If code expects a string foreground
value then it is not designed to DTRT with a cons etc.  This should be
obvious, no?

Re: About the :distant-foreground face attribute

[David Engster]

Darren Hoo writes:
> David Engster <deng@randomsample.de> writes:
>> Eli Zaretskii writes:
>>> The solution should be able to cope with the need to dynamically
>>> decide which color is used as a foreground, based on the current
>>> background.  It also needs to support the possibility that a face will
>>> want to force use of a specific fixed foreground color, regardless of
>>> the background.  (Jan, did I miss some additional requirements?)  If
>>> you can propose a cleaner solution that satisfies these requirements,
>>> please do, and let's discuss that.
>>
>> I still wonder why it is necessary in the first place.
>>
>> From my experience, editors usually simply drop font-lock when you mark
>> text (at least Eclipse does that, and in gvim it seems to be
>> configurable), and I think that is a good default. When you mark a
>> region, you want to apply some kind of operation on it, after which the
>> region will be gone and font lock is restored.
>>
>
> That's quite contrary to my experience. Even for old emacs like emacs22
> font-lock is kept on marked text with transient-mark-mode enabled.
> As to other tools like Eclipse, Netbeans,Xcode selected text does not
> lose syntax highlight either. 

My Eclipse does that. I use the Zenburn color theme, though. Maybe it is
configurable, I don't know. Anyway, I think that it is the right
*default* behavior: making the region clearly visible and not caring
about font lock.

> Especially for Eclipse I remembered that when I used it about a decade
> ago copying selected code from Eclipse to other WYSIWYG applications
> like MS PowerPoint the syntax highlight is also copied ie, rich formatted.

And what do those editors do when the highlight background color is very
similar to one of the font-lock colors?

> I find myself get an uneasy feeling when font-lock is lost on marked
> text.

Well, I don't.

-David

Re: About the :distant-foreground face attribute

[Eli Zaretskii]

> Date: Thu, 9 Jan 2014 13:12:59 -0800 (PST)
> From: Drew Adams <drew.adams@oracle.com>
> Cc: monnier@iro.umontreal.ca, cyd@gnu.org, jan.h.d@swipnet.se,
>         emacs-devel@gnu.org
> 
> > Why do you assume that the previous form will not be accepted?  Of
> > course, it will be.
> 
> Accepted by what?  Are you thinking of `defface'?  Why would you assume
> that I assume that?

What else can you possibly think of, when we are talking about
defface?

> That's not the point.  It's about being accepted by, recognized by, and
> useful for other, existing code.

Other existing code should never see this.  We are talking about
defface, and defface alone.  At least I was.

RE: About the :distant-foreground face attribute

[Drew Adams]

> > The solution should be able to cope with the need to dynamically
> > decide which color is used as a foreground, based on the current
> > background.  It also needs to support the possibility that a face
> > will want to force use of a specific fixed foreground color,
> > regardless of the background.  (Jan, did I miss some additional
> > requirements?) If you can propose a cleaner solution that satisfies
> > these requirements, please do, and let's discuss that.

IMO, there are no such "requirements", and no bug to be "fixed" here.

> I still wonder why it is necessary in the first place.

I don't think it was.

> From my experience, editors usually simply drop font-lock when you
> mark text (at least Eclipse does that, and in gvim it seems to be
> configurable), and I think that is a good default.

Yes, of course; it is the normal, and expected, behavior.

Users should be able to easily see which text is selected - each
character.  Selection highlighting should be (and is, everywhere
else that I am aware of) "on top", taking precedence over other
highlighting.

> When you mark a region, you want to apply some kind of operation on
> it, after which the region will be gone and font lock is restored.
> 
> If you really really want font-lock on a marked region, then you
> will have to choose a region background which will play well with
> your color theme.
>
> Introducing a new face attribute for such a small annoyance looks
> like overkill to me.

It's not just the chosen fix that is misguided.  It is the belief
that other highlighting (such as font lock) should "show through" the
selection highlighting.  This should have been tossed as "not a bug".

RE: About the :distant-foreground face attribute

[Drew Adams]

> >> Please don't.  That too would break code that expects :foreground
> >> to be as it is now.
> >
> > Why do you assume that the previous form will not be accepted?  Of
> > course, it will be.
> 
> What Drew is worrying about, I think, is that third-party code, or
> old versions of Emacs, will barf when they come across the new
> :foreground form in user customizations or themes.

Yes, that is exactly what motivated my reply there.

But that is not the main point I argue.  The main point is that this
feature is misguided.  A selection (e.g. Emacs region) highlight
_should_ override other highlighting.  You need to be able to see
clearly which text has been selected, each and every character.

IIUC, this enhancement request came about because some platforms
impose default region backgrounds that are inappropriate, being the
same as or too close to the default foreground.

My answer to that would be: (a) not Emacs's problem (a la Eli) -
lobby the platform, (b) too bad, (c) let the user customize face
`region' to get a better background.

Users can define both the foreground and background of face `region'.
It should be trivial to do that so there is no conflict and all text
selected is easily readable.  End of story, no?

Why is it "necessary" that font lock highlighting show through the
text selection (region)?  Answer: it's not.  It's not only not
necessary (YAGNI), but it is wrong (misguided).  The selection
_should_ take precedence.

> I don't know whether this would be a major problem in practice.
> Personally, I agree with David Engster that
> 
>   If you really really want font-lock on a marked region, then you
>   will have to choose a region background which will play well with
>   your color theme.  Introducing a new face attribute for such a small
>   annoyance looks like overkill to me.
> 
> But IF people feel really strongly about having this feature, doing
> it by adding a new :foreground type seems like the least bad option,
> from a code cleanliness perspective.

It is backward incompatible.  It solves a non-problem.  It imposes
unconventional, unexpected, confusing UI interaction that can reduce
a user's ability to tell just which characters have been selected.
Not a bug.

RE: About the :distant-foreground face attribute

[Drew Adams]

> > From my experience, editors usually simply drop font-lock when you
> > mark text (at least Eclipse does that, and in gvim it seems to be
> > configurable), and I think that is a good default. When you mark a
> > region, you want to apply some kind of operation on it, after
> > which the region will be gone and font lock is restored.
> 
> That's quite contrary to my experience. Even for old emacs like
> emacs22 font-lock is kept on marked text with transient-mark-mode
> enabled.  As to other tools like Eclipse, Netbeans,Xcode selected
> text does not lose syntax highlight either.

Yes and no.  You might think that, just because (a) by default, face
`region' does not specify a foreground color and (b) most font locking
(maybe all default font-locking) does not affect the background.

> Especially for Eclipse I remembered that when I used it about a
> decade ago copying selected code from Eclipse to other WYSIWYG
> applications like MS PowerPoint the syntax highlight is also copied
> ie, rich formatted.

But this is the point: does the selection highlighting take precedence
over the underlying text highlighting (syntax highlighting or other)?

My guess is that in *all* of the cases you cite the answer is yes.
Selection highlighting _should_ take precedence, so you can tell
clearly which text has been selected.  When you are selecting text,
that is generally more important than other highlighting considerations.

> I must say here that my experience with color theme is way much
> better and much less flaws than before it's integerated thanks to
> the work of all the developers especially cyd.

I'm not sure what you mean by that.  Color theme was not integrated
into Emacs.  It is still a separate library (and it still works
with Emacs 24.4).  What CYD did was to enhance Emacs custom themes
to do some of what color themes do (in a different way), and to do
some things that color themes do not do.  `color-theme.el' is here:
http://www.nongnu.org/color-theme .  See also
http://www.emacswiki.org/emacs/ColorTheme.

> I do. I find myself get an uneasy feeling when font-lock is lost on
> marked text.

It was lost in the context you describe (IIUC), because the platform
provided a bad default color for the `region' background.  This was
not the right "fix" for that problem, IMO (IIUC).

RE: About the :distant-foreground face attribute

[Drew Adams]

> > > Why do you assume that the previous form will not be accepted?
> > > Of course, it will be.
> >
> > Accepted by what?  Are you thinking of `defface'?  Why would you
> > assume that I assume that?
> 
> What else can you possibly think of, when we are talking about
> defface?

Yes, and why would you assume that I assume that the previous form
would not be accepted by (an updated) `defface'?

Of course I assume the opposite, as I said.  I assume that you 
will DTRT for defface, to handle the new form.  `defface' is not
the problem I see.

> > That's not the point.  It's about being accepted by, recognized
> > by, and useful for other, existing code.
> 
> Other existing code should never see this.  We are talking about
> defface, and defface alone.  At least I was.

And I was not.  I was talking, as Yidong pointed out, about
existing code trying to deal with a redefined `foreground'
attribute value (soon to be allowed to be a cons perhaps).

First, the idea was to use an additional face attribute; lately,
the talk is about a redefined attribute value, allowing it to be
a list (of two strings), in addition to a string.  IIUC.
Mille excuses, if I did not understand the latest proposal correctly.

Re: About the :distant-foreground face attribute

[Darren Hoo]

David Engster <deng@randomsample.de> writes:


> My Eclipse does that. I use the Zenburn color theme, though. Maybe it is
> configurable, I don't know. Anyway, I think that it is the right
> *default* behavior: making the region clearly visible and not caring
> about font lock.

I see. I never used customized Eclipse color theme. I assume Zenburn might
be a dark theme.

I think that by default (No color theme) Emacs should work as what it
did before. But with color themes especially for the dark ones I would
not insist on the the region font lock. 

>> Especially for Eclipse I remembered that when I used it about a decade
>> ago copying selected code from Eclipse to other WYSIWYG applications
>> like MS PowerPoint the syntax highlight is also copied ie, rich formatted.
>
> And what do those editors do when the highlight background color is very
> similar to one of the font-lock colors?
>

I am quite happy with Intellij Idea's dark theme (Dracula), I have used
it for quite a while and have never seen illegilbe selected text.
But I don't know how it does well so I can not provide any useful
information here.

Re: About the :distant-foreground face attribute

[David Engster]

Chong Yidong writes:
> Eli Zaretskii <eliz@gnu.org> writes:
>
>>> Please don't.  That too would break code that expects :foreground to
>>> be as it is now.
>>
>> Why do you assume that the previous form will not be accepted?  Of
>> course, it will be.
>
> What Drew is worrying about, I think, is that third-party code, or old
> versions of Emacs, will barf when they come across the new :foreground
> form in user customizations or themes.

I'm not too fond of that for the same reason.

I'm wondering: We already can set different face attributes depending on
DISPLAY's 'background' property, which can be 'light' or 'dark'. Say the
user is working with a 'dark' background by default, but we now detect
that one of the font-lock faces has not enough contrast when highlighted
by the region: why not simply switch to the face that is defined for
'light' background instead?

-David

Re: About the :distant-foreground face attribute

[David Engster]

David Engster writes:
> I'm wondering: We already can set different face attributes depending on
> DISPLAY's 'background' property, which can be 'light' or 'dark'. Say the
> user is working with a 'dark' background by default, but we now detect
> that one of the font-lock faces has not enough contrast when highlighted
> by the region: why not simply switch to the face that is defined for
> 'light' background instead?

Hey look, a tumbleweed!

I'm assuming everybody's simply stunned by my ingenious proposal?

(;-), etc.)

-David

Re: About the :distant-foreground face attribute

[Jan Djärv]

Hello.

12 jan 2014 kl. 12:14 skrev David Engster <deng@randomsample.de>:

> David Engster writes:
>> I'm wondering: We already can set different face attributes depending on
>> DISPLAY's 'background' property, which can be 'light' or 'dark'. Say the
>> user is working with a 'dark' background by default, but we now detect
>> that one of the font-lock faces has not enough contrast when highlighted
>> by the region: why not simply switch to the face that is defined for
>> 'light' background instead?
> 
> Hey look, a tumbleweed!
> 
> I'm assuming everybody's simply stunned by my ingenious proposal?
> 

A theme does not have to suppy colors for properties light or dark.

> (;-), etc.)


	Jan D.

Re: About the :distant-foreground face attribute

[David Engster]

Jan Djärv writes:
> 12 jan 2014 kl. 12:14 skrev David Engster <deng@randomsample.de>:
>
>> David Engster writes:
>>> I'm wondering: We already can set different face attributes depending on
>>> DISPLAY's 'background' property, which can be 'light' or 'dark'. Say the
>>> user is working with a 'dark' background by default, but we now detect
>>> that one of the font-lock faces has not enough contrast when highlighted
>>> by the region: why not simply switch to the face that is defined for
>>> 'light' background instead?
>> 
>> Hey look, a tumbleweed!
>> 
>> I'm assuming everybody's simply stunned by my ingenious proposal?
>> 
>
> A theme does not have to suppy colors for properties light or dark.

So? It also does not have to supply the distant-foreground attribute.

-David

Re: About the :distant-foreground face attribute

[Jan Djärv]

Hello.

12 jan 2014 kl. 13:21 skrev David Engster <deng@randomsample.de>:

> Jan Djärv writes:
>> 12 jan 2014 kl. 12:14 skrev David Engster <deng@randomsample.de>:
>> 
>>> David Engster writes:
>>>> I'm wondering: We already can set different face attributes depending on
>>>> DISPLAY's 'background' property, which can be 'light' or 'dark'. Say the
>>>> user is working with a 'dark' background by default, but we now detect
>>>> that one of the font-lock faces has not enough contrast when highlighted
>>>> by the region: why not simply switch to the face that is defined for
>>>> 'light' background instead?
>>> 
>>> Hey look, a tumbleweed!
>>> 
>>> I'm assuming everybody's simply stunned by my ingenious proposal?
>>> 
>> 
>> A theme does not have to suppy colors for properties light or dark.
> 
> So? It also does not have to supply the distant-foreground attribute.

Right.  But distant-foreground is implemented, you proposal is not and does not add anything except moving colors to some other place in the defface definition.  Also, for a theme that does have a dark and light version, there is no guarantee that applying the dark version on the light version (or vice versa) is consistent with the theme look.

	Jan D.

Re: About the :distant-foreground face attribute

[David Engster]

Jan Djärv writes:
> Hello.
>
> 12 jan 2014 kl. 13:21 skrev David Engster <deng@randomsample.de>:
>
>> Jan Djärv writes:
>>> 12 jan 2014 kl. 12:14 skrev David Engster <deng@randomsample.de>:
>>> 
>>>> David Engster writes:
>>>>> I'm wondering: We already can set different face attributes depending on
>>>>> DISPLAY's 'background' property, which can be 'light' or 'dark'. Say the
>>>>> user is working with a 'dark' background by default, but we now detect
>>>>> that one of the font-lock faces has not enough contrast when highlighted
>>>>> by the region: why not simply switch to the face that is defined for
>>>>> 'light' background instead?
>>>> 
>>>> Hey look, a tumbleweed!
>>>> 
>>>> I'm assuming everybody's simply stunned by my ingenious proposal?
>>>> 
>>> 
>>> A theme does not have to suppy colors for properties light or dark.
>> 
>> So? It also does not have to supply the distant-foreground attribute.
>
> Right.  But distant-foreground is implemented,

It can be removed again. That is what this thread is about.

> you proposal is not and does not add anything except moving colors to
> some other place in the defface definition.

Exactly. I want to fix the original bug by Darren without the need to
introduce another face attribute.

> Also, for a theme that does have a dark and light
> version, there is no guarantee that applying the dark version on the
> light version (or vice versa) is consistent with the theme look.

A consistent color theme will choose the region's background color in a
way that the font-lock colors are always visible in the first place. The
problem is with the default colors. We can easily add a 'dark'/'light'
foreground attribute to our default font-lock colors that work well with
the default region background from GTK/NS.

-David

Re: About the :distant-foreground face attribute

[Jan Djärv]

Hello.

12 jan 2014 kl. 14:07 skrev David Engster <deng@randomsample.de>:

> A consistent color theme will choose the region's background color in a
> way that the font-lock colors are always visible in the first place.

That is impossible in principle.  Any mode (or elisp code) can define a new face with new colors that didn't even exist when the theme was designed.  Distant-foreground gives theme designers a way to cope with this, even if no theme has done so yet.  And in a way that is consistent with other theme colors.  Dark/light just don't do that.  EOD.

	Jan D.

Re: About the :distant-foreground face attribute

[Chong Yidong]

David Engster <deng@randomsample.de> writes:

>> I'm wondering: We already can set different face attributes depending on
>> DISPLAY's 'background' property, which can be 'light' or 'dark'. Say the
>> user is working with a 'dark' background by default, but we now detect
>> that one of the font-lock faces has not enough contrast when highlighted
>> by the region: why not simply switch to the face that is defined for
>> 'light' background instead?
>
> Hey look, a tumbleweed!
>
> I'm assuming everybody's simply stunned by my ingenious proposal?

This would not work with customized faces, because Custom sets them with
a DISPLAY spec of t by default.  Anyway, this proposal would be hard to
implement technically, because DISPLAY spec is handled (in Lisp) before
the foreground and background attributes are settled on (which takes
face inheritance into account and is done in C).

Instead, here's a compromise proposal:

- Allow :foreground to take the value of (fallback COLOR) or something
  like that, which would be equivalent to setting :foreground to
  unspecified and :distant-foreground to COLOR.

  (We still need a replacement term for "distant foreground".  As
  mentioned before, this term sounds nonsensical.)

- In order to avoid incompatibilities, set the :foreground of the
  `region' face to "*_selection_fg_color".  In other words, avoid using
  the above feature in the `region' face, at least for Emacs 24.4.

  The rationale is that (i) we can live with having a fixed foreground
  color for the `region' face, since that was the case in Emacs 24.3 on
  GTK anyway.  And (ii) not using this feature immediately gives
  third-party packages a "transition period" to adapt to its presence,
  without immediately failing by encountering it in a standard face.

If there are no objections, I can implement this.

RE: About the :distant-foreground face attribute

[Drew Adams]

> Instead, here's a compromise proposal:
> 
> - Allow :foreground to take the value of (fallback COLOR) or
>   something like that, which would be equivalent to setting
>   :foreground to unspecified and :distant-foreground to COLOR.

I think you are saying to set the face's foreground attribute to
whatever the dwim thingie calculates, which is what I suggested too.
Is that what you mean?

>   (We still need a replacement term for "distant foreground".  As
>   mentioned before, this term sounds nonsensical.)

Yes, if a term is needed at all.  IIUC, there would be no new face
attribute, at least.  The ordinary foreground attribute would be
given whatever clever value is deemed more appropriate, right?

So if a new term is needed for this clever color, then that would
be only for, say, a function name and its doc, not for a new face
attribute - right?

> - In order to avoid incompatibilities, set the :foreground of the
>   `region' face to "*_selection_fg_color".

I don't know what the latter is.  Not that I necessarily need to.
I've already expressed my concerns, and am hoping that they are
taken into account in some way.  I have confidence that you, at
least, understand what my concerns are and will think about them.

>   In other words, avoid using the above feature in the `region'
>   face, at least for Emacs 24.4.

I guess you are saying that the region foreground will not be
adapted automatically to compensate for an unfortunate region
background default choice.  For Emacs 24.4, at least.  Is that
right?

>   The rationale is that (i) we can live with having a fixed
>   foreground color for the `region' face, since that was the
>   case in Emacs 24.3 on GTK anyway.

Can't we live with that as a continuing feature?  That sounds
like TRT, to me.

Let's not forget that the user can define the `region' face
any way s?he wants.  It is not only the foreground that can
"conflict", and it is not only the background that might be
defined.

Similarly, font-lock highlighting can use faces with backgrounds
defined, as well as foregrounds.  Would this clever feature
do the same thing for letting font-lock backgrounds show through
as it aims to do for letting font-lock foregrounds show through?
If not, why not?

There is nothing inherently asymmetrical between foreground and
background, for either face `region' or font-locking.

>   And (ii) not using this feature immediately gives
>   third-party packages a "transition period" to adapt to its
>   presence,

Which means what?

>   without immediately failing by encountering it in a standard
>   face.

What is it that will eventually be encountered in a face,
standard or otherwise?  Are back to an additional face attribute?
Sorry, but I don't understand the proposal.  What is it, in
concrete terms (user terms, Lisp terms)?  What will 3rd-party
code need to know?  What adaptation is needed, in general terms
at least?

> If there are no objections, I can implement this.

I replied so that you know that I don't yet understand what is
being proposed.  I understand that you had problems with this
new feature, as implemented (you filed the bug report).  So I
suppose that you will DTRT.  Still, I would like to understand
what the change will be, if possible.

Re: About the :distant-foreground face attribute

[Jan Djärv]

Hello.

12 jan 2014 kl. 21:14 skrev Chong Yidong <cyd@gnu.org>:

> Instead, here's a compromise proposal:
> 
> - Allow :foreground to take the value of (fallback COLOR) or something
>  like that, which would be equivalent to setting :foreground to
>  unspecified and :distant-foreground to COLOR.
> 
>  (We still need a replacement term for "distant foreground".  As
>  mentioned before, this term sounds nonsensical.)
> 

There is a function called distant_color in the source, thats where it comes from.

> - In order to avoid incompatibilities, set the :foreground of the
>  `region' face to "*_selection_fg_color".  In other words, avoid using
>  the above feature in the `region' face, at least for Emacs 24.4.
> 
>  The rationale is that (i) we can live with having a fixed foreground
>  color for the `region' face, since that was the case in Emacs 24.3 on
>  GTK anyway.  And (ii) not using this feature immediately gives
>  third-party packages a "transition period" to adapt to its presence,
>  without immediately failing by encountering it in a standard face.
> 
> If there are no objections, I can implement this.

Count this as an objection. (i) means reopening a fixed bug BTW, don't forget that.
I don't think this should be done at all, but lets just consider 24.4:

1) We are talking about a replacing a feature that has no outstanding bugs, and has been in place for about 2 months.
2) The only reason too replace it is some personal feelings about "clean design", based on unknown principles, that are not documented in any Emacs or GNU document.
3) The code for this proposal will be messier.  Distant-foreground is quite separate in the code, you can easily find any place where the source handles to it.  This proposal suggests modifying foreground, thus changing a lot of places where foreground are handeled.  Not only is this bound to introduce bugs, but the feature is not as easily seen in the source.

And all this during a feature freeze?  It makes feature freeze kind of pointless if any feature can be replaced willy nilly based on a persons design feelings.

But this is Stefans call.

	Jan D.

Re: About the :distant-foreground face attribute

[Stefan Monnier]

> - Allow :foreground to take the value of (fallback COLOR) or something
>   like that, which would be equivalent to setting :foreground to
>   unspecified and :distant-foreground to COLOR.

This syntax implies that a face can't set both :foreground
and :distant-foreground at the same time, even tho the combination of
the two is meaningful (it means "use color X if that's readable and
color Y otherwise").

So I think it makes more sense to use a syntax that can specify both,
such as (COLOR . FALLBACK) or (choose COLOR1 COLOR2).

Now, what happens in the following case:
- face1 sets foreground to (choose COLOR1 COLOR2)
- face2 sets foreground to COLOR3 and inherits from face1.
Is the resulting "merge" equivalent to COLOR3 or to (choose COLOR3
COLOR2) or to (choose COLOR3 COLOR1 COLOR2)?
I think, to be a useful feature, the merge can't be COLOR3, so it would
have to be (choose COLOR3 COLOR2) or (choose COLOR3 COLOR1 COLOR2).
Is that really better than what we have now?

But now I wonder: what's the benefit from folding this "alternate" color
into :foreground compared to having it in a new
property :distant-foreground?

>   (We still need a replacement term for "distant foreground".  As
>   mentioned before, this term sounds nonsensical.)

Agreed.  Maybe :alternative-foreground?


        Stefan

RE: About the :distant-foreground face attribute

[Drew Adams]

> Count this as an objection. (i) means reopening a fixed bug BTW,
> don't forget that.

On the contrary.  It fixed a non-bug, and introduced a regression.
It forces Lisp code to access and manipulate actual face
foregrounds (what is really shown, not counting effects of other
faces etc.) in a new, kludgy way.

> 1) We are talking about a replacing a feature that has no
> outstanding bugs, and has been in place for about 2 months.

Do you want me to file a bug for this regression?

> 2) The only reason too replace it is some personal feelings about
> "clean design", based on unknown principles, that are not documented
> in any Emacs or GNU document.

Among the reasons to remove this regression are: (a) it breaks
existing Lisp code and complicates future code, (b) it is lousy UI,
going counter to selection in other apps, (c) it is not needed for
anything, (d) it treats foreground differently than background
(asymmetric), and so does not solve the problem it purports to,
since both `region' and font-lock can specify either foreground or
background, or both, (e) it tries to work around a non-Emacs problem
of certain platforms providing poor default selection background
colors.

> 3) The code for this proposal will be messier.  Distant-foreground
> is quite separate in the code, you can easily find any place where
> the source handles to it.  This proposal suggests modifying
> foreground, thus changing a lot of places where foreground are
> handeled.  Not only is this bound to introduce bugs, but the
> feature is not as easily seen in the source.

The introduction of an additional face attribute that is not
independent of attribute foreground, and that affects the
perceived foreground of the face, and so breaks the one-to-one
relation between a face's foreground attribute and the face's
perceived foreground, is an ill-conceived, error-prone hack.

It complicates any Lisp code that tries to deal with the actual
face foreground based on its foreground attribute.  And that's in
the best of cases: future code.  Existing code is simply broken,
if it expects the face's foreground attribute and appearance to
correspond.

> And all this during a feature freeze?

This new "feature" should never have been approved.  It is simply
an unfortunate regression.  It should be pulled out pronto.

> It makes feature freeze kind of pointless if any feature can be
> replaced willy nilly based on a persons design feelings.

Feature freeze is the time to test the product, including changes
that have been made.  This change does not pass the smell test,
let alone any usability tests for Lisp code.  It will even
confuse users in UI terms, when they try to check the foreground
at some position.

> But this is Stefans call.

Re: About the :distant-foreground face attribute

[chad]

On 12 Jan 2014, at 14:59, Stefan Monnier <monnier@iro.umontreal.ca> wrote:
>  (We still need a replacement term for "distant foreground".  As
>>  mentioned before, this term sounds nonsensical.)
> 
> Agreed.  Maybe :alternative-foreground?

I hate to shed the bike, but maybe “contrast” is a helpful word here?

~Chad

[PATCH] Re: About the :distant-foreground face attribute

[Daniel Colascione]

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

On 01/08/2014 09:43 AM, Eli Zaretskii wrote:
>> From: Chong Yidong <cyd@gnu.org>
>> Cc: jan.h.d@swipnet.se,  emacs-devel@gnu.org
>> Date: Wed, 08 Jan 2014 13:24:17 +0800
>>
>> Eli Zaretskii <eliz@gnu.org> writes:
>>
>>>> The feature does not fit well with the design of the rest of the
>>>> face-handling code.  We already have a mechanism for checking to see
>>>> when to use a particular face: the DISPLAY element in a face spec.  The
>>>> :distant-foreground face attribute, by its very existence, is redundant
>>>> with what the DISPLAY element was meant to do.  This adds extra
>>>> complexity to the design, for no good reason.

The attached patch might be another solution to the problem. It replaces 
:distant-foreground with :contrast-function, which punts the actual 
contrast logic to lisp by calling the named function during face 
realization. (Performance isn't a problem in practice because we cache 
face realizations.) In lisp, we implement four low-contrast-mitigation 
policies: do not adjust for contrast, adjust automatically (by adjusting 
CIE L*A*B color space L values), adjust automatically (by adjusting the 
V values in HSV color space), or just set the foreground to a specific 
color if the contrast dips below a certain point (the current 
:distant-foreground behavior). Both the policy and the parameters (well, 
the override color) are customizable on a per-face basis; when merging 
faces, the one with the highest priority sets the whole behavior.

The patch uses the CIE L*A*B colorspace algorithm by default. It 
produces surprisingly good results, at least in my tests, adapting 
automatically to light and dark backgrounds while preserving the hues of 
theme foreground colors.

(Changing themes nukes the face property right now, so you'll have to 
reset it each time.)

[-- Attachment #2: adaptive-face.patch --]
[-- Type: text/x-patch, Size: 18485 bytes --]

=== modified file 'lisp/color.el'
--- lisp/color.el	2014-01-01 07:43:34 +0000
+++ lisp/color.el	2014-01-13 12:18:53 +0000
@@ -114,6 +114,26 @@
        (color-hue-to-rgb m1 m2 H)
        (color-hue-to-rgb m1 m2 (mod (- H (/ 1.0 3)) 1))))))
 
+(defun color-hsv-to-rgb (H S V)
+  "Convert hue, saturation and value to their RGB representation.
+H, S, and V should each be numbers between 0.0 and 1.0, inclusive.
+Return a list (RED GREEN BLUE), where each element is between 0.0 and 1.0,
+inclusive."
+  (let* ((V (float V))
+         (H (/ (* 3 H) float-pi))
+         (i (floor H))
+         (ff (- H i))
+         (P (* V (- 1.0 S)))
+         (Q (* V (- 1.0 (* S ff))))
+         (T (* V (- 1.0 (* S (- 1.0 ff))))))
+    (cond
+     ((= i 0) (list V T P))
+     ((= i 1) (list Q V P))
+     ((= i 2) (list P V T))
+     ((= i 3) (list P Q V))
+     ((= i 4) (list T P V))
+     (t       (list V P Q)))))
+
 (defun color-complement-hex (color)
   "Return the color that is the complement of COLOR, in hexadecimal format."
   (apply 'color-rgb-to-hex (color-complement color)))

=== modified file 'lisp/cus-face.el'
--- lisp/cus-face.el	2014-01-01 07:43:34 +0000
+++ lisp/cus-face.el	2014-01-13 12:54:58 +0000
@@ -230,6 +230,32 @@
 		   :help-echo "Name of bitmap file."
 		   :must-match t)))
 
+    (:contrast-function
+     (choice :tag "Low contrast behavior"
+             :help-echo "How do we make colors legible?"
+             (const :tag "Do not adjust" nil)
+             (const :tag "Automatic (CIE)" face-contrast-automatic-cie)
+             (const :tag "Automatic (HSV)" face-contrast-automatic-hsv)
+             (color :tag "Override foreground"))
+     ;; filter to make value suitable for customize
+     (lambda (real-value)
+       (or
+        ;; We're loaded too early to use pcase
+        (and (eq (car-safe real-value) 'lambda)
+             (equal (car (cdr real-value)) '(fg bg))
+             (null (cdr (cdr (cdr real-value))))
+             (let ((form (car (cdr (cdr real-value)))))
+               (and (eq (car-safe form) 'face-contrast-fg-override-cie)
+                    (stringp (car (cdr form)))
+                    (car (cdr form)))))
+        real-value))
+     ;; filter to make customized-value suitable for storing
+     (lambda (cus-value)
+       (if (stringp cus-value)
+           `(lambda (fg bg)
+              (face-contrast-fg-override-cie ,cus-value fg bg))
+         cus-value)))
+
     (:inherit
      (repeat :tag "Inherit"
 	     :help-echo "List of faces to inherit attributes from."

=== modified file 'lisp/faces.el'
--- lisp/faces.el	2014-01-01 07:43:34 +0000
+++ lisp/faces.el	2014-01-13 12:57:49 +0000
@@ -274,8 +274,6 @@
     (:weight (".attributeWeight" . "Face.AttributeWeight"))
     (:slant (".attributeSlant" . "Face.AttributeSlant"))
     (:foreground (".attributeForeground" . "Face.AttributeForeground"))
-    (:distant-foreground
-     (".attributeDistantForeground" . "Face.AttributeDistantForeground"))
     (:background (".attributeBackground" . "Face.AttributeBackground"))
     (:overline (".attributeOverline" . "Face.AttributeOverline"))
     (:strike-through (".attributeStrikeThrough" . "Face.AttributeStrikeThrough"))
@@ -288,7 +286,9 @@
     (:bold (".attributeBold" . "Face.AttributeBold"))
     (:italic (".attributeItalic" . "Face.AttributeItalic"))
     (:font (".attributeFont" . "Face.AttributeFont"))
-    (:inherit (".attributeInherit" . "Face.AttributeInherit"))))
+    (:inherit (".attributeInherit" . "Face.AttributeInherit"))
+    (:contrast-function (".attributeContrastFunction" .
+                         "Face.ContrastFunction"))))
   "List of X resources and classes for face attributes.
 Each element has the form (ATTRIBUTE ENTRY1 ENTRY2...) where ATTRIBUTE is
 the name of a face attribute, and each ENTRY is a cons of the form
@@ -1070,7 +1070,8 @@
     (:foreground . "foreground color")
     (:background . "background color")
     (:stipple . "background stipple")
-    (:inherit . "inheritance"))
+    (:inherit . "inheritance")
+    (:contrast-function "contrast function"))
   "An alist of descriptive names for face attributes.
 Each element has the form (ATTRIBUTE-NAME . DESCRIPTION) where
 ATTRIBUTE-NAME is a face attribute name (a keyword symbol), and
@@ -1351,7 +1352,6 @@
 		  (:weight . "Weight")
 		  (:slant . "Slant")
 		  (:foreground . "Foreground")
-		  (:distant-foreground . "DistantForeground")
 		  (:background . "Background")
 		  (:underline . "Underline")
 		  (:overline . "Overline")
@@ -1361,7 +1361,8 @@
 		  (:stipple . "Stipple")
 		  (:font . "Font")
 		  (:fontset . "Fontset")
-		  (:inherit . "Inherit")))
+		  (:inherit . "Inherit")
+                  (:contrast-function . "Contrast-function")))
 	(max-width (apply #'max (mapcar #'(lambda (x) (length (cdr x)))
 					attrs))))
     (help-setup-xref (list #'describe-face face)
@@ -1919,6 +1920,93 @@
 		 ((memq ':background face) (cadr (memq ':background face)))))
 	  (t nil))))			; Invalid face value.
 
+(defcustom face-contrast-minimum-de2000 20.0
+  "Threshold to activate low-contrast behavior in face rendering.
+Units are as reported by `color-cie-de2000'."
+  :group 'faces
+  :type 'float
+  :version "24.4")
+
+(defcustom face-contrast-minimum-color-distance 30000
+  "Threshold to activate low-contrast behavior in face rendering.
+Units are as reported by `color-distance'."
+  :group 'faces
+  :type 'integer
+  :version "24.4")
+
+(defun face-xcolor-to-rgb (color)
+  "Convert an XColor triplet to a float RGB triplet."
+  (let ((maxv (float (car (color-values "#ffffff")))))
+    (mapcar (lambda (x) (/ x maxv)) color)))
+
+(defun face-contrast-fg-override-cie (color fg bg)
+  "Override a face foreground in low contrast situations."
+  (when (require 'color nil t)
+    (when (< (color-cie-de2000
+              (apply #'color-srgb-to-lab
+                     (face-xcolor-to-rgb fg))
+              (apply #'color-srgb-to-lab
+                     (face-xcolor-to-rgb bg)))
+             face-contrast-minimum-de2000)
+      (cons color nil))))
+
+(defun face-contrast-automatic-cie (fg bg)
+  "If colors differ by too much, make contrasting colors.
+Adjust the value component of FG until it achieves a minimum
+contrast against BG. Against dark backgrounds, prefer light
+colors and vice versa."
+  (when (require 'color nil t)
+    (let* ((fg-rgb (face-xcolor-to-rgb fg))
+           (fg-lab (apply #'color-srgb-to-lab fg-rgb))
+           (bg-lab (apply #'color-srgb-to-lab
+                          (face-xcolor-to-rgb bg))))
+      (when (< (color-cie-de2000 fg-lab bg-lab) face-contrast-minimum-de2000)
+        (let ((step (if (< (car bg-lab) 50.0) +2 -2)))
+          (while (and (setf (car fg-lab) (+ step (car fg-lab)))
+                      (<= 0 (car fg-lab) 100)
+                      (< (color-cie-de2000 fg-lab bg-lab)
+                         face-contrast-minimum-de2000))))
+        (when (< (car fg-lab) 0)
+          (setf (car fg-lab) 0))
+        (when (> (car fg-lab) 100)
+          (setf (car fg-lab) 100))
+        (cons
+         (apply #'color-rgb-to-hex
+                (mapcar #'color-clamp
+                        (apply #'color-lab-to-srgb fg-lab)))
+         nil)))))
+
+(defun face-contrast-fg-override-hsv (color fg bg)
+  "Override a face foreground in low contrast situations."
+  (when (< (color-distance fg bg) face-contrast-minimum-color-distance)
+    (cons color nil)))
+
+(defun face-contrast-automatic-hsv (fg bg)
+  "If colors differ by too much, make contrasting colors.
+Adjust the value component of FG until it achieves a minimum
+contrast against BG. Against dark backgrounds, prefer light
+colors and vice versa."
+  (when (and (< (color-distance fg bg) face-contrast-minimum-color-distance)
+             (require 'color nil t))
+    (let* ((fg-rgb (face-xcolor-to-rgb fg))
+           (fg-hsv (apply #'color-rgb-to-hsv fg-rgb))
+           (bg-hsv (apply #'color-rgb-to-hsv
+                          (face-xcolor-to-rgb bg))))
+      (let ((step (if (< (car (cdr (cdr bg-hsv))) 0.5) +0.1 -0.1))
+            (h (car fg-hsv))
+            (s (car (cdr fg-hsv)))
+            (v (car (cdr (cdr fg-hsv))))
+            (new-fg))
+        (while (and (setf v (+ v step))
+                    (<= 0 v 1.0)
+                    (setf new-fg
+                          (apply #'color-rgb-to-hex
+                                 (mapcar #'color-clamp
+                                         (color-hsv-to-rgb h s v))))
+                    (< (color-distance new-fg bg)
+                       face-contrast-minimum-color-distance)))
+        (cons (or new-fg fg) nil)))))
+
 \f
 ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
 ;;; Frame creation.
@@ -2256,19 +2344,22 @@
 ;; if background is light.
 (defface region
   '((((class color) (min-colors 88) (background dark))
-     :background "blue3")
+     :background "blue3"
+     :contrast-function face-contrast-automatic-cie)
     (((class color) (min-colors 88) (background light) (type gtk))
-     :distant-foreground "gtk_selection_fg_color"
-     :background "gtk_selection_bg_color")
+     :background "gtk_selection_bg_color"
+     :contrast-function face-contrast-automatic-cie)
     (((class color) (min-colors 88) (background light) (type ns))
-     :distant-foreground "ns_selection_fg_color"
-     :background "ns_selection_bg_color")
+     :background "ns_selection_bg_color"
+     :contrast-function face-contrast-automatic-cie)
     (((class color) (min-colors 88) (background light))
-     :background "lightgoldenrod2")
+     :background "lightgoldenrod2"
+     :contrast-function face-contrast-automatic-cie)
     (((class color) (min-colors 16) (background dark))
      :background "blue3")
     (((class color) (min-colors 16) (background light))
-     :background "lightgoldenrod2")
+     :background "lightgoldenrod2"
+     :contrast-function face-contrast-automatic-cie)
     (((class color) (min-colors 8))
      :background "blue" :foreground "white")
     (((type tty) (class mono))

=== modified file 'src/dispextern.h'
--- src/dispextern.h	2014-01-01 07:43:34 +0000
+++ src/dispextern.h	2014-01-13 08:13:06 +0000
@@ -1556,7 +1556,7 @@
   LFACE_FONT_INDEX,
   LFACE_INHERIT_INDEX,
   LFACE_FONTSET_INDEX,
-  LFACE_DISTANT_FOREGROUND_INDEX,
+  LFACE_CONTRAST_FUNCTION_INDEX,
   LFACE_VECTOR_SIZE
 };
 

=== modified file 'src/xfaces.c'
--- src/xfaces.c	2014-01-01 07:43:34 +0000
+++ src/xfaces.c	2014-01-13 09:26:17 +0000
@@ -292,7 +292,7 @@
 static Lisp_Object QCfont, QCbold, QCitalic;
 static Lisp_Object QCreverse_video;
 static Lisp_Object QCoverline, QCstrike_through, QCbox, QCinherit;
-static Lisp_Object QCfontset, QCdistant_foreground;
+static Lisp_Object QCfontset, QCcontrast_function;
 
 /* Symbols used for attribute values.  */
 
@@ -1263,8 +1263,6 @@
 
 #ifdef HAVE_WINDOW_SYSTEM
 
-#define NEAR_SAME_COLOR_THRESHOLD 30000
-
 /* Load colors for face FACE which is used on frame F.  Colors are
    specified by slots LFACE_BACKGROUND_INDEX and LFACE_FOREGROUND_INDEX
    of ATTRS.  If the background color specified is not supported on F,
@@ -1276,6 +1274,8 @@
 {
   Lisp_Object fg, bg, dfg;
   XColor xfg, xbg;
+  Lisp_Object contrast_function;
+  Lisp_Object adj_cons;
 
   bg = attrs[LFACE_BACKGROUND_INDEX];
   fg = attrs[LFACE_FOREGROUND_INDEX];
@@ -1303,14 +1303,30 @@
   face->background = load_color2 (f, face, bg, LFACE_BACKGROUND_INDEX, &xbg);
   face->foreground = load_color2 (f, face, fg, LFACE_FOREGROUND_INDEX, &xfg);
 
-  dfg = attrs[LFACE_DISTANT_FOREGROUND_INDEX];
-  if (!NILP (dfg) && !UNSPECIFIEDP (dfg)
-      && color_distance (&xbg, &xfg) < NEAR_SAME_COLOR_THRESHOLD)
+  contrast_function = attrs[LFACE_CONTRAST_FUNCTION_INDEX];
+  if (!NILP (contrast_function) && !UNSPECIFIEDP (contrast_function))
     {
-      if (EQ (attrs[LFACE_INVERSE_INDEX], Qt))
-        face->background = load_color (f, face, dfg, LFACE_BACKGROUND_INDEX);
-      else
-        face->foreground = load_color (f, face, dfg, LFACE_FOREGROUND_INDEX);
+      /* Give lisp a chance to adjust the generated colors.  */
+      adj_cons = safe_call2 (contrast_function,
+                             list3 (make_number (xfg.red),
+                                    make_number (xfg.green),
+                                    make_number (xfg.blue)),
+                             list3 (make_number (xbg.red),
+                                    make_number (xbg.green),
+                                    make_number (xbg.blue)));
+
+      if (CONSP (adj_cons))
+        {
+          if (STRINGP (XCAR (adj_cons)))
+              face->foreground =
+              load_color2 (f, face, XCAR (adj_cons),
+                           LFACE_FOREGROUND_INDEX, &xfg);
+
+          if (STRINGP (XCDR (adj_cons)))
+            face->background =
+              load_color2 (f, face, XCDR (adj_cons),
+                         LFACE_BACKGROUND_INDEX, &xbg);
+        }
     }
 }
 
@@ -1742,8 +1758,8 @@
 #define LFACE_FONT(LFACE)	    AREF ((LFACE), LFACE_FONT_INDEX)
 #define LFACE_INHERIT(LFACE)	    AREF ((LFACE), LFACE_INHERIT_INDEX)
 #define LFACE_FONTSET(LFACE)	    AREF ((LFACE), LFACE_FONTSET_INDEX)
-#define LFACE_DISTANT_FOREGROUND(LFACE) \
-  AREF ((LFACE), LFACE_DISTANT_FOREGROUND_INDEX)
+#define LFACE_CONTRAST_FUNCTION(LFACE) \
+  AREF ((LFACE), LFACE_CONTRAST_FUNCTION_INDEX)
 
 /* Non-zero if LFACE is a Lisp face.  A Lisp face is a vector of size
    LFACE_VECTOR_SIZE which has the symbol `face' in slot 0.  */
@@ -1806,9 +1822,6 @@
   eassert (UNSPECIFIEDP (attrs[LFACE_FOREGROUND_INDEX])
 	   || IGNORE_DEFFACE_P (attrs[LFACE_FOREGROUND_INDEX])
 	   || STRINGP (attrs[LFACE_FOREGROUND_INDEX]));
-  eassert (UNSPECIFIEDP (attrs[LFACE_DISTANT_FOREGROUND_INDEX])
-	   || IGNORE_DEFFACE_P (attrs[LFACE_DISTANT_FOREGROUND_INDEX])
-	   || STRINGP (attrs[LFACE_DISTANT_FOREGROUND_INDEX]));
   eassert (UNSPECIFIEDP (attrs[LFACE_BACKGROUND_INDEX])
 	   || IGNORE_DEFFACE_P (attrs[LFACE_BACKGROUND_INDEX])
 	   || STRINGP (attrs[LFACE_BACKGROUND_INDEX]));
@@ -1817,6 +1830,10 @@
 	   || NILP (attrs[LFACE_INHERIT_INDEX])
 	   || SYMBOLP (attrs[LFACE_INHERIT_INDEX])
 	   || CONSP (attrs[LFACE_INHERIT_INDEX]));
+  eassert (UNSPECIFIEDP (attrs[LFACE_CONTRAST_FUNCTION_INDEX])
+	   || IGNORE_DEFFACE_P (attrs[LFACE_CONTRAST_FUNCTION_INDEX])
+	   || NILP (attrs[LFACE_CONTRAST_FUNCTION_INDEX])
+	   || FUNCTIONP (attrs[LFACE_CONTRAST_FUNCTION_INDEX]));
 #ifdef HAVE_WINDOW_SYSTEM
   eassert (UNSPECIFIEDP (attrs[LFACE_STIPPLE_INDEX])
 	   || IGNORE_DEFFACE_P (attrs[LFACE_STIPPLE_INDEX])
@@ -2074,8 +2091,8 @@
   int i;
 
   for (i = 1; i < LFACE_VECTOR_SIZE; ++i)
-    if (i != LFACE_FONT_INDEX && i != LFACE_INHERIT_INDEX
-        && i != LFACE_DISTANT_FOREGROUND_INDEX)
+    if (i != LFACE_FONT_INDEX && i != LFACE_INHERIT_INDEX &&
+        i != LFACE_CONTRAST_FUNCTION_INDEX)
       if ((UNSPECIFIEDP (attrs[i]) || IGNORE_DEFFACE_P (attrs[i])))
 	break;
 
@@ -2476,13 +2493,6 @@
 		  else
 		    err = 1;
 		}
-	      else if (EQ (keyword, QCdistant_foreground))
-		{
-		  if (STRINGP (value))
-		    to[LFACE_DISTANT_FOREGROUND_INDEX] = value;
-		  else
-		    err = 1;
-		}
 	      else if (EQ (keyword, QCbackground))
 		{
 		  if (STRINGP (value))
@@ -2525,6 +2535,13 @@
 					err_msgs, named_merge_points))
 		    err = 1;
 		}
+              else if (EQ (keyword, QCcontrast_function))
+                {
+                  if (NILP (value) || FUNCTIONP (value))
+                    to[LFACE_CONTRAST_FUNCTION_INDEX] = value;
+                  else
+                    err = 1;
+                }
 	      else
 		err = 1;
 
@@ -3039,23 +3056,6 @@
       old_value = LFACE_FOREGROUND (lface);
       ASET (lface, LFACE_FOREGROUND_INDEX, value);
     }
-  else if (EQ (attr, QCdistant_foreground))
-    {
-      /* Compatibility with 20.x.  */
-      if (NILP (value))
-	value = Qunspecified;
-      if (!UNSPECIFIEDP (value) && !IGNORE_DEFFACE_P (value))
-	{
-	  /* Don't check for valid color names here because it depends
-	     on the frame (display) whether the color will be valid
-	     when the face is realized.  */
-	  CHECK_STRING (value);
-	  if (SCHARS (value) == 0)
-	    signal_error ("Empty distant-foreground color value", value);
-	}
-      old_value = LFACE_DISTANT_FOREGROUND (lface);
-      ASET (lface, LFACE_DISTANT_FOREGROUND_INDEX, value);
-    }
   else if (EQ (attr, QCbackground))
     {
       /* Compatibility with 20.x.  */
@@ -3073,6 +3073,15 @@
       old_value = LFACE_BACKGROUND (lface);
       ASET (lface, LFACE_BACKGROUND_INDEX, value);
     }
+  else if (EQ (attr, QCcontrast_function))
+    {
+      if (!UNSPECIFIEDP (value) && !IGNORE_DEFFACE_P (value)
+          && !FUNCTIONP (value) && !NILP (value))
+        signal_error ("Invalid contrast function", value);
+
+      old_value = LFACE_CONTRAST_FUNCTION (lface);
+      ASET (lface, LFACE_CONTRAST_FUNCTION_INDEX, value);
+    }
   else if (EQ (attr, QCstipple))
     {
 #if defined (HAVE_WINDOW_SYSTEM)
@@ -3700,8 +3709,6 @@
     value = LFACE_INVERSE (lface);
   else if (EQ (keyword, QCforeground))
     value = LFACE_FOREGROUND (lface);
-  else if (EQ (keyword, QCdistant_foreground))
-    value = LFACE_DISTANT_FOREGROUND (lface);
   else if (EQ (keyword, QCbackground))
     value = LFACE_BACKGROUND (lface);
   else if (EQ (keyword, QCstipple))
@@ -3714,6 +3721,8 @@
     value = LFACE_FONT (lface);
   else if (EQ (keyword, QCfontset))
     value = LFACE_FONTSET (lface);
+  else if (EQ (keyword, QCcontrast_function))
+    value = LFACE_CONTRAST_FUNCTION (lface);
   else
     signal_error ("Invalid face attribute name", keyword);
 
@@ -4741,9 +4750,6 @@
       || (!UNSPECIFIEDP (attrs[LFACE_FOREGROUND_INDEX])
 	  && face_attr_equal_p (attrs[LFACE_FOREGROUND_INDEX],
 				def_attrs[LFACE_FOREGROUND_INDEX]))
-      || (!UNSPECIFIEDP (attrs[LFACE_DISTANT_FOREGROUND_INDEX])
-	  && face_attr_equal_p (attrs[LFACE_DISTANT_FOREGROUND_INDEX],
-				def_attrs[LFACE_DISTANT_FOREGROUND_INDEX]))
       || (!UNSPECIFIEDP (attrs[LFACE_BACKGROUND_INDEX])
 	  && face_attr_equal_p (attrs[LFACE_BACKGROUND_INDEX],
 				def_attrs[LFACE_BACKGROUND_INDEX]))
@@ -6406,7 +6412,7 @@
   DEFSYM (QCwidth, ":width");
   DEFSYM (QCfont, ":font");
   DEFSYM (QCfontset, ":fontset");
-  DEFSYM (QCdistant_foreground, ":distant-foreground");
+  DEFSYM (QCcontrast_function, ":contrast-function");
   DEFSYM (QCbold, ":bold");
   DEFSYM (QCitalic, ":italic");
   DEFSYM (QCoverline, ":overline");

Re: [PATCH] Re: About the :distant-foreground face attribute

[Jan Djärv]

13 jan 2014 kl. 14:13 skrev Daniel Colascione <dancol@dancol.org>:

> <adaptive-face.patch>

Re: [PATCH] Re: About the :distant-foreground face attribute

[Jan Djärv]

Hello.

13 jan 2014 kl. 14:13 skrev Daniel Colascione <dancol@dancol.org>:

> The attached patch might be another solution to the problem. It replaces :distant-foreground with :contrast-function, which punts the actual contrast logic to lisp by calling the named function during face realization. (Performance isn't a problem in practice because we cache face realizations.) In lisp, we implement four low-contrast-mitigation policies: do not adjust for contrast, adjust automatically (by adjusting CIE L*A*B color space L values), adjust automatically (by adjusting the V values in HSV color space), or just set the foreground to a specific color if the contrast dips below a certain point (the current :distant-foreground behavior). Both the policy and the parameters (well, the override color) are customizable on a per-face basis; when merging faces, the one with the highest priority sets the whole behavior.

The main complaint (as I see it) was that a new attribute was added.  So does this.

> 
> The patch uses the CIE L*A*B colorspace algorithm by default.

Do not change the defaults please.  Reinstate the *_selection_fg_color.  They are system defined and should be honored.

> It produces surprisingly good results, at least in my tests, adapting automatically to light and dark backgrounds while preserving the hues of theme foreground colors.
> 
> (Changing themes nukes the face property right now, so you'll have to reset it each time.)
> <adaptive-face.patch>


@@ -1070,7 +1070,8 @@
     (:foreground . "foreground color")
     (:background . "background color")
     (:stipple . "background stipple")
-    (:inherit . "inheritance"))
+    (:inherit . "inheritance")
+    (:contrast-function "contrast function"))
   "An alist of descriptive names for face attributes.
 Each element has the form (ATTRIBUTE-NAME . DESCRIPTION) where
 ATTRIBUTE-NAME is a face attribute name (a keyword symbol), and
@@ -1351,7 +1352,6 @@


There is a . missing after :contrast-function.

Have you checked that no event handling code accesses a face fore/background?  You can not call a lisp function in that context.

	Jan D.

Re: [PATCH] Re: About the :distant-foreground face attribute

[Daniel Colascione]

On 01/13/2014 08:33 AM, Jan Djärv wrote:
> 13 jan 2014 kl. 14:13 skrev Daniel Colascione <dancol@dancol.org>:
>
>> The attached patch might be another solution to the problem. It replaces :distant-foreground with :contrast-function, which punts the actual contrast logic to lisp by calling the named function during face realization. (Performance isn't a problem in practice because we cache face realizations.) In lisp, we implement four low-contrast-mitigation policies: do not adjust for contrast, adjust automatically (by adjusting CIE L*A*B color space L values), adjust automatically (by adjusting the V values in HSV color space), or just set the foreground to a specific color if the contrast dips below a certain point (the current :distant-foreground behavior). Both the policy and the parameters (well, the override color) are customizable on a per-face basis; when merging faces, the one with the hig
 hest priority sets the whole behavior.
>
> The main complaint (as I see it) was that a new attribute was added.
So does this.

No. The main complaint is that we're adding a brittle face attribute 
with a very confusing name, not that just that we're adding an 
attribute. This patch allows for flexible policy and treats face 
foreground and background colors identically.

>> The patch uses the CIE L*A*B colorspace algorithm by default.
>
> Do not change the defaults please. Reinstate the
> *_selection_fg_color.
They are system defined and should be honored.

There are two sane defaults: the 24.3 behavior, where we always use the 
system selection foreground and background, and my proposed behavior, 
where we use the fontified foreground and automatically adjust it so 
that it's legible. The current behavior is worse because it uses the 
system selection foreground only sometimes and doesn't preserve theme 
hues when possible.

>
>> It produces surprisingly good results, at least in my tests, adapting automatically to light and dark backgrounds while preserving the hues of theme foreground colors.
>>
>> (Changing themes nukes the face property right now, so you'll have to reset it each time.)
>> <adaptive-face.patch>
>
>
> @@ -1070,7 +1070,8 @@
>       (:foreground . "foreground color")
>       (:background . "background color")
>       (:stipple . "background stipple")
> -    (:inherit . "inheritance"))
> +    (:inherit . "inheritance")
> +    (:contrast-function "contrast function"))
>     "An alist of descriptive names for face attributes.
>   Each element has the form (ATTRIBUTE-NAME . DESCRIPTION) where
>   ATTRIBUTE-NAME is a face attribute name (a keyword symbol), and
> @@ -1351,7 +1352,6 @@
>
>
> There is a . missing after :contrast-function.

Thanks.

> Have you checked that no event handling code accesses a face
> fore/background?  You can not call a lisp function in that context.

We only run this code when we're realizing faces, not when we're just 
accessing their attributes. We call lisp from merge_face_heights in the 
same context, so this use should be safe as well.

Re: [PATCH] Re: About the :distant-foreground face attribute

[Jan Djärv]

Hello.

13 jan 2014 kl. 19:41 skrev Daniel Colascione <dancol@dancol.org>:

> On 01/13/2014 08:33 AM, Jan Djärv wrote:
>> 13 jan 2014 kl. 14:13 skrev Daniel Colascione <dancol@dancol.org>:
>>> The patch uses the CIE L*A*B colorspace algorithm by default.
>> 
>> Do not change the defaults please. Reinstate the
>> *_selection_fg_color.
>> They are system defined and should be honored.
> 
> There are two sane defaults: the 24.3 behavior, where we always use the system selection foreground and background, and my proposed behavior, where we use the fontified foreground and automatically adjust it so that it's legible. The current behavior is worse because it uses the system selection foreground only sometimes and doesn't preserve theme hues when possible.

What theme hues?  The default theme is not really a theme as selection colors are taken from system settings.  And this is correct IMHO, any application that doesn't do so by default (i.e. no user configuration has been set in the application) is seriously broken.

If you talk about other themes, they can set :distant-foreground to a real color of their choosing and not rely on some automatically generated one which most probably don't fit the theme anyway.  Automatically generated colors are a crutch which should be avoided if possible, certainly not recommended.

Of course the 24.3 has been changed, that default is no longer valid due to bug #15668 and it is pointless to use that as an argument unless you mean that the bug fix should be reverted.

	Jan D.

Re: [PATCH] Re: About the :distant-foreground face attribute

[Daniel Colascione]

On 01/13/2014 01:29 PM, Jan Djärv wrote:
> 13 jan 2014 kl. 19:41 skrev Daniel Colascione <dancol@dancol.org>:
>
>> On 01/13/2014 08:33 AM, Jan Djärv wrote:
>>> 13 jan 2014 kl. 14:13 skrev Daniel Colascione <dancol@dancol.org>:
>>>> The patch uses the CIE L*A*B colorspace algorithm by default.
>>>
>>> Do not change the defaults please. Reinstate the
>>> *_selection_fg_color.
>>> They are system defined and should be honored.
>>
>> There are two sane defaults: the 24.3 behavior, where we always use the system selection foreground and background, and my proposed behavior, where we use the fontified foreground and automatically adjust it so that it's legible. The current behavior is worse because it uses the system selection foreground only sometimes and doesn't preserve theme hues when possible.
>
> What theme hues? The default theme is not really a theme as
> selection
colors are taken from system settings. And this is correct IMHO, any
application that doesn't do so by default (i.e. no user configuration
has been set in the application) is seriously broken.

Yes, but font-lock colors are specified with explicit colors (even in 
the default "theme"), and we want to preserve these even in the presence 
of a selection. We are *already* not honoring the system-specified 
foreground selection color. At the same time, we want to make sure that 
highlighted text is legible against a background of whatever the system 
selection color happens to be. The best way to do that is to 
automatically shift the foreground colors in value, but not hue, so that 
they remain legible while being recognizably the same color.

I think the results are actually pretty good. If a particular theme 
doesn't want to use automatic color adjustment, it can specify its own 
:contrast-function and do whatever it wants.

I would also support a scheme where, by default, 'region' sets 
foreground *and* background colors to the system selection colors and 
other faces don't show through. But we didn't decide to go in that 
direction.

> If you talk about other themes, they can set :distant-foreground to
> a
real color of their choosing and not rely on some automatically
generated one which most probably don't fit the theme anyway.
Automatically generated colors are a crutch which should be avoided if
possible, certainly not recommended.

There's no way that themes can take into account all the possible colors 
users and packages might use. Automatic contrast adjustment can do that.

If you want the :distant-foreground behavior, it can be accommodated in 
this patch. This patch also permits other schemes that some users might 
find more useful. We should push policy to user customization when 
possible instead of hardcoding policy in the logic of face attributes.

RE: [PATCH] Re: About the :distant-foreground face attribute

[Drew Adams]

> Yes, but font-lock colors are specified with explicit colors (even
> in the default "theme"), and we want to preserve these even in the
> presence of a selection. 

I, for one, do not want that.  I want region highlighting to
correctly show which characters have been selected - each character.
Region highlighting should never be overridden by font-lock
highlighting.  UI-101: Intro to User Interfaces.

> At the same time, we want to make sure that highlighted text is
> legible against a background of whatever the system selection color
> happens to be.

Then choose the `region' background accordingly.  If Emacs cannot
do that automatically in the case of some platforms, too bad - let
users compensate by setting `region' manually.  They should always
be the ultimate judge of what works best for them.

> The best way to do that is to automatically shift the foreground
> colors in value, but not hue, so that they remain legible while
> being recognizably the same color.

No.  The best way to do that is to let users do it - let their
preferences rule.  Users do not need Emacs changing whatever values
they set for the `region' face.

And if you say that your new feature does not override user preference
for `region', how can you determine that?  You cannot just compare the
current value to the standard value, since the standard value might in
fact be just what the user wants.  Who are we to say we know better
than the user?

> I think the results are actually pretty good.

That does not sound like a ringing endorsement.  Does that mean that
only 25% of user preferences are ignored?  10%?

> If a particular theme doesn't want to use automatic color adjustment,
> it can specify its own :contrast-function and do whatever it wants.

So if it does not specify a :contrast-function, you take that as a
license to impose a standard one, instead of as a preference to not
perform any :contrast-function twiddling at all?

> I would also support a scheme where, by default, 'region' sets
> foreground *and* background colors to the system selection colors
> and other faces don't show through. But we didn't decide to go in that
> direction.

How about supporting the time-worn "scheme" whereby users and Lisp
code can specify clearly what the region highlighting foreground and
background are, literally, with nothing hidden behind their backs
playing tricks on them?  IOW, treat `region' like other faces, and
treat its foreground and background equally.

> > If you talk about other themes, they can set :distant-foreground
> > to a real color of their choosing and not rely on some automatically
> > generated one which most probably don't fit the theme anyway.
> > Automatically generated colors are a crutch which should be avoided
> > if possible, certainly not recommended.
> 
> There's no way that themes can take into account all the possible
> colors users and packages might use. Automatic contrast adjustment
> can do that.

Bzzzzzt!  No, but thanks for playing.  Automatic contrast adjustment
can never respect all user (or package) `region' specs, precisely
because it *automatically adjusts* the appearance away from what users
(or packages) specify.

> If you want the :distant-foreground behavior, it can be accommodated
> in this patch. This patch also permits other schemes that some users
> might find more useful. We should push policy to user customization
> when possible instead of hardcoding policy in the logic of face
> attributes.

We should leave face specs defined by users and packages well enough
alone.  Let them decide what they want - and let them get what they
want, without the "benefit" of behind-the-back twiddling.  Users
deserve honest transparency, not parlor magic tricks.

Re: [PATCH] Re: About the :distant-foreground face attribute

[Daniel Colascione]

On 01/13/2014 03:01 PM, Drew Adams wrote:
>> Yes, but font-lock colors are specified with explicit colors (even
>> in the default "theme"), and we want to preserve these even in the
>> presence of a selection.
>
> I, for one, do not want that.  I want region highlighting to
> correctly show which characters have been selected - each character.
> Region highlighting should never be overridden by font-lock
> highlighting.  UI-101: Intro to User Interfaces.

Okay, good for you. You can configure Emacs to act that way.

>> At the same time, we want to make sure that highlighted text is
>> legible against a background of whatever the system selection color
>> happens to be.
>
> Then choose the `region' background accordingly.  If Emacs cannot
> do that automatically in the case of some platforms, too bad - let
> users compensate by setting `region' manually.  They should always
> be the ultimate judge of what works best for them.

If we choose a region background that works with traditional font-lock 
colors, that background color cannot come from the system. If we want 
the region background color to come from the system, we have to have 
some way of making it contrast with the foreground. You cannot 
simultaneously "choose the `region' background accordingly" and respect 
system preferences.

>> The best way to do that is to automatically shift the foreground
>> colors in value, but not hue, so that they remain legible while
>> being recognizably the same color.
>
> No.  The best way to do that is to let users do it - let their
> preferences rule.  Users do not need Emacs changing whatever values
> they set for the `region' face.

Emacs won't change any colors users set on the region face. If a user 
sets the region's foreground and background colors, Emacs will use those 
colors for the selection. This is not a difficult concept to grasp. We 
are talking specifically about the case where users do *not* specify a 
foreground color for region. For better or worse, it seems that Emacs 
will be shipping this way, so we should make this configuration work as 
well as we can.

> And if you say that your new feature does not override user preference
> for `region', how can you determine that?  You cannot just compare the
> current value to the standard value, since the standard value might in
> fact be just what the user wants.  Who are we to say we know better
> than the user?

I have no idea what you are talking about. The current value of what? 
The standard value of what?

>> I think the results are actually pretty good.
>
> That does not sound like a ringing endorsement.  Does that mean that
> only 25% of user preferences are ignored?  10%?

I said I found the results acceptable. Why are you trying to read 
meaning that isn't present into my post?

>> If a particular theme doesn't want to use automatic color adjustment,
>> it can specify its own :contrast-function and do whatever it wants.
>
> So if it does not specify a :contrast-function, you take that as a
> license to impose a standard one, instead of as a preference to not
> perform any :contrast-function twiddling at all?

Emacs adjusts colors only when a :contrast-function is set for some face 
applying to a particular character and that face isn't overridden by one 
that sets :contrast-function to nil.

>> I would also support a scheme where, by default, 'region' sets
>> foreground *and* background colors to the system selection colors
>> and other faces don't show through. But we didn't decide to go in that
>> direction.
>
> How about supporting the time-worn "scheme" whereby users and Lisp
> code can specify clearly what the region highlighting foreground and
> background are, literally, with nothing hidden behind their backs
> playing tricks on them?  IOW, treat `region' like other faces, and
> treat its foreground and background equally.

M-x customize-face RET region RET

Set foreground and background. Uncheck :constrast-function if you want, 
although if selecting good colors is as good as you claim, the contrast 
function will never go into effect anyway.

>>> If you talk about other themes, they can set :distant-foreground
>>> to a real color of their choosing and not rely on some automatically
>>> generated one which most probably don't fit the theme anyway.
>>> Automatically generated colors are a crutch which should be avoided
>>> if possible, certainly not recommended.
>>
>> There's no way that themes can take into account all the possible
>> colors users and packages might use. Automatic contrast adjustment
>> can do that.
>
> Bzzzzzt!  No, but thanks for playing.

You're really elevating the level of discourse around here, aren't you?

> Automatic contrast adjustment
> can never respect all user (or package) `region' specs, precisely
> because it *automatically adjusts* the appearance away from what users
> (or packages) specify.>

Of course users specify it --- it's part of the damn face. It's 
specified *right there*. If a user or package doesn't want automatic 
contrast adjustment, either don't ask for it or explicitly turn it off.

>> If you want the :distant-foreground behavior, it can be accommodated
>> in this patch. This patch also permits other schemes that some users
>> might find more useful. We should push policy to user customization
>> when possible instead of hardcoding policy in the logic of face
>> attributes.
>
> We should leave face specs defined by users and packages well enough
> alone.  Let them decide what they want - and let them get what they
> want, without the "benefit" of behind-the-back twiddling.  Users
> deserve honest transparency, not parlor magic tricks.

Yes, users should be in control. That's precisely what this change 
allows. Can you please try to understand it before haranguing it?

Re: [PATCH] Re: About the :distant-foreground face attribute

[Stefan Monnier]

> If you want the :distant-foreground behavior, it can be accommodated in this
> patch. This patch also permits other schemes that some users might find more
> useful. We should push policy to user customization when possible instead of
> hardcoding policy in the logic of face attributes.

FWIW, I like the idea of being able to compute the color dynamically.
I also would welcome a way to specify "color filters", e.g. a face which
"darkens the foreground color".  IOW the equivalent of the
floating-point :height settings, but for colors.

But I'd first like to hear Yidong's opinion on why it'd be better to
fold the "alternate background" feature into the :foreground attribute
rather than having it as a separate attribute.


        Stefan

Re: [PATCH] Re: About the :distant-foreground face attribute

[Daniel Colascione]

On 01/13/2014 03:57 PM, Stefan Monnier wrote:
>> If you want the :distant-foreground behavior, it can be accommodated in this
>> patch. This patch also permits other schemes that some users might find more
>> useful. We should push policy to user customization when possible instead of
>> hardcoding policy in the logic of face attributes.
>
> FWIW, I like the idea of being able to compute the color dynamically.
> I also would welcome a way to specify "color filters", e.g. a face which
> "darkens the foreground color".  IOW the equivalent of the
> floating-point :height settings, but for colors.

You can write something like that in my setup --- we actually call 
:contrast-function on every face realization. There's no reason it has 
to act only on certain conditions, although that's what all the existing 
implementations do.

We could generalize the mechanism --- maybe we could rename 
:contrast-function to :filter. Then, in merge_face_vectors, we call the 
supplied filter function with a vector (or alist?) of face attributes. 
It returns the new set of attributes and we use those instead of the 
ones we came up with. The filter function could use `color-values' to 
get RGB information from the raw face color attributes.

Doing this on every face lookup would probably be too expensive, but 
maybe we can (in face_at_buffer_position) first compute properties 
without filters and look that up in the cache. If we don't find it, we 
can then compute the properties again, this time applying filters, and 
cache the result.

RE: [PATCH] Re: About the :distant-foreground face attribute

[Drew Adams]

> > Region highlighting should never be overridden by font-lock
> > highlighting.
> 
> You can configure Emacs to act that way.

Emacs should act that way by default, as it always has.  Anyone
who wants automatic foreground twiddling for a given face should
ask for that explicitly.  Whether that twiddling is to accommodate
font-locking or for some other reason.

(Likewise, for face filtering, if this feature ends up going
that way in order to generalize the function and give it access
to and effect over all face attributes.)

> > Then choose the `region' background accordingly.  If Emacs cannot
> > do that automatically in the case of some platforms, too bad - let
> > users compensate by setting `region' manually.  They should always
> > be the ultimate judge of what works best for them.
> 
> If we choose a region background that works with traditional font-
> lock colors, that background color cannot come from the system.

Just don't do that.  Certainly not by default.

> If we want the region background color to come from the system,
> we have to have some way of making it contrast with the
> foreground.

What has Emacs done in the past?  Since Emacs can specify the
default background and foreground, it should be trivial for it
to come up with two colors that contrast.  And even two colors
that contrast and are different from the default (e.g. system)
background.

If Emacs on some system (how many are a real problem?) really
cannot find a default `region' background and foreground that
work, then punt and let the user try.  How have we gotten by
for almost 4 decades without this feature?

It's hard to believe that a contrasting pair of default
colors cannot be found.  Just take the font-lock nonsense
out of the equation, and there should be no problem - that's
my guess.

> Emacs won't change any colors users set on the region face.
> If a user sets the region's foreground and background colors,
> Emacs will use those colors for the selection. 

Glad to hear you say that.

And what if the default `region' foreground and background
are exactly what the user wants?  Does s?he have to jump
through a hoop to "set" face attributes to what they already
were, just to say "Hands off!"?  She shouldn't have to.

> We are talking specifically about the case where users do
> *not* specify a foreground color for region.

And what does "not specify" mean?  Does it mean only that
the value has not changed from the standard (default) value?
Or does it mean that users somehow explicitly let you know
that they do not mind if you twiddle their region?

A face being equal to its default setting does not imply
that the user gives Emacs license to change it.

IMO, any such feature should be opt-in, not opt-out.  A
user should not need to explicitly do anything to stop
Emacs from twiddling her region.  She should ask for
twiddling if she wants that.

> Emacs adjusts colors only when a :contrast-function is set
> for some face applying to a particular character and that
> face isn't overridden by one that sets :contrast-function to
> nil.

OK, that sounds a bit better, at least.  So if any face has a
nil :twiddle-me, er sorry, :contrast-function attribute, and
that face is merged with a face that has a non-nil one, the nil
one wins and there is no twiddling.  Is that right?

It is important that no face, including `region', have a
non-nil :contrast-function by default.

> Set foreground and background. Uncheck :constrast-function if
> you want

So it is easy for a user or Lisp code to turn this off.  Good.
Now let's turn it off by default.  It will be just as easy for
a user to turn it on, if s?he wants.

> If a user or package doesn't want automatic contrast
> adjustment, either don't ask for it or explicitly turn it off.

Don't ask for it (i.e., do nothing) sounds good, to keep it off.
Off by default.  Explicitly turn it on if you want it.

What you describe now sounds a bit like what I suggested a week
ago:

>> And that user control should be *per face*.  One should not
>> be obliged to choose either preventing the overriding or
>> allowing it for all faces.  The choice should be a function
>> of the particular face.  Now *that* could be done using a
>> new face attribute, if you want.  (Or a function.)

This is OK as long as any long-existing faces such as `region'
will not have non-nil :contrast-function attributes by default.
Let users who really want their region twiddled opt into that. 

But it still makes life more complicated for Lisp code that
wants to get or set the actual appearance of the face.  Whereas
before code needed only to get or set attribute :foreground,
now it will need to also check for a non-nil :contrast-function
and apply that.

Still, this sounds better than what has been proposed so far.

Re: [PATCH] Re: About the :distant-foreground face attribute

[Stefan Monnier]

>>> If you want the :distant-foreground behavior, it can be accommodated
>>> in this patch. This patch also permits other schemes that some users
>>> might find more useful. We should push policy to user customization
>>> when possible instead of hardcoding policy in the logic of
>>> face attributes.
>> FWIW, I like the idea of being able to compute the color dynamically.
>> I also would welcome a way to specify "color filters", e.g. a face which
>> "darkens the foreground color".  IOW the equivalent of the
>> floating-point :height settings, but for colors.
> You can write something like that in my setup --- we actually
> call :contrast-function on every face realization. There's no reason it has
> to act only on certain conditions, although that's what all the existing
> implementations do.

There are some differences, tho, w.r.t what the function sees:
does it see the fully merged face, or does it only see the face
merged up to the point where the filter appears.

The point of :distant-foreground is that it looks at the fully merged
face, whereas the floating-point :height settings apply to the face
merged up to the point where that :height appears.  The difference is
that "higher up" attributes can still override the result.

Both behaviors make sense in different circumstances.


        Stefan

Re: [PATCH] Re: About the :distant-foreground face attribute

[Stefan Monnier]

> Emacs should act that way by default, as it always has.

"Always" as in "ever since 24.3", that is.
'enuf said,


        Stefan

Re: [PATCH] Re: About the :distant-foreground face attribute

[Daniel Colascione]

It sounds like we're in broad agreement on the mechanism. Now let's talk 
about default policy.

On 01/13/2014 05:32 PM, Drew Adams wrote:
>>> Region highlighting should never be overridden by font-lock
>>> highlighting.
>>
>> You can configure Emacs to act that way.
>
> Emacs should act that way by default, as it always has.  Anyone
> who wants automatic foreground twiddling for a given face should
> ask for that explicitly.  Whether that twiddling is to accommodate
> font-locking or for some other reason.

I don't have a strong opinion about the default, although I lean toward 
the behavior #3 (3b specifically) below since it looks blingy and might 
help attract new users.

>>> Then choose the `region' background accordingly.  If Emacs cannot
>>> do that automatically in the case of some platforms, too bad - let
>>> users compensate by setting `region' manually.  They should always
>>> be the ultimate judge of what works best for them.

I still don't understand exactly what you're proposing. Here are our 
options:

1) We can ship Emacs with hard-coded defaults for region's foreground 
and background colors and ignore system settings. This arrangement will 
produce legible text, and there's no need to adjust region's colors.

2) We can ship Emacs such that it uses the system selection foreground 
and background colors for region. This arrangement will also produce 
legible text, and there's no need to adjust region's colors.

3) We can ship Emacs such that it uses the system selection background 
color and uses font-lock colors for foreground text. This arrangement 
will sometimes produce an illegible result because the system background 
color is chosen independently of font-lock's face colors. We can

   3a) do nothing and make users customize either their system selection 
color, Emacs region face, or font-lock color scheme

   3b) Automatically adjust foreground colors when the region face is in 
effect so that they're legible against the system selection background 
color. This is the policy my patch makes Emacs use by default and it's 
the one I prefer.

   3c) Switch to the system selection foreground color when the 
foreground color we would otherwise use would be illegible against the 
system selection background color. My patch can support this policy, and 
it's the only policy :distant-foreground supports.

   3d) Adjust the region face background color until it's legible 
against all font-lock faces? I *think* this is what you're proposing, 
but it doesn't make any sense. If we diverge from the system selection 
background color, we've already lost the integration benefit and should 
just use option #1 above.

So to put the discussion in more concrete terms: say I run a stock GTK3 
Emacs on a system with a background color exactly equal to "Blue1", 
which is the color we use for font-lock-function-name-face. I visit 
xfaces.c, search for "UNSPECIFIEDP", and hit C-a C-SPC C-e. What colors 
do you propose we use for the text "UNSPECIFIEDP" on that line?

>> Emacs won't change any colors users set on the region face.
>> If a user sets the region's foreground and background colors,
>> Emacs will use those colors for the selection.
>
> And what if the default `region' foreground and background
> are exactly what the user wants?  Does s?he have to jump
> through a hoop to "set" face attributes to what they already
> were, just to say "Hands off!"?  She shouldn't have to.

If we ship configuration 3b, we won't set a foreground color and we'll 
use the specified region background color (which will come from the 
system) and the font-lock supplied foreground unless the foreground 
would be illegible against the background. In this case, we adjust the 
foreground color until it's legible.

If a user wants to customize the selection color and uses M-x 
customize-face RET region RET, she'll be able to set foreground and 
background independently. If she unchecks :contrast-function, we will 
always use those colors exactly.

If she leaves :contrast-function set, Emacs will always use her 
specified colors unless, again, the combination would be illegible, and 
in this case, Emacs will adjust the foreground color until it becomes 
legible. But why would she select a foreground and background colors 
combination that is illegible in the first place?

>> We are talking specifically about the case where users do
>> *not* specify a foreground color for region.
>
> And what does "not specify" mean?  Does it mean only that
> the value has not changed from the standard (default) value?
> Or does it mean that users somehow explicitly let you know
> that they do not mind if you twiddle their region?

It depends on the configuration we ship.

> A face being equal to its default setting does not imply
> that the user gives Emacs license to change it.

I disagree. If our user leaves a face at the default setting, she's 
giving us *explicit license* to use whatever heuristics we think work best.

> IMO, any such feature should be opt-in, not opt-out.  A
> user should not need to explicitly do anything to stop
> Emacs from twiddling her region.  She should ask for
> twiddling if she wants that.

I'm in favor of shipping defaults that create a good user experience. If 
a user never expressed a preference with respect to foreground and 
background colors, there's no contract to violate.

>> Emacs adjusts colors only when a :contrast-function is set
>> for some face applying to a particular character and that
>> face isn't overridden by one that sets :contrast-function to
>> nil.
>
> OK, that sounds a bit better, at least.  So if any face has a
> nil :twiddle-me, er sorry, :contrast-function attribute, and
> that face is merged with a face that has a non-nil one, the nil
> one wins and there is no twiddling.  Is that right?

Right now, it depends on the order in which the faces are merged. The 
last face that specifies a :contrast-function gets to control the 
contrast behavior. Right now, region is the only face that specifies a 
contrast-function, and I can't think of another good use case at the moment.

> It is important that no face, including `region', have a
> non-nil :contrast-function by default.

I disagree. Putting it on :region by default is perfectly fine. Remember 
that the attribute has no effect unless the result would otherwise be 
illegible.

>>> And that user control should be *per face*.  One should not
>>> be obliged to choose either preventing the overriding or
>>> allowing it for all faces.  The choice should be a function
>>> of the particular face.  Now *that* could be done using a
>>> new face attribute, if you want.  (Or a function.)
>
> This is OK as long as any long-existing faces such as `region'
> will not have non-nil :contrast-function attributes by default.
> Let users who really want their region twiddled opt into that.
>
> But it still makes life more complicated for Lisp code that
> wants to get or set the actual appearance of the face.  Whereas
> before code needed only to get or set attribute :foreground,
> now it will need to also check for a non-nil :contrast-function
> and apply that.

The C code already has functionality to compute the exact face used to 
render a particular location in a particular buffer in a particular 
frame. We could expose this functionality to lisp if there were a good 
use case. But right now, I don't understand why lisp code would need to 
know the post-adjustment colors used for display. Can you explain why 
we'd want to know?

Also, today, any lisp code that wants to mimic the redisplay face 
combination logic needs to take into account text properties, 
frame-local variables, overlays, display attributes, and so on. It would 
be a big job, and I'm not aware of anyone who's done it.

> Still, this sounds better than what has been proposed so far.

Thanks.

Re: [PATCH] Re: About the :distant-foreground face attribute

[Daniel Colascione]

On 01/13/2014 05:45 PM, Stefan Monnier wrote:
>>>> If you want the :distant-foreground behavior, it can be accommodated
>>>> in this patch. This patch also permits other schemes that some users
>>>> might find more useful. We should push policy to user customization
>>>> when possible instead of hardcoding policy in the logic of
>>>> face attributes.
>>> FWIW, I like the idea of being able to compute the color dynamically.
>>> I also would welcome a way to specify "color filters", e.g. a face which
>>> "darkens the foreground color".  IOW the equivalent of the
>>> floating-point :height settings, but for colors.
>> You can write something like that in my setup --- we actually
>> call :contrast-function on every face realization. There's no reason it has
>> to act only on certain conditions, although that's what all the existing
>> implementations do.
>
> There are some differences, tho, w.r.t what the function sees:
> does it see the fully merged face, or does it only see the face
> merged up to the point where the filter appears.

Indeed. Well, let's honor the long tradition of choosing the policy 
that's easier to implement. :-)

We can support our hypothetical :filter (which we should rename 
:post-realize-filter) without too much change to the face logic using 
this scheme:

1) Define LFACE_FILTER_FUNCTION as a possibly-empty list of filter functions

2) When we combine faces A and B, A+B's LFACE_FILTER_FUNCTION is the 
concatenation of A's filter list and B's filter list. As an exception,
if B's filter list is of the form (t . OVERRIDE-FILTER-LIST), then A+B's 
filter list is just OVERRIDE-FILTER-LIST.

3) On realization, we pass the resulting face attributes through each 
filter function, then use the resulting attributes for display.

Later, if we need to, we can add a regular :filter that works 
face-by-face, like height.

Re: [PATCH] Re: About the :distant-foreground face attribute

[Jan D.]

Hello.

Your mailer does not quote replies properly, which is distracting.  Can 
you fix that?

Daniel Colascione skrev 2014-01-13 22:36:
> On 01/13/2014 01:29 PM, Jan Djärv wrote:
>> 13 jan 2014 kl. 19:41 skrev Daniel Colascione <dancol@dancol.org>:
>>
>>> On 01/13/2014 08:33 AM, Jan Djärv wrote:
>>>> 13 jan 2014 kl. 14:13 skrev Daniel Colascione <dancol@dancol.org>:
>>>>> The patch uses the CIE L*A*B colorspace algorithm by default.
>>>>
>>>> Do not change the defaults please. Reinstate the
>>>> *_selection_fg_color.
>>>> They are system defined and should be honored.
>>>
>>> There are two sane defaults: the 24.3 behavior, where we always use
>>> the system selection foreground and background, and my proposed
>>> behavior, where we use the fontified foreground and automatically
>>> adjust it so that it's legible. The current behavior is worse because
>>> it uses the system selection foreground only sometimes and doesn't
>>> preserve theme hues when possible.
>>
>> What theme hues? The default theme is not really a theme as
>> selection
> colors are taken from system settings. And this is correct IMHO, any
> application that doesn't do so by default (i.e. no user configuration
> has been set in the application) is seriously broken.
>
> Yes, but font-lock colors are specified with explicit colors (even in
> the default "theme"), and we want to preserve these even in the presence
> of a selection. We are *already* not honoring the system-specified
> foreground selection color. At the same time, we want to make sure that
> highlighted text is legible against a background of whatever the system
> selection color happens to be. The best way to do that is to
> automatically shift the foreground colors in value, but not hue, so that
> they remain legible while being recognizably the same color.

Given the use case at hand, we know for a fact that the background is 
the region background, so I don't understand why a calculated foreground 
is needed.  Just pick one that matches the background.
There might be other use cases where a calculated foreground makes 
sense, but my imagination fails me here.

> I would also support a scheme where, by default, 'region' sets
> foreground *and* background colors to the system selection colors and
> other faces don't show through. But we didn't decide to go in that
> direction.

FWIW, here Eclipse, XCode and Visual Studio all shows the (equivalent 
of) foreground color from font lock face and background from region when 
selecting text with the mouse, so it is not as Eamcs is breaking new 
ground here.

>
>> If you talk about other themes, they can set :distant-foreground to
>> a
> real color of their choosing and not rely on some automatically
> generated one which most probably don't fit the theme anyway.
> Automatically generated colors are a crutch which should be avoided if
> possible, certainly not recommended.
>
> There's no way that themes can take into account all the possible colors
> users and packages might use. Automatic contrast adjustment can do that.

Again, I really don't see this use case.  Do you have one?

>
> If you want the :distant-foreground behavior, it can be accommodated in
> this patch. This patch also permits other schemes that some users might
> find more useful.

I know that, this is just about defaults.

> We should push policy to user customization when
> possible instead of hardcoding policy in the logic of face attributes.

I don't think we do that, users can still customize faces as they see fit.

	Jan D.

Re: [PATCH] Re: About the :distant-foreground face attribute

[Daniel Colascione]

On 01/13/2014 11:47 PM, Jan D. wrote:
> Your mailer does not quote replies properly, which is distracting.  Can
> you fix that?

It looks fine to me.

> Daniel Colascione skrev 2014-01-13 22:36:
>> On 01/13/2014 01:29 PM, Jan Djärv wrote:
>>> 13 jan 2014 kl. 19:41 skrev Daniel Colascione <dancol@dancol.org>:
>>>
>>>> On 01/13/2014 08:33 AM, Jan Djärv wrote:
>>>>> 13 jan 2014 kl. 14:13 skrev Daniel Colascione <dancol@dancol.org>:
>>>>>> The patch uses the CIE L*A*B colorspace algorithm by default.
>>>>>
>>>>> Do not change the defaults please. Reinstate the
>>>>> *_selection_fg_color.
>>>>> They are system defined and should be honored.
>>>>
>>>> There are two sane defaults: the 24.3 behavior, where we always use
>>>> the system selection foreground and background, and my proposed
>>>> behavior, where we use the fontified foreground and automatically
>>>> adjust it so that it's legible. The current behavior is worse because
>>>> it uses the system selection foreground only sometimes and doesn't
>>>> preserve theme hues when possible.
>>>
>>> What theme hues? The default theme is not really a theme as
>>> selection
>> colors are taken from system settings. And this is correct IMHO, any
>> application that doesn't do so by default (i.e. no user configuration
>> has been set in the application) is seriously broken.
>>
>> Yes, but font-lock colors are specified with explicit colors (even in
>> the default "theme"), and we want to preserve these even in the presence
>> of a selection. We are *already* not honoring the system-specified
>> foreground selection color. At the same time, we want to make sure that
>> highlighted text is legible against a background of whatever the system
>> selection color happens to be. The best way to do that is to
>> automatically shift the foreground colors in value, but not hue, so that
>> they remain legible while being recognizably the same color.
>
> Given the use case at hand, we know for a fact that the background is
> the region background, so I don't understand why a calculated foreground
> is needed.  Just pick one that matches the background.
> There might be other use cases where a calculated foreground makes
> sense, but my imagination fails me here.

Calculated foreground colors look better: they resemble the font-lock 
colors on which they're based.

>> I would also support a scheme where, by default, 'region' sets
>> foreground *and* background colors to the system selection colors and
>> other faces don't show through. But we didn't decide to go in that
>> direction.
>
> FWIW, here Eclipse, XCode and Visual Studio all shows the (equivalent
> of) foreground color from font lock face and background from region when
> selecting text with the mouse, so it is not as Eamcs is breaking new
> ground here.
>
>>
>>> If you talk about other themes, they can set :distant-foreground to
>>> a
>> real color of their choosing and not rely on some automatically
>> generated one which most probably don't fit the theme anyway.
>> Automatically generated colors are a crutch which should be avoided if
>> possible, certainly not recommended.
>>
>> There's no way that themes can take into account all the possible colors
>> users and packages might use. Automatic contrast adjustment can do that.
>
> Again, I really don't see this use case.  Do you have one?

I just mentioned one. See my other email, the one where I laid out the 
options for Emacs configuration defaults.

>> We should push policy to user customization when
>> possible instead of hardcoding policy in the logic of face attributes.
>
> I don't think we do that, users can still customize faces as they see fit.

:distant-foreground has far too narrow a justification to warrant being 
a face property by itself.

Re: [PATCH] Re: About the :distant-foreground face attribute

[Chong Yidong]

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

> But I'd first like to hear Yidong's opinion on why it'd be better to
> fold the "alternate background" feature into the :foreground attribute
> rather than having it as a separate attribute.

1. Each aspect of a face should be controlled by one attribute, like
   :height (which can take either relative or absolute heights), and
   unlike :font vs :family (which has been a source of annoyance).

2. As currently implemented, :distant-foreground is the only attribute
   that can be `unspecified' even for the default face.  This breaks a
   nice and easily understandable rule, that `face-attribute' returns
   something specific for each attribute of the default face.

3. Neatness aside, if the alternative color stuff is supposed to be
   disabled by setting :distant-foreground to `unspecified', as it is
   now, then one cannot inherit from a face and then disable the
   alternative color specified by the parent.  If :distant-foreground is
   `unspecified' in the inheriting face, the parent's non-`unspecified'
   value of :distant-foreground takes effect.

   I suppose one could get around 2. and 3. by using a value of nil to
   disable the alternative color.  This then breaks the symmetry between
   :foreground and :distant-foreground, as the former does not take nil
   values (although `internal-set-lisp-face-attribute' accepts a nil
   value, that's only for 20.x background compatibility).

Re: [PATCH] Re: About the :distant-foreground face attribute

[Jan D.]

Daniel Colascione skrev 2014-01-14 09:18:
> On 01/13/2014 11:47 PM, Jan D. wrote:
>> Your mailer does not quote replies properly, which is distracting.  Can
>> you fix that?
>
> It looks fine to me.

It seems to quote only the first line, and not subsequent lines.  You 
can easily see this in the web archive for emacs-devel.


>>
>> Given the use case at hand, we know for a fact that the background is
>> the region background, so I don't understand why a calculated foreground
>> is needed.  Just pick one that matches the background.
>> There might be other use cases where a calculated foreground makes
>> sense, but my imagination fails me here.
>
> Calculated foreground colors look better: they resemble the font-lock
> colors on which they're based.

For the region case, that would imply the possibility of different 
foregrounds for marked text, none which is the actual font-lock color. 
That is just confusing.  Also, "look better" is just subjective, a theme 
designer should deside what looks best, not Emacs code.

>> Again, I really don't see this use case.  Do you have one?
>
> I just mentioned one. See my other email, the one where I laid out the
> options for Emacs configuration defaults.

That is not a use case.  A use case describes what an actual user does 
and in what situation automatically calculates colors enters the picture.

>
>>> We should push policy to user customization when
>>> possible instead of hardcoding policy in the logic of face attributes.
>>
>> I don't think we do that, users can still customize faces as they see
>> fit.
>
> :distant-foreground has far too narrow a justification to warrant being
> a face property by itself.

Also, just an opinion, not based on some documented design rule in any 
Emacs or GNU document.  IMHO :stipple is as narrow.

	Jan D.

Re: [PATCH] Re: About the :distant-foreground face attribute

[Jan D.]

Hi.

Chong Yidong skrev 2014-01-14 09:47:
> Stefan Monnier <monnier@iro.umontreal.ca> writes:
>
>> But I'd first like to hear Yidong's opinion on why it'd be better to
>> fold the "alternate background" feature into the :foreground attribute
>> rather than having it as a separate attribute.
>
> 1. Each aspect of a face should be controlled by one attribute, like
>     :height (which can take either relative or absolute heights), and
>     unlike :font vs :family (which has been a source of annoyance).

The same can be said for :background and :stipple.

	Jan D.

Re: [PATCH] Re: About the :distant-foreground face attribute

[Daniel Colascione]

On 01/14/2014 01:34 AM, Jan D. wrote:
>>> Given the use case at hand, we know for a fact that the background is
>>> the region background, so I don't understand why a calculated foreground
>>> is needed.  Just pick one that matches the background.
>>> There might be other use cases where a calculated foreground makes
>>> sense, but my imagination fails me here.
>>
>> Calculated foreground colors look better: they resemble the font-lock
>> colors on which they're based.
>
> For the region case, that would imply the possibility of different
> foregrounds for marked text, none which is the actual font-lock color.

It *is* the same color in the sense that the code we generate has the 
same hue. How on earth is that worse than changing arbitrary font-locked 
pieces of text to the system selection foreground color?

You seem to object to any feature that renders a face in a color other 
than what's literally written in the face spec. But that's exactly what 
happens when we put gtk_selection_bg_color in a color spec! Why are you 
fine with allowing gtk_selection_bg_color as a color (producing a color 
that's presumably workable but unknown to a theme writer) but adamantly 
opposed to a feature that selects good colors another way?

I've produced working code that allows Emacs to act how you prefer *and* 
how I prefer based on user configuration. What is your problem with that 
code? Have you responded to the numerous objections to 
:distant-foreground with anything other than retrenchment?

> Also, "look better" is just subjective, a theme
> designer should deside what looks best, not Emacs code.

And a theme designer can opt not to use the feature or to explicitly 
turn it off --- but a theme designer should never have to bother, 
because the feature only activates when the contrast is awful, which 
happens only when a theme designer screws up or deliberately relies on 
system colors.

Theme designers can specify what exactly gets rendered in both systems. 
Why do you care so much about letting theme designers loosen bounds 
using a special-purpose face attribute (which you don't even bother to 
mirror for changing backgrounds) instead of a general-purpose mechanism?

>>> Again, I really don't see this use case.  Do you have one?
>>
>> I just mentioned one. See my other email, the one where I laid out the
>> options for Emacs configuration defaults.
>
> That is not a use case.  A use case describes what an actual user does
> and in what situation automatically calculates colors enters the picture.

This entire thread describes the use case. There are abundant examples. 
Your objection here is baseless.

>>>> We should push policy to user customization when
>>>> possible instead of hardcoding policy in the logic of face attributes.
>>>
>>> I don't think we do that, users can still customize faces as they see
>>> fit.
>>
>> :distant-foreground has far too narrow a justification to warrant being
>> a face property by itself.
>
> Also, just an opinion, not based on some documented design rule in any
> Emacs or GNU document.

So only GNU documents count now, not reasoned arguments? What 
"documented design rule in any Emacs or GNU document" justifies 
:distant-foreground?

>  IMHO :stipple is as narrow.

:stipple is nothing like what we're talking about.

Re: [PATCH] Re: About the :distant-foreground face attribute

[Jan D.]

Daniel Colascione skrev 2014-01-14 11:44:
> On 01/14/2014 01:34 AM, Jan D. wrote:
>>>> Given the use case at hand, we know for a fact that the background is
>>>> the region background, so I don't understand why a calculated
>>>> foreground
>>>> is needed.  Just pick one that matches the background.
>>>> There might be other use cases where a calculated foreground makes
>>>> sense, but my imagination fails me here.
>>>
>>> Calculated foreground colors look better: they resemble the font-lock
>>> colors on which they're based.
>>
>> For the region case, that would imply the possibility of different
>> foregrounds for marked text, none which is the actual font-lock color.
>
> It *is* the same color in the sense that the code we generate has the
> same hue. How on earth is that worse than changing arbitrary font-locked
> pieces of text to the system selection foreground color?

Because the system color foreground is (presumably) choosen to look good 
together with the system color background.

>
> You seem to object to any feature that renders a face in a color other
> than what's literally written in the face spec.

It may seem that way to you, but it is false.

> But that's exactly what
> happens when we put gtk_selection_bg_color in a color spec! Why are you
> fine with allowing gtk_selection_bg_color as a color (producing a color
> that's presumably workable but unknown to a theme writer) but adamantly
> opposed to a feature that selects good colors another way?

I'm objecting to having the system default colors replaced with 
generated colors for the default case.  What themes other than the 
default does is irrelevant.  We are just talking about defaults here.

>
> I've produced working code that allows Emacs to act how you prefer *and*
> how I prefer based on user configuration. What is your problem with that
> code?

I have no problem with the code, just the defaults.

> Have you responded to the numerous objections to
> :distant-foreground with anything other than retrenchment?

So far the objections have been mostly opinions.  If there was a 
concrete case or a bug it exposes that would be another issue.
The most concrete objection is the case of inheriting faces Yidong 
brought up in another mail, which indeed is a real problem.


>> Also, "look better" is just subjective, a theme
>> designer should deside what looks best, not Emacs code.
>
> And a theme designer can opt not to use the feature or to explicitly
> turn it off --- but a theme designer should never have to bother,
> because the feature only activates when the contrast is awful, which
> happens only when a theme designer screws up or deliberately relies on
> system colors.

Why should a theme designer rely on system colors?  That does not sound 
like a good theme.  Again, the default should rely on system colors, not 
arbitrary themes.

>
> Theme designers can specify what exactly gets rendered in both systems.
> Why do you care so much about letting theme designers loosen bounds
> using a special-purpose face attribute (which you don't even bother to
> mirror for changing backgrounds) instead of a general-purpose mechanism?

One could think of changing the background instead of the foreground, 
but the bug report was about the foreground, so the implementation 
matches.  There was no need to implement a general mechanism for this 
specific bug report.

>> That is not a use case.  A use case describes what an actual user does
>> and in what situation automatically calculates colors enters the picture.
>
> This entire thread describes the use case. There are abundant examples.
> Your objection here is baseless.

There is a lot of handwaving and describing of mechanisms, but not one 
real world example, i.e. what major mode and which font lock faces and 
possible other faces are involved, and what does the user do to trigger 
the generated color.

>>> :distant-foreground has far too narrow a justification to warrant being
>>> a face property by itself.
>>
>> Also, just an opinion, not based on some documented design rule in any
>> Emacs or GNU document.
>
> So only GNU documents count now, not reasoned arguments?

No, but if documentation of some design rule existed it would give 
weight to your argument.  But your statement about :distant-foreground 
being too narrow is just your personal opinion which you have not 
motivated with any reasoned argument, just asserted.

	Jan D.

Re: [PATCH] Re: About the :distant-foreground face attribute

[Stefan Monnier]

> The most concrete objection is the case of inheriting faces Yidong brought
> up in another mail, which indeed is a real problem.

Can someone clarify the problem?
Is there an alternative suggestion which solves the original problem
while at the same time fixing this one?


        Stefan

Re: [PATCH] Re: About the :distant-foreground face attribute

[Jan Djärv]

Hello.

14 jan 2014 kl. 18:56 skrev Stefan Monnier <monnier@iro.umontreal.ca>:

>> The most concrete objection is the case of inheriting faces Yidong brought
>> up in another mail, which indeed is a real problem.
> 
> Can someone clarify the problem?

The region face defines :distant-foreground.  A face that inherits region can not override that attribute so that is undefined, it can only be overridden to another color.

> Is there an alternative suggestion which solves the original problem
> while at the same time fixing this one?

Yidong hinted at a fix, i.e. let nil be a valid value, like for :stipple.  The argument against was that :distant-foreground would have nil as a valid value, but :foreground does not, i.e. they do not have the same values.
The authors of the alternate implementations must answer for their cases.

	Jan D.

Re: [PATCH] Re: About the :distant-foreground face attribute

[Daniel Colascione]

On 01/14/2014 10:06 AM, Jan Djärv wrote:
> 14 jan 2014 kl. 18:56 skrev Stefan Monnier <monnier@iro.umontreal.ca>:
>
>>> The most concrete objection is the case of inheriting faces Yidong brought
>>> up in another mail, which indeed is a real problem.
>>
>> Can someone clarify the problem?
>
> The region face defines :distant-foreground. A face that inherits
> region can not override that attribute so that is undefined, it can only
> be overridden to another color.
>
>> Is there an alternative suggestion which solves the original problem
>> while at the same time fixing this one?
>
> Yidong hinted at a fix, i.e. let nil be a valid value, like for
> :stipple.  The argument against was that :distant-foreground would
> have nil as a valid value, but :foreground does not, i.e. they do not
> have the same values. The authors of the alternate implementations
> must answer for their cases.

My proposal does this and more. We should use it instead of the current 
:distant-foreground code regardless of the default policy we choose to 
use with it.

Re: [PATCH] Re: About the :distant-foreground face attribute

[Daniel Colascione]

On 01/14/2014 03:44 AM, Jan D. wrote:
> Daniel Colascione skrev 2014-01-14 11:44:
>> On 01/14/2014 01:34 AM, Jan D. wrote:
>>>>> Given the use case at hand, we know for a fact that the background is
>>>>> the region background, so I don't understand why a calculated
>>>>> foreground
>>>>> is needed.  Just pick one that matches the background.
>>>>> There might be other use cases where a calculated foreground makes
>>>>> sense, but my imagination fails me here.
>>>>
>>>> Calculated foreground colors look better: they resemble the font-lock
>>>> colors on which they're based.
>>>
>>> For the region case, that would imply the possibility of different
>>> foregrounds for marked text, none which is the actual font-lock color.
>>
>> It *is* the same color in the sense that the code we generate has the
>> same hue. How on earth is that worse than changing arbitrary font-locked
>> pieces of text to the system selection foreground color?
>
> Because the system color foreground is (presumably) choosen to look good
> together with the system color background.

Yes, and a color we algorithmically generate from a font-lock face will 
*also* look good against that background color, but 1) will be distinct 
from other faces replaced for lack of contrast, and 2) will be visually 
similar to the pre-highlight face. Have you tried the patch?

>> But that's exactly what
>> happens when we put gtk_selection_bg_color in a color spec! Why are you
>> fine with allowing gtk_selection_bg_color as a color (producing a color
>> that's presumably workable but unknown to a theme writer) but adamantly
>> opposed to a feature that selects good colors another way?
>
> I'm objecting to having the system default colors replaced with
> generated colors for the default case.  What themes other than the
> default does is irrelevant.  We are just talking about defaults here.

We already replace the system default colors with colors from font-lock 
faces.

>> I've produced working code that allows Emacs to act how you prefer *and*
>> how I prefer based on user configuration. What is your problem with that
>> code?
>
> I have no problem with the code, just the defaults.
>
>> Have you responded to the numerous objections to
>> :distant-foreground with anything other than retrenchment?
>
> So far the objections have been mostly opinions.  If there was a
> concrete case or a bug it exposes that would be another issue.
> The most concrete objection is the case of inheriting faces Yidong
> brought up in another mail, which indeed is a real problem.

Problems with usability in UI or API design don't result in clear "bugs" 
in the same way that logic errors or crashes do. Fixing them relies on 
judgment, and that relies chiefly on informed opinion. You can't dismiss 
others' opinions about this feature while not subjecting your own to the 
same treatment.

>> And a theme designer can opt not to use the feature or to explicitly
>> turn it off --- but a theme designer should never have to bother,
>> because the feature only activates when the contrast is awful, which
>> happens only when a theme designer screws up or deliberately relies on
>> system colors.
>
> Why should a theme designer rely on system colors?  That does not sound
> like a good theme.  Again, the default should rely on system colors, not
> arbitrary themes.

You are not responding to the point I make in the text you quote. I 
don't know why a theme designer would use system colors either. That's 
not the point. I'm saying that explicitly-chosen themes should never 
trigger the code under discussion if they're well-designed.

>>> That is not a use case.  A use case describes what an actual user does
>>> and in what situation automatically calculates colors enters the
>>> picture.
>>
>> This entire thread describes the use case. There are abundant examples.
>> Your objection here is baseless.
>
> There is a lot of handwaving and describing of mechanisms, but not one
> real world example, i.e. what major mode and which font lock faces and
> possible other faces are involved, and what does the user do to trigger
> the generated color.

Message-ID: <52D4A23E.4030802@dancol.org>
From: Daniel Colascione <dancol@dancol.org>
> So to put the discussion in more concrete terms: say I run a stock
> GTK3 Emacs on a system with a background color exactly equal to
> "Blue1", which is the color we use for font-lock-function-name-face.
> I visit xfaces.c, search for "UNSPECIFIEDP", and hit C-a C-SPC C-e.
> What colors do you propose we use for the text "UNSPECIFIEDP" on that
> line?

>
>>>> :distant-foreground has far too narrow a justification to warrant being
>>>> a face property by itself.
>>>
>>> Also, just an opinion, not based on some documented design rule in any
>>> Emacs or GNU document.
>>
>> So only GNU documents count now, not reasoned arguments?
>
> No, but if documentation of some design rule existed it would give
> weight to your argument.  But your statement about :distant-foreground
> being too narrow is just your personal opinion which you have not
> motivated with any reasoned argument, just asserted.

I'm not the only one who's expressed concerns about the design of the 
feature.

Re: [PATCH] Re: About the :distant-foreground face attribute

[John Yates]

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

My lurker's interpretation is that there are two model of emacs usage at
odds here:
1) emacs is my environement (My emacs controls the horizontal and the
vertical. OS themes be damned! Just stay out of its way.)
2) emacs as a good citizen in a OS themed setting

The latter is likely the expectation of (most?) modern newbies.  They use
many windowed applications that display text and perform simple
manipulations using a foreground color, background color and selection
highlight color specified by an OS theme.  They may well have exploited
their ability to adjust the OS theme.  But being newbies they have yet to
embark on customizing emacs' colors.  Hence they expect fontlocked text to
cohabit comfortably with their OS theme's background and selection colors.

/john


On Tue, Jan 14, 2014 at 12:56 PM, Stefan Monnier
<monnier@iro.umontreal.ca>wrote:

> > The most concrete objection is the case of inheriting faces Yidong
> brought
> > up in another mail, which indeed is a real problem.
>
> Can someone clarify the problem?
> Is there an alternative suggestion which solves the original problem
> while at the same time fixing this one?
>
>
>         Stefan
>
>

[-- Attachment #2: Type: text/html, Size: 1654 bytes --]

RE: [PATCH] Re: About the :distant-foreground face attribute

[Drew Adams]

>>>> Region highlighting should never be overridden by font-lock
>>>> highlighting.
>>> 
>>> You can configure Emacs to act that way.
>>
>> Emacs should act that way by default, as it always has.
> 
> "Always" as in "ever since 24.3", that is. 'enuf said,

Really?  I don't see font-lock highlighting overriding region
highlighting in Emacs 23, 22, 21, or 20 either.  And I don't
recall it ever doing so for Emacs back to The Beginning.

RE: [PATCH] Re: About the :distant-foreground face attribute

[Drew Adams]

> FWIW, I like the idea of being able to compute the color
> dynamically. I also would welcome a way to specify "color filters",
> e.g. a face which "darkens the foreground color".  IOW the equivalent
> of the floating-point :height settings, but for colors.

Instead of :contrast-function (since this should not be limited to
foreground contrast), I guess you are essentially arguing to generalize
this to a :filter face attribute with a function value that would take
the whole face into account, as it is currently defined, and would act
on whatever attributes it wants, to update (modify) the face.

Is that right?  If so, that is not a filter, which is something that
just rejects some individuals from a collection (applies a predicate).
That is a transformation.  The function-valued attribute would be
better named :transformer or :transform-function.

But why have only one such function for a given face - its designated
transformation function, cast in bronze as an attribute?  Why wouldn't
code just apply whatever transformations it wanted?

Daniel proposes that a :filter attribute value be a _list_ of
transformation functions.  But the same question arises: why favor
one list of such functions by putting into an attribute value.

The same questions can be posed for whatever foreground-enhancing
transformation you are currently envisioning.  Why have a face
attribute for that?  Why not just perform whatever foreground
modification your code thinks is needed, directly?  What is the value
in promoting that particular transformation function into a face
attribute? 

Is it to give users control over this feature by using Customize?
If so, why not do that as we usually do: provide your (hopefully
optional, hopefully opt-in) feature via a user option?  With the option
turned on by the user, your automatic foreground enhancing would take
effect for some list of faces (e.g. just `(region)').

RE: [PATCH] Re: About the :distant-foreground face attribute

[Drew Adams]

> > Emacs should act that way by default, as it always has.  Anyone
> > who wants automatic foreground twiddling for a given face should
> > ask for that explicitly.  Whether that twiddling is to accommodate
> > font-locking or for some other reason.
> 
> I don't have a strong opinion about the default, although I lean
> toward the behavior #3 (3b specifically) below since it looks
> blingy and might help attract new users.

Bof.  Today's sparkling blingy is tomorrow's annoying.

But the real problem is selection highlighting not showing you exactly
what is selected - each char, because font-locking can override it,
making it seem to disappear here or there.

> I still don't understand exactly what you're proposing. Here are our
> options:
> 
> 1) We can ship Emacs with hard-coded defaults for region's
> foreground and background colors and ignore system settings. This
> arrangement will produce legible text, and there's no need to adjust
> region's colors.
> 
> 2) We can ship Emacs such that it uses the system selection
> foreground and background colors for region. This arrangement will
> also produce legible text, and there's no need to adjust region's
> colors.

Yes.  #1 or #2 - only.  I personally do not care about or for #2, but
if it is important to some users or some platforms, that's OK by me -
if, as you say, it presents no problems.

> 3) We can ship Emacs such that it uses the system selection
> background color and uses font-lock colors for foreground text.
> This arrangement will sometimes produce an illegible result...

Not #3, IMO.  There is no need to give priority to font-lock
highlighting over the selection highlighting.  That's just asking
for trouble, and will confuse users.  Font-lock can use any colors
for either foreground or background, so there will always the
possibility that some selection highlighting will "disappear", not
showing you some chars as having been selected.

And YAGNI.  Sounds like cool bling to you (today).  Not useful.
Problematic.  Just one opinion, of course.

> So to put the discussion in more concrete terms: say I run a stock
> GTK3 Emacs on a system with a background color exactly equal to
> "Blue1", which is the color we use for font-lock-function-name-face.
> I visit xfaces.c, search for "UNSPECIFIEDP", and hit C-a C-SPC C-e.
> What colors do you propose we use for the text "UNSPECIFIEDP" on
> that line?

See #1 and #2 above.

And note that the "problem", of text highlighted by the selection
(region) having foreground and background the same, is the same
problem you will anyway encounter for Isearch.  In your example,
even when searching you can run into the same problem.

Will you be proposing that Isearch highlighting too let font-lock
highlighting show through?  There is a reason we give Isearch
highlighting such a high priority: it should clearly _highlight_
text - even text that might be font-locked.

And will you be proposing the same thing for secondary-selection
highlighting?  And what about Isearch over text that is in the
region or the secondary selection?

Font lock highlighting is not about selecting text.  It should not
(certainly not by default) show through a selection highlight.

> > A face being equal to its default setting does not imply
> > that the user gives Emacs license to change it.
> 
> I disagree. If our user leaves a face at the default setting, she's
> giving us *explicit license* to use whatever heuristics we think
> work best.

And how does she specify to you that the default colors are exactly
what she happens to prefer?  How does she produce the same effect as
if the default colors were different and she customized them to be
what the default colors actually are?  How does she tell you
"Hands off!  This is really what I want!"?

> > IMO, any such feature should be opt-in, not opt-out.  A
> > user should not need to explicitly do anything to stop
> > Emacs from twiddling her region.  She should ask for
> > twiddling if she wants that.
> 
> I'm in favor of shipping defaults that create a good user
> experience. 

A novel opinion, to be sure. ;-)

> If a user never expressed a preference with respect to foreground
> and background colors, there's no contract to violate.

And how does she express that preference, if it happens to coincide
with the default colors?

What you are saying is like saying that you think you have a license
to change the value of option `default-frame-alist' automatically,
if the current value is nil, because that's also the default value.

I do not see it that way.  Coincidence of the current value with the
default value does not imply that the user does not care.  That's
just faulty logic.  And perhaps over-eagerness to do clever dwimmy
things for the user's "benefit".

> >> Emacs adjusts colors only when a :contrast-function is set
> >> for some face applying to a particular character and that
> >> face isn't overridden by one that sets :contrast-function to
> >> nil.
> >
> > OK, that sounds a bit better, at least.  So if any face has a
> > nil :twiddle-me, er sorry, :contrast-function attribute, and
> > that face is merged with a face that has a non-nil one, the nil
> > one wins and there is no twiddling.  Is that right?
> 
> Right now, it depends on the order in which the faces are merged.
> The last face that specifies a :contrast-function gets to control the
> contrast behavior.

Users are going to have trouble with that complication.  Too much
wondering about what happened and why.  Of course, I could be wrong.

> Right now, region is the only face that specifies a
> contrast-function, and I can't think of another good use case at
> the moment.

Maybe that in itself should tell you something...

> > But it still makes life more complicated for Lisp code that
> > wants to get or set the actual appearance of the face.  Whereas
> > before code needed only to get or set attribute :foreground,
> > now it will need to also check for a non-nil :contrast-function
> > and apply that.
> 
> I don't understand why lisp code would need to know the
> post-adjustment colors used for display. Can you explain why
> we'd want to know?

Lisp code that checks or changes a given color attribute is trying
to check or affect the actual appearance of the face (again, not
considering merges with other faces etc.).

Doing this breaks the one-to-one relationship between the face's
attributes and its appearance.  That means that the Lisp code cannot
just examine or set `foreground' and `background' colors.  It will
need to be changed to also invoke the :contrast-function.  Or if
that takes effect automatically, it will need to be aware that that
can happen, and perhaps even accommodate it to cancel it out
(depending on the purpose of the Lisp code).

Consider code that does something based on distance of the current
foreground appearance from some given value - the same kind of thing
you are doing with a given :contrast-function, for instance.  It
will no longer be sufficient to just measure the distance to the
current `foreground' attribute value.  Now, it will need to invoke
the current :contrast-function too, to accurately estimate the
distance.

> Also, today, any lisp code that wants to mimic the redisplay face
> combination logic needs to take into account text properties,
> frame-local variables, overlays, display attributes, and so on. It
> would be a big job, and I'm not aware of anyone who's done it.

I specifically spoke of the color attributes for an individual face,
and separated this from combining faces and other display properties.

Consider a Lisp function that helps you customize the foreground
and background colors of an individual face.  What it checks and what
it produces should reflect the face appearance (again, in isolation).

Consider a function that lets you modify the foreground of a given
face incrementally, showing you the effect, WYSIWYG-style, in your
buffer of C code or whatever.  Maybe it increments hue or saturation
or the blue component.  With your feature, what you see by its
changing attribute `foreground' will presumably "jump" when the
:contrast-function decides that the new value would be too close to
the background.

I don't say that these things are insurmountable.  I'm just saying
that this adds something new to take into account.  It might or
might not break some existing code (in terms of intention, at least).

Re: [PATCH] Re: About the :distant-foreground face attribute

[Jan Djärv]

Hello.

14 jan 2014 kl. 19:47 skrev Daniel Colascione <dancol@dancol.org>:

> On 01/14/2014 03:44 AM, Jan D. wrote:
>> Daniel Colascione skrev 2014-01-14 11:44:
>>> On 01/14/2014 01:34 AM, Jan D. wrote:
>>>>>> Given the use case at hand, we know for a fact that the background is
>>>>>> the region background, so I don't understand why a calculated
>>>>>> foreground
>>>>>> is needed.  Just pick one that matches the background.
>>>>>> There might be other use cases where a calculated foreground makes
>>>>>> sense, but my imagination fails me here.
>>>>> 
>>>>> Calculated foreground colors look better: they resemble the font-lock
>>>>> colors on which they're based.
>>>> 
>>>> For the region case, that would imply the possibility of different
>>>> foregrounds for marked text, none which is the actual font-lock color.
>>> 
>>> It *is* the same color in the sense that the code we generate has the
>>> same hue. How on earth is that worse than changing arbitrary font-locked
>>> pieces of text to the system selection foreground color?
>> 
>> Because the system color foreground is (presumably) choosen to look good
>> together with the system color background.
> 
> Yes, and a color we algorithmically generate from a font-lock face will *also* look good against that background color, but 1) will be distinct from other faces replaced for lack of contrast, and 2) will be visually similar to the pre-highlight face. Have you tried the patch?

No.  If Emacs generates a color, Emacs desides what looks good.  If the system defines a color, the system (or the user if customized) desides what looks good.  I don't think it matters what I think about colors generated by your patch, I might even think they look better than many system defined colors.  But as a principle I think the desision is not Emacs to make *by default*.  Users may of course apply customizations to Emacs and change it.

> 
>>> But that's exactly what
>>> happens when we put gtk_selection_bg_color in a color spec! Why are you
>>> fine with allowing gtk_selection_bg_color as a color (producing a color
>>> that's presumably workable but unknown to a theme writer) but adamantly
>>> opposed to a feature that selects good colors another way?
>> 
>> I'm objecting to having the system default colors replaced with
>> generated colors for the default case.  What themes other than the
>> default does is irrelevant.  We are just talking about defaults here.
> 
> We already replace the system default colors with colors from font-lock faces.

We at least try to use text selection colors and tool tip colors where we can.  For some ports of Emacs this is harder (i.e. not using Gtk+).  I haven't seen any system defined colors that corresponds exactly to an Emacs font-lock face, Emacs defines tons of faces that most GUI system don't.  So yes, we replace system colors, when Emacs can't figure out the system color.  What else should Emacs do?

> 
>>> I've produced working code that allows Emacs to act how you prefer *and*
>>> how I prefer based on user configuration. What is your problem with that
>>> code?
>> 
>> I have no problem with the code, just the defaults.
>> 
>>> Have you responded to the numerous objections to
>>> :distant-foreground with anything other than retrenchment?
>> 
>> So far the objections have been mostly opinions.  If there was a
>> concrete case or a bug it exposes that would be another issue.
>> The most concrete objection is the case of inheriting faces Yidong
>> brought up in another mail, which indeed is a real problem.
> 
> Problems with usability in UI or API design don't result in clear "bugs" in the same way that logic errors or crashes do. Fixing them relies on judgment, and that relies chiefly on informed opinion. You can't dismiss others' opinions about this feature while not subjecting your own to the same treatment.

I have not put forth any usability or design argument for the current implementation, it is a bug fix plain and simple.  Informed opinion sounds like a copout to avoid having to motivate your opinion.  If you can't motivate your informed opinion, it is not worth anything.  Design can be motivated, and so can usability, there are tons of books on the subjects.  I have not read any such arguments here, just assertions that this is bad.
Take "attribute is to narrow".  It may very well be, but on what motivation?  How can I know if I want to add another attribute if it will be to narrow too?  Because person X and Y says so?  That would be OK if X and Y where appointed design chiefs for Emacs or this aspect of Emacs.  I don't think any value of X or Y has been so appointed, except for the Emacs maintainers.

> 
>>> And a theme designer can opt not to use the feature or to explicitly
>>> turn it off --- but a theme designer should never have to bother,
>>> because the feature only activates when the contrast is awful, which
>>> happens only when a theme designer screws up or deliberately relies on
>>> system colors.
>> 
>> Why should a theme designer rely on system colors?  That does not sound
>> like a good theme.  Again, the default should rely on system colors, not
>> arbitrary themes.
> 
> You are not responding to the point I make in the text you quote. I don't know why a theme designer would use system colors either. That's not the point. I'm saying that explicitly-chosen themes should never trigger the code under discussion if they're well-designed.

Okay.  I don't know why that is relevant though.

> 
>>>> That is not a use case.  A use case describes what an actual user does
>>>> and in what situation automatically calculates colors enters the
>>>> picture.
>>> 
>>> This entire thread describes the use case. There are abundant examples.
>>> Your objection here is baseless.
>> 
>> There is a lot of handwaving and describing of mechanisms, but not one
>> real world example, i.e. what major mode and which font lock faces and
>> possible other faces are involved, and what does the user do to trigger
>> the generated color.
> 
> Message-ID: <52D4A23E.4030802@dancol.org>
> From: Daniel Colascione <dancol@dancol.org>
>> So to put the discussion in more concrete terms: say I run a stock
>> GTK3 Emacs on a system with a background color exactly equal to
>> "Blue1", which is the color we use for font-lock-function-name-face.
>> I visit xfaces.c, search for "UNSPECIFIEDP", and hit C-a C-SPC C-e.
>> What colors do you propose we use for the text "UNSPECIFIEDP" on that
>> line?

You can do that today and see which color gets used.
It is the one Emacs trunk uses today, the Gtk selection foreground color.

> 
>> 
>>>>> :distant-foreground has far too narrow a justification to warrant being
>>>>> a face property by itself.
>>>> 
>>>> Also, just an opinion, not based on some documented design rule in any
>>>> Emacs or GNU document.
>>> 
>>> So only GNU documents count now, not reasoned arguments?
>> 
>> No, but if documentation of some design rule existed it would give
>> weight to your argument.  But your statement about :distant-foreground
>> being too narrow is just your personal opinion which you have not
>> motivated with any reasoned argument, just asserted.
> 
> I'm not the only one who's expressed concerns about the design of the feature.

Good, then you should be able to motivate your design rules with no effort.

	Jan D,

Re: [PATCH] Re: About the :distant-foreground face attribute

[Daniel Colascione]

On 01/14/2014 12:01 PM, Jan Djärv wrote:
> 14 jan 2014 kl. 19:47 skrev Daniel Colascione <dancol@dancol.org>:
>
>> On 01/14/2014 03:44 AM, Jan D. wrote:
>>> Daniel Colascione skrev 2014-01-14 11:44:
>>>> On 01/14/2014 01:34 AM, Jan D. wrote:
>>>>>>> Given the use case at hand, we know for a fact that the background is
>>>>>>> the region background, so I don't understand why a calculated
>>>>>>> foreground
>>>>>>> is needed.  Just pick one that matches the background.
>>>>>>> There might be other use cases where a calculated foreground makes
>>>>>>> sense, but my imagination fails me here.
>>>>>>
>>>>>> Calculated foreground colors look better: they resemble the font-lock
>>>>>> colors on which they're based.
>>>>>
>>>>> For the region case, that would imply the possibility of different
>>>>> foregrounds for marked text, none which is the actual font-lock color.
>>>>
>>>> It *is* the same color in the sense that the code we generate has the
>>>> same hue. How on earth is that worse than changing arbitrary font-locked
>>>> pieces of text to the system selection foreground color?
>>>
>>> Because the system color foreground is (presumably) choosen to look good
>>> together with the system color background.
>>
>> Yes, and a color we algorithmically generate from a font-lock face will *also* look good against that background color, but 1) will be distinct from other faces replaced for lack of contrast, and 2) will be visually similar to the pre-highlight face. Have you tried the patch?
>
> No.  If Emacs generates a color, Emacs desides what looks good.  If
> the system defines a color, the system (or the user if customized)
> desides what looks good.  I don't think it matters what I think about
> colors generated by your patch, I might even think they look better
> than many system defined colors.  But as a principle I think the
> desision is not Emacs to make *by default*.  Users may of course
> apply customizations to Emacs and change it.

In 24.4, Emacs has already been changed to override the system selection 
foreground color with various font-lock faces. Why is it okay to do that 
when there's no contrast problem, but suddenly, when there's a contrast 
problem, we can say that the principle of following system colors is 
important? You're applying this principle very selectively. If you're 
going to override the system color selection, you need to do it well and 
consistently, and automatic contrast adjustment is the best way to go 
about solving the inevitable contrast problems that arise when you 
combine colors you control with colors you don't.

Re: [PATCH] Re: About the :distant-foreground face attribute

[Daniel Colascione]

I don't feel we're converging on a consensus solution. If you're not 
going to budge on this, I propose simply reverting the code to what it 
was in 24.3 and unconditionally using the system selection foreground 
and background colors. I'd rather have that than :distant-foreground and 
its hard-coded policy.

Also, can you please configure *your* mailer to wrap long lines?

On 01/14/2014 12:01 PM, Jan Djärv wrote:
>>>> But that's exactly what
>>>> happens when we put gtk_selection_bg_color in a color spec! Why are you
>>>> fine with allowing gtk_selection_bg_color as a color (producing a color
>>>> that's presumably workable but unknown to a theme writer) but adamantly
>>>> opposed to a feature that selects good colors another way?
>>>
>>> I'm objecting to having the system default colors replaced with
>>> generated colors for the default case.  What themes other than the
>>> default does is irrelevant.  We are just talking about defaults here.
>>
>> We already replace the system default colors with colors from font-lock faces.
> We at least try to use text selection colors and tool tip colors
> where
> we can.

That's simply not true. We're now replacing the system selection 
foreground with colors from font-lock faces. This entire thread is about 
solving the problems that arise from this decision.

> For some ports of Emacs this is harder (i.e. not using Gtk+). I
> haven't seen any system defined colors that corresponds exactly to an
> Emacs font-lock face, Emacs defines tons of faces that most GUI system
> don't. So yes, we replace system colors, when Emacs can't figure out the
> system color. What else should Emacs do?

Replacing the system selection foreground color is a choice. If we're 
going to make the leap to overriding system color preferences, we 
shouldn't feel constrained by needing to respect system preferences.

>>>> I've produced working code that allows Emacs to act how you prefer *and*
>>>> how I prefer based on user configuration. What is your problem with that
>>>> code?
>>>
>>> I have no problem with the code, just the defaults.
>>>
>>>> Have you responded to the numerous objections to
>>>> :distant-foreground with anything other than retrenchment?
>>>
>>> So far the objections have been mostly opinions.  If there was a
>>> concrete case or a bug it exposes that would be another issue.
>>> The most concrete objection is the case of inheriting faces Yidong
>>> brought up in another mail, which indeed is a real problem.
>>
>> Problems with usability in UI or API design don't result in clear
>>"bugs" in the same way that logic errors or crashes do. Fixing them
>> relies on judgment, and that relies chiefly on informed opinion. You
>> can't dismiss others' opinions about this feature while not subjecting
>> your own to the same treatment.
> I have not put forth any usability or design argument for the
> current
> implementation,

Your position seems inconsistent. You say that it's okay to override 
system colors with explicitly-specified RGB values, but not okay to 
override these preferences with generated colors. You argue that the 
system selection foreground is presumably chosen to look good against 
the system selection background, but ignore the fact that font-lock 
faces aren't.

> it is a bug fix plain and simple. Informed opinion
> sounds like a copout to avoid having to motivate your opinion.

Bullshit. My patch is just as much a bugfix as :distant-foreground. What 
makes your bugfix better than mine?

> That would be OK if X
> and Y where appointed design chiefs for Emacs or this aspect of Emacs. I
> don't think any value of X or Y has been so appointed, except for the
> Emacs maintainers.

And Stefan hasn't stated a position, except to say that the general idea 
of algorithmically-derived face color is acceptable in some circumstances.

>>>> And a theme designer can opt not to use the feature or to explicitly
>>>> turn it off --- but a theme designer should never have to bother,
>>>> because the feature only activates when the contrast is awful, which
>>>> happens only when a theme designer screws up or deliberately relies on
>>>> system colors.
>>>
>>> Why should a theme designer rely on system colors?  That does not sound
>>> like a good theme.  Again, the default should rely on system colors, not
>>> arbitrary themes.
>>
>> You are not responding to the point I make in the text you quote.
>> I
>> don't know why a theme designer would use system colors either. That's
>> not the point. I'm saying that explicitly-chosen themes should never
>> trigger the code under discussion if they're well-designed.
>
> Okay.  I don't know why that is relevant though.

It's relevant because you're arguing as if we arbitrarily replace 
user-specified colors with algorithmically-derived colors. We only do 
that when the colors we would otherwise use are not legible against each 
other. Any well-designed theme will never trigger this code because it 
will never produce an illegible color combination. If it did, it would 
be better for users for Emacs to override the theme and produce text 
users can read.

In any case, themes probably won't set :contrast-function, so it won't 
matter.

>>>>> That is not a use case.  A use case describes what an actual user does
>>>>> and in what situation automatically calculates colors enters the
>>>>> picture.
>>>>
>>>> This entire thread describes the use case. There are abundant examples.
>>>> Your objection here is baseless.
>>>
>>> There is a lot of handwaving and describing of mechanisms, but not one
>>> real world example, i.e. what major mode and which font lock faces and
>>> possible other faces are involved, and what does the user do to trigger
>>> the generated color.
>>
>> Message-ID: <52D4A23E.4030802@dancol.org>
>> From: Daniel Colascione <dancol@dancol.org>
>>> So to put the discussion in more concrete terms: say I run a stock
>>> GTK3 Emacs on a system with a background color exactly equal to
>>> "Blue1", which is the color we use for font-lock-function-name-face.
>>> I visit xfaces.c, search for "UNSPECIFIEDP", and hit C-a C-SPC C-e.
>>> What colors do you propose we use for the text "UNSPECIFIEDP" on that
>>> line?
>
> You can do that today and see which color gets used.
> It is the one Emacs trunk uses today, the Gtk selection foreground color.
>
>>
>>>
>>>>>> :distant-foreground has far too narrow a justification to warrant being
>>>>>> a face property by itself.
>>>>>
>>>>> Also, just an opinion, not based on some documented design rule in any
>>>>> Emacs or GNU document.
>>>>
>>>> So only GNU documents count now, not reasoned arguments?
>>>
>>> No, but if documentation of some design rule existed it would give
>>> weight to your argument.  But your statement about :distant-foreground
>>> being too narrow is just your personal opinion which you have not
>>> motivated with any reasoned argument, just asserted.
>>
>> I'm not the only one who's expressed concerns about the design of the feature.
>
> Good, then you should be able to motivate your design rules with no effort.

This is incredibly frustrating. I've *already* "motivated" my concerns, 
as have Drew Adams and Chong Yidong. Briefly, the proposed face 
attribute addresses one specific problem and not easily-foreseeable 
related problems, hard-codes a particular policy choice for solving that 
problem. It requires that we support this particular hard-coded policy 
choice for all time, since we can't easily remove face attributes once 
we've added them. Automatically adjusting colors for contrast is subtle 
and applicable to *any* situation where we need to combine faces for 
disparate sources.

Further, a single foreground-color attribute is inadequate for the 
general case, since there's no guarantee that the "distant" color 
contrasts against the background any better than the normal color does. 
In practice, your feature is only good for the region face, since there, 
we can specify the system selection background. That's what I mean when 
I say it's "too narrow": your solution is a special-case hack that 
doesn't fully address the issues surrounding the problem it means to 
solve. If we fully solve the problem after shipping distant-foreground, 
we'll just have to support *both* solutions forever.

Re: [PATCH] About the :distant-foreground face attribute

[Jan Djärv]

Hello.

14 jan 2014 kl. 21:39 skrev Daniel Colascione <dancol@dancol.org>:

> I don't feel we're converging on a consensus solution.

On defaults, no.

> If you're not going to budge on this, I propose simply reverting the code to what it was in 24.3 and unconditionally using the system selection foreground and background colors. I'd rather have that than :distant-foreground and its hard-coded policy.

Well, my position is not to revert, so by doing that nobody is happy.  Why is that good?
Anyway, as I said before, this is Stefans call.  But I always assumed we are talking post-release here as your code clearly don't belong in a feature freeze.  Again, Stefans call.

> 
> Also, can you please configure *your* mailer to wrap long lines?

No can do, I though all modern mailers could wrap text.  I'll make shorter lines.

	Jan D.

Re: [PATCH] About the :distant-foreground face attribute

[Jan Djärv]

14 jan 2014 kl. 21:06 skrev Daniel Colascione <dancol@dancol.org>:

> On 01/14/2014 12:01 PM, Jan Djärv wrote:
>> 
>> No.  If Emacs generates a color, Emacs desides what looks good.  If
>> the system defines a color, the system (or the user if customized)
>> desides what looks good.  I don't think it matters what I think about
>> colors generated by your patch, I might even think they look better
>> than many system defined colors.  But as a principle I think the
>> desision is not Emacs to make *by default*.  Users may of course
>> apply customizations to Emacs and change it.
> 
> In 24.4, Emacs has already been changed to override the system selection foreground color with various font-lock faces.

Which font lock faces are you talking about?  No system I know of defines
system colors for things like comment face, function face etc.

> Why is it okay to do that when there's no contrast problem, but suddenly, when there's a contrast problem, we can say that the principle of following system colors is important?

The principle of following system foreground is only important if system 
background is used.
This is currently for NS/Gtk+ only.  For Lucid/Motif/No toolkit, we don't use
system colors at all, because they are not known and can not be known, 
because the API to get them is not available.

> You're applying this principle very selectively.

System background + contrast problem => system foreground.
How is that selectively, it is a clear rule.

> If you're going to override the system color selection, you need to do it well and consistently, and automatic contrast adjustment is the best way to go about solving the inevitable contrast problems that arise when you combine colors you control with colors you don't.

Again, I don't know what faces except region faces a system defines colors 
for, so when you say Emacs overrides system colors for font-lock faces, I 
don't know what faces you are talking about.
If there are system colors that are applicable as font-lock face colors, then 
that is news to me.

	Jan D.

RE: [PATCH] About the :distant-foreground face attribute

[Drew Adams]

> But I always assumed we are talking post-release here as your code
> clearly don't belong in a feature freeze.

Nor does your code that introduced this regression.  Please restore
the behavior as it was before this "fix" for a nonexistent "bug".

Re: [PATCH] About the :distant-foreground face attribute

[Daniel Colascione]

On 01/14/2014 02:05 PM, Jan Djärv wrote:
> 14 jan 2014 kl. 21:06 skrev Daniel Colascione <dancol@dancol.org>:
>
>> On 01/14/2014 12:01 PM, Jan Djärv wrote:
>>>
>>> No.  If Emacs generates a color, Emacs desides what looks good.  If
>>> the system defines a color, the system (or the user if customized)
>>> desides what looks good.  I don't think it matters what I think about
>>> colors generated by your patch, I might even think they look better
>>> than many system defined colors.  But as a principle I think the
>>> desision is not Emacs to make *by default*.  Users may of course
>>> apply customizations to Emacs and change it.
>>
>> In 24.4, Emacs has already been changed to override the system selection foreground color with various font-lock faces.
>
> Which font lock faces are you talking about?  No system I know of defines
> system colors for things like comment face, function face etc.
>
>> Why is it okay to do that when there's no contrast problem, but suddenly, when there's a contrast problem, we can say that the principle of following system colors is important?
>
> The principle of following system foreground is only important if system
> background is used.

Then why don't we always use the system *selection* foreground color 
when we use the system *selection* background? When we apply font-lock 
foreground colors to a region you've highlighted, we're overriding your 
system's selection foreground color. Why is it okay to override that 
color in some situations but not others?

> This is currently for NS/Gtk+ only.  For Lucid/Motif/No toolkit, we don't use
> system colors at all, because they are not known and can not be known,
> because the API to get them is not available.
>
>> You're applying this principle very selectively.
>
> System background + contrast problem => system foreground.
> How is that selectively, it is a clear rule.

It's clear, but arbitrary. Another clear and arbitrary rule is this:

   System background + contrast problem => adjust foreground.

You haven't presented any justification for your arbitrary rule being 
better than my arbitrary rule.

Re: [PATCH] Re: About the :distant-foreground face attribute

[Stefan Monnier]

> My lurker's interpretation is that there are two model of emacs usage at
> odds here:
> 1) emacs is my environement (My emacs controls the horizontal and the
> vertical. OS themes be damned! Just stay out of its way.)
> 2) emacs as a good citizen in a OS themed setting

No: Emacs-24.3 uses the system's fg highlight color for various reasons,
but it's a regression and we should fix it.  As pointed out, most other
text editors only use the system's bg color for highlighting but leave
the fg color unchanged during highlighting.  That's on the average
a better choice (even if some people like it less: they can Customize
this aspect very easily).

The issue is how to make sure that this "system bg + font-lock
foreground" always results in readable text.


        Stefan

Re: [PATCH] Re: About the :distant-foreground face attribute

[Daniel Colascione]

Thanks. This is productive.

On 01/14/2014 11:32 AM, Drew Adams wrote:
>>> Emacs should act that way by default, as it always has.  Anyone
>>> who wants automatic foreground twiddling for a given face should
>>> ask for that explicitly.  Whether that twiddling is to accommodate
>>> font-locking or for some other reason.
>>
>> I don't have a strong opinion about the default, although I lean
>> toward the behavior #3 (3b specifically) below since it looks
>> blingy and might help attract new users.
>
> Bof.  Today's sparkling blingy is tomorrow's annoying.

Fair enough, but it looks like we're going to go with some variant of #3 
(according to Stefan's latest message), so we might as well choose the 
least objectionable #3 variant.

> And note that the "problem", of text highlighted by the selection
> (region) having foreground and background the same, is the same
> problem you will anyway encounter for Isearch.  In your example,
> even when searching you can run into the same problem.
>
> Will you be proposing that Isearch highlighting too let font-lock
> highlighting show through?  There is a reason we give Isearch
> highlighting such a high priority: it should clearly _highlight_
> text - even text that might be font-locked.

If I had my druthers, we'd turn on :contrast-function for the default 
face. It's not generally possible for packages written by arbitrary 
third parties to coordinate color choices in the presence of arbitrary 
themes. Most packages can base colors on existing font-lock faces, but 
font-lock has a limited built-in palette and plenty of packages define 
entirely new faces with arbitrary colors. Automatic adjustment can make 
these faces readable no matter the circumstances of their use.

I'm trying to be conservative by applying automatic contrast adjustment 
only to region right now, since it's this face, being based on system 
colors, that has the highest risk of being unreadable.

>>> A face being equal to its default setting does not imply
>>> that the user gives Emacs license to change it.
>>
>> I disagree. If our user leaves a face at the default setting, she's
>> giving us *explicit license* to use whatever heuristics we think
>> work best.
>
> And how does she specify to you that the default colors are exactly
> what she happens to prefer?  How does she produce the same effect as
> if the default colors were different and she customized them to be
> what the default colors actually are?  How does she tell you
> "Hands off!  This is really what I want!"?

She shouldn't have to, because if she chooses reasonable colors, the 
automatic adjustment will never go into effect. If she really wants to 
customize the region face so that it is illegible, she can uncheck 
:contrast-function.

>> If a user never expressed a preference with respect to foreground
>> and background colors, there's no contract to violate.
>
> And how does she express that preference, if it happens to coincide
> with the default colors?

See above.

> What you are saying is like saying that you think you have a license
> to change the value of option `default-frame-alist' automatically,
> if the current value is nil, because that's also the default value.

Well, yes, we do have such a license. By this logic, we can't change any 
default ever.

>> Right now, it depends on the order in which the faces are merged.
>> The last face that specifies a :contrast-function gets to control the
>> contrast behavior.
>
> Users are going to have trouble with that complication.  Too much
> wondering about what happened and why.  Of course, I could be wrong.

I don't think it's any more confusing than other aspects of the face 
model --- or CSS, for that matter. Sure, the model is complex, but it's 
powerful.

>> Right now, region is the only face that specifies a
>> contrast-function, and I can't think of another good use case at
>> the moment.
>
> Maybe that in itself should tell you something...

See the discussion of generality above. Personally, I'd like to apply 
automatic adjustment (or other filters) more broadly, but for now, it 
makes sense to be conservative, which means applying the adjustment only 
to region.

>>> But it still makes life more complicated for Lisp code that
>>> wants to get or set the actual appearance of the face.  Whereas
>>> before code needed only to get or set attribute :foreground,
>>> now it will need to also check for a non-nil :contrast-function
>>> and apply that.
>>
>> I don't understand why lisp code would need to know the
>> post-adjustment colors used for display. Can you explain why
>> we'd want to know?
>
> Lisp code that checks or changes a given color attribute is trying
> to check or affect the actual appearance of the face (again, not
> considering merges with other faces etc.).
>
> Doing this breaks the one-to-one relationship between the face's
> attributes and its appearance.

But there's only a one-to-one relationship if the face is fully 
specified --- this patch doesn't change that.

> That means that the Lisp code cannot
> just examine or set `foreground' and `background' colors.  It will
> need to be changed to also invoke the :contrast-function.  Or if
> that takes effect automatically, it will need to be aware that that
> can happen, and perhaps even accommodate it to cancel it out
> (depending on the purpose of the Lisp code).
>
> Consider code that does something based on distance of the current
> foreground appearance from some given value - the same kind of thing
> you are doing with a given :contrast-function, for instance.  It
> will no longer be sufficient to just measure the distance to the
> current `foreground' attribute value.  Now, it will need to invoke
> the current :contrast-function too, to accurately estimate the
> distance.
>
>> Also, today, any lisp code that wants to mimic the redisplay face
>> combination logic needs to take into account text properties,
>> frame-local variables, overlays, display attributes, and so on. It
>> would be a big job, and I'm not aware of anyone who's done it.
>
> I specifically spoke of the color attributes for an individual face,
> and separated this from combining faces and other display properties.
>
> Consider a Lisp function that helps you customize the foreground
> and background colors of an individual face.  What it checks and what
> it produces should reflect the face appearance (again, in isolation).
>
> Consider a function that lets you modify the foreground of a given
> face incrementally, showing you the effect, WYSIWYG-style, in your
> buffer of C code or whatever.  Maybe it increments hue or saturation
> or the blue component.  With your feature, what you see by its
> changing attribute `foreground' will presumably "jump" when the
> :contrast-function decides that the new value would be too close to
> the background.

Yes, that's what would happen, but an editor of this sort --- and I'm 
not aware of any that currently exist --- can just explicitly turn off 
the adjustment, either at the face level or through an overlay in the 
preview area. We should optimize emacs as a text editor, not a precise 
color picker or a floor wax. :-)

RE: [PATCH] Re: About the :distant-foreground face attribute

[Drew Adams]

> >> If a user never expressed a preference with respect to foreground
> >> and background colors, there's no contract to violate.
> >
> > And how does she express that preference, if it happens to
> > coincide with the default colors?
> 
> See above.

So she has to opt out.  She has to realize that she really wants
the default color attribute values, which she never really sees
because the automatic tweaking is on by default.  And then she
has to explicitly opt out, to get those colors she has finally
realized she wants.

Just turn it off by default.  For those few platforms that are
ill-designed enough to present a problem, she will figure out
whether she wants to remedy that problem by opting into the
automatic twiddling.  If there really is a visible problem on
her platform then she will see it and will be able to make an
informed choice.  Call out this clever compensation in the doc.

> > What you are saying is like saying that you think you have a
> > license to change the value of option `default-frame-alist'
> > automatically, if the current value is nil, because that's also
> > the default value.
> 
> Well, yes, we do have such a license. By this logic, we can't change
> any default ever.

Did you notice "automatically" and "current value" in that sentence?
I'm not talking about the license of Emacs Dev to redefine the
_default_ value.  I'm talking about license to change the _current_
value of an option, on the fly, automatically, behind the user's
back... - just because the current value happens to coincide with
the default value (`nil' in this case).

A user's not customizing an option away from the default value is
not implicit permission for Emacs to twiddle the current value to
something different.

> >>> But it still makes life more complicated for Lisp code that
> >>> wants to get or set the actual appearance of the face.  Whereas
> >>> before code needed only to get or set attribute :foreground,
> >>> now it will need to also check for a non-nil :contrast-function
> >>> and apply that.
> >>
> >> I don't understand why lisp code would need to know the
> >> post-adjustment colors used for display. Can you explain why
> >> we'd want to know?
> >
> > Lisp code that checks or changes a given color attribute is trying
> > to check or affect the actual appearance of the face (again, not
> > considering merges with other faces etc.).
> >
> > Doing this breaks the one-to-one relationship between the face's
> > attributes and its appearance.
> 
> But there's only a one-to-one relationship if the face is fully
> specified --- this patch doesn't change that.

That's not true AFAIK, considering the face in isolation.  (I said
that I was abstracting from complications of face merging and
interactions with other display properties.)

> > Consider a Lisp function that helps you customize the foreground
> > and background colors of an individual face.  What it checks and
> > what it produces should reflect the face appearance (again, in
> > isolation).
> >
> > Consider a function that lets you modify the foreground of a given
> > face incrementally, showing you the effect, WYSIWYG-style, in your
> > buffer of C code or whatever.  Maybe it increments hue or
> > saturation or the blue component.  With your feature, what you see
> > by its changing attribute `foreground' will presumably "jump" when
> > the :contrast-function decides that the new value would be too close
> > to the background.
> 
> Yes, that's what would happen, but an editor of this sort --- and
> I'm not aware of any that currently exist

Doesn't matter whether (a) you are aware of that or (b) whether any
actually do exist currently.  The point is that any Lisp code like
that will likely have its life complicated, and so will users of such
code.

Oh, and yes, in fact there are "editors of this sort".  Both DoReMi
and Icicles have commands and other functions that do this kind of
thing.  They give you WYSIWYG incremental tweaking of face attributes.

> --- can just explicitly turn off the adjustment, 

Just don't turn it on by default.  Let users opt in, if they want.

Commands like those I described should probably *not* turn this
off - not without user say-so.  If it is on because the user wants
it on then the commands should show what the real effect of
incrementing various attribute is - WYSIWYG, even if that means
passing through discontinuous jumps (there are already some
discontinuities in the hue model, BTW).  The commands should not
show you a false result, which will then be overridden by The
Twiddler as soon as the user has chosen the look she wants.

> We should optimize emacs as a text editor, not a precise color
> picker or a floor wax. :-)

Dunno what that means.  It's not either/or: text editor or color
picker.  If you are proposing to fiddle with Emacs's choosing
colors, _especially_ if done automatically in an opaque manner
for the user, then you should take seriously such color picking.

I thought this feature was being argued for because it supposedly
provides more reasonable color picking - getting a better
foreground color in some platform-specific, corner cases.  If you
now make fun of "precise color picking" then I have even less
confidence in this whole proposed ball of wax. ;-)

Re: [PATCH] Re: About the :distant-foreground face attribute

[Daniel Colascione]

On 01/14/2014 04:31 PM, Drew Adams wrote:
>>>> If a user never expressed a preference with respect to foreground
>>>> and background colors, there's no contract to violate.
>>>
>>> And how does she express that preference, if it happens to
>>> coincide with the default colors?
>>
>> See above.
>
> So she has to opt out.  She has to realize that she really wants
> the default color attribute values, which she never really sees
> because the automatic tweaking is on by default.

She sees the default attributes --- system selection background, 
font-lock foreground --- almost all the time. Can you give me a concrete 
example of a situation in which this scheme might be problematic?

> And then she
> has to explicitly opt out, to get those colors she has finally
> realized she wants.

We're talking about a default Emacs configuration. Users want legible 
text, and they want Emacs to look like their other applications. We're 
obliging them.

> Just turn it off by default.  For those few platforms that are
> ill-designed enough to present a problem, she will figure out
> whether she wants to remedy that problem by opting into the
> automatic twiddling.  If there really is a visible problem on
> her platform then she will see it and will be able to make an
> informed choice.  Call out this clever compensation in the doc.

Whether we have a contrast problem has absolutely nothing to do with how 
well the system is designed. The variable here is the default selection 
background. Or are you claiming that any system that provides a 
selection background color that conflicts with an Emacs font-lock face 
is badly designed? That's a pretty Emacs-centric view of the world.

I strongly disagree that we should show users illegible text and tell 
them how to correct the problem in the documentation. Users don't change 
defaults when evaluating a product. Emacs should work by default. Users 
should have to opt into breaking their editing environment.

>>> What you are saying is like saying that you think you have a
>>> license to change the value of option `default-frame-alist'
>>> automatically, if the current value is nil, because that's also
>>> the default value.
>>
>> Well, yes, we do have such a license. By this logic, we can't change
>> any default ever.
>
> Did you notice "automatically" and "current value" in that sentence?
> I'm not talking about the license of Emacs Dev to redefine the
> _default_ value.  I'm talking about license to change the _current_
> value of an option, on the fly, automatically, behind the user's
> back... - just because the current value happens to coincide with
> the default value (`nil' in this case).
>
> A user's not customizing an option away from the default value is
> not implicit permission for Emacs to twiddle the current value to
> something different.

I honestly don't understand what you're talking about. Perhaps you can 
rephrase it? If a user has customized an option, we don't touch it. If a 
user hasn't customized an option and has relied on the default, we can 
modify the default values when we update Emacs. All customization 
variables work this way. Faces are not special. Users with custom themes 
won't see a difference.

`nil' isn't a foreground color. It's the absence of a foreground color. 
If a user has customized region and set the region foreground color to 
nil, she's explicitly asked Emacs to combine the region foreground with 
whatever other face happens to be in the buffer. We're just performing 
this combination in a way that yields visible text. Nobody has ever 
deliberately told Emacs to make text illegible when highlighting the 
selection.

You're going on about some kind of intellectual purity and ignoring the 
reality of what users will actually see.

RE: [PATCH] Re: About the :distant-foreground face attribute

[Drew Adams]

One more try.

> >>> What you are saying is like saying that you think you have a
> >>> license to change the value of option `default-frame-alist'
> >>> automatically, if the current value is nil, because that's
> >>> also the default value.
> >>
> >> Well, yes, we do have such a license. By this logic, we can't
> >> change any default ever.
> >
> > Did you notice "automatically" and "current value" in that
> > sentence?  I'm not talking about the license of Emacs Dev to
> > redefine the _default_ value.

Please read that sentence again, because you keep going on about
how Emacs can change the default value.  That's not the point.

> > I'm talking about license to change the _current_ value of an
> > option, on the fly, automatically, behind the user's back...
> > - just because the current value happens to coincide with
> > the default value (`nil' in this case).
> >
> > A user's not customizing an option away from the default value
> > is not implicit permission for Emacs to twiddle the current
> > value to something different.
> 
> I honestly don't understand what you're talking about...
> If a user hasn't customized an option and has relied on the
> default, we can modify the default values when we update Emacs.

Yes, yes.  Emacs Dev can change the default value.  No one says
otherwise.  What the code being added now does is more than that.
It changes the appearance of the face on the fly - the current
state, not the default value.

And the rationale you gave for dynamically changing the face
appearance was that the user had not customized the face away
from the default spec, so she must not care.  That does not
follow.

My point was that a license to change the value dynamically
does not come from the user not changing the default value.

The default value for option `foo' is 42.  The user does not
change that.  That fact alone does not let you presume that
the user does not mind if you change the value of `foo' on
the fly in some contexts to 3000.

Yes, the analogy is not exact. You are not changing the
`foreground' attribute itself.  You are changing the face's
foreground appearance.  The `foreground' attribute says nil
or "LightBlue" or whatever the default defface specifies,
but on the fly the apparent foreground sometimes ends up
being "HelloKittyPink" (even though the attribute value has
not changed).

You can do that, but your being able to do that does not
follow from the user not having customized the option.

Re: [PATCH] About the :distant-foreground face attribute

[Eli Zaretskii]

> Date: Tue, 14 Jan 2014 14:06:08 -0800 (PST)
> From: Drew Adams <drew.adams@oracle.com>
> Cc: Eli Zaretskii <eliz@gnu.org>, Chong Yidong <cyd@gnu.org>,
> 	emacs-devel <emacs-devel@gnu.org>
> 
> > But I always assumed we are talking post-release here as your code
> > clearly don't belong in a feature freeze.
> 
> Nor does your code that introduced this regression.

Jan committed his changes before the freeze, so your argument is
invalid.

Re: [PATCH] Re: About the :distant-foreground face attribute

[Daniel Colascione]

On 01/14/2014 07:07 PM, Drew Adams wrote:
>> I honestly don't understand what you're talking about...
>> If a user hasn't customized an option and has relied on the
>> default, we can modify the default values when we update Emacs.
>
> Yes, yes.  Emacs Dev can change the default value.  No one says
> otherwise.  What the code being added now does is more than that.
> It changes the appearance of the face on the fly - the current
> state, not the default value.
>
> And the rationale you gave for dynamically changing the face
> appearance was that the user had not customized the face away
> from the default spec, so she must not care.  That does not
> follow.

You're contradicting yourself. You acknowledge that changing defaults is 
legitimate. Then you say that a user's failure to customize a face does 
not give us license to change the way that face looks by default when we 
update Emacs. That's a bizarre claim. Are we supposed to never update 
faces, or are we supposed to telepathically know whether the user is 
happy with the previous version's default?

Let's talk about the case where users have customized *something*.

> The default value for option `foo' is 42.  The user does not
> change that.  That fact alone does not let you presume that
> the user does not mind if you change the value of `foo' on
> the fly in some contexts to 3000.

A better analogy is this: Emacs comes with a foo parameter, and a user 
has customized it to 42. We ship Emacs with a new parameter, 
automatically-choose-foo, and default this parameter to t. Under certain 
conditions, when 42 is useless and when automatically-choose-foo is t, 
we use 50 instead of the user's configured 42. I think we agree that 
this kind of automatic-option-overriding behavior is confusing and that 
we should have a very good reason before adopting it, weighing the 
benefit of automatically-choose-foo against user surprise.

In the particular case of face color, we have a good enough reason.

> Yes, the analogy is not exact. You are not changing the
> `foreground' attribute itself.  You are changing the face's
> foreground appearance.  The `foreground' attribute says nil
> or "LightBlue" or whatever the default defface specifies,
> but on the fly the apparent foreground sometimes ends up
> being "HelloKittyPink" (even though the attribute value has
> not changed).

Yes, if a user has customized font-lock-function-name-face to 
HelloKittyPink and she runs Emacs on a system where the GTK selection 
background color is also HelloKittyPink, then under my proposed scheme, 
we'll render function names that appear in the selection using a 
slightly different shade of pink or red. If our user is unhappy with 
this rendering, she can either change font-lock-function-name-face's 
foreground or change the selection background either by changing the 
system default background or by changing the region face in Emacs. Once 
she changes either such that the font-lock-function-name-face-on-region 
combination becomes legible, we use the specified colors without any 
adjustment. In the meantime, though, isn't it better for text to be 
legible than not?

You would seem to prefer that our user's function names just disappear 
when selected and justify this preference with references to data model 
purity. Users don't care about purity.

+--------+--------------------+------------------+
|        | Good contrast      | Bad Contrast     |
+--------+--------------------+------------------+
| Auto   | Exact color        | Color changed    |
| Adjust | Happy              | Maybe happy?     |
+--------+--------------------+------------------+
| Static | Exact color        | Text invisible   |
|        | Happy              | Unhappy          |
+--------+--------------------+------------------+

Note that we're doing this adjustment only for region. rainbow-mode 
performs a similar adjustment, using a different mechanism, for very 
similar reasons.

RE: [PATCH] About the :distant-foreground face attribute

[Drew Adams]

> > > But I always assumed we are talking post-release here as your
> > > code clearly don't belong in a feature freeze.
> >
> > Nor does your code that introduced this regression.
> 
> Jan committed his changes before the freeze, so your argument is
> invalid.

OK, so it belongs in the pretest.  But it is a bug, and should be
fixed before the release.  One opinion.

Re: [PATCH] About the :distant-foreground face attribute

[Stefan Monnier]

> Well, my position is not to revert, so by doing that nobody is happy.
> Why is that good?  Anyway, as I said before, this is Stefans call.
> But I always assumed we are talking post-release here as your code
> clearly don't belong in a feature freeze.  Again, Stefans call.

In the short term, indeed, Daniel's patch is too new for 24.4.
But in the longer term, I like the idea of being able to write Elisp
code to fine-tune the colors.

So the :distant-foreground might not be useful after 24.4.

IOW, if we keep it for 24.4, I think we should hide it so we can remove
it later.  Not sure how to do that, tho.

What do you guys think?


        Stefan

Re: [PATCH] About the :distant-foreground face attribute

[Daniel Colascione]

On 01/14/2014 08:22 PM, Stefan Monnier wrote:
>> Well, my position is not to revert, so by doing that nobody is happy.
>> Why is that good?  Anyway, as I said before, this is Stefans call.
>> But I always assumed we are talking post-release here as your code
>> clearly don't belong in a feature freeze.  Again, Stefans call.
>
> In the short term, indeed, Daniel's patch is too new for 24.4.
> But in the longer term, I like the idea of being able to write Elisp
> code to fine-tune the colors.
>
> So the :distant-foreground might not be useful after 24.4.
>
> IOW, if we keep it for 24.4, I think we should hide it so we can remove
> it later.  Not sure how to do that, tho.
>
> What do you guys think?

Right. There's a dilemma here --- that's why I'm suggesting we should 
revert to 24.3 behavior and, for the next release, implement the 
functionality in a more general way. I don't think the original bug is a 
release blocker.

Re: [PATCH] Re: About the :distant-foreground face attribute

[Yuri Khan]

On Wed, Jan 15, 2014 at 2:32 AM, Drew Adams <drew.adams@oracle.com> wrote:
>
> And note that the "problem", of text highlighted by the selection
> (region) having foreground and background the same, is the same
> problem you will anyway encounter for Isearch.  In your example,
> even when searching you can run into the same problem.
>
> Will you be proposing that Isearch highlighting too let font-lock
> highlighting show through?


The problem is in fact deeper than that.

We have region highlighting. We have Isearch highlighting the current
match and all other matches. We have hl-line and
highlight-current-line. We have an option in whitespace-mode and a
highlight-beyond-fill-column mode to highlight things that exceed a
preconfigured line length.

We have ediff highlighting deleted text, inserted text, merged text,
word-refined deleted/inserted/merged text, active difference, even and
odd other differences. We have MuMaMo-like modes highlighting runs of
code in a different programming or markup language.

In all of these cases, I as a user would like (1) syntax highlighting
(font-lock) to be retained and visible through the highlight; (2) all
resulting color combinations to be of suitable contrast and proper
brightness relationship; (3) all that without a combinatorial
explosion of having to define a separate pair of colors for each
combination of a font-lock face with a highlight face.


Importantly, the word “suitable” in the above sentence does not always
mean “suitably high”. I use global-whitespace-mode to visualize
spaces, line breaks and tabs. For these, I configure a low-contrast
foreground. I want them to stay low-contrast *and* still visible, no
matter which highlight is overlaid.

As an example: My default background is aluminium6 (#f7f8f5, hereafter
all colors are from the extended Tango palette[1]). My region color is
skyblue5 (#97c4f0). I use hl-line with background of aluminium5
(#ecf0eb). I have to choose a foreground color for the whitespace
marks so that it is sufficiently low contrast on all these backgrounds
as not to distract from the main text; and that it is visible on and
darker than each of these backgrounds.

For display on the default background, aluminium4 (#d3d7cf) would be
about right. But if I choose that, whitespace markers become lighter
than the region background, which is visually unpleasant. So I’m
forced to take the darker shade, aluminium3 (#babdb6), even though it
is now a bit too visible on the default background.

[1]: http://emilis.info/other/extended_tango/


Thinking about it, I feel I might appreciate a system that would not
use fixed absolute foreground colors but blends instead. E.g. in my
light background setting, region would multiply both the background
and the foreground by #9ccafa. This would turn the default
black-on-aluminium6 into black-on-skyblue5 while turning the
whitespace marks from aluminium4 into just a bit lighter than
skyblue4. Similarly, hl-line would multiply the colors by #f4f7f5.

(The multiplicative model is only good for light backgrounds. Dark
backgrounds would need some other combination formula, possibly
something similar to (1-(1-x)*(1-y)). Alpha blending might seem more
general, but yields lower overall contrast. I also don’t have a
solution for fixed-palette 8-, 16-, 88- and 256-color displays.)

RE: [PATCH] Re: About the :distant-foreground face attribute

[Drew Adams]

> >> I honestly don't understand what you're talking about...
> >> If a user hasn't customized an option and has relied on the
> >> default, we can modify the default values when we update Emacs.
> >
> > Yes, yes.  Emacs Dev can change the default value.  No one says
> > otherwise.  What the code being added now does is more than that.
> > It changes the appearance of the face on the fly - the current
> > state, not the default value.
> >
> > And the rationale you gave for dynamically changing the face
> > appearance was that the user had not customized the face away
> > from the default spec, so she must not care.  That does not
> > follow.
> 
> You're contradicting yourself. You acknowledge that changing
> defaults is legitimate. Then you say that a user's failure to
> customize a face does not give us license to change the way that
> face looks by default
             ^^^^^^^^^^

No.  You added "by default".  You keep doing that.  And I keep
explaining that it is the _on-the-fly appearance change_ that
does not follow from a user not customizing the default.

> when we update Emacs. 

And you keep repeating that, though I have made it clear that
this is not about your having the right to change the default
value when you update Emacs.  I couldn't be clearer about that.

> That's a bizarre claim. Are we supposed to never update faces,
> or are we supposed to telepathically know whether the user is
> happy with the previous version's default?

It's not about the default.  It's about a user not having
changed the value from the default being taken as an assumption
that the user does not care whether you change the face
appearance later, on the fly, in some contexts.

Not customizing away from the default value says nothing about
the user not caring about the appearance.

> > The default value for option `foo' is 42.  The user does not
> > change that.  That fact alone does not let you presume that
> > the user does not mind if you change the value of `foo'
> > on the fly in some contexts to 3000.
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^

Note: "on the fly in some contexts".  Nothing about changing
the default value.

> A better analogy is this: Emacs comes with a foo parameter,
> and a user has customized it to 42. 

No, you have not understood my point, so you are coming up with
stuff that is off the mark.

You have already reassured us all that if the user customizes
face `region' then you will not override that user preference
with on-the-fly foreground twiddling.  So no one is talking
about the case where the user customizes, since that case is
apparently not changed - you will continue to respect the user
customization, right?

The analogy was for the case where the user does *not* change
the default value - does not customize the face (the option,
in the analogy).  That lack of customization does not imply
not caring about the value and thus give implicit permission
to change it on the fly when you think that is appropriate.

> > Yes, the analogy is not exact. You are not changing the
> > `foreground' attribute itself.  You are changing the face's
> > foreground appearance.  The `foreground' attribute says nil
> > or "LightBlue" or whatever the default defface specifies,
> > but on the fly the apparent foreground sometimes ends up
> > being "HelloKittyPink" (even though the attribute value has
> > not changed).
> 
> Yes, if a user has customized font-lock-function-name-face
> to HelloKittyPink

No, this is about the `region' face.  My analogy was wrt the
thing that you will be twiddling, which is the `region'
foreground.  Customization of other faces or options is not
the question.

> Once she changes either such that the
> font-lock-function-name-face-on-region combination becomes
> legible, we use the specified colors without any adjustment.

Such that the _combination becomes legible_?  That is now the
price of your respecting a user customization of `region'?
It is not enough that she customizes `region', to have you
leave her region highlighting alone?  That customization now has
to result in a combination that you find legible?

In that case, there is no respect of the user's customization.
That is respect only when respect doesn't matter and has no cost
or effect.

Previously you said categorically that if the user customizes
`region' then you will attempt no adjustment.  Now you say that
if she does that, AND if no adjustment is needed, then you will
perform no adjustment!

You are kind enough to say that if you find that no adjustment
is needed you will offer a dispensation from adjusting.  When
your adjustment would amount to a no-op you will kindly forego it.
Gee, thanks.

> In the meantime, though, isn't it better for text to be
> legible than not?

Isn't it better to let the user decide what appearance she
wants, whether you think it is appropriate or not?

People have several times mentioned the fact that certain
libraries intentionally specify foreground and background
to be the same color for some faces.  This change flies in the
face of that use case, even if you limit it to `region' for now
(you said you would prefer to generalize it, but would limit it
for now).

> You would seem to prefer that our user's function names
> just disappear when selected

It's not about what colors I prefer for the face.  It's about
what the user prefers.

> Users don't care about purity.

Whatever that means.  No idea what purity you think I'm
requesting.

But I shudder when I see blanket declarations that users
don't care about this or that, even something so meaninglessly
abstract as "purity".

Re: [PATCH] Re: About the :distant-foreground face attribute

[Daniel Colascione]

On 01/14/2014 08:59 PM, Drew Adams wrote:
>> That's a bizarre claim. Are we supposed to never update faces,
>> or are we supposed to telepathically know whether the user is
>> happy with the previous version's default?
>
> It's not about the default.  It's about a user not having
> changed the value from the default being taken as an assumption
> that the user does not care whether you change the face
> appearance later, on the fly, in some contexts.

So your objection is just to the same face appearing in different colors 
in different contexts? You must loathe XEmacs specifiers, frame-local 
face-remapping tables, and the display-spec portion of face specifications.

>>> Yes, the analogy is not exact. You are not changing the
>>> `foreground' attribute itself.  You are changing the face's
>>> foreground appearance.  The `foreground' attribute says nil
>>> or "LightBlue" or whatever the default defface specifies,
>>> but on the fly the apparent foreground sometimes ends up
>>> being "HelloKittyPink" (even though the attribute value has
>>> not changed).
>>
>> Yes, if a user has customized font-lock-function-name-face
>> to HelloKittyPink
>
> No, this is about the `region' face.  My analogy was wrt the
> thing that you will be twiddling, which is the `region'
> foreground.  Customization of other faces or options is not
> the question.

The foreground of the `region' face isn't being changed. In this 
instance, `region' doesn't have a foreground face set. If the user 
customizes the region face, this entire discussion doesn't apply 
because, at that point, the user can set whatever colors are desired and 
(if necessary) turn off the contrast adjustment.

>> Once she changes either such that the
>> font-lock-function-name-face-on-region combination becomes
>> legible, we use the specified colors without any adjustment.
>
> Such that the _combination becomes legible_?  That is now the
> price of your respecting a user customization of `region'?
> It is not enough that she customizes `region', to have you
> leave her region highlighting alone?  That customization now has
> to result in a combination that you find legible?
>
> In that case, there is no respect of the user's customization.
> That is respect only when respect doesn't matter and has no cost
> or effect.
>
> Previously you said categorically that if the user customizes
> `region' then you will attempt no adjustment.  Now you say that
> if she does that, AND if no adjustment is needed, then you will
> perform no adjustment!
>
> You are kind enough to say that if you find that no adjustment
> is needed you will offer a dispensation from adjusting.  When
> your adjustment would amount to a no-op you will kindly forego it.
> Gee, thanks.
>
>> In the meantime, though, isn't it better for text to be
>> legible than not?
>
> Isn't it better to let the user decide what appearance she
> wants, whether you think it is appropriate or not?
>
> People have several times mentioned the fact that certain
> libraries intentionally specify foreground and background
> to be the same color for some faces.  This change flies in the
> face of that use case, even if you limit it to `region' for now
> (you said you would prefer to generalize it, but would limit it
> for now).

The purpose of using a face with identical foreground and background 
colors would be better served with a text or overlay property. Besides: 
even in 24.3, highlighting a part of the buffer marked with such a face 
makes the hidden text visible.

>> You would seem to prefer that our user's function names
>> just disappear when selected
>
> It's not about what colors I prefer for the face.  It's about
> what the user prefers.

Do you really think users would prefer invisible text?

>> Users don't care about purity.
>
> Whatever that means.  No idea what purity you think I'm
> requesting.
>
> But I shudder when I see blanket declarations that users
> don't care about this or that, even something so meaninglessly
> abstract as "purity".

Your entire objection to this feature seems to be driven by some kind of 
Platonic ideal of settings as values that Emacs uses and consistently 
and that never change without explicit user interaction. Preserving 
these properties isn't worth much, especially if it leads to a worse 
user experience.

RE: [PATCH] Re: About the :distant-foreground face attribute

[Drew Adams]

> Your entire objection to this feature seems to be driven by some kind of
> Platonic ideal of settings as values that Emacs uses and consistently
> and that never change without explicit user interaction. Preserving
> these properties isn't worth much, especially if it leads to a worse
> user experience.

I give up.

Re: [PATCH] About the :distant-foreground face attribute

[Jan Djärv]

Hello.

14 jan 2014 kl. 23:14 skrev Daniel Colascione <dancol@dancol.org>:

>> 
>>> Why is it okay to do that when there's no contrast problem, but suddenly, when there's a contrast problem, we can say that the principle of following system colors is important?
>> 
>> The principle of following system foreground is only important if system
>> background is used.
> 
> Then why don't we always use the system *selection* foreground color when we use the system *selection* background? When we apply font-lock foreground colors to a region you've highlighted, we're overriding your system's selection foreground color. Why is it okay to override that color in some situations but not others?

Because of bug http://debbugs.gnu.org/cgi/bugreport.cgi?bug=15668.
Note that, as I said, Xcode, Eclipse and Visual Studio all does this as well, so it is not unheard of.

> 
>> This is currently for NS/Gtk+ only.  For Lucid/Motif/No toolkit, we don't use
>> system colors at all, because they are not known and can not be known,
>> because the API to get them is not available.
>> 
>>> You're applying this principle very selectively.
>> 
>> System background + contrast problem => system foreground.
>> How is that selectively, it is a clear rule.
> 
> It's clear, but arbitrary. Another clear and arbitrary rule is this:
> 
>  System background + contrast problem => adjust foreground.
> 
> You haven't presented any justification for your arbitrary rule being better than my arbitrary rule.

I have stated my justification as such: User and system configurations should override Emacs settings and code in the default case, i.e. when the user has made no explicit customization in Emacs.

	Jan D.

Re: [PATCH] About the :distant-foreground face attribute

[Jan Djärv]

Hello.

15 jan 2014 kl. 05:25 skrev Daniel Colascione <dancol@dancol.org>:

> On 01/14/2014 08:22 PM, Stefan Monnier wrote:
>>> Well, my position is not to revert, so by doing that nobody is happy.
>>> Why is that good?  Anyway, as I said before, this is Stefans call.
>>> But I always assumed we are talking post-release here as your code
>>> clearly don't belong in a feature freeze.  Again, Stefans call.
>> 
>> In the short term, indeed, Daniel's patch is too new for 24.4.
>> But in the longer term, I like the idea of being able to write Elisp
>> code to fine-tune the colors.
>> 
>> So the :distant-foreground might not be useful after 24.4.
>> 
>> IOW, if we keep it for 24.4, I think we should hide it so we can remove
>> it later.  Not sure how to do that, tho.
>> 
>> What do you guys think?
> 
> Right. There's a dilemma here --- that's why I'm suggesting we should revert to 24.3 behavior and, for the next release, implement the functionality in a more general way. I don't think the original bug is a release blocker.

Just mark and document :distant-foreground as deprecated if you know now that it will be changed.  I do think the bug fix should be in the release.  Why should we make Emacs have less usability that Eclipse, XCode and Visual Studio?  Makes no sense to me.

	Jan D.

Re: [PATCH] Re: About the :distant-foreground face attribute

[Jan D.]

Hello.

Yuri Khan skrev 2014-01-15 05:50:

>
> We have region highlighting. We have Isearch highlighting the current
> match and all other matches. We have hl-line and
> highlight-current-line. We have an option in whitespace-mode and a
> highlight-beyond-fill-column mode to highlight things that exceed a
> preconfigured line length.
>
> We have ediff highlighting deleted text, inserted text, merged text,
> word-refined deleted/inserted/merged text, active difference, even and
> odd other differences. We have MuMaMo-like modes highlighting runs of
> code in a different programming or markup language.
>
> In all of these cases, I as a user would like (1) syntax highlighting
> (font-lock) to be retained and visible through the highlight; (2) all
> resulting color combinations to be of suitable contrast and proper
> brightness relationship; (3) all that without a combinatorial
> explosion of having to define a separate pair of colors for each
> combination of a font-lock face with a highlight face.
>
>
> Importantly, the word “suitable” in the above sentence does not always
> mean “suitably high”. I use global-whitespace-mode to visualize
> spaces, line breaks and tabs. For these, I configure a low-contrast
> foreground. I want them to stay low-contrast *and* still visible, no
> matter which highlight is overlaid.
>
> As an example: My default background is aluminium6 (#f7f8f5, hereafter
> all colors are from the extended Tango palette[1]). My region color is
> skyblue5 (#97c4f0). I use hl-line with background of aluminium5
> (#ecf0eb). I have to choose a foreground color for the whitespace
> marks so that it is sufficiently low contrast on all these backgrounds
> as not to distract from the main text; and that it is visible on and
> darker than each of these backgrounds.
>
> For display on the default background, aluminium4 (#d3d7cf) would be
> about right. But if I choose that, whitespace markers become lighter
> than the region background, which is visually unpleasant. So I’m
> forced to take the darker shade, aluminium3 (#babdb6), even though it
> is now a bit too visible on the default background.
>
> [1]: http://emilis.info/other/extended_tango/
>
>
> Thinking about it, I feel I might appreciate a system that would not
> use fixed absolute foreground colors but blends instead. E.g. in my
> light background setting, region would multiply both the background
> and the foreground by #9ccafa. This would turn the default
> black-on-aluminium6 into black-on-skyblue5 while turning the
> whitespace marks from aluminium4 into just a bit lighter than
> skyblue4. Similarly, hl-line would multiply the colors by #f4f7f5.
>
> (The multiplicative model is only good for light backgrounds. Dark
> backgrounds would need some other combination formula, possibly
> something similar to (1-(1-x)*(1-y)). Alpha blending might seem more
> general, but yields lower overall contrast. I also don’t have a
> solution for fixed-palette 8-, 16-, 88- and 256-color displays.)
>

As you note, the lack of contrast is a general problem for all cases
when more than one face can be applied.  But no user has been
sufficiently annoyed to file a bug report except for the selection
case, so it has been an ignored/unknown problem.  In hindsight, I kind
of regret trying to fix this and thus put focus on the issue.
I should have done as has been done in the past, just ignore the whole
issue.

	Jan D.

Re: [PATCH] Re: About the :distant-foreground face attribute

[Daniel Colascione]

On 01/14/2014 11:50 PM, Jan D. wrote:
> As you note, the lack of contrast is a general problem for all cases
> when more than one face can be applied.  But no user has been
> sufficiently annoyed to file a bug report except for the selection
> case, so it has been an ignored/unknown problem.  In hindsight, I kind
> of regret trying to fix this and thus put focus on the issue.
> I should have done as has been done in the past, just ignore the whole
> issue.

Yuri is right. We should do more to try to fix this issue. I'm still at 
a loss as to why you and Drew are so adamantly opposed to generating colors.

Re: [PATCH] About the :distant-foreground face attribute

[Daniel Colascione]

On 01/14/2014 10:33 PM, Jan Djärv wrote:
>>> This is currently for NS/Gtk+ only.  For Lucid/Motif/No toolkit, we don't use
>>> system colors at all, because they are not known and can not be known,
>>> because the API to get them is not available.
>>>
>>>> You're applying this principle very selectively.
>>>
>>> System background + contrast problem => system foreground.
>>> How is that selectively, it is a clear rule.
>>
>> It's clear, but arbitrary. Another clear and arbitrary rule is this:
>>You're merely stated your conclusion. You think it's okay
>>   System background + contrast problem => adjust foreground.
>>
>> You haven't presented any justification for your arbitrary rule being better than my arbitrary rule.
>
> I have stated my justification as such: User and system
> configurations should override Emacs settings and code in the default
> case, i.e. when the user has made no explicit customization in
> Emacs.

You haven't stated any justification at all. All you've done is 
re-assert your position. You clearly think it's okay for Emacs to 
override the foreground selection color in some circumstances, otherwise 
you'd support keeping the 24.3 behavior. You're justifying your 
particular strategy for handling contrast inherent in the new behavior 
by invoking a principle you *JUST VIOLATED* by creating the problematic 
new behavior.

We have to do something about contrast problems. Why do you think your 
solution produces better results than mine? If, to render something the 
user can actually read, we have to choose a foreground color other than 
what normal face logic would produce, why use the system selection 
foreground color instead of some other color we algorithmically create?

Please don't invoke the "system settings should override Emacs" crap 
again: that idea clearly doesn't hold in the case we're talking about.

Re: [PATCH] Re: About the :distant-foreground face attribute

[Jan D.]

Hello.

Daniel Colascione skrev 2014-01-15 08:52:
> On 01/14/2014 11:50 PM, Jan D. wrote:
>> As you note, the lack of contrast is a general problem for all cases
>> when more than one face can be applied.  But no user has been
>> sufficiently annoyed to file a bug report except for the selection
>> case, so it has been an ignored/unknown problem.  In hindsight, I kind
>> of regret trying to fix this and thus put focus on the issue.
>> I should have done as has been done in the past, just ignore the whole
>> issue.
>
> Yuri is right. We should do more to try to fix this issue. I'm still at
> a loss as to why you and Drew are so adamantly opposed to generating
> colors.

Just for the case when a configured color is available, i.e. when
system selection fore- and background color exists we should not be
generating any one of them as a substitute.  And I think a defined
color is always preferrable to a generated.

But if no suitable defined color exists, generated is better than
unreadable text.

	Jan D.

Re: [PATCH] About the :distant-foreground face attribute

[Jan D.]

Daniel Colascione skrev 2014-01-15 09:05:
> On 01/14/2014 10:33 PM, Jan Djärv wrote:
>> I have stated my justification as such: User and system
>> configurations should override Emacs settings and code in the default
>> case, i.e. when the user has made no explicit customization in
>> Emacs.
>
> You haven't stated any justification at all.  All you've done is
> re-assert your position.

And my position is what I say above.

> You clearly think it's okay for Emacs to
> override the foreground selection color in some circumstances, otherwise
> you'd support keeping the 24.3 behavior.

Yes I do, otherwise I wouldn't have fixed the bug report.

> You're justifying your
> particular strategy for handling contrast inherent in the new behavior
> by invoking a principle you *JUST VIOLATED* by creating the problematic
> new behavior.

No need to shout. There is no behaviour that is problematic AFAIK, you
are objecting on basis of design, not because the behaviour of letting
font lock faces shine through is problematic in some way.

I did not violate the principle by this code.  As I said, when there is
a choice between system/user defined color and generated colors, the
first should be preferred.  This does not in any way imply that
user/system colors shall always be used in all circumstances they might 
be used.  For example if we show font-lock foreground and
selection background we are not considering user/system color for the 
foreground because we are providing a feature (font-lock) where system
color is not applicable.

There exists a default font size for text in the system also.  We are
using that by default.  But info makes headers in a larger font size.
This does not mean that we have overridden the system font size, and
are violating the principle of using system font size.  It is a totally
separate feature where system font size is not applicable.  And so is
the case of marking font-locked text.

>
> We have to do something about contrast problems. Why do you think your
> solution produces better results than mine?

As I said before "better" is subjective, and from my point of view, it
has nothing to do with "better".

> If, to render something the
> user can actually read, we have to choose a foreground color other than
> what normal face logic would produce, why use the system selection
> foreground color instead of some other color we algorithmically create?
>
> Please don't invoke the "system settings should override Emacs" crap
> again: that idea clearly doesn't hold in the case we're talking about.

Yes it does.

	Jan D.

Re: [PATCH] Re: About the :distant-foreground face attribute

[Stefan Monnier]

> I give up.

That came a bit late, but better late than never,


        Stefan

Re: [PATCH] About the :distant-foreground face attribute

[Stefan Monnier]

> What do you guys think?
              ^^^^

Sorry for being imprecise: "guys" was meant to exclude Jan, Daniel, and
Drew whose opinion we already know.


        Stefan

Re: [PATCH] About the :distant-foreground face attribute

[Stefan Monnier]

Guys!  You're bikeshedding!

None of you is either wrong or right or going to convince the other.
Luckily it doesn't matter, because it's me you're supposed to convinced,
and you've already given me your arguments, so you can stop.
Thank you both for your code and for your opinion.  I really do value
both of them.  But repeating your opinion is not helping.


        Stefan

Re: [PATCH] About the :distant-foreground face attribute

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: Daniel Colascione <dancol@dancol.org>,  Eli Zaretskii <eliz@gnu.org>,  Chong Yidong <cyd@gnu.org>,  emacs-devel <emacs-devel@gnu.org>
> Date: Tue, 14 Jan 2014 23:22:34 -0500
> 
> > Well, my position is not to revert, so by doing that nobody is happy.
> > Why is that good?  Anyway, as I said before, this is Stefans call.
> > But I always assumed we are talking post-release here as your code
> > clearly don't belong in a feature freeze.  Again, Stefans call.
> 
> In the short term, indeed, Daniel's patch is too new for 24.4.
> But in the longer term, I like the idea of being able to write Elisp
> code to fine-tune the colors.
> 
> So the :distant-foreground might not be useful after 24.4.
> 
> IOW, if we keep it for 24.4, I think we should hide it so we can remove
> it later.  Not sure how to do that, tho.
> 
> What do you guys think?

How about changing just the name of the attribute for now?  We could
then keep the implementation, and after the branch extend it to accept
more flexible forms as the value of the attribute.  Then we will both
fix the original bug, and avoid the risk of introducing short-lived,
non forward-compatible attributes.

Re: [PATCH] About the :distant-foreground face attribute

[Eli Zaretskii]

> Date: Tue, 14 Jan 2014 20:25:39 -0800
> From: Daniel Colascione <dancol@dancol.org>
> Cc: Eli Zaretskii <eliz@gnu.org>, Chong Yidong <cyd@gnu.org>,
> 	emacs-devel <emacs-devel@gnu.org>
> 
> Right. There's a dilemma here --- that's why I'm suggesting we should 
> revert to 24.3 behavior and, for the next release, implement the 
> functionality in a more general way. I don't think the original bug is a 
> release blocker.

Reverting a bugfix sounds too drastic a measure to me.

Re: [PATCH] About the :distant-foreground face attribute

[Stefan Monnier]

> How about changing just the name of the attribute for now?  We could
> then keep the implementation, and after the branch extend it to accept
> more flexible forms as the value of the attribute.

I could go along with that, but I'm not sure what name to use.
We could either use an obscure name like `:deprecated-at-birth', or try
to guess what will be an appropriate name for the subsequent feature,
which could be `:filter' but could also be something else.

Of course, we still need to address the inheritance issue.


        Stefan

Re: [PATCH] About the :distant-foreground face attribute

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: jan.h.d@swipnet.se,  dancol@dancol.org,  cyd@gnu.org,  emacs-devel@gnu.org
> Date: Wed, 15 Jan 2014 11:17:24 -0500
> 
> We could either use an obscure name like `:deprecated-at-birth', or try
> to guess what will be an appropriate name for the subsequent feature,

I had the latter in mind.

RE: [PATCH] Re: About the :distant-foreground face attribute

[Drew Adams]

> > As you note, the lack of contrast is a general problem for all cases
> > when more than one face can be applied.  But no user has been
> > sufficiently annoyed to file a bug report except for the selection
> > case, so it has been an ignored/unknown problem.  In hindsight, I kind
> > of regret trying to fix this and thus put focus on the issue.
> > I should have done as has been done in the past, just ignore the whole
> > issue.
> 
> Yuri is right. We should do more to try to fix this issue.

As the one who pointed out that this "issue" is not limited to `region'
highlighting, since there are other kinds of selection highlighting in
Emacs, and so this "issue" is not limited to `region' highlighting, let me
repeat why I mentioned that:

There is NO issue/problem, even with the several such kinds of selection
highlighting, if you just drop the notion that selection highlighting
should not mask font-lock highlighting.

IOW, just drop trying to fix the non-bug, and there is no problem.  The
various selection highlightings (isearch, region, secondary-selection,
etc.) just need to work out any priority conflicts that might arise
among them.  And so far, there are no such conflicts, AFAIK.

It is only by introducing the desire to see font-lock highlighting along
with such selection highlighting that you create a nightmare scenario.

> I'm still at a loss as to why you and Drew are so adamantly opposed to
> generating colors.

I am not opposed to generating colors.  I do it all the time.  I've
written a fair amount of code that does that in various ways.

Re: [PATCH] About the :distant-foreground face attribute

[Stefan Monnier]

>> We could either use an obscure name like `:deprecated-at-birth', or try
>> to guess what will be an appropriate name for the subsequent feature,
> I had the latter in mind.

My guess so far is :filter.
What's yours?


        Stefan

Re: [PATCH] About the :distant-foreground face attribute

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: jan.h.d@swipnet.se,  dancol@dancol.org,  cyd@gnu.org,  emacs-devel@gnu.org
> Date: Wed, 15 Jan 2014 12:33:37 -0500
> 
> My guess so far is :filter.

Why "filter"?  Is it supposed to filter something?

> What's yours?

How about :color-adjust?

Re: [PATCH] About the :distant-foreground face attribute

[Stefan Monnier]

> Why "filter"?  Is it supposed to filter something?

Like menu-item's :filter.

>> What's yours?
> How about :color-adjust?

So, you guess that it will be limited to colors?
It might indeed be better to limit it to colors.


        Stefan

Re: [PATCH] About the :distant-foreground face attribute

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: jan.h.d@swipnet.se,  dancol@dancol.org,  cyd@gnu.org,  emacs-devel@gnu.org
> Date: Wed, 15 Jan 2014 13:43:39 -0500
> 
> > Why "filter"?  Is it supposed to filter something?
> 
> Like menu-item's :filter.

Never understood that one, either.

> > How about :color-adjust?
> 
> So, you guess that it will be limited to colors?

Yes.

Re: [PATCH] About the :distant-foreground face attribute

[Josh]

On Wed, Jan 15, 2014 at 11:06 AM, Eli Zaretskii <eliz@gnu.org> wrote:
>> From: Stefan Monnier <monnier@iro.umontreal.ca>
>> Cc: jan.h.d@swipnet.se,  dancol@dancol.org,  cyd@gnu.org,  emacs-devel@gnu.org
>> Date: Wed, 15 Jan 2014 13:43:39 -0500
>>
>> > Why "filter"?  Is it supposed to filter something?
>>
>> Like menu-item's :filter.
>
> Never understood that one, either.
>
>> > How about :color-adjust?
>>
>> So, you guess that it will be limited to colors?
>
> Yes.

Though this does seem likely, it might be wise to adopt a less
specific name for this attribute now in case other appropriate uses
of this adjustment mechanism later come to light.  If we were to
use a name like :attribute-filters or :attribute-adjusters we'd retain
flexibility for the future even though we might only support
adjusting foreground color initially (or indeed ever).

Re: [PATCH] About the :distant-foreground face attribute

[Eli Zaretskii]

> From: Josh <josh@foxtail.org>
> Date: Wed, 15 Jan 2014 12:05:05 -0800
> Cc: Stefan Monnier <monnier@iro.umontreal.ca>, Jan Djärv <jan.h.d@swipnet.se>, 
> 	Daniel Colascione <dancol@dancol.org>, Chong Yidong <cyd@gnu.org>, emacs-devel <emacs-devel@gnu.org>
> 
> Though this does seem likely, it might be wise to adopt a less
> specific name for this attribute now in case other appropriate uses
> of this adjustment mechanism later come to light.  If we were to
> use a name like :attribute-filters or :attribute-adjusters we'd retain
> flexibility for the future even though we might only support
> adjusting foreground color initially (or indeed ever).

It has been a long tradition of Emacs not to generalize before there's
a clear and present need to.

Re: [PATCH] About the :distant-foreground face attribute

[Daniel Colascione]

On 01/15/2014 12:40 PM, Eli Zaretskii wrote:
>> From: Josh <josh@foxtail.org>
>> Date: Wed, 15 Jan 2014 12:05:05 -0800
>> Cc: Stefan Monnier <monnier@iro.umontreal.ca>, Jan Djärv <jan.h.d@swipnet.se>,
>> 	Daniel Colascione <dancol@dancol.org>, Chong Yidong <cyd@gnu.org>, emacs-devel <emacs-devel@gnu.org>
>>
>> Though this does seem likely, it might be wise to adopt a less
>> specific name for this attribute now in case other appropriate uses
>> of this adjustment mechanism later come to light.  If we were to
>> use a name like :attribute-filters or :attribute-adjusters we'd retain
>> flexibility for the future even though we might only support
>> adjusting foreground color initially (or indeed ever).
>
> It has been a long tradition of Emacs not to generalize before there's
> a clear and present need to.

Sure. The problem with :adjust-colors or some other specific name, 
though, is what happens to ordering if we introduce other kinds of 
filters. If we have :adjust-colors and some hypothetical :adjust-foo, 
the order in which the adjustments are made becomes unclear. If we have 
a single list of filters, ordering is natural.

Also, can we please include "post-" in the name of the attribute? I want 
to emphasize that the filtering happens *after* face merging.

Re: [PATCH] About the :distant-foreground face attribute

[Eli Zaretskii]

> Date: Wed, 15 Jan 2014 13:03:29 -0800
> From: Daniel Colascione <dancol@dancol.org>
> CC: jan.h.d@swipnet.se, monnier@iro.umontreal.ca, cyd@gnu.org, 
>  emacs-devel@gnu.org
> 
> > It has been a long tradition of Emacs not to generalize before there's
> > a clear and present need to.
> 
> Sure. The problem with :adjust-colors or some other specific name, 
> though, is what happens to ordering if we introduce other kinds of 
> filters. If we have :adjust-colors and some hypothetical :adjust-foo, 
> the order in which the adjustments are made becomes unclear.

Again, why should we be bothered by hypothetical attributes?  Let's
bother about them when they become a reality, or at least come close.

> Also, can we please include "post-" in the name of the attribute? I want 
> to emphasize that the filtering happens *after* face merging.

Is it wise to expose implementation details in the interfaces?

Re: [PATCH] About the :distant-foreground face attribute

[Daniel Colascione]

On 01/15/2014 01:12 PM, Eli Zaretskii wrote:
>> Date: Wed, 15 Jan 2014 13:03:29 -0800
>> From: Daniel Colascione <dancol@dancol.org>
>> CC: jan.h.d@swipnet.se, monnier@iro.umontreal.ca, cyd@gnu.org,
>>   emacs-devel@gnu.org
>>
>>> It has been a long tradition of Emacs not to generalize before there's
>>> a clear and present need to.
>>
>> Sure. The problem with :adjust-colors or some other specific name,
>> though, is what happens to ordering if we introduce other kinds of
>> filters. If we have :adjust-colors and some hypothetical :adjust-foo,
>> the order in which the adjustments are made becomes unclear.
>
> Again, why should we be bothered by hypothetical attributes?  Let's
> bother about them when they become a reality, or at least come close.

We're not bothering with them now. We're making sure we won't *be* 
bothered by them later.

>> Also, can we please include "post-" in the name of the attribute? I want
>> to emphasize that the filtering happens *after* face merging.
>
> Is it wise to expose implementation details in the interfaces?

Generally no, but I don't see how you can avoid it. Whether the 
filtering happens after face composition or face-by-face during 
attribute merging affects the values the filter function sees. It's part 
of the interface.

Re: [PATCH] About the :distant-foreground face attribute

[John Yates]

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

:post-merge-adjust ?
:final-adjust ?

/john


On Wed, Jan 15, 2014 at 4:03 PM, Daniel Colascione <dancol@dancol.org>wrote:

> On 01/15/2014 12:40 PM, Eli Zaretskii wrote:
>
>> From: Josh <josh@foxtail.org>
>>> Date: Wed, 15 Jan 2014 12:05:05 -0800
>>> Cc: Stefan Monnier <monnier@iro.umontreal.ca>, Jan Djärv <
>>> jan.h.d@swipnet.se>,
>>>         Daniel Colascione <dancol@dancol.org>, Chong Yidong <cyd@gnu.org>,
>>> emacs-devel <emacs-devel@gnu.org>
>>>
>>> Though this does seem likely, it might be wise to adopt a less
>>> specific name for this attribute now in case other appropriate uses
>>> of this adjustment mechanism later come to light.  If we were to
>>> use a name like :attribute-filters or :attribute-adjusters we'd retain
>>> flexibility for the future even though we might only support
>>> adjusting foreground color initially (or indeed ever).
>>>
>>
>> It has been a long tradition of Emacs not to generalize before there's
>> a clear and present need to.
>>
>
> Sure. The problem with :adjust-colors or some other specific name, though,
> is what happens to ordering if we introduce other kinds of filters. If we
> have :adjust-colors and some hypothetical :adjust-foo, the order in which
> the adjustments are made becomes unclear. If we have a single list of
> filters, ordering is natural.
>
> Also, can we please include "post-" in the name of the attribute? I want
> to emphasize that the filtering happens *after* face merging.
>
>

[-- Attachment #2: Type: text/html, Size: 2430 bytes --]

RE: [PATCH] About the :distant-foreground face attribute

[Drew Adams]

> what happens to ordering if we introduce other kinds of
> filters. If we have :adjust-colors and some hypothetical :adjust-foo,
> the order in which the adjustments are made becomes unclear. If we have
> a single list of filters, ordering is natural.

...
> Whether the filtering happens after face composition or face-by-face during
> attribute merging affects the values the filter function sees. 

None of the functions being considered for the attribute value are
filters.  A filter filters.  These are adjusters, transformers,
whatever, but not filters.

Re: [PATCH] About the :distant-foreground face attribute

[Stefan Monnier]

>> Why "filter"?  Is it supposed to filter something?

I think it really comes from Unix shells's notion of a "filter".

Maybe it's not a filter in the usual sense, but within
the context of the GNU system, I don't think it's that far fetched.

In any case I see it's not very popular, so my guess was probably wrong,


        Stefan