how to track down "invalid face attribute" errors?

how to track down "invalid face attribute" errors?

[Eric S Fraga]

Hello all,

some package I'm using is leading to errors of the form

  Invalid face attribute :foreground nil [54 times]

can anybody suggest how I can track down which package/function is
causing this?  Debugging on error doesn't catch this.  All web searches
lead me to specific packages that have had this error in the past (none
of which I use).

Thank you,
eric

-- 
Eric S Fraga via gnus (Emacs 29.0.50 2022-09-22) on Debian 11.4

Re: how to track down "invalid face attribute" errors?

[Emanuel Berg]

Eric S Fraga wrote:

> some package I'm using is leading to errors of the form
>
>   Invalid face attribute :foreground nil [54 times]
>
> can anybody suggest how I can track down which
> package/function is causing this? Debugging on error doesn't
> catch this. All web searches lead me to specific packages
> that have had this error in the past (none of which I use).

Sounds like that error message should be changed to include
what face causes trouble, if possible?

BTW here is a function to loop thru all faces and set them to
something, try for example

;; (set-all-faces "red")

Maybe you can modify that to retrieve attributes instead?

That could be cool ...

(defun set-all-faces (fg &optional bg weight)
  (let ((backg (or bg     "black"))
        (wght  (or weight 'normal))
        (faces) )
    (mapatoms (lambda (s)
                (when (facep s)
                  (push (symbol-name s) faces) )))
    (dolist (f faces)
      (set-face-attribute (intern f) nil
                          :foreground fg)
                          :background backg
                          :weight     wght
                          :italic     nil) ))

-- 
underground experts united
https://dataswamp.org/~incal

Re: how to track down "invalid face attribute" errors?

[Eli Zaretskii]

> From: Eric S Fraga <e.fraga@ucl.ac.uk>
> Date: Sun, 25 Sep 2022 13:03:03 +0100
> 
> some package I'm using is leading to errors of the form
> 
>   Invalid face attribute :foreground nil [54 times]
> 
> can anybody suggest how I can track down which package/function is
> causing this?  Debugging on error doesn't catch this.  All web searches
> lead me to specific packages that have had this error in the past (none
> of which I use).

Can you run Emacs under GDB?  If so, put a breakpoint on the line in
xfaces.c which emits this message, and use the backtrace to find out
which face causes it.

Re: how to track down "invalid face attribute" errors?

[Felix Dietrich]

Hi Eric,

Eric S Fraga <e.fraga@ucl.ac.uk> writes:

> some package I'm using is leading to errors of the form
>
>   Invalid face attribute :foreground nil [54 times]
>
> can anybody suggest how I can track down which package/function is
> causing this?  Debugging on error doesn't catch this.

I am just going to brainstorm some ideas here:

1. Go through every buffer and check for every buffer position whether
any anonymous face set in a text property or overlay property has a
:foreground attribute that is nil.  This might give you an idea about
which packages could be responsible.  I do not have code for that, but
could probably concoct something.


2. Grep through your configuration and your installed packages for
":foreground" and see if it is paired with nil somewhere.  One of the
various functions to add text properties [1] or ‘overlay-put’ should be
close by.


3. Deactivate groups of packages and see what happens.  Might be a bit
to tedious and error prone if you have a lot installed and need to
manage their shared dependencies.


4. A bit of a harebrained sledgehammer approach (which might lead
nowhere): run Emacs in a debugger, set a watchpoint on ‘add_to_log’
being called with a ‘format’ argument starting with "Invalid face
attribute", then, on break, analyse the stack, and see if you can figure
out the culprit.


Footnotes:
[1]  (info "(elisp) Changing Properties")

     <https://www.gnu.org/software/emacs/manual/html_node/elisp/Changing-Properties.html>

-- 
Felix Dietrich

Re: how to track down "invalid face attribute" errors?

[Fraga, Eric]

Hi Felix,

Thank you for the various suggestions.  They are all rather cumbersome
but all make sense as well.  I'm doing the disable various packages in
turn at the moment.

Thanks again,
eric

-- 
Eric S Fraga via gnus (Emacs 29.0.50 2022-09-22) on Debian 11.4

Re: how to track down "invalid face attribute" errors?

[Eric S Fraga]

Hi Eli,

On Sunday, 25 Sep 2022 at 17:01, Eli Zaretskii wrote:
> Can you run Emacs under GDB?  If so, put a breakpoint on the line in
> xfaces.c which emits this message, and use the backtrace to find out
> which face causes it.

Currently playing around by disabling packages to try to track this down
but I will try this if all else fails.  It's been years (decades?) since
I last used gdb...

Thank you,
eric

-- 
Eric S Fraga via gnus (Emacs 29.0.50 2022-09-22) on Debian 11.4

Re: how to track down "invalid face attribute" errors?

[Eli Zaretskii]

> From: Eric S Fraga <e.fraga@ucl.ac.uk>
> Date: Sun, 25 Sep 2022 16:53:52 +0100
> 
> On Sunday, 25 Sep 2022 at 17:01, Eli Zaretskii wrote:
> > Can you run Emacs under GDB?  If so, put a breakpoint on the line in
> > xfaces.c which emits this message, and use the backtrace to find out
> > which face causes it.
> 
> Currently playing around by disabling packages to try to track this down
> but I will try this if all else fails.  It's been years (decades?) since
> I last used gdb...

If you have many dozens of packages to try disabling, it might be
faster to ask for instructions for running under GDB, because that
will give you an instant and accurate answer.

Your call.

Re: how to track down "invalid face attribute" errors?

[Eric S Fraga]

On Sunday, 25 Sep 2022 at 19:06, Eli Zaretskii wrote:
> If you have many dozens of packages to try disabling, it might be
> faster to ask for instructions for running under GDB, because that
> will give you an instant and accurate answer.

Thank you.  In any case, I have now found it (working on the last
instantiated package first basis ;-)).

-- 
Eric S Fraga via gnus (Emacs 29.0.50 2022-09-22) on Debian 11.4

Re: how to track down "invalid face attribute" errors?

[Eric S Fraga]

On Sunday, 25 Sep 2022 at 15:00, Emanuel Berg wrote:
> Sounds like that error message should be changed to include
> what face causes trouble, if possible?

That sounds useful.

-- 
Eric S Fraga via gnus (Emacs 29.0.50 2022-09-26) on Debian 11.5

Re: how to track down "invalid face attribute" errors?

[Robert Pluim]

>>>>> On Mon, 26 Sep 2022 10:04:09 +0100, Eric S Fraga <e.fraga@ucl.ac.uk> said:

    Eric> On Sunday, 25 Sep 2022 at 15:00, Emanuel Berg wrote:
    >> Sounds like that error message should be changed to include
    >> what face causes trouble, if possible?

    Eric> That sounds useful.

I think youʼre looking for the `debug-on-message' variable, since this
is a log in *Messages*, not an error.

(If you have a reproducer, we can look at changing the message)

Robert
-- 

Re: how to track down "invalid face attribute" errors?

[Fraga, Eric]

On Monday, 26 Sep 2022 at 11:40, Robert Pluim wrote:
> I think youʼre looking for the `debug-on-message' variable, since this
> is a log in *Messages*, not an error.

Excellent.  Definitely what I was looking for (and knew about at some
point in the past but forgot completely).

-- 
Eric S Fraga via gnus (Emacs 29.0.50 2022-09-26) on Debian bullseye/sid

Re: how to track down "invalid face attribute" errors?

[Felix Dietrich]

Robert Pluim <rpluim@gmail.com> writes:

> On Mon, 26 Sep 2022 10:04:09 +0100, Eric S Fraga <e.fraga@ucl.ac.uk> said:
>
>> On Sunday, 25 Sep 2022 at 15:00, Emanuel Berg wrote:
>>
>>> Sounds like that error message should be changed to include
>>> what face causes trouble, if possible?
>
> I think youʼre looking for the `debug-on-message' variable, since this
> is a log in *Messages*, not an error.

Would this work in this case?  The variable ‘debug-on-message’ seems to
be handled in the C-function ‘set_message’ which, on a cursory glance, I
cannot see being called from ‘add_to_log’ or below.  Therefore,
‘debug-on-message’ may be bypassed in this case here.

> (If you have a reproducer, we can look at changing the message)

Here is one:

#+begin_src emacs-lisp
  (let ((buf (generate-new-buffer "*test*"))
        (s (propertize "Hello World" 'face '(:foreground nil))))
    (with-current-buffer buf
      (insert s))
    (display-buffer buf))
#+end_src

Upon evaluation, the resulting output in the “*Messages*” buffer also
shows another issues with a missing newline between messages (in Emacs
28.1 at least).

#+begin_example
#<window 54 on *test*<2>>Invalid face attribute :foreground nil
Invalid face attribute :foreground nil
#+end_example

Maybe a value of nil for the :foreground attribute in an anonymous face
should be handled the same way as it is in a face defined with
‘defface’, that is being treated as the symbol ‘unspecified’?

#+begin_src emacs-lisp
  (progn
    (defface my/test '((t . (:foreground nil))) "test face")
    (face-attribute 'my/test :foreground))
#+end_src

#+RESULTS:
: unspecified


-- 
Felix Dietrich

Re: how to track down "invalid face attribute" errors?

[Robert Pluim]

>>>>> On Tue, 27 Sep 2022 13:12:12 +0200, Felix Dietrich <felix.dietrich@sperrhaken.name> said:

    Felix> Robert Pluim <rpluim@gmail.com> writes:
    >> On Mon, 26 Sep 2022 10:04:09 +0100, Eric S Fraga <e.fraga@ucl.ac.uk> said:
    >> 
    >>> On Sunday, 25 Sep 2022 at 15:00, Emanuel Berg wrote:
    >>> 
    >>>> Sounds like that error message should be changed to include
    >>>> what face causes trouble, if possible?
    >> 
    >> I think youʼre looking for the `debug-on-message' variable, since this
    >> is a log in *Messages*, not an error.

    Felix> Would this work in this case?  The variable ‘debug-on-message’ seems to
    Felix> be handled in the C-function ‘set_message’ which, on a cursory glance, I
    Felix> cannot see being called from ‘add_to_log’ or below.  Therefore,
    Felix> ‘debug-on-message’ may be bypassed in this case here.

In that case gdb and something like this

gdb emacs
> b add_to_log
> commands
> bt
> c
> end
> run

add_to_log is not called that often, so this shouldnʼt create too much
output.

    >> (If you have a reproducer, we can look at changing the message)

    Felix> Here is one:

    Felix> #+begin_src emacs-lisp
    Felix>   (let ((buf (generate-new-buffer "*test*"))
    Felix>         (s (propertize "Hello World" 'face '(:foreground nil))))
    Felix>     (with-current-buffer buf
    Felix>       (insert s))
    Felix>     (display-buffer buf))
    Felix> #+end_src

Thereʼs no actual face there for Emacs to log, what the low-level code
receives is "(:foreground nil)"

    Felix> Upon evaluation, the resulting output in the “*Messages*” buffer also
    Felix> shows another issues with a missing newline between messages (in Emacs
    Felix> 28.1 at least).

    Felix> #+begin_example
    Felix> #<window 54 on *test*<2>>Invalid face attribute :foreground nil
    Felix> Invalid face attribute :foreground nil
    Felix> #+end_example

Itʼs still there in emacs-29, but itʼs deep in the guts of the elisp
engine, so I havenʼt found exactly where it comes from.

    Felix> Maybe a value of nil for the :foreground attribute in an anonymous face
    Felix> should be handled the same way as it is in a face defined with
    Felix> ‘defface’, that is being treated as the symbol ‘unspecified’?

    Felix> #+begin_src emacs-lisp
    Felix>   (progn
    Felix>     (defface my/test '((t . (:foreground nil))) "test face")
    Felix>     (face-attribute 'my/test :foreground))
    Felix> #+end_src

    Felix> #+RESULTS:
    Felix> : unspecified

There was recently a long thread about this on emacs-devel, which I
didnʼt read :-) It might be worth suggesting this there.

Robert
-- 

Re: how to track down "invalid face attribute" errors?

[Stefan Monnier via Users list for the GNU Emacs text editor]

Emanuel Berg [2022-09-25 15:00:31] wrote:
> Sounds like that error message should be changed to include
> what face causes trouble, if possible?

IIUC this message comes when the face is not a symbol but a plist of
properties, so there's a good chance that printing the face will just
say (:foreground nil) and thus won't help you.  A better option might be
to include the position of the problem, so you can then go and
investigate the text properties and overlays at that position.


        Stefan

Re: how to track down "invalid face attribute" errors?

[Robert Pluim]

>>>>> On Wed, 28 Sep 2022 07:41:38 -0400, Stefan Monnier via Users list for the GNU Emacs text editor <help-gnu-emacs@gnu.org> said:

    Stefan> Emanuel Berg [2022-09-25 15:00:31] wrote:
    >> Sounds like that error message should be changed to include
    >> what face causes trouble, if possible?

    Stefan> IIUC this message comes when the face is not a symbol but a plist of
    Stefan> properties, so there's a good chance that printing the face will just
    Stefan> say (:foreground nil) and thus won't help you.  A better option might be
    Stefan> to include the position of the problem, so you can then go and
    Stefan> investigate the text properties and overlays at that position.

Yes, (:foreground nil) is what you get. And by the time the log is
produced in `merge_face_ref', thereʼs no trace left of the position
weʼre looking at.

Robert
-- 

Re: how to track down "invalid face attribute" errors?

[Emanuel Berg]

Stefan Monnier via Users list for the GNU Emacs text editor wrote:

>> Sounds like that error message should be changed to include
>> what face causes trouble, if possible?
>
> IIUC this message comes when the face is not a symbol but
> a plist of properties, so there's a good chance that
> printing the face will just say (:foreground nil) and thus
> won't help you. [...]

Okay, but why are the face used before they even have a name?

Isn't it better to initialize the faces first, and if it is
attempted to define them using invalid attributes, then that
would be an error?

-- 
underground experts united
https://dataswamp.org/~incal

Re: how to track down "invalid face attribute" errors?

[Felix Dietrich]

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

Hello Emanuel,

Emanuel Berg <incal@dataswamp.org> writes:

> Stefan Monnier via Users list for the GNU Emacs text editor wrote:
>
>>> Sounds like that error message should be changed to include
>>> what face causes trouble, if possible?
>>
>> IIUC this message comes when the face is not a symbol but
>> a plist of properties, so there's a good chance that
>> printing the face will just say (:foreground nil) and thus
>> won't help you. [...]
>
> Okay, but why are the face used before they even have a name?

Those are *anonymous* faces.  You can attach these to text or overlay
properties.  If I recall correctly, the Emacs Lisp manual recommends
using named faces but recognises that anonymous faces might have some
uses.

> Isn't it better to initialize the faces first, and if it is
> attempted to define them using invalid attributes, then that
> would be an error?

This already happens to a degree for named faces: if you use ‘defface’
to create a face with :foreground set to nil, subsequent calls to
‘face-attribute’ for :foreground on that face will return ‘unspecified’:

#+begin_src emacs-lisp
  (progn
    (defface my/test '((t . (:foreground nil))) "test face")
    (face-attribute 'my/test :foreground))
#+end_src

#+RESULTS:
: unspecified


If I skimmed the code correctly, this is done in
“xfaces.c:internal-set-lisp-face-attribute” around line 3344 (note that
there is also a check to ensure that the :foreground attribute is a
string):

#+NAME: xfaces.c:3344
#+begin_src c
  else if (EQ (attr, QCforeground))
    {
      /* Compatibility with 20.x.  */
      if (NILP (value))
        value = Qunspecified;
      if (!UNSPECIFIEDP (value) && !IGNORE_DEFFACE_P (value))
        {
          /* Don't check for valid color names here because it depends
             on the frame (display) whether the color will be valid
             when the face is realized.  */
          CHECK_STRING (value);
          if (SCHARS (value) == 0)
            signal_error ("Empty foreground color value", value);
        }
#+end_src


In “xfaces.c:merge_face_ref”, around line 2728, on the other hand,
everything but strings is considered an error for the :foreground
attribute:

#+NAME: xfaces.c:2728
#+begin_src c
              else if (EQ (keyword, QCforeground))
                {
                  if (STRINGP (value))
                    to[LFACE_FOREGROUND_INDEX] = value;
                  else
                    err = true;
                }
#+end_src c

Maybe just a missing check to allow NILP for the :foreground value in
“xfaces.c:2728” (completely untested):


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Patch to also allow NILP for :foreground in merge_face_ref --]
[-- Type: text/x-patch, Size: 428 bytes --]

diff --git a/src/xfaces.c b/src/xfaces.c
index f7ee19195f..ab624a8d87 100644
--- a/src/xfaces.c
+++ b/src/xfaces.c
@@ -2727,7 +2727,9 @@ merge_face_ref (struct window *w,
 		}
 	      else if (EQ (keyword, QCforeground))
 		{
-		  if (STRINGP (value))
+		  if (NILP (value))
+		    to[LFACE_FOREGROUND_INDEX] = Qunspecified;
+		  else if (STRINGP (value))
 		    to[LFACE_FOREGROUND_INDEX] = value;
 		  else
 		    err = true;

[-- Attachment #3: Type: text/plain, Size: 20 bytes --]


-- 
Felix Dietrich

Re: how to track down "invalid face attribute" errors?

[Emanuel Berg]

Felix Dietrich wrote:

>>> IIUC this message comes when the face is not a symbol but
>>> a plist of properties, so there's a good chance that
>>> printing the face will just say (:foreground nil) and thus
>>> won't help you. [...]
>>
>> Okay, but why are the face used before they even have
>> a name?
>
> Those are *anonymous* faces. You can attach these to text or
> overlay properties. If I recall correctly, the Emacs Lisp
> manual recommends using named faces but recognises that
> anonymous faces might have some uses.

Okay, what use cases are they?

And even so, if they are anonymous faces, why are they
producing a warning for an invalid value, which should then be
part of their definition? Shouldn't that be an error?

Or can you somehow refer to the face, even if it is anonymous,
an make it complete at some later stage?

Oh no, this whole thing ...

-- 
underground experts united
https://dataswamp.org/~incal

Re: how to track down "invalid face attribute" errors?

[Felix Dietrich]

Felix Dietrich <felix.dietrich@sperrhaken.name> writes:

> Emanuel Berg <incal@dataswamp.org> writes:
>
>> Isn't it better to initialize the faces first, and if it is
>> attempted to define [a face] using invalid attributes, then that
>> would be an error?
>
> This already happens to a degree for named faces: if you use ‘defface’
> to create a face with :foreground set to nil, subsequent calls to
> ‘face-attribute’ for :foreground on that face will return ‘unspecified’:
>
> #+begin_src emacs-lisp
>   (progn
>     (defface my/test '((t . (:foreground nil))) "test face")
>     (face-attribute 'my/test :foreground))
> #+end_src

I was not correct with my assumption: when you ‘defface’ a face with an
invalid :foreground value this will produce an error, but enough of the
face with the invalid value remains that later creations of new frames
using ‘make-frame-command’ will fail with that same error:

#+begin_src emacs-lisp
  (progn
    (ignore-errors
      (defface my/test03 '((t . (:foreground nil))) "test face"))
    (make-frame-command))
#+end_src

This produces the error “(wrong-type-argument stringp :bla)”.

-- 
Felix Dietrich

Re: how to track down "invalid face attribute" errors?

[Felix Dietrich]

Felix Dietrich <felix.dietrich@sperrhaken.name> writes:

> Felix Dietrich <felix.dietrich@sperrhaken.name> writes:
>
>> Emanuel Berg <incal@dataswamp.org> writes:
>>
>>> Isn't it better to initialize the faces first, and if it is
>>> attempted to define [a face] using invalid attributes, then that
>>> would be an error?
>>
>> This already happens to a degree for named faces: if you use ‘defface’
>> to create a face with :foreground set to nil, subsequent calls to
>> ‘face-attribute’ for :foreground on that face will return ‘unspecified’:
>>
>> #+begin_src emacs-lisp
>>   (progn
>>     (defface my/test '((t . (:foreground nil))) "test face")
>>     (face-attribute 'my/test :foreground))
>> #+end_src
>
> I was not correct with my assumption: when you ‘defface’ a face with an
> invalid :foreground value this will produce an error, but enough of the
> face with the invalid value remains that later creations of new frames
> using ‘make-frame-command’ will fail with that same error:
>
> #+begin_src emacs-lisp
>   (progn
>     (ignore-errors
>       (defface my/test03 '((t . (:foreground nil))) "test face"))
>     (make-frame-command))
> #+end_src
>
> This produces the error “(wrong-type-argument stringp :bla)”.

I copied the wrong snippet from my experiments.  It should have been
this:

#+begin_src emacs-lisp
  (progn
    (ignore-errors
      (defface my/test '((t . (:foreground :bla))) "test face"))
    (make-frame-command))
#+end_src

-- 
Felix Dietrich

Re: how to track down "invalid face attribute" errors?

[Felix Dietrich]

Hello Robert,

Emanuel Berg <incal@dataswamp.org> writes:
>
> Eric S Fraga <e.fraga@ucl.ac.uk> writes:
>
>> some package I'm using is leading to errors of the form
>> 
>>   Invalid face attribute :foreground nil [54 times]
>> 
>> can anybody suggest how I can track down which package/function is
>> causing this?  Debugging on error doesn't catch this.
>
> Sounds like that error message should be changed to include
> what face causes trouble, if possible?


Robert Pluim <rpluim@gmail.com> writes:
>
> Felix Dietrich <felix.dietrich@sperrhaken.name> writes:
>
>> Robert Pluim <rpluim@gmail.com> writes:
>>>
>>> (If you have a reproducer, we can look at changing the message)
>>
>> Here is one:
>>
>> #+begin_src emacs-lisp
>>   (let ((buf (generate-new-buffer "*test*"))
>>         (s (propertize "Hello World" 'face '(:foreground nil))))
>>     (with-current-buffer buf
>>       (insert s))
>>     (display-buffer buf))
>> #+end_src
>
> Thereʼs no actual face there for Emacs to log, what the low-level code
> receives is "(:foreground nil)"

But “xfaces.c:merge_face_ref” (which produces the error message) has a
“struct window w” parameter.  I believe in cases where it is not NULL,
which it isnʼt when running the above code, this could be used to derive
the buffer and the bufferʼs position using the struct members “contents”
and “pointm’: 1. “contents” stores, for leaf windows, the windows
buffer; its descriptions mentions other possible value types it may hold
[1]; I do not know in how far these other types have to be considered
here.  2. “pointm” refers to a marker holding the current buffer
position in window; its description states that it is “used only when
the window is not selected”.  Therefore, it is possible that, if the
window “w” is the selected one, the point would have to be taken from
another source (perhaps the “PT” macro in “buffer.h”).

Also note that, if the position would be added to the log entry,
multiple log entries would not be merged anymore.  Additionally, with
the code above, the position is actually after the text (at
‘point-max’); therefore, this might not be useful information to add.

I am not sure if the whole thing is worth the effort or would complicate
the creation of a simple log entry for a, hopefully, rare case to much.

Anyway, perhaps, if there is any interest in adding information to the
“Invalid face attribute” log message, further discussion should move to
a wishlist bug report?


Robert Pluim <rpluim@gmail.com> writes:
>
> Felix Dietrich <felix.dietrich@sperrhaken.name> writes
>>
>> Upon evaluation, the resulting output in the “*Messages*” buffer also
>> shows another issues with a missing newline between messages (in Emacs
>> 28.1 at least).
>
>> #+begin_example
>> #<window 54 on *test*<2>>Invalid face attribute :foreground nil
>> Invalid face attribute :foreground nil
>> #+end_example
>
> Itʼs still there in emacs-29, but itʼs deep in the guts of the elisp
> engine, so I havenʼt found exactly where it comes from.

If there isnʼt one already, I try to remember to file a minor bug report
sometime in the next couple of days.


>> Maybe a value of nil for the :foreground attribute in an anonymous face
>> should be handled the same way as it is in a face defined with
>> ‘defface’, that is being treated as the symbol ‘unspecified’?
>
>> #+begin_src emacs-lisp
>>   (progn
>>     (defface my/test '((t . (:foreground nil))) "test face")
>>     (face-attribute 'my/test :foreground))
>> #+end_src
>
>> #+RESULTS:
>> : unspecified
>
> There was recently a long thread about this on emacs-devel, which I
> didnʼt read :-) It might be worth suggesting this there.

I donʼt seem to be able to find it.


Footnotes:
[1]  “For a leaf window or a tooltip window this is the buffer shown
      in the window; for a combination window this is the first of
      its child windows; for a pseudo window showing the menu bar or
      tool bar this is nil.  It is a buffer for a minibuffer window
      as well.”
      — Description of the “contents” member of “struct window”
        copied from the file “window.h”

-- 
Felix Dietrich

Re: how to track down "invalid face attribute" errors?

[Robert Pluim]

>>>>> On Wed, 05 Oct 2022 14:32:04 +0200, Felix Dietrich <felix.dietrich@sperrhaken.name> said:

    Felix> But “xfaces.c:merge_face_ref” (which produces the error message) has a
    Felix> “struct window w” parameter.  I believe in cases where it is not NULL,
    Felix> which it isnʼt when running the above code, this could be used to derive
    Felix> the buffer and the bufferʼs position using the struct members “contents”
    Felix> and “pointm’: 1. “contents” stores, for leaf windows, the windows
    Felix> buffer; its descriptions mentions other possible value types it may hold
    Felix> [1]; I do not know in how far these other types have to be considered
    Felix> here.  2. “pointm” refers to a marker holding the current buffer
    Felix> position in window; its description states that it is “used only when
    Felix> the window is not selected”.  Therefore, it is possible that, if the
    Felix> window “w” is the selected one, the point would have to be taken from
    Felix> another source (perhaps the “PT” macro in “buffer.h”).

Weʼre not necessarily examining the faces of the character at (point),
weʼre going through redisplay of the buffer, I thought?

    Felix> Also note that, if the position would be added to the log entry,
    Felix> multiple log entries would not be merged anymore.  Additionally, with
    Felix> the code above, the position is actually after the text (at
    Felix> ‘point-max’); therefore, this might not be useful information to add.

    Felix> I am not sure if the whole thing is worth the effort or would complicate
    Felix> the creation of a simple log entry for a, hopefully, rare case to much.

Itʼs more effort than Iʼm willing to go to, at least :-)

    Felix> Anyway, perhaps, if there is any interest in adding information to the
    Felix> “Invalid face attribute” log message, further discussion should move to
    Felix> a wishlist bug report?

Yes. That will get it in front of people who know the relevant code
better than I do.

Robert
-- 

Re: how to track down "invalid face attribute" errors?

[Eli Zaretskii]

> From: Felix Dietrich <felix.dietrich@sperrhaken.name>
> Cc: help-gnu-emacs@gnu.org
> Date: Wed, 05 Oct 2022 14:32:04 +0200
> 
> But “xfaces.c:merge_face_ref” (which produces the error message) has a
> “struct window w” parameter.  I believe in cases where it is not NULL,
> which it isnʼt when running the above code, this could be used to derive
> the buffer and the bufferʼs position using the struct members “contents”
> and “pointm’:

Face merging has nothing to do with point, it just considers the face
attributes.  The commentary to merge_face_ref says how the window
pointer is used:

  /* Merge face attributes from the lisp `face reference' FACE_REF on
     frame F into the face attribute vector TO as appropriate for
     window W; W is used only for filtering face specs.
	       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

If you want to understand what that filtering is about, look at
filter_face_ref, where you will clearly see that it never accesses the
window's buffer or its point.

So when this function is called, there's no reason to believe that the
buffer position of point has anything to do with the offending face.
The most frequent face merging is performed by redisplay, which
doesn't move point and doesn't make the window on whose display it is
working the selected window.

> Anyway, perhaps, if there is any interest in adding information to the
> “Invalid face attribute” log message, further discussion should move to
> a wishlist bug report?

Definitely.  This discussion doesn't belong to the help list.

Re: how to track down "invalid face attribute" errors?

[Felix Dietrich]

Eli Zaretskii <eliz@gnu.org> writes:

> Face merging has nothing to do with point, it just considers the face
> attributes.  The commentary to merge_face_ref says how the window
> pointer is used:

Robert Pluim <rpluim@gmail.com> writes:

> Weʼre not necessarily examining the faces of the character at (point),
> weʼre going through redisplay of the buffer, I thought?

Uh, I blundered there in my examination and my thinking: when I ran
Emacs in a debugger with a breakpoint on “xfaces.c:merge_face_ref”, the
breakpoint was hit when I moved the cursor into or out of a position
with text that had an invalid property, and I made, from a position of
lacking understanding, a wrong assumption.  Thanks to you two for
clearing that up.

> If you want to understand what that filtering is about, look at
> filter_face_ref, where you will clearly see that it never accesses the
> window's buffer or its point.
>
> So when this function is called, there's no reason to believe that the
> buffer position of point has anything to do with the offending face.
> The most frequent face merging is performed by redisplay, which
> doesn't move point and doesn't make the window on whose display it is
> working the selected window.

Thank you for the clarification.

>> Anyway, perhaps, if there is any interest in adding information to the
>> “Invalid face attribute” log message, further discussion should move to
>> a wishlist bug report?
>
> Definitely.  This discussion doesn't belong to the help list.

The bug report can be found here:

  bug#58312 <https://debbugs.gnu.org/cgi/bugreport.cgi?bug=58312>

-- 
Felix Dietrich