unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#58312: Improve “Invalid face attribute” error message
@ 2022-10-05 18:42 Felix Dietrich
  2022-10-06 12:44 ` Lars Ingebrigtsen
  0 siblings, 1 reply; 4+ messages in thread
From: Felix Dietrich @ 2022-10-05 18:42 UTC (permalink / raw)
  To: 58312

Package: emacs
Severity: wishlist

Could the log message “Invalid face attribute” created by
“xfaces.c:merge_face_ref” be extended to also include the buffer or
other identifying information about its origin?  This request is
motivated by a recent issue posted to the help-gnu-emacs list [1].

When Emacs encounters an anonymous face with an invalid face attribute
value in a buffer, it will write a log entry to the “*Messages*” buffer.
This message takes the form “Invalid face attribute ATTRIBUTE VALUE” and
lacks any information that would allow easy identification of the
invalid faceʼs place of origin.  The log entry is induced by the
function “xfaces.c:merge_face_ref”.  This function takes as its first
argument a “struct window” which could potentially be used to retrieve
the causing buffer using its “contents” member (note, though, that it
may be NULL).

Here is a snippet that will cause the “Invalid face attribute” message
to be written to the “*Messages*” buffer when evaluated:

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

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

Two things I noticed exploring the issue:

1. the value I use in the above example for the :foreground attribute
nil is handled specially in “xfaces.c:internal-set-lisp-face-attribute”
(accompanied by a comment stating “Compatibility with 20.x.”): it is
changed to ‘unspecified’, but in “xfaces.c:merge_face_ref” nil is
considered an error;

2. there is a newline missing between the log entries for the result of
the evaluation and the error entries in the “*Messages*” buffer.  This
is bug #58311 <https://debbugs.gnu.org/cgi/bugreport.cgi?bug=58311>.


Footnotes:
[1]  From: Eric S Fraga
     Subject: how to track down "invalid face attribute" errors?
     Date: Sun, 25 Sep 2022 13:03:03 +0100
     Message-ID: <87k05rr79k.fsf@ucl.ac.uk>

     <https://lists.gnu.org/archive/html/help-gnu-emacs/2022-09/msg00415.html>

-- 
Felix Dietrich





^ permalink raw reply	[flat|nested] 4+ messages in thread

* bug#58312: Improve “Invalid face attribute” error message
  2022-10-05 18:42 bug#58312: Improve “Invalid face attribute” error message Felix Dietrich
@ 2022-10-06 12:44 ` Lars Ingebrigtsen
  2024-01-10 10:50   ` Stefan Kangas
  0 siblings, 1 reply; 4+ messages in thread
From: Lars Ingebrigtsen @ 2022-10-06 12:44 UTC (permalink / raw)
  To: Felix Dietrich; +Cc: 58312

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

> Could the log message “Invalid face attribute” created by
> “xfaces.c:merge_face_ref” be extended to also include the buffer or
> other identifying information about its origin?

Perhaps include the window name?  That would be less ambiguous than the
buffer name.






^ permalink raw reply	[flat|nested] 4+ messages in thread

* bug#58312: Improve “Invalid face attribute” error message
  2022-10-06 12:44 ` Lars Ingebrigtsen
@ 2024-01-10 10:50   ` Stefan Kangas
  2024-01-10 13:38     ` Eli Zaretskii
  0 siblings, 1 reply; 4+ messages in thread
From: Stefan Kangas @ 2024-01-10 10:50 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: Felix Dietrich, Po Lu, eliz, 58312

Lars Ingebrigtsen <larsi@gnus.org> writes:

> Felix Dietrich <felix.dietrich@sperrhaken.name> writes:
>
>> Could the log message “Invalid face attribute” created by
>> “xfaces.c:merge_face_ref” be extended to also include the buffer or
>> other identifying information about its origin?
>
> Perhaps include the window name?  That would be less ambiguous than the
> buffer name.

That makes sense to me.  Eli or Po Lu, WDYT about this change?





^ permalink raw reply	[flat|nested] 4+ messages in thread

* bug#58312: Improve “Invalid face attribute” error message
  2024-01-10 10:50   ` Stefan Kangas
@ 2024-01-10 13:38     ` Eli Zaretskii
  0 siblings, 0 replies; 4+ messages in thread
From: Eli Zaretskii @ 2024-01-10 13:38 UTC (permalink / raw)
  To: Stefan Kangas; +Cc: felix.dietrich, luangruo, larsi, 58312

> From: Stefan Kangas <stefankangas@gmail.com>
> Date: Wed, 10 Jan 2024 02:50:46 -0800
> Cc: Felix Dietrich <felix.dietrich@sperrhaken.name>, 58312@debbugs.gnu.org, eliz@gnu.org, 
> 	Po Lu <luangruo@yahoo.com>
> 
> Lars Ingebrigtsen <larsi@gnus.org> writes:
> 
> > Felix Dietrich <felix.dietrich@sperrhaken.name> writes:
> >
> >> Could the log message “Invalid face attribute” created by
> >> “xfaces.c:merge_face_ref” be extended to also include the buffer or
> >> other identifying information about its origin?
> >
> > Perhaps include the window name?  That would be less ambiguous than the
> > buffer name.
> 
> That makes sense to me.  Eli or Po Lu, WDYT about this change?

Patches welcome, but please take into account the fact that the window
is not always known there: merge_face_ref can be called with its first
argument NULL.





^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2024-01-10 13:38 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-10-05 18:42 bug#58312: Improve “Invalid face attribute” error message Felix Dietrich
2022-10-06 12:44 ` Lars Ingebrigtsen
2024-01-10 10:50   ` Stefan Kangas
2024-01-10 13:38     ` Eli Zaretskii

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).