GNU bug report logs - #16694
Regression by commit 115663 (bzr): Theme face attributes no longer take precedence over X resources

Previous Next

Full log

View this message in rfc822 format

From: Barry OReilly <gundaetiapo <at> gmail.com>
To: Matthias Dahl <ml_emacs-lists <at> binary-island.eu>
Cc: Eli Zaretskii <eliz <at> gnu.org>, Clemens Schüller <cs.mlists+bug-gnu-emacs <at> mailbox.org>, 16694 <at> debbugs.gnu.org
Subject: bug#16694: bugs #16694/#16378: Patches
Date: Wed, 26 Mar 2014 19:49:29 -0400

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

In face-set-after-frame-default, a function untouched in Chong's
changes, there is:

  (let ((window-system-p (memq (window-system frame) '(x w32))))
[...]
        ;; Initialize faces from face spec and custom theme.
        (face-spec-recalc face frame)
        ;; X resources for the default face are applied during
        ;; `x-create-frame'.
        (and (not (eq face 'default)) window-system-p
         (make-face-x-resource-internal face frame))

But the

  (and (not (eq face 'default)) window-system-p ...)

is completely pointless, since Chong's changes have face-spec-recalc
call make-face-x-resource-internal unconditionally. On a hunch, I
tried:

diff --git a/lisp/faces.el b/lisp/faces.el
index e008993..1150d8f 100644
--- a/lisp/faces.el
+++ b/lisp/faces.el
@@ -1642,7 +1642,9 @@ then the override spec."
       (face-spec-set-2 face frame spec))
     (setq spec (face-spec-choose (get face 'face-override-spec) frame))
     (face-spec-set-2 face frame spec))
-  (make-face-x-resource-internal face frame))
+  (and (not (eq face 'default))
+       (memq (window-system frame) '(x w32))
+       (make-face-x-resource-internal face frame)))

 (defun face-spec-set-2 (face frame spec)
   "Set the face attributes of FACE on FRAME according to SPEC."

with no other patches, and './src/emacs -r' worked as expected.

Should face-spec-recalc take responsibility for calling
make-face-x-resource-internal, or leave it to the caller as before the
offending changes? A patch along those lines also allows './src/emacs
-r' to work:

diff --git a/lisp/faces.el b/lisp/faces.el
index e008993..2f8560a 100644
--- a/lisp/faces.el
+++ b/lisp/faces.el
@@ -1615,7 +1615,8 @@ function for its other effects."
   ;; Initialize the face if it does not exist, then recalculate.
   (make-empty-face face)
   (dolist (frame (frame-list))
-    (face-spec-recalc face frame)))
+    (face-spec-recalc face frame)
+    (make-face-x-resource-internal face frame)))

 (defun face-spec-recalc (face frame)
   "Reset the face attributes of FACE on FRAME according to its specs.
@@ -1641,8 +1642,7 @@ then the override spec."
       (setq spec (face-spec-choose (face-default-spec face) frame))
       (face-spec-set-2 face frame spec))
     (setq spec (face-spec-choose (get face 'face-override-spec) frame))
-    (face-spec-set-2 face frame spec))
-  (make-face-x-resource-internal face frame))
+    (face-spec-set-2 face frame spec)))

 (defun face-spec-set-2 (face frame spec)
   "Set the face attributes of FACE on FRAME according to SPEC."

Someone who knows this code better than me should decide what's The
Right Thing.

[Message part 2 (text/html, inline)]

Previous Next

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