all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
From: Matthias Dahl <matthias.dahl@binary-island.eu>
To: Stefan Monnier <monnier@IRO.UMontreal.CA>
Cc: gundaetiapo@gmail.com, 16434@debbugs.gnu.org
Subject: bug#16434: bug#16694: bugs #16694/#16378: Patches
Date: Sun, 27 Apr 2014 10:22:27 +0200	[thread overview]
Message-ID: <535CBE43.7000803@binary-island.eu> (raw)
In-Reply-To: <jwvha5jtvhr.fsf-monnier+emacsbugs@gnu.org>

[-- Attachment #1: Type: text/plain, Size: 1059 bytes --]

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

[-- Attachment #2: emacs24-0001-PATCH-lisp-faces.el-Apply-X-resources-after-defface-.patch --]
[-- Type: text/x-patch, Size: 3355 bytes --]

From 829b21a2e9cbcdab8d59f9b7f3b87c112088e729 Mon Sep 17 00:00:00 2001
From: Matthias Dahl <matthias.dahl@binary-island.eu>
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  <matthias.dahl@binary-island.eu>
+
+	* 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  <theonewiththeevillook@yahoo.fr>
 
 	* 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


[-- Attachment #3: master-0001-lisp-faces.el-Apply-X-resources-after-defface-spec.patch --]
[-- Type: text/x-patch, Size: 3341 bytes --]

From 093207c4c79012615377695eb8eba31283aded5d Mon Sep 17 00:00:00 2001
From: Matthias Dahl <matthias.dahl@binary-island.eu>
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  <matthias.dahl@binary-island.eu>
+
+	* 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  <monnier@iro.umontreal.ca>
 
 	* 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


  reply	other threads:[~2014-04-27  8:22 UTC|newest]

Thread overview: 60+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-02-08 10:06 bug#16694: Regression by commit 115663 (bzr): Theme face attributes no longer take precedence over X resources Matthias Dahl
2014-02-12 22:21 ` Glenn Morris
2014-02-14 19:17   ` Matthias Dahl
2014-02-20 18:27     ` Eli Zaretskii
2014-02-20 21:47       ` Stefan Monnier
2014-02-21  9:07         ` Eli Zaretskii
2014-02-21 17:36           ` Stefan Monnier
2014-02-23 16:46           ` Matthias Dahl
2014-02-23 17:18             ` Eli Zaretskii
2014-02-24 19:29               ` Matthias Dahl
2014-02-26 15:49                 ` bug#16378: Empty face settings ignored (was bug#16694: Regression by commit 115663 (bzr): Theme face attributes no longer take precedence over X resources) Stefan Monnier
2014-02-27 19:05                   ` Matthias Dahl
2014-03-02 14:26                   ` Matthias Dahl
2014-03-02 16:56                     ` Eli Zaretskii
2014-03-05 16:14 ` bug#16694: bugs #16694/#16378: Patches Matthias Dahl
2014-03-16 19:13   ` bug#16378: " Matthias Dahl
2014-03-17 14:33     ` Eli Zaretskii
2014-03-21 18:05 ` bug#16694: " Barry OReilly
2014-03-22  8:30   ` Eli Zaretskii
2014-03-23 17:04     ` Matthias Dahl
2014-03-24 23:42 ` Barry OReilly
2014-03-24 23:49   ` Clemens Schüller
2014-03-25 14:17     ` Barry OReilly
2014-03-25 15:51       ` Eli Zaretskii
2014-03-25 16:17         ` Barry OReilly
2014-03-25 19:09           ` Matthias Dahl
2014-03-26 23:49             ` Barry OReilly
2014-03-27 14:22               ` Stefan Monnier
2014-03-28 14:59                 ` Barry OReilly
2014-03-28 15:15                   ` bug#16434: " Matthias Dahl
2014-04-01 17:15                     ` Barry OReilly
2014-04-01 18:17                       ` Matthias Dahl
2014-04-02 15:04                       ` Matthias Dahl
2014-04-02 16:47                         ` Barry OReilly
2014-04-02 18:36                           ` bug#16434: " Matthias Dahl
2014-04-02 19:34                             ` Barry OReilly
2014-04-05  7:52                         ` bug#16378: " Eli Zaretskii
2014-04-05 15:48                           ` Stefan Monnier
2014-04-05 16:15                             ` Eli Zaretskii
2014-04-07  9:58                               ` bug#16434: " Matthias Dahl
2014-04-09  9:49                               ` Matthias Dahl
2014-04-12 11:37                                 ` bug#16378: " Matthias Dahl
2014-04-12 14:32                                   ` Eli Zaretskii
2014-04-12 14:57                                     ` Matthias Dahl
2014-04-23 15:51                                       ` bug#16434: " Eli Zaretskii
2014-04-23 18:11                                         ` Matthias Dahl
2014-04-24  0:36                                           ` Stefan Monnier
2014-04-27  8:22                                             ` Matthias Dahl [this message]
2014-04-28 16:38                                               ` Eli Zaretskii
2014-04-28 18:36                                                 ` Matthias Dahl
2014-04-28 19:18                                                   ` Eli Zaretskii
2014-04-30 18:34                                                     ` Matthias Dahl
2014-03-26 15:30           ` Eli Zaretskii
2014-03-26 16:03             ` Glenn Morris
2014-03-26 13:39 ` bug#16694: Strange background color problem in gentoo Linux Barry OReilly
2014-03-26 15:11   ` Joakim Tjernlund
2014-03-26 16:49     ` Barry OReilly
2014-03-26 18:12       ` bug#16694: Strange background color problem in gentoo GNU/Linux Joakim Tjernlund
2014-03-26 15:58   ` bug#16694: Strange background color problem in gentoo Linux Clemens Schüller
2014-03-26 18:18     ` Joakim Tjernlund

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=535CBE43.7000803@binary-island.eu \
    --to=matthias.dahl@binary-island.eu \
    --cc=16434@debbugs.gnu.org \
    --cc=gundaetiapo@gmail.com \
    --cc=monnier@IRO.UMontreal.CA \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this external index

	https://git.savannah.gnu.org/cgit/emacs.git
	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.