GNU bug report logs -
#19912
facemenu-add-face: does not handle 'face being set to a property list
Previous Next
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.
To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 19912 in the body.
You can then email your comments to 19912 AT debbugs.gnu.org in the normal way.
Toggle the display of automated, internal messages from the tracker.
Report forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#19912; Package
emacs.
(Sat, 21 Feb 2015 12:13:01 GMT)
Full text and
rfc822 format available.
Acknowledgement sent
to
Ivan Shmakov <ivan <at> siamics.net>:
New bug report received and forwarded. Copy sent to
bug-gnu-emacs <at> gnu.org.
(Sat, 21 Feb 2015 12:13:01 GMT)
Full text and
rfc822 format available.
Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (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 2 (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,
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#19912; Package
emacs.
(Wed, 25 Feb 2015 07:06:02 GMT)
Full text and
rfc822 format available.
Message #8 received at 19912 <at> debbugs.gnu.org (full text, mbox):
[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.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#19912; Package
emacs.
(Wed, 25 Feb 2015 17:25:02 GMT)
Full text and
rfc822 format available.
Message #11 received at 19912 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Please consider the (once again) revised patch MIMEd.
* 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.
--
FSF associate member #7257 np. Isle of Avalon — Iron Maiden … 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,17 @@ defun face-nontrivial-p (face &optional frame)
(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 keyword."
+ ;; The logic of merge_face_ref (xfaces.c) is recreated here.
+ (and (listp face-or-list)
+ (not (memq (car face-or-list)
+ '(foreground-color background-color)))
+ (not (keywordp (car face-or-list)))))
+
+
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
;;; Setting face attributes from X resources.
@@ -1922,11 +1933,7 @@ defun face-at-point (&optional thing multiple)
(get-char-property (point) 'face))))
(cond ((facep faceprop)
(push faceprop faces))
- ((and (listp faceprop)
- ;; Don't treat an attribute spec as a list of faces.
- (not (keywordp (car faceprop)))
- (not (memq (car faceprop)
- '(foreground-color background-color))))
+ ((face-list-p faceprop)
(dolist (face faceprop)
(if (facep face)
(push face faces))))))
Reply sent
to
Ivan Shmakov <ivan <at> siamics.net>:
You have taken responsibility.
(Thu, 26 Feb 2015 18:14:02 GMT)
Full text and
rfc822 format available.
Notification sent
to
Ivan Shmakov <ivan <at> siamics.net>:
bug acknowledged by developer.
(Thu, 26 Feb 2015 18:14:02 GMT)
Full text and
rfc822 format available.
Message #16 received at 19912-done <at> debbugs.gnu.org (full text, mbox):
>>>>> 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
bug archived.
Request was from
Debbugs Internal Request <help-debbugs <at> gnu.org>
to
internal_control <at> debbugs.gnu.org.
(Fri, 27 Mar 2015 11:24:04 GMT)
Full text and
rfc822 format available.
This bug report was last modified 10 years and 137 days ago.
Previous Next
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.