Subject: facemenu-add-face: does not handle 'face being set to a property list

From: Ivan Shmakov <[email-masked]>
Date: Sat, 21 Feb 2015 12:12:28 +0000

Package:  emacs
Severity: minor
Tags: patch

	As currently implemented, facemenu-add-face doesn’t handle the
	case of the 'face property value being a property list, like:

(with-temp-buffer
  (insert "Hello, world!")
  (put-text-property 3 11 'face '(:weight bold))
  (facemenu-add-face 'italic 5 7)
  (buffer-string))

	The relevant part of the backtrace is like:

  check-face(:weight)
  facemenu-active-faces((italic :weight bold) #<frame F1 0xb497d0>)
  facemenu-add-face(italic 5 7)

	With the patch MIMEd, the example produces the expected result:

#("Hello, world!"
  2 4 (face (:weight bold))
  4 6 (face (italic (:weight bold)))
  6 10 (face (:weight bold)))

	* 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’.

-- 
FSF associate member #7257  http://boycottsystemd.org/  … 3013 B6A0 230E 334A
--- a/lisp/facemenu.el
+++ b/lisp/facemenu.el
@@ -732,9 +732,17 @@ defun facemenu-add-face (face &optional start end)
                                  face
                                (facemenu-active-faces
                                 (cons face
-                                      (if (listp prev)
-                                          prev
-                                        (list prev)))
+                                      (if (or (atom prev)
+					      (not (symbolp (car prev)))
+                                              (memq (car prev)
+						    '(foreground-color
+						      background-color))
+                                              (let ((n (symbol-name
+							(car prev))))
+						(and (> (length n)  0)
+						     (eq ?: (aref n 0)))))
+					  (list prev)
+					prev))
                                 ;; Specify the selected frame
                                 ;; because nil would mean to use
                                 ;; the new-frame default settings,