Subject: bug#61521: closed (Re: bug#61521: "default" is now the first item returned from (font-faces), breaking various code.)

From: [email-masked] (GNU bug Tracking System)
Date: Fri, 17 Feb 2023 08:30:03 +0000

Your bug report

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

which was filed against the emacs package, has been closed.

The explanation is attached below, along with your original report.
If you require more details, please reply to [email-masked].

-- 
61521: https://debbugs.gnu.org/cgi/bugreport.cgi?bug=61521
GNU Bug Tracking System
Contact [email-masked] with problems

Subject: Re: bug#61521: "default" is now the first item returned from (font-faces), breaking various code.
From: Eli Zaretskii <[email-masked]>
To: Brennan Vincent <[email-masked]>
Cc: [email-masked]
Date: Fri, 17 Feb 2023 10:29:39 +0200
List-Id: <debbugs-submit.debbugs.gnu.org>

> Date: Wed, 15 Feb 2023 11:19:54 -0500
> Cc: [email-masked]
> From: Brennan Vincent <[email-masked]>
> 
> > 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.

Subject: "default" is now the first item returned from (font-faces), breaking various code.
From: Brennan Vincent <[email-masked]>
To: [email-masked]
Date: Tue, 14 Feb 2023 19:31:30 -0500
List-Id: <debbugs-submit.debbugs.gnu.org>

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.