GNU bug report logs - #19912
facemenu-add-face: does not handle 'face being set to a property list

Previous Next

Package: emacs;

Reported by: Ivan Shmakov <ivan <at> siamics.net>

Date: Sat, 21 Feb 2015 12:13:01 UTC

Severity: minor

Tags: patch

Done: Ivan Shmakov <ivan <at> siamics.net>

Bug is archived. No further changes may be made.

Full log


View this message in rfc822 format

From: Ivan Shmakov <ivan <at> siamics.net>
To: 19912 <at> debbugs.gnu.org
Subject: bug#19912: facemenu-add-face: does not handle 'face being set to a property list 
Date: Wed, 25 Feb 2015 07:05:20 +0000
[Message part 1 (text/plain, inline)]
>>>>> Ivan Shmakov <ivan <at> siamics.net> writes:

[…]

 > * lisp/facemenu.el (facemenu-add-face): Follow the (stricter) logic
 > of merge_face_ref when determining whether the value of the 'face
 > property is a sole face or a list thereof.  (Bug#???)

 > (Tested on c4e2be4587ec, 2015-02-16 07:22:46 UTC.)

 > Alternatively, a suitable, Lisp-callable predicate may be
 > split off ‘merge_face_ref’ (src/faces.c) and be used ine
 > ‘facemenu-add-face’.

	… Or should such a predicate be introduced to faces.el instead?
	The merge_face_ref () logic is a bit arcane, so I’d rather keep
	it confined to the inner parts of Emacs (whether faces.c,
	faces.el, or similar.)

	Please consider the revised patch MIMEd.

	* lisp/faces.el (face-list-p): New function.
	* lisp/facemenu.el (facemenu-add-face): Use it.  (Bug#19912)

	Unless there be objections, I hope to push this new change
	to ‘master’ within the next day or two.

-- 
FSF associate member #7257  http://boycottsystemd.org/  … 3013 B6A0 230E 334A
[Message part 2 (text/diff, inline)]
--- a/lisp/facemenu.el
+++ b/lisp/facemenu.el
@@ -732,7 +732,7 @@ defun facemenu-add-face (face &optional start end)
                                  face
                                (facemenu-active-faces
                                 (cons face
-                                      (if (listp prev)
+                                      (if (face-list-p prev)
                                           prev
                                         (list prev)))
                                 ;; Specify the selected frame
--- a/lisp/faces.el
+++ b/lisp/faces.el
@@ -273,6 +273,22 @@
   (not (internal-lisp-face-empty-p face frame)))
 
 
+(defun face-list-p (face-or-list)
+  "True if FACE-OR-LIST is a list of faces.
+Return nil if FACE-OR-LIST is a non-nil atom, or a cons cell whose car
+is either 'foreground-color, 'background-color, or a symbol starting
+with a colon (:)."
+  ;; The logic of merge_face_ref (xfaces.c) is recreated here.
+  (or (null face-or-list)
+      (and (consp face-or-list)
+	   (symbolp (car face-or-list))
+	   (not (memq (car face-or-list)
+		      '(foreground-color background-color)))
+	   (let ((n (symbol-name (car face-or-list))))
+	     (or (zerop (length n))
+		 (not (eq ?: (aref n 0))))))))
+
+
 
 ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
 ;;; Setting face attributes from X resources.

This bug report was last modified 10 years and 138 days ago.

Previous Next


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