list-faces-display

list-faces-display

[Juanma Barranquero]

The new regexp argument for list-faces-display is not documented on
NEWS. Should it? And if yes, in what section? "Changes in Specialized
Modes and Packages in Emacs 22.1" and "Lisp Changes in Emacs 22.1"
both seem in order.

Somewhat related: on 1999-07-21 Gerd committed lots and lots of
changes (possibly he was synchronizing from a branch). faces.el got an
almost complete rewrite, and in particular list-faces-display lost the
following ability:

1999-01-11  Richard Stallman  <rms@psilocin.ai.mit.edu>

        * faces.el (list-faces-display): Improve the formatting
        by computing the maximum length required for any face-name.

(I'm attaching a patch that shows the changes done to
list-faces-display; line numbers are wrong because I've cut out just
the code for the function)

Question is, should this change be reverted? And, it is that a bugfix
or a new feature?

                    /L/e/k/t/u


--- faces-1.138	Mon May 30 13:35:15 2005
+++ faces-1.139	Mon May 30 13:35:29 2005
@@ -1,2 +1 @@
-\f
  (defvar list-faces-sample-text
@@ -5,4 +4,6 @@
 
-;; The name list-faces would be more consistent, but let's avoid a conflict
-;; with Lucid, which uses that name differently.
+
+;; The name list-faces would be more consistent, but let's avoid a
+;; conflict with Lucid, which uses that name differently.
+
  (defun list-faces-display ()
@@ -10,16 +11,8 @@
 The sample text is a string that comes from the variable
-`list-faces-sample-text'.
-
-It is possible to give a particular face name different appearances in
-different frames.  This command shows the appearance in the
-selected frame."
+`list-faces-sample-text'."
   (interactive)
-  (let ((faces (sort (face-list) (function string-lessp)))
+  (let ((faces (sort (face-list) #'string-lessp))
 	(face nil)
 	(frame (selected-frame))
-	disp-frame window
-        (face-name-max-length
-         (car (sort (mapcar (function string-width)
-			    (mapcar (function symbol-name) (face-list)))
-                    (function >)))))
+	disp-frame window)
     (with-output-to-temp-buffer "*Faces*"
@@ -31,6 +24,3 @@
  	  (setq faces (cdr faces))
-	  (insert (format 
-                   (format "%%-%ds "
-                           face-name-max-length)
-                   (symbol-name face)))
+	  (insert (format "%25s " (face-name face)))
 	  (let ((beg (point)))
@@ -43,3 +33,3 @@
  	    (while (not (eobp))
-	      (insert-char ?  (1+ face-name-max-length))
+	      (insert "                          ")
 	      (forward-line 1))))

Re: list-faces-display

[Juanma Barranquero]

Anyway, this is the patch I propose, if deemed a fix and not a feature.

-- 
                    /L/e/k/t/u


Index: faces.el
===================================================================
RCS file: /cvsroot/emacs/emacs/lisp/faces.el,v
retrieving revision 1.310
diff -u -2 -r1.310 faces.el
--- faces.el	31 May 2005 11:10:43 -0000	1.310
+++ faces.el	31 May 2005 11:07:42 -0000
@@ -1155,16 +1155,21 @@
   (interactive (list (and current-prefix-arg
                           (read-string "List faces matching regexp: "))))
-  (let ((faces (sort (face-list) #'string-lessp))
+  (let ((all-faces (zerop (length regexp)))
 	(frame (selected-frame))
+	(max-length 0)
+	face line-format
  	disp-frame window face-name)
-    (when (> (length regexp) 0)
-      (setq faces
-            (delq nil
-                  (mapcar (lambda (f)
-                            (when (string-match regexp (symbol-name f))
-                              f))
-                          faces)))
-      (unless faces
-        (error "No faces matching \"%s\"" regexp)))
+    (setq faces
+	  (delq nil
+		(mapcar (lambda (f)
+			  (let ((s (symbol-name f)))
+			    (when (or all-faces (string-match regexp s))
+			      (setq max-length (max (length s) max-length))
+			      f)))
+			(sort (face-list) #'string-lessp))))
+    (unless faces
+      (error "No faces matching \"%s\"" regexp))
+    (setq max-length (1+ max-length)
+	  line-format (format "%%-%ds" max-length))
     (with-output-to-temp-buffer "*Faces*"
       (save-excursion
@@ -1181,5 +1186,5 @@
  	(dolist (face faces)
  	  (setq face-name (symbol-name face))
-	  (insert (format "%25s " face-name))
+	  (insert (format line-format face-name))
 	  ;; Hyperlink to a customization buffer for the face.  Using
 	  ;; the help xref mechanism may not be the best way.
@@ -1206,5 +1211,5 @@
 	    (forward-line 1)
  	    (while (not (eobp))
-	      (insert "                          ")
+	      (insert-char ?\s max-length)
 	      (forward-line 1))))
  	(goto-char (point-min)))

Re: list-faces-display

[Juanma Barranquero]

> +       face line-format

That would be

> +       faces line-format

of course.

                    /L/e/k/t/u

Re: list-faces-display

[Richard Stallman]

Please do reinstall the 1999 code that got lost.

    The new regexp argument for list-faces-display is not documented on
    NEWS. Should it? And if yes, in what section? "Changes in Specialized
    Modes and Packages in Emacs 22.1" and "Lisp Changes in Emacs 22.1"
    both seem in order.

The latter is better--this isn't big enough to be called a
"specialized mode or package".

Re: list-faces-display

[Juanma Barranquero]

> Please do reinstall the 1999 code that got lost.

OK (I'll install the patch I sent, which is a reworking of it).

> The latter is better--this isn't big enough to be called a
> "specialized mode or package".

No, but it is a "change in a specialized mode or package". The problem
with describing the new feature only on Lisp Changes (though it also
is that) is that it misses the fact that the user-level command can
now be called with C-u to force it to ask for a regexp.

-- 
                    /L/e/k/t/u

Re: list-faces-display

[Richard Stallman]

    > The latter is better--this isn't big enough to be called a
    > "specialized mode or package".

    No, but it is a "change in a specialized mode or package".

list-faces-display is not big enough to be called a "specialized mode
or package".

     The problem
    with describing the new feature only on Lisp Changes (though it also
    is that) is that it misses the fact that the user-level command can
    now be called with C-u to force it to ask for a regexp.

Then put it in the user-level command changes page.