RE: About the :distant-foreground face attribute

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

[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

[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.

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

[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

[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]

> 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

[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

[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

[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

[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

[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

[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

[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

[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

[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

[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

[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

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

[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

[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

[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

[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

[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

[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]

> > 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

[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]

> 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

[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 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.