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

Previous Next

Full log

View this message in rfc822 format

From: help-debbugs <at> gnu.org (GNU bug Tracking System)
To: Ivan Shmakov <ivan <at> siamics.net>
Subject: bug#19912: closed (Re: bug#19912: facemenu-add-face: does not
 handle 'face being set to a property list )
Date: Thu, 26 Feb 2015 18:14:02 +0000

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

Your bug report

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

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 19912 <at> debbugs.gnu.org.

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

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

From: Ivan Shmakov <ivan <at> siamics.net>
To: 19912-done <at> debbugs.gnu.org
Subject: Re: bug#19912: facemenu-add-face: does not handle 'face being set to
 a property list 
Date: Thu, 26 Feb 2015 18:12:52 +0000

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

 > * lisp/faces.el (face-list-p): Split from face-at-point.
 > (face-at-point): Use it.
 > * lisp/facemenu.el (facemenu-add-face): Likewise.  (Bug#19912)

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

	Pushed, closing.

commit 619fc5c197ebef5444aed24fe30657989fc2a839
CommitDate: 2015-02-26 18:09:48 +0000

    Fix 'face property handling in facemenu-add-face.
    
    * lisp/faces.el (face-list-p): Split from face-at-point.
    (face-at-point): Use it.
    * lisp/facemenu.el (facemenu-add-face): Likewise.
    
    Fixes: debbugs:19912

-- 
FSF associate member #7257  http://boycottsystemd.org/  … 3013 B6A0 230E 334A

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

From: Ivan Shmakov <ivan <at> siamics.net>
To: submit <at> debbugs.gnu.org
Subject: facemenu-add-face: does not handle 'face being set to a property list
Date: Sat, 21 Feb 2015 12:12:28 +0000

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

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

[Message part 5 (text/diff, inline)]

--- 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,

Previous Next

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