Subject: Re: bug#16694: bugs #16694/#16378: Patches

From: Matthias Dahl <[email-masked]>
Date: Sun, 27 Apr 2014 10:22:27 +0200

Hello @all...

> Would it help to turn the X resource settings into a theme which can
> then be maybe more easily stacked at the right place?

Stefan, thanks for the idea. With the way things are currently wired,
this would most certainly mess the logic up. :(

Eli, could you please test the attached patches? Everything is rather
self-explanatory and the fix as simplistic as possible. I tested it for
all known possible regressions, but everything works just fine here. It
looks like it is the right thing to do (tm). Unfortunately, I was not
able to test it on win32 (yeah, I know, sorry) for obvious reasons :)
but based on what you said, it should fix the toolbar coloring just fine.

If there are no regressions reported from other parties and if it fixes
the issues on win32, from my side just go ahead and apply it to master
and emacs-24.

Hoping very much for positive feedback. ;-)

So long,
Matthias

-- 
Dipl.-Inf. (FH) Matthias Dahl | Software Engineer | binary-island.eu
 services: custom software [desktop, mobile, web], server administration
>From 829b21a2e9cbcdab8d59f9b7f3b87c112088e729 Mon Sep 17 00:00:00 2001
From: Matthias Dahl <[email-masked]>
Date: Sun, 27 Apr 2014 10:03:40 +0200
Subject: [PATCH 4/4] [PATCH] lisp/faces.el: Apply X resources after defface
 spec

Apply X resources only after the the defface spec has been applied. Thus,
X resources are no longer overriden by the defface spec which also fixes
issues on win32 where the toolbar coloring was wrong because it is set
through X resources and was (wrongfully) overriden.
---
 lisp/ChangeLog |  8 ++++++++
 lisp/faces.el  | 18 ++++++++++++------
 2 files changed, 20 insertions(+), 6 deletions(-)

diff --git a/lisp/ChangeLog b/lisp/ChangeLog
index bcb649f..cfe61ba 100644
--- a/lisp/ChangeLog
+++ b/lisp/ChangeLog
@@ -1,3 +1,11 @@
+2014-04-27  Matthias Dahl  <[email-masked]>
+
+	* faces.el (face-spec-recalc): Apply X resources only after the
+	the defface spec has been applied. Thus, X resources are no longer
+	overriden by the defface spec which also fixes issues on win32 where
+	the toolbar coloring was wrong because it is set through X resources
+	and was (wrongfully) overriden.
+
 2014-04-25  Nicolas Richard  <[email-masked]>
 
 	* battery.el (battery-update): Handle the case where battery
diff --git a/lisp/faces.el b/lisp/faces.el
index 88b8748..df31e0d 100644
--- a/lisp/faces.el
+++ b/lisp/faces.el
@@ -1641,18 +1641,22 @@ function for its other effects."
 
 (defun face-spec-recalc (face frame)
   "Reset the face attributes of FACE on FRAME according to its specs.
-After the reset, the specs are applied from the following sources in this order:
-  X resources (if applicable)
+The following sources are applied in this order:
+
+  face reset to default values if it's the default face, otherwise set
+  to unspecifed (through `face-spec-reset-face`)
    |
   (theme and user customization)
-    or, if nonexistent or does not match the current frame,
+    or: if none of the above exist, do not match the current frame or
+        did inherit from the defface spec instead of overwriting it
+        entirely, the following is applied instead:
   (defface default spec)
+  (X resources (if applicable))
    |
   defface override spec"
   (while (get face 'face-alias)
     (setq face (get face 'face-alias)))
   (face-spec-reset-face face frame)
-  (make-face-x-resource-internal face frame)
   ;; If FACE is customized or themed, set the custom spec from
   ;; `theme-face' records.
   (let ((theme-faces (get face 'theme-face))
@@ -1666,10 +1670,12 @@ After the reset, the specs are applied from the following sources in this order:
 	    (setq theme-face-applied t))))
     ;; If there was a spec applicable to FRAME, that overrides the
     ;; defface spec entirely (rather than inheriting from it).  If
-    ;; there was no spec applicable to FRAME, apply the defface spec.
+    ;; there was no spec applicable to FRAME, apply the defface spec
+    ;; as well as any applicable X resources.
     (unless theme-face-applied
       (setq spec (face-spec-choose (face-default-spec face) frame))
-      (face-spec-set-2 face frame spec))
+      (face-spec-set-2 face frame spec)
+      (make-face-x-resource-internal face frame))
     (setq spec (face-spec-choose (get face 'face-override-spec) frame))
     (face-spec-set-2 face frame spec)))
 
-- 
1.9.2

>From 093207c4c79012615377695eb8eba31283aded5d Mon Sep 17 00:00:00 2001
From: Matthias Dahl <[email-masked]>
Date: Sun, 27 Apr 2014 09:55:51 +0200
Subject: [PATCH] lisp/faces.el: Apply X resources after defface spec

Apply X resources only after the the defface spec has been applied. Thus,
X resources are no longer overriden by the defface spec which also fixes
issues on win32 where the toolbar coloring was wrong because it is set
through X resources and was (wrongfully) overriden.
---
 lisp/ChangeLog |  8 ++++++++
 lisp/faces.el  | 18 ++++++++++++------
 2 files changed, 20 insertions(+), 6 deletions(-)

diff --git a/lisp/ChangeLog b/lisp/ChangeLog
index c81f99e..c982540 100644
--- a/lisp/ChangeLog
+++ b/lisp/ChangeLog
@@ -1,3 +1,11 @@
+2014-04-27  Matthias Dahl  <[email-masked]>
+
+	* faces.el (face-spec-recalc): Apply X resources only after the
+	the defface spec has been applied. Thus, X resources are no longer
+	overriden by the defface spec which also fixes issues on win32 where
+	the toolbar coloring was wrong because it is set through X resources
+	and was (wrongfully) overriden.
+
 2014-04-25  Stefan Monnier  <[email-masked]>
 
 	* progmodes/perl-mode.el (perl--syntax-exp-intro-regexp): New var.
diff --git a/lisp/faces.el b/lisp/faces.el
index 9c11547..b58575d 100644
--- a/lisp/faces.el
+++ b/lisp/faces.el
@@ -1652,18 +1652,22 @@ function for its other effects."
 
 (defun face-spec-recalc (face frame)
   "Reset the face attributes of FACE on FRAME according to its specs.
-After the reset, the specs are applied from the following sources in this order:
-  X resources (if applicable)
+The following sources are applied in this order:
+
+  face reset to default values if it's the default face, otherwise set
+  to unspecifed (through `face-spec-reset-face`)
    |
   (theme and user customization)
-    or, if nonexistent or does not match the current frame,
+    or: if none of the above exist, do not match the current frame or
+        did inherit from the defface spec instead of overwriting it
+        entirely, the following is applied instead:
   (defface default spec)
+  (X resources (if applicable))
    |
   defface override spec"
   (while (get face 'face-alias)
     (setq face (get face 'face-alias)))
   (face-spec-reset-face face frame)
-  (make-face-x-resource-internal face frame)
   ;; If FACE is customized or themed, set the custom spec from
   ;; `theme-face' records.
   (let ((theme-faces (get face 'theme-face))
@@ -1677,10 +1681,12 @@ After the reset, the specs are applied from the following sources in this order:
 	    (setq theme-face-applied t))))
     ;; If there was a spec applicable to FRAME, that overrides the
     ;; defface spec entirely (rather than inheriting from it).  If
-    ;; there was no spec applicable to FRAME, apply the defface spec.
+    ;; there was no spec applicable to FRAME, apply the defface spec
+    ;; as well as any applicable X resources.
     (unless theme-face-applied
       (setq spec (face-spec-choose (face-default-spec face) frame))
-      (face-spec-set-2 face frame spec))
+      (face-spec-set-2 face frame spec)
+      (make-face-x-resource-internal face frame))
     (setq spec (face-spec-choose (get face 'face-override-spec) frame))
     (face-spec-set-2 face frame spec)))
 
-- 
1.9.2