bug#15900: 24.3.50; foreground-color-at-point returns wrong results

bug#15900: 24.3.50; foreground-color-at-point returns wrong results

[Michael Heerdegen]

Hello,

`foreground-color-at-point' doesn't return the expected result when
there are several faces at point present.

For example, on the red subject in a Gnus article buffer, or on a blue
link in w3m, it returns "black".  Looking at the code, I see that
`foreground-color-at-point' uses

  (face-at-point)

i.e., it looks only at one face and disregards the others.  This is
especially meaningless when this first face doesn't specify any
foreground at all.


Thanks,

Michael.

P.S. Some background: I'm working on an addition to stripe-buffer.el
that changes the foreground color continuously, instead of changing the
background.  This is for better readability.

I want to keep the foreground colors already present, so that e.g. links
in w3m are still recognizable.  Paragraphs in italic can be colored
OTOH.

So, what I need is a reliable `foreground-color-at-point'.  Tips and
alternatives welcome.



In GNU Emacs 24.3.50.1 (x86_64-unknown-linux-gnu, GTK+ Version 3.8.4)
 of 2013-11-13 on drachen
Windowing system distributor `The X.Org Foundation', version 11.0.11403000
System Description:	Debian GNU/Linux testing (jessie)

Configured using:
 `configure --prefix=/usr/local/built/'

bug#15900: 24.3.50; foreground-color-at-point returns wrong results

[Michael Heerdegen]

Michael Heerdegen <michael_heerdegen@web.de> writes:

> Hello,
>
> `foreground-color-at-point' doesn't return the expected result when
> there are several faces at point present.
>
> For example, on the red subject in a Gnus article buffer, or on a blue
> link in w3m, it returns "black".  Looking at the code, I see that
> `foreground-color-at-point' uses
>
>   (face-at-point)
>
> i.e., it looks only at one face and disregards the others.  This is
> especially meaningless when this first face doesn't specify any
> foreground at all.

Even

  (face-at-point nil 'multiple)

is not correct.  Evaluated in w3m with point on a link, i get

==>  (w3m-current-anchor)

This is what I get for M-1 C-x =   (excerpt)

,----------------------------------------------------------------------
| There is an overlay here:
|  From 7785 to 7798
|   evaporate            t
|   face                 w3m-current-anchor
|   w3m-momentary-overlay t
| 
| 
| There are text properties here:
|   balloon-help         nil
|   face                 (w3m-anchor)
|   help-echo            [Show]
|   keymap               [Show]
|   mouse-face           highlight
|   rear-nonsticky       t
|   w3m-anchor-sequence  75
|   w3m-anchor-title     nil
|   w3m-balloon-help     [Show]
|   w3m-href-anchor      [Show]
|   w3m-name-anchor      [Show]
|   w3m-name-anchor2     ("showmodes" "gb_70")
|   w3m-safe-url-regexp  nil
`----------------------------------------------------------------------

`face-at-point' disregards the face text property, it only returns the
face overlay property.

> So, what I need is a reliable `foreground-color-at-point'.  Tips and
> alternatives welcome.

I try to use something like this as a workaround now:

--8<---------------cut here---------------start------------->8---
(cl-some
 (lambda (face) (not (memq (face-foreground face nil t)
		      `(nil ,(face-attribute 'default :foreground)))))
 (cl-mapcan (lambda (face-or-list) (if (facep face-or-list)
				  (list face-or-list)
				face-or-list))
	    (list
	     (get-text-property (point) 'face)
	     (get-text-property (point) 'font-lock-face))))
--8<---------------cut here---------------end--------------->8---


Regards,

Michael.

bug#15900: 24.3.50; foreground-color-at-point returns wrong results

[Eli Zaretskii]

> From: Michael Heerdegen <michael_heerdegen@web.de>
> Date: Fri, 15 Nov 2013 03:04:56 +0100
> 
> `foreground-color-at-point' doesn't return the expected result when
> there are several faces at point present.
> 
> For example, on the red subject in a Gnus article buffer, or on a blue
> link in w3m, it returns "black".  Looking at the code, I see that
> `foreground-color-at-point' uses
> 
>   (face-at-point)
> 
> i.e., it looks only at one face and disregards the others.  This is
> especially meaningless when this first face doesn't specify any
> foreground at all.

As you later discovered, even (face-at-point nil t) doesn't do the
job.

Which doesn't surprise me a bit: this kind of things cannot be done
reliably from Lisp, even at a price of the kind of obfuscated code
that face-at-point and foreground-color-at-point use.  It is much
simpler to write a C primitive that simulates the display, then
returns the resulting face at a given character position, a simple and
straightforward task on the C level, something the display engine does
all the time.

> P.S. Some background: I'm working on an addition to stripe-buffer.el
> that changes the foreground color continuously, instead of changing the
> background.  This is for better readability.
> 
> I want to keep the foreground colors already present, so that e.g. links
> in w3m are still recognizable.  Paragraphs in italic can be colored
> OTOH.
> 
> So, what I need is a reliable `foreground-color-at-point'.  Tips and
> alternatives welcome.

Why can't you detect that a portion of text is covered by specific
properties (e.g., one of a list of known properties), and leave those
portions alone?

bug#15900: 24.3.50; foreground-color-at-point returns wrong results

[Michael Heerdegen]

Hello,

CCing Drew cause the Change Log mentions him with regard to
`foreground-color-at-point'.

Eli Zaretskii <eliz@gnu.org> writes:

> As you later discovered, even (face-at-point nil t) doesn't do the
> job.
>
> Which doesn't surprise me a bit: this kind of things cannot be done
> reliably from Lisp, even at a price of the kind of obfuscated code
> that face-at-point and foreground-color-at-point use.  It is much
> simpler to write a C primitive that simulates the display, then
> returns the resulting face at a given character position, a simple and
> straightforward task on the C level, something the display engine does
> all the time.

That sounds good.  Can we just do that?

> > P.S. Some background: I'm working on an addition to stripe-buffer.el
> > that changes the foreground color continuously, instead of changing the
> > background.  This is for better readability.
> > 
> > I want to keep the foreground colors already present, so that e.g. links
> > in w3m are still recognizable.  Paragraphs in italic can be colored
> > OTOH.
> > 
> > So, what I need is a reliable `foreground-color-at-point'.  Tips and
> > alternatives welcome.
>
> Why can't you detect that a portion of text is covered by specific
> properties (e.g., one of a list of known properties), and leave those
> portions alone?

What do you mean with "properties" - text and overlay properties?  If
faces are among them, I still must figure out if one of these faces
changes the foreground.  If not, i.e., if a face e.g. just underlines, I
do want to color the text nevertheless.  Probably I didn't understand
what you meant. This expression works ok for me:

--8<---------------cut here---------------start------------->8---
(cl-some
 (lambda (face) (not (memq (face-foreground face nil t)
		      `(nil
			,(face-attribute 'default :foreground)
			"unspecified-fg" "unspecified-bg"))))
 (cl-mapcan (lambda (face-or-list) (if (facep face-or-list)
				  (list face-or-list)
				face-or-list))
	    (list
	     (get-text-property (point) 'face)
	     (get-text-property (point) 'font-lock-face))))
--8<---------------cut here---------------end--------------->8---

Overlays are not covered by this - which is not even necessary, because
overlays have priorities that can be controlled.


Thanks,

Michael.

bug#15900: 24.3.50; foreground-color-at-point returns wrong results

[Drew Adams]

> CCing Drew cause the Change Log mentions him with regard to
> `foreground-color-at-point'.

Thanks, Michael.  I wasn't reading this thread.

1. Let me first butt in with this preliminary comment, having now
diff'd the current code (as of an MS Windows snapshot from 11/12)
against the Emacs 24.3 code (which is the same as what I submitted).

I think it is a mistake to fudge `face-at-point' so that it returns
a face named in the buffer.  That is NOT at all "the face of the
character after point", which is what the doc string first line
says.  And it is (was) not the intention of `face-at-point' at all.
See below for a suggestion, for those who want a face named at point.

In addition, that argument should not be called THING.  It is NOT
(as is usually the case for an argument so named) a symbol or a
string naming an Emacs THING, as in `thingatpt.el'.  It is just a
Boolean which if non-nil means to try first for a face name at point.  
IOW, it should be called something like CHECK-FOR-FACE-NAMED-IN-TEXT.

2. Now on to the addition of argument MULTIPLE, which is mainly what
this bug is about, IIUC.

In the bug report, Michael said this:

  it looks only at one face and disregards the others.  This is
  especially meaningless when this first face doesn't specify any
  foreground at all.

Yes, `face-at-point' returned only the first of a list of faces.

No, it is not meaningless for `face-at-point' to return a face that
does not specify a foreground.  Or does not specify a background.
Or an underline.  Or any other face attribute.  By default, it was
designed to return face `default' if no other face was found at
point but if another face was found it returned that face.

`face-at-point' should give you a face at point, regardless of the
attributes that face might specify explicitly.

Does it make sense for `foreground-color-at-point' to return nil
if the face returned by `face-at-point' specifies no foreground?

Yes.  It was designed to return the color specified by a given
face, and if no such color was specified, to return nil.  But it
uses `face-at-point', which looks at face `default' if no other
face is found at point.

If there is a non-`default' face at point, then its foreground
should be used, and if there is none, then nil should be the result.

3. But MULTIPLE is something else.  It aims to give you all of the
faces at point, whether used as a text property or an overlay
property.  Like THING, MULTIPLE was added after I contributed the
code.  But unlike THING, MULTIPLE is a good addition.

I can imagine that it might be useful to optionally get only faces
used as a text property or only those used as an overlay property.
Michael's use case is apparently one such.

Perhaps `*face-at-point' should provide an argument for this.
(That would certainly be more useful than the THING argument,
which has nothing to do with the properties at point.)

It looks like there is a bug wrt the use of `get-char-property'.
`get-char-property' itself always favors an overlay property over
the same property used as a text property at the same position.

Furthermore, this important part of the `get-char-property'
behavior is not even mentioned in the doc string (it is mentioned
in the Elisp manual).  This DOC BUG should be the first thing to
be fixed.

Given this limitation, it seems that the right thing for
`face-at-point' to do is to gather faces using both
`get-text-property' and `get-overlay-property', and not
`get-char-property'.  And it might be good to add an argument that
lets you specify one or the other of these or both.

Given that fix, MULTIPLE should DTRT.  And Michael should be
have what he needs.

4. So I would propose the following:

a. REMOVE the misnamed argument THING altogether.  It does not
belong in `face-at-point'.  Create a function
`face-named-at-point' that returns the face named at point.
Anyone wanting the behavior of first-look-for-face-named-...
would use (or (face-named-at-point...) (face-at-point...)).
The time to make this change is NOW, before 24.4 is released.

b. Add an optional argument TYPE, with a nil value meaning
both overlay and text property.  Possible values would be
`overlay', `text', and nil.

c. TYPE would be taken into account regardless of the value
of MULTIPLE.  For non-nil MULTIPLE the function would return a
list of all faces at point, whether in overlays or the `face'
text property.  `face-at-point' would use `get-text-property'
or `get-overlay-property', or both, and not `get-char-property'.

> > As you later discovered, even (face-at-point nil t) doesn't
> > do the job.  Which doesn't surprise me a bit: this kind of
> > things cannot be done reliably from Lisp

I don't see any basis for saying that.  See above.  Sounds
pretty straightforward in Lisp, to me.  What am I missing?

> > even at a price of the kind of obfuscated code that
> > face-at-point and foreground-color-at-point use. 

Care to back that up?  Just where do you see obfuscation?

> > It is much simpler to write a C primitive that simulates
> > the display, then returns the resulting face at a given
> > character position, a simple and straightforward task on
> > the C level, something the display engine does all the
> > time.

Overengineering, IMO.  Not needed for `face-at-point', that
I can see.

bug#15900: 24.3.50; foreground-color-at-point returns wrong results

[Eli Zaretskii]

> Date: Fri, 15 Nov 2013 16:26:15 -0800 (PST)
> From: Drew Adams <drew.adams@oracle.com>
> Cc: 15900@debbugs.gnu.org
> 
> > > As you later discovered, even (face-at-point nil t) doesn't
> > > do the job.  Which doesn't surprise me a bit: this kind of
> > > things cannot be done reliably from Lisp
> 
> I don't see any basis for saying that.  See above.  Sounds
> pretty straightforward in Lisp, to me.  What am I missing?

You are missing the fact that only part of what the display engine
does to compute the appearance of a given character on display is
exposed to Lisp.

> > > even at a price of the kind of obfuscated code that
> > > face-at-point and foreground-color-at-point use. 
> 
> Care to back that up?  Just where do you see obfuscation?

Everywhere.  I don't understand why that code does what it does, for
starters.  It looks to me as a series of kludges one upon another,
with the sole purpose of outsmarting the display engine.  These kinds
of things should never be done in Lisp, because they all will look
like that: complicated, fragile, and unreliable.

> > > It is much simpler to write a C primitive that simulates
> > > the display, then returns the resulting face at a given
> > > character position, a simple and straightforward task on
> > > the C level, something the display engine does all the
> > > time.
> 
> Overengineering, IMO.

In my book, a simple and elegant solution is always better than a
complex and inelegant one.  That's the opposite of over-engineering.

bug#15900: 24.3.50; foreground-color-at-point returns wrong results

[Eli Zaretskii]

> From: Michael Heerdegen <michael_heerdegen@web.de>
> Cc: 15900@debbugs.gnu.org, Drew Adams <drew.adams@oracle.com>
> Date: Fri, 15 Nov 2013 23:18:11 +0100
> 
> > As you later discovered, even (face-at-point nil t) doesn't do the
> > job.
> >
> > Which doesn't surprise me a bit: this kind of things cannot be done
> > reliably from Lisp, even at a price of the kind of obfuscated code
> > that face-at-point and foreground-color-at-point use.  It is much
> > simpler to write a C primitive that simulates the display, then
> > returns the resulting face at a given character position, a simple and
> > straightforward task on the C level, something the display engine does
> > all the time.
> 
> That sounds good.  Can we just do that?

For some value of "we", yes.  If "we" includes me and you, then it
will have to be you, as my plate is pretty much full these days,
sorry.  In my defense, I can tell that this should be a nice exercise
for someone who wants to get accustomed to hacking the display engine,
as it shouldn't be too hard, and there's abundant example code that
does similar things.

> > > P.S. Some background: I'm working on an addition to stripe-buffer.el
> > > that changes the foreground color continuously, instead of changing the
> > > background.  This is for better readability.
> > > 
> > > I want to keep the foreground colors already present, so that e.g. links
> > > in w3m are still recognizable.  Paragraphs in italic can be colored
> > > OTOH.
> > > 
> > > So, what I need is a reliable `foreground-color-at-point'.  Tips and
> > > alternatives welcome.
> >
> > Why can't you detect that a portion of text is covered by specific
> > properties (e.g., one of a list of known properties), and leave those
> > portions alone?
> 
> What do you mean with "properties" - text and overlay properties?

Yes.

> If faces are among them, I still must figure out if one of these
> faces changes the foreground.

You can know them in advance, I think.  Your example talks about
links, which use a known face.  I presume there are only a few faces
that needs such a special treatment, which would make the list of them
quite short.

IOW, why not test against a known list of properties that you want to
leave alone, instead of digging into their color?

> If not, i.e., if a face e.g. just underlines, I do want to color the
> text nevertheless.

The face used by links is different not only in its underline, but
also in its color.  If you want links to remain instantly
recognizable, you should probably keep their appearance intact
wholesale, not just the underline, otherwise how would the user
distinguish between a link and underlined text?

> Probably I didn't understand what you meant.

More probable that I didn't understand what you meant.  Hopefully the
above tells enough about my misunderstanding to allow you to correct
me.

bug#15900: 24.3.50; foreground-color-at-point returns wrong results

[Michael Heerdegen]

Eli Zaretskii <eliz@gnu.org> writes:

> > That sounds good.  Can we just do that?
>
> For some value of "we", yes.  If "we" includes me and you, then it
> will have to be you, as my plate is pretty much full these days,
> sorry.  In my defense, I can tell that this should be a nice exercise
> for someone who wants to get accustomed to hacking the display engine,
> as it shouldn't be too hard, and there's abundant example code that
> does similar things.

Sorry Eli, but I can't do it.  I'm a real noob to C.

> > If faces are among them, I still must figure out if one of these
> > faces changes the foreground.
>
> You can know them in advance, I think.  Your example talks about
> links, which use a known face.  I presume there are only a few faces
> that needs such a special treatment, which would make the list of them
> quite short.
>
> IOW, why not test against a known list of properties that you want to
> leave alone, instead of digging into their color?

I think the missing information you didn't have is that this is a
general mode, it must work in any Emacs buffer.  w3m was only an example
- info, man, and gnus are others.  So, testing for hardcoded face or
property lists is not really an option.

> > If not, i.e., if a face e.g. just underlines, I do want to color the
> > text nevertheless.
>
> The face used by links is different not only in its underline, but
> also in its color.  If you want links to remain instantly
> recognizable, you should probably keep their appearance intact
> wholesale, not just the underline, otherwise how would the user
> distinguish between a link and underlined text?

Yes, that's what I actually do ;-)

> > Probably I didn't understand what you meant.
>
> More probable that I didn't understand what you meant.  Hopefully the
> above tells enough about my misunderstanding to allow you to correct
> me.

I think that you thought that what I do is w3m specific, but it's not.
It should work in any buffer, with any modes.  And it should change the
foreground color for every piece of text that has the default foreground
color.


Thanks, and regards,

Michael.

bug#15900: 24.3.50; foreground-color-at-point returns wrong results

[Eli Zaretskii]

> From: Michael Heerdegen <michael_heerdegen@web.de>
> Cc: 15900@debbugs.gnu.org,  drew.adams@oracle.com
> Date: Sun, 17 Nov 2013 03:33:47 +0100
> 
> > > If faces are among them, I still must figure out if one of these
> > > faces changes the foreground.
> >
> > You can know them in advance, I think.  Your example talks about
> > links, which use a known face.  I presume there are only a few faces
> > that needs such a special treatment, which would make the list of them
> > quite short.
> >
> > IOW, why not test against a known list of properties that you want to
> > leave alone, instead of digging into their color?
> 
> I think the missing information you didn't have is that this is a
> general mode, it must work in any Emacs buffer.  w3m was only an example
> - info, man, and gnus are others.  So, testing for hardcoded face or
> property lists is not really an option.

I still don't see why it isn't an option, even for a general-purpose
mode.  The list of faces that need such special treatment must be
quite short, and it can be a defcustom.

bug#15900: 24.3.50; foreground-color-at-point returns wrong results

[Michael Heerdegen]

Eli Zaretskii <eliz@gnu.org> writes:

> > > IOW, why not test against a known list of properties that you want to
> > > leave alone, instead of digging into their color?
> > 
> > I think the missing information you didn't have is that this is a
> > general mode, it must work in any Emacs buffer.  w3m was only an example
> > - info, man, and gnus are others.  So, testing for hardcoded face or
> > property lists is not really an option.
>
> I still don't see why it isn't an option, even for a general-purpose
> mode.  The list of faces that need such special treatment must be
> quite short, and it can be a defcustom.

From the user's point of view, yes, it's an option.  But you mentioned
it saying it would be easier to realize in Lisp.  This is what I have:

(cl-some
 (lambda (face) (not (memq (face-foreground face nil t)
		      `(nil
			,(face-attribute 'default :foreground)
			"unspecified-fg" "unspecified-bg"))))
 (cl-mapcan (lambda (face-or-list) (if (facep face-or-list)
				  (list face-or-list)
				face-or-list))
	    (list
	     (get-text-property (point) 'face)
	     (get-text-property (point) 'font-lock-face))))

For your solution, I can use a different predicate, but would still have
to find all faces at point, no?  What do I miss?


Regards,

Michael.

bug#15900: 24.3.50; foreground-color-at-point returns wrong results

[Eli Zaretskii]

> From: Michael Heerdegen <michael_heerdegen@web.de>
> Cc: 15900@debbugs.gnu.org,  drew.adams@oracle.com
> Date: Sun, 17 Nov 2013 06:35:03 +0100
> 
> For your solution, I can use a different predicate, but would still have
> to find all faces at point, no?  What do I miss?

I don't know, and I don't know what am I missing, either.

When you say "all the faces at point", what exactly do you mean by
that?  There can only be one 'face' text property at point, so do you
mean faces that come from overlays?  If that is what you mean, then
why isn't get-char-property all you need to have your answer?  I feel
I'm missing something very basic here.

bug#15900: 24.3.50; foreground-color-at-point returns wrong results

[Lars Ingebrigtsen]

Michael Heerdegen <michael_heerdegen@web.de> writes:

> `foreground-color-at-point' doesn't return the expected result when
> there are several faces at point present.
>
> For example, on the red subject in a Gnus article buffer, or on a blue
> link in w3m, it returns "black".  Looking at the code, I see that
> `foreground-color-at-point' uses
>
>   (face-at-point)
>
> i.e., it looks only at one face and disregards the others.  This is
> especially meaningless when this first face doesn't specify any
> foreground at all.

It looks like this was fixed a couple years later in this commit:

commit 7ccedcb486ee4e37da54dd82a8557c80616d9467
Author:     Artur Malabarba <bruce.connor.am@gmail.com>
AuthorDate: Fri Oct 30 15:00:37 2015 +0000

    * lisp/faces.el: Refactor common code and fix a bug
    
    (faces--attribute-at-point): New function.  Fix a bug when the
    face at point is a list of faces and the desired attribute is not
    on the first one.
    (foreground-color-at-point, background-color-at-point): Use it.

And I've verified that (foreground-color-at-point) seems to return
sensible results in the test cases in Emacs 29, so I'm closing this bug
report.

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

bug#15900: 24.3.50; foreground-color-at-point returns wrong results

[Drew Adams]

> > > > As you later discovered, even (face-at-point nil t) doesn't
> > > > do the job.  Which doesn't surprise me a bit: this kind of
> > > > things cannot be done reliably from Lisp
> >
> > I don't see any basis for saying that.  See above.  Sounds
> > pretty straightforward in Lisp, to me.  What am I missing?
> 
> You are missing the fact that only part of what the display engine
> does to compute the appearance of a given character on display is
> exposed to Lisp.

Please be specific.  Just what, in the current context, cannot
be done?

The point of `face-at-point' is to give Lisp code a face at point.
Clearly Lisp code can have access only to faces that it can access!
What else is needed here, for `face-at-point'?

> > Just where do you see obfuscation?
> 
> Everywhere.  I don't understand why that code does what it does, for
> starters.  It looks to me as a series of kludges one upon another,
> with the sole purpose of outsmarting the display engine.  

Generalities.  What things?  In what way do you think it is trying
to outsmart the display engine?  If you criticize the code, that's
good - but please speak about it specifically.  Otherwise, there is
no way to know what you are talking about, and no one can learn
anything other than the fact that you don't understand the code.

> These kinds of things should never be done in Lisp, because they
> all will look like that: complicated, fragile, and unreliable.

What kinds of things?  What things?  Why should they, whatever they
are "never be done in Lisp"?  I ask you to point to something
specific that is problematic, and you just reply "Everywhere" and
"these things" and "should never be done".  That's not constructive.
Talk about obfuscation!

> In my book, a simple and elegant solution is always better than a
> complex and inelegant one.  That's the opposite of over-engineering.

No one would disagree with that first sentence.  Just a platitude,
unfortunately.  Please explain what complexity and inelegance you
perceive, so we all can learn, instead of just mouthing "obfuscation"
"everywhere".

And while you're at it, please describe the Lisp-level function(s)
that you propose to provide in C.  I have no doubt they will be
useful - I am confident in you for that.  My questions are about
your problems with the code of `face-at-point', not about your
ability to provide something useful to Lisp from C.  I look forward
to some real, helpful information about both.

bug#15900: 24.3.50; foreground-color-at-point returns wrong results

[Eli Zaretskii]

> Date: Sat, 16 Nov 2013 08:20:01 -0800 (PST)
> From: Drew Adams <drew.adams@oracle.com>
> Cc: michael_heerdegen@web.de, 15900@debbugs.gnu.org
> 
> > > > > As you later discovered, even (face-at-point nil t) doesn't
> > > > > do the job.  Which doesn't surprise me a bit: this kind of
> > > > > things cannot be done reliably from Lisp
> > >
> > > I don't see any basis for saying that.  See above.  Sounds
> > > pretty straightforward in Lisp, to me.  What am I missing?
> > 
> > You are missing the fact that only part of what the display engine
> > does to compute the appearance of a given character on display is
> > exposed to Lisp.
> 
> Please be specific.  Just what, in the current context, cannot
> be done?

What foreground-color-at-point tries to do.

> The point of `face-at-point' is to give Lisp code a face at point.

I wasn't talking about face-at-point (although it, too, has its
measure of difficulties, as the face of a character can come from
umpteen different sources).  This discussion is about
foreground-color-at-point.

> Clearly Lisp code can have access only to faces that it can access!
> What else is needed here, for `face-at-point'?

I was not talking about face-at-point.

> > > Just where do you see obfuscation?
> > 
> > Everywhere.  I don't understand why that code does what it does, for
> > starters.  It looks to me as a series of kludges one upon another,
> > with the sole purpose of outsmarting the display engine.  
> 
> Generalities.  What things?  In what way do you think it is trying
> to outsmart the display engine?

It tries to second-guess what the display engine would do given the
faces and overlays at or around point.  It is much better to ask the
display engine to provide that information directly.

> > These kinds of things should never be done in Lisp, because they
> > all will look like that: complicated, fragile, and unreliable.
> 
> What kinds of things?  What things?

foreground-color-at-point, of course.

> Why should they, whatever they are "never be done in Lisp"?

Because, for the umpteenth time, Lisp code is not given enough
information to do that.

> please describe the Lisp-level function(s) that you propose to
> provide in C

foreground-color-at-point, obviously.  Maybe also
background-color-at-point, for symmetry.  I would also propose
face-at-point, but that's another discussion.

bug#15900: 24.3.50; foreground-color-at-point returns wrong results

[Drew Adams]

> > For your solution, I can use a different predicate, but would
> > still have to find all faces at point, no?  What do I miss?
> 
> I don't know, and I don't know what am I missing, either.
> 
> When you say "all the faces at point", what exactly do you mean by
> that?  There can only be one 'face' text property at point, so do
> you mean faces that come from overlays?  If that is what you mean,
> then why isn't get-char-property all you need to have your answer?
> I feel I'm missing something very basic here.

I think he means that the value of property `face' (whether from
a text property or an overlay property) can be a list of faces.
Normally, the attributes of the faces in the list are merged for
display.  See (elisp) `Special Properties' and (elisp `Overlay
Properties'.

bug#15900: 24.3.50; foreground-color-at-point returns wrong results

[Eli Zaretskii]

> Date: Sun, 17 Nov 2013 09:29:57 -0800 (PST)
> From: Drew Adams <drew.adams@oracle.com>
> Cc: 15900@debbugs.gnu.org, drew.adams@oracle.com
> 
> > When you say "all the faces at point", what exactly do you mean by
> > that?  There can only be one 'face' text property at point, so do
> > you mean faces that come from overlays?  If that is what you mean,
> > then why isn't get-char-property all you need to have your answer?
> > I feel I'm missing something very basic here.
> 
> I think he means that the value of property `face' (whether from
> a text property or an overlay property) can be a list of faces.

Yes, but then you get all that list in a single call to
get-char-property, right?  So where's the difficulty in collecting
"all the faces at point"?

bug#15900: 24.3.50; foreground-color-at-point returns wrong results

[Michael Heerdegen]

Hello,

> > I think he means that the value of property `face' (whether from
> > a text property or an overlay property) can be a list of faces.

Indeed.  Plus, I want to look at the font-lock-face property.

> Yes, but then you get all that list in a single call to
> get-char-property, right?  So where's the difficulty in collecting
> "all the faces at point"?

There's no difficulty.  Eli, did you ever read the code snip I had
included?  Here it is again:

(cl-some
 (lambda (face) (not (memq (face-foreground face nil t)
		      `(nil
			,(face-attribute 'default :foreground)
			"unspecified-fg" "unspecified-bg"))))
 (cl-mapcan (lambda (face-or-list) (if (facep face-or-list)
				  (list face-or-list)
				face-or-list))
	    (list
	     (get-text-property (point) 'face)
	     (get-text-property (point) 'font-lock-face))))

I think what you mean, and what I do in the code, are quite the same
(except for the predicate maybe).  I appreciate your help, but please
tell me if the snipped is ok or if you mean something else, and if so,
what.  I've just the feeling that we to talk at cross-purposes.


Thanks,

Michael.

bug#15900: 24.3.50; foreground-color-at-point returns wrong results

[Eli Zaretskii]

> From: Michael Heerdegen <michael_heerdegen@web.de>
> Cc: Drew Adams <drew.adams@oracle.com>,  15900@debbugs.gnu.org
> Date: Sun, 17 Nov 2013 23:38:11 +0100
> 
> Hello,
> 
> > > I think he means that the value of property `face' (whether from
> > > a text property or an overlay property) can be a list of faces.
> 
> Indeed.  Plus, I want to look at the font-lock-face property.
> 
> > Yes, but then you get all that list in a single call to
> > get-char-property, right?  So where's the difficulty in collecting
> > "all the faces at point"?
> 
> There's no difficulty.

Then I don't understand why you still want to analyze colors instead
of analyzing faces.  This is what my suggestion boiled down to:
instead of trying to figure out the foreground color of the text at
point, just compare its face(s) with a list of known faces whose
appearance you don't want to override.

> Eli, did you ever read the code snip I had included?

Of course, I did.  But it still talks about colors, something I
suggested to avoid.

bug#15900: 24.3.50; foreground-color-at-point returns wrong results

[Michael Heerdegen]

Eli Zaretskii <eliz@gnu.org> writes:

> > There's no difficulty.
>
> Then I don't understand why you still want to analyze colors instead
> of analyzing faces.  This is what my suggestion boiled down to:
> instead of trying to figure out the foreground color of the text at
> point, just compare its face(s) with a list of known faces whose
> appearance you don't want to override.

Ok, I think now I understand your point.  And I see that my approach has
its difficulties: faces can be customized (a foreground attribute can be
removed or added), faces can even be frame dependent, etc.  What I do is
just a heuristic.  Mmh, let me think about it.  Thanks for your opinion.


Regards,

Michael.

bug#15900: 24.3.50; foreground-color-at-point returns wrong results

[Drew Adams]

> > Please be specific.  Just what, in the current context, cannot
> > be done?
> 
> What foreground-color-at-point tries to do.

Please be specific.  You have said nothing about it, so far.

> > The point of `face-at-point' is to give Lisp code a face at point.
> 
> I wasn't talking about face-at-point (although it, too, has its
> measure of difficulties, as the face of a character can come from
> umpteen different sources).  This discussion is about
> foreground-color-at-point.

Fine.  Replace `face-at-point' with `foreground-at-point' in my
questions to you: Please be specific about the `foreground-at-point'
code.  So far, you have said nothing concrete about it.

> > > Everywhere.  I don't understand why that code does what it does,
> > > for starters.  It looks to me as a series of kludges one upon
> > > another, with the sole purpose of outsmarting the display engine.
> >
> > Generalities.  What things?  In what way do you think it is trying
> > to outsmart the display engine?
> 
> It tries to second-guess what the display engine would do given the
> faces and overlays at or around point.  

How so?  Please point to something in the code that you think "tries
to second-guess what the display engine would do..."  Stop
hand-waving, please.

> It is much better to ask the display engine to provide that
> information directly.

What information?  Please point to specifics in the Lisp code.

> > > These kinds of things should never be done in Lisp, because they
> > > all will look like that: complicated, fragile, and unreliable.
> >
> > What kinds of things?  What things?
> 
> foreground-color-at-point, of course.

Again, just hand-waving.  Just what things in the code of
`foreground-color-at-point' are problematic, for you?

> > Why should they, whatever they are, "never be done in Lisp"?
> 
> Because, for the umpteenth time, Lisp code is not given enough
> information to do that.

To do what?  What information are you hinting about?

Please stop with the generalities about "Lisp code" and "these
kinds of things" and  "information", and get down to Earth with
your criticism of the `foreground-color-at-point' code.  You claim
that the code is obfuscated.  I want to learn just how that is.
What is unclear about it?

> > please describe the Lisp-level function(s) that you propose to
> > provide in C
> 
> foreground-color-at-point, obviously.

I see.  And what is the spec of what that function would do?
What would it do differently?  Just what is the problem you would
be trying to solve by moving the implementation of this function
to C?

Please don't just reply that you will remove obfuscation or some
such.  Be specific about the problems you see in the Lisp code
and how you will solve those problems by moving the code to C.

I'm sure that if you do that I will understand, and will likely be
convinced.  But so far you have just been hand-waving, and its
hard to learn anything from that.

Clearly, if Lisp code cannot be used to accomplish something that
C code can, then we want to that something in C.  So far, you have
said nothing about what that something is.  Except for generalities:
get the foreground color at point or some such.  Speak about the
specific differences, please.

The lisp code for `foreground-color-at-point' uses `face-at-point'
(which you say you don't see as the problem) and text properties
`read-face-name' and `face'.  That is the design - simple, and
perhaps too simple for all purposes.

But I don't see obfuscation there - it's pretty clear what is
going on.  It would be clearer perhaps if the doc string said
explicitly that the foreground color returned, if any, is taken
from `face-at-point' or text property `read-face-name' or `face',
in that order.

I'm certainly willing to believe that those three things will not
always provide all possible information about the foreground
color at point.  But there is no confusion or hiding of just what
the code does, IMO.  It confuses me that you say the code is
confusing, so I ask you, "How so?"

If you become specific in this discussion then there might be
some hope of light, instead of just smoke.

bug#15900: 24.3.50; foreground-color-at-point returns wrong results

[Eli Zaretskii]

> Date: Sat, 16 Nov 2013 09:47:29 -0800 (PST)
> From: Drew Adams <drew.adams@oracle.com>
> Cc: michael_heerdegen@web.de, 15900@debbugs.gnu.org
> 
> > > Please be specific.  Just what, in the current context, cannot
> > > be done?
> > 
> > What foreground-color-at-point tries to do.
> 
> Please be specific.

This is silly.  You do know the purpose of foreground-color-at-point,
don't you?  Here's a scoop in case you didn't: it's trying to return
the foreground color of the character at point.  This is what I say
cannot be reliably done in Lisp: you cannot reliably determine the
colors of any specific character at any specific buffer position.

Now, how more specific than that can one be??

> > It is much better to ask the display engine to provide that
> > information directly.
> 
> What information?

The color of the character at point, of course.

> > > please describe the Lisp-level function(s) that you propose to
> > > provide in C
> > 
> > foreground-color-at-point, obviously.
> 
> I see.  And what is the spec of what that function would do?

Same one as the Lisp implementation, except that it will do that
correctly and reliably.  IOW, the implementation will change
completely, while the API will remain unchanged, and the contract will
also remain unchanged, except that it will now be always honored.

> What would it do differently?

It would simulate the display, as if Emacs actually did display the
character at point, and return the color produced by that.  You cannot
do that in Lisp.

> Just what is the problem you would be trying to solve by moving the
> implementation of this function to C?

The current Lisp code collects face information at or around point,
and tries to deduce from that how the character will be displayed.
Instead, the C implementation will actually display it (without
putting the result on the glass).  The result is guaranteed to be
correct.  By contrast, the Lisp implementation cannot promise such
correctness, because it in effect tries to reproduce the logic used by
the display engine, and the result is a different implementation, that
cannot possibly be 100% compatible with what the display does.

> I'm sure that if you do that I will understand, and will likely be
> convinced.  But so far you have just been hand-waving, and its
> hard to learn anything from that.

There's a lot of experience in hacking the display engine behind that
hand-waving, so you may wish to accept what I'm saying.  Or not, I
really don't care much.

> Clearly, if Lisp code cannot be used to accomplish something that
> C code can, then we want to that something in C.  So far, you have
> said nothing about what that something is.  Except for generalities:
> get the foreground color at point or some such.  Speak about the
> specific differences, please.

I can't: you lack too much knowledge for me to discuss the details.
Just read xdisp.c, if you really want to know.  I assure you that once
you've done that, it will become crystal clear to you that producing
the color at point is almost trivial on the C level, since code that
simulates display is readily available and widely used by many Emacs
features.  Reimplementing that in Lisp is waste of energy.

> The lisp code for `foreground-color-at-point' uses `face-at-point'
> (which you say you don't see as the problem) and text properties
> `read-face-name' and `face'.  That is the design - simple, and
> perhaps too simple for all purposes.

Except that a face of a character can come from many different places,
and there could be several different faces in effect at a given buffer
position, some coming from text properties, others from overlays,
still others from display strings, etc. etc.  And what if the
character at point is not even displayed, e.g. if it's covered by some
invisibility spec?  Why try to untangle all this complexity, when the
display engine already does?

> But I don't see obfuscation there - it's pretty clear what is
> going on.

It's wrong, for starters -- this is why this discussion started.  And
it's nowhere near clear, either; but I'm not going to try to convince
you in that.

> It would be clearer perhaps if the doc string said explicitly that
> the foreground color returned, if any, is taken from `face-at-point'
> or text property `read-face-name' or `face', in that order.

As a user of this API, I don't really care.  I want to know the
foreground color of the character at point, and I don't care how the
implementation does that.  So any clarifications in the doc string
about implementation details will not help me, as a user, to
understand the answer to a simple question: does this API produce what
it promises, or doesn't it?

> I'm certainly willing to believe that those three things will not
> always provide all possible information about the foreground
> color at point.

I'm trying to explain that you shouldn't even _try_ doing that in
Lisp.  If you do, you will just waste effort, and likely come up with
unreliable results, because what the display engine does cannot be
reliably reimplemented in Lisp, not with the current design.

bug#15900: 24.3.50; foreground-color-at-point returns wrong results

[Drew Adams]

Fair enough.  Now you've made clear the intent.  Thank you for
the effort.

I agree that it would be worthwhile to have the true foreground
color at point, and not just the foreground color of a face at
point.  Looking forward to it.