GNU bug report logs - #61521
"default" is now the first item returned from (font-faces), breaking various code.

Previous Next

Full log

View this message in rfc822 format

From: help-debbugs <at> gnu.org (GNU bug Tracking System)
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: tracker <at> debbugs.gnu.org
Subject: bug#61521: closed ("default" is now the first item returned from
 (font-faces), breaking various code.)
Date: Fri, 17 Feb 2023 08:30:02 +0000

[Message part 1 (text/plain, inline)]

Your message dated Fri, 17 Feb 2023 10:29:39 +0200
with message-id <83pma81yq4.fsf <at> gnu.org>
and subject line Re: bug#61521: "default" is now the first item returned from (font-faces), breaking various code.
has caused the debbugs.gnu.org bug report #61521,
regarding "default" is now the first item returned from (font-faces), breaking various code.
to be marked as done.

(If you believe you have received this mail in error, please contact
help-debbugs <at> gnu.org.)


-- 
61521: https://debbugs.gnu.org/cgi/bugreport.cgi?bug=61521
GNU Bug Tracking System
Contact help-debbugs <at> gnu.org with problems

[Message part 2 (message/rfc822, inline)]

From: Brennan Vincent <brennan <at> umanwizard.com>
To: bug-gnu-emacs <at> gnu.org
Subject: "default" is now the first item returned from (font-faces), breaking
 various code.
Date: Tue, 14 Feb 2023 19:31:30 -0500

Various code seems to expect "default" to be the /last/ item in the list
returned by that function, not the first. For example, this comment in faces.el:

  ;; The `reverse' is so that `default' goes first.
  (dolist (face (nreverse (face-list)))


Also, org-html-htmlize-generate-css does not work when default comes first in
the list (as it skips processing all fonts after default).

I am not sure why this was changed and if the change was intentional, but it can
be fixed by changing the "<" to a ">" in the last line of face-list, so I
suspect it might have been a mistake.

diff --git lisp/faces.el lisp/faces.el
index 4933b495a6c..e998dc504e5 100644
--- lisp/faces.el
+++ lisp/faces.el
@@ -199,7 +199,7 @@ face-list
     (maphash (lambda (face spec)
                (push `(,(car spec) . ,face) faces))
              face--new-frame-defaults)
-    (mapcar #'cdr (sort faces (lambda (f1 f2) (< (car f1) (car f2)))))))
+    (mapcar #'cdr (sort faces (lambda (f1 f2) (> (car f1) (car f2)))))))

 (defun make-face (face)
   "Define a new face with name FACE, a symbol.

[Message part 3 (message/rfc822, inline)]

From: Eli Zaretskii <eliz <at> gnu.org>
To: Brennan Vincent <brennan <at> umanwizard.com>
Cc: 61521-done <at> debbugs.gnu.org
Subject: Re: bug#61521: "default" is now the first item returned from
 (font-faces), breaking various code.
Date: Fri, 17 Feb 2023 10:29:39 +0200

> Date: Wed, 15 Feb 2023 11:19:54 -0500
> Cc: 61521 <at> debbugs.gnu.org
> From: Brennan Vincent <brennan <at> umanwizard.com>
> 
> > Good question.  AFAICT, the sorting was added when we switched from
> > storing faces in alists to storing them in hash tables.  It probably
> > sorted faces to be more compatible with what face-list returned before
> > the switch to hash table.  So I suspect the order we have now is
> > simply a bug, and we do need to change the order of sorting to get
> > back the original order.
> 
> I tend to agree. Sorry for not explaining this reasoning more fully in my
> original message.
> 
> Here's what I suspect happened (not 100% sure, it's just a theory):
> 
> (1) Initially set of faces was stored as a list, so it was naturally maintained
> in the inverse order that things were added to it (thus default would be at the
> end).
> 
> (2) Now faces are stored in a hash table whose key is the face and whose value
> contains various pieces of data, including the face ID.
> 
> (3) This face ID is allocated in increasing order (see e.g. this code in xfaces.c:
>       Lisp_Object face_id = make_fixnum (next_lface_id);
>       lface_id_to_name[next_lface_id] = face;
>       Fput (face, Qface, face_id);
>       ++next_lface_id;
> 
> (4) Thus, `face-list` and `frame-face-alist` sorted the faces by face ID in
> order to maintain the old ordering behavior. However, the author accidentally
> inverted the comparison when doing so.
> 
> 
> > Gregory, any counter-arguments?

No further comments, so I've now installed the proposed changes on the
emacs-29 branch, and I'm boldly closing this bug as done.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.