GNU bug report logs -
#16434
Regression: emacs --reverse-video broken
Previous Next
To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 16434 in the body.
You can then email your comments to 16434 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#16434; Package
emacs.
(Tue, 14 Jan 2014 00:43:02 GMT)
Full text and
rfc822 format available.
Acknowledgement sent
to
Barry OReilly <gundaetiapo <at> gmail.com>:
New bug report received and forwarded. Copy sent to
bug-gnu-emacs <at> gnu.org.
(Tue, 14 Jan 2014 00:43:02 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)]
Used to be that emacs --reverse-video would open Emacs with black
background. Now it opens with white background as though the flag
wasn't specified. I Git bisected the Savannah repo to:
15e14b165dcbc6566a0459b0d5e66f89080f569e is the first bad commit
commit 15e14b165dcbc6566a0459b0d5e66f89080f569e
Author: Chong Yidong <cyd <at> gnu.org>
Date: Sat Dec 21 23:31:09 2013 +0800
Don't make faces when loading Custom themes.
* custom.el (custom-theme-recalc-face): Do nothing if the face is
undefined. Thus, theme settings for undefined faces do not take
effect until the faces are defined with defface, the same as with
theme variables.
* faces.el (face-spec-set): Use face-spec-recalc in all cases.
(face-spec-reset-face): Don't assign extra properties in temacs.
(face-spec-recalc): Apply X resources too.
[Message part 2 (text/html, inline)]
Forcibly Merged 16434 16443.
Request was from
Glenn Morris <rgm <at> gnu.org>
to
control <at> debbugs.gnu.org.
(Tue, 14 Jan 2014 16:57:01 GMT)
Full text and
rfc822 format available.
Forcibly Merged 16434 16440 16443.
Request was from
Glenn Morris <rgm <at> gnu.org>
to
control <at> debbugs.gnu.org.
(Tue, 14 Jan 2014 16:58:02 GMT)
Full text and
rfc822 format available.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#16434; Package
emacs.
(Fri, 28 Mar 2014 15:16:01 GMT)
Full text and
rfc822 format available.
Message #16 received at 16434 <at> debbugs.gnu.org (full text, mbox):
Hello @all.
Sorry for my late reply. I was _really_ busy this week. :(
@Barry: Thanks for all your investigative work. Unfortunately I fear you
are somewhat barking up the wrong tree (= function). :)
I am pretty sure I've found the real culprit but the fix is a bit more
involved, if I am right. I still need to do a few more tests and think
about a solution that works without side-effects and is as non-invasive
as possible.
If nobody minds, give me a few more days (still a bit busy here) and I
will see if I can get a patch out for testing asap.
Thanks for the patience in advance. :-)
So long,
Matthias
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#16434; Package
emacs.
(Tue, 01 Apr 2014 17:16:02 GMT)
Full text and
rfc822 format available.
Message #19 received at 16434 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
> I am pretty sure I've found the real culprit but the fix is a bit
> more involved, if I am right.
It would be nice to get a fix into the upcoming pretest, announced at:
http://lists.gnu.org/archive/html/emacs-devel/2014-03/msg01243.html
Could you explain how the patch I proposed would be wrong to install,
even if it doesn't solve all ill symptoms? (No one has reported
whether or not it solves theme problems.)
[Message part 2 (text/html, inline)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#16434; Package
emacs.
(Tue, 01 Apr 2014 18:19:01 GMT)
Full text and
rfc822 format available.
Message #22 received at 16434 <at> debbugs.gnu.org (full text, mbox):
Hello Barry...
> It would be nice to get a fix into the upcoming pretest, announced at:
I agree-- that goes for all the mentioned bugs in the subject btw. ;-)
So I hope we can delay the pretest until those patches are applied.
> Could you explain how the patch I proposed would be wrong to install,
> even if it doesn't solve all ill symptoms? (No one has reported
> whether or not it solves theme problems.)
That whole part of Emacs is not as easy to modify as it might look and
things do tend to have rather unexpected side-effects there-- speaking
from experience. :)
So I honestly would like to keep as much untouched (with my other
patches applied) as possible to fix this bug as well.
The problem is: The inverse video logic is not handled in faces.el but
outside of it. And for X, after the frame has been created, the default
face needs to be left alone, otherwise the inverse video is lost. I've a
few ideas that I just need to test.
I've set aside some time tomorrow for this, so I'll get back to everyone
later that day after I have either cooked up something that works fine
or I've given up and banged my head against the wall. ;)
Sorry for the delay...
Matthias
--
Dipl.-Inf. (FH) Matthias Dahl | Software Engineer | binary-island.eu
services: custom software [desktop, mobile, web], server administration
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#16434; Package
emacs.
(Wed, 02 Apr 2014 15:05:01 GMT)
Full text and
rfc822 format available.
Message #25 received at 16434 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Hello @all...
Like promised, attached revised versions of my first three patches which
had a nasty and very embarrassing bug/brain-fart as well as a potential
fix for bug #16434.
I'd appreciate testing and any feedback, so that we (as in who ever is
in charge and can do this) commit this for the pretest. :)
Keeping my fingers crossed...
So long,
Matthias
[0001-lisp-faces.el-Fix-application-of-X-resource-attribut.patch (text/x-patch, attachment)]
[0002-lisp-faces.el-Fix-empty-face-handling.patch (text/x-patch, attachment)]
[0003-lisp-faces.el-Centralize-no-init-from-x-resources-ch.patch (text/x-patch, attachment)]
[0004-lisp-faces.el-Fix-reverse-video-for-X-window-system.patch (text/x-patch, attachment)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#16434; Package
emacs.
(Wed, 02 Apr 2014 16:48:02 GMT)
Full text and
rfc822 format available.
Message #28 received at 16434 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Hi Matthias, thank you for taking the time to update the patch series.
Patch 3 undoes something patch 1 introduced, perhaps because you
intended a subset of the patches for the emacs-24 branch and the rest
for trunk? Could you clarify that?
I applied all 4 patches and find './src/emacs -r' works correctly.
However, the behavior of './src/emacs -nw -r' is different from
emacs-24.3. I have my gnome-terminal configured to white on black, so
24.3's './src/emacs -nw -r' starts with white background. With your
patch, './src/emacs -nw -r' starts with black background. Not that I
mind the -nw behavior, but I suspect the behavioral difference is
unintended.
+ (when (and (eq face 'default)
+ (frame-parameter frame 'reverse))
+ (let ((fg (face-attribute face :foreground frame))
+ (bg (face-attribute face :background frame)))
+ (set-face-attribute face frame :foreground bg :background fg))))))
Wouldn't there already be a place in code responsible for the swap?
(x-handle-reverse-video and tty-handle-reverse-video?) I'm unsure why
the fix would entail a new place in code responsible for it.
For my information, could you confirm the effect my patch has on your
theme problem?
[Message part 2 (text/html, inline)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#16434; Package
emacs.
(Wed, 02 Apr 2014 18:37:02 GMT)
Full text and
rfc822 format available.
Message #31 received at 16434 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Hello Barry,
I'm on the run but I wanted to answer you this very day, so please
excuse my brief mail. :(
> Patch 3 undoes something patch 1 introduced, perhaps because you
> intended a subset of the patches for the emacs-24 branch and the rest
> for trunk? Could you clarify that?
Thanks for taking a closer look. :) I usually write small contained
patches which work iteratively together which makes it a lot easier to
track down bugs at a later stage with git bisect or whatever.
So sometimes patch x introduces something that is needed for problem x
which patch x+1 slightly modifies or revises to solve problem x+1. :)
> For my information, could you confirm the effect my patch has on your
> theme problem?
I'm sorry but you lost me there. Could you please elaborate more what it
is you want me to comment on? Sorry... and thanks.
Last but not least: Attached you find a new patch for the problem which
is still the fourth in the series but since the last 3 haven't changed,
those are omitted from this mail but still required.
The patch basically restores the behavior (wrt reverse video) to what
Emacs 24.3 did without undoing any of the other work and fixes. I've
tested everything as far as I could and all your test cases work just
fine on my machine.
I hope this one is a keeper.
Thanks for testing and your patience. Again, sorry for the short mail.
So long,
Matthias
--
Dipl.-Inf. (FH) Matthias Dahl | Software Engineer | binary-island.eu
services: custom software [desktop, mobile, web], server administration
[0004-lisp-faces.el-Fix-reverse-video-for-X-window-system.patch (text/x-patch, attachment)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#16434; Package
emacs.
(Wed, 02 Apr 2014 19:36:01 GMT)
Full text and
rfc822 format available.
Message #34 received at 16434 <at> debbugs.gnu.org (full text, mbox):
>> For my information, could you confirm the effect my patch has on
>> your theme problem?
> I'm sorry but you lost me there. Could you please elaborate more
> what it is you want me to comment on? Sorry... and thanks.
I wanted to know if the following happened to fix the problem
described at http://debbugs.gnu.org/cgi/bugreport.cgi?bug=16694#5 :
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."
> The patch basically restores the behavior (wrt reverse video) to
> what Emacs 24.3 did without undoing any of the other work and fixes.
> I've tested everything as far as I could and all your test cases
> work just fine on my machine.
Confirmed, thanks. I have no further objections to the patch series.
Thank you for the work you put into it.
Reply sent
to
Eli Zaretskii <eliz <at> gnu.org>:
You have taken responsibility.
(Sat, 05 Apr 2014 07:53:06 GMT)
Full text and
rfc822 format available.
Notification sent
to
Barry OReilly <gundaetiapo <at> gmail.com>:
bug acknowledged by developer.
(Sat, 05 Apr 2014 07:53:07 GMT)
Full text and
rfc822 format available.
Message #39 received at 16434-done <at> debbugs.gnu.org (full text, mbox):
> Date: Wed, 02 Apr 2014 17:04:19 +0200
> From: Matthias Dahl <ml_emacs-lists <at> binary-island.eu>
> CC: Clemens Schüller
> <cs.mlists+bug-gnu-emacs <at> mailbox.org>, 16694 <at> debbugs.gnu.org,
> Stefan Monnier <monnier <at> iro.umontreal.ca>,
> Eli Zaretskii <eliz <at> gnu.org>, 16434 <at> debbugs.gnu.org
>
> Like promised, attached revised versions of my first three patches which
> had a nasty and very embarrassing bug/brain-fart as well as a potential
> fix for bug #16434.
>
> I'd appreciate testing and any feedback, so that we (as in who ever is
> in charge and can do this) commit this for the pretest. :)
Thanks. I applied to the emacs-24 branch the first 2 patches, the
part of the 3rd patch that does not introduce backward
incompatibilities, and the 4th patch you sent in a later message.
Please confirm that the result will DTRT. Meanwhile, I'm marking
those bugs as "done".
In the future, please also provide ChangeLog entries for the changes
that could be dropped into the respective ChangeLog files. TIA.
The incompatible part of the 3rd patch, reproduced below, remains
uncommitted. I understand the motivation for it, but the emacs-24
branch shouldn't introduce incompatible changes at this time. If
Stefan agrees with applying this part to the trunk, I will do that.
============================================================
Backwards incompatible change: make-face previously accepted
no-init-from-resources as an optional parameter which has now
been removed. There were no other users within Emacs itself. And this
parameter shouldn't have been there in the first place, imho.
diff --git a/lisp/faces.el b/lisp/faces.el
index 8536c08..28205d2 100644
--- a/lisp/faces.el
+++ b/lisp/faces.el
@@ -149,13 +149,10 @@ REGISTRY, ALTERNATIVE1, ALTERNATIVE2, and etc."
"Return a list of all defined faces."
(mapcar #'car face-new-frame-defaults))
-(defun make-face (face &optional no-init-from-resources)
+(defun make-face (face)
"Define a new face with name FACE, a symbol.
Do not call this directly from Lisp code; use `defface' instead.
-
-If NO-INIT-FROM-RESOURCES is non-nil, don't initialize face
-attributes from X resources. If FACE is already known as a face,
-leave it unmodified. Return FACE."
+If FACE is already known as a face, leave it unmodified. Return FACE."
(interactive (list (read-from-minibuffer
"Make face: " nil nil t 'face-name-history)))
(unless (facep face)
@@ -166,8 +163,7 @@ leave it unmodified. Return FACE."
(when (fboundp 'facemenu-add-new-face)
(facemenu-add-new-face face))
;; Define frame-local faces for all frames from X resources.
- (unless no-init-from-resources
- (make-face-x-resource-internal face)))
+ (make-face-x-resource-internal face))
face)
(defun make-empty-face (face)
@@ -175,7 +171,7 @@ leave it unmodified. Return FACE."
Do not call this directly from Lisp code; use `defface' instead."
(interactive (list (read-from-minibuffer
"Make empty face: " nil nil t 'face-name-history)))
- (make-face face 'no-init-from-resources))
+ (make-face face))
(defun copy-face (old-face new-face &optional frame new-frame)
"Define a face named NEW-FACE, which is a copy of OLD-FACE.
Reply sent
to
Eli Zaretskii <eliz <at> gnu.org>:
You have taken responsibility.
(Sat, 05 Apr 2014 07:53:08 GMT)
Full text and
rfc822 format available.
Notification sent
to
"Sebastien Vauban" <sva-news <at> mygooglest.com>:
bug acknowledged by developer.
(Sat, 05 Apr 2014 07:53:09 GMT)
Full text and
rfc822 format available.
Reply sent
to
Eli Zaretskii <eliz <at> gnu.org>:
You have taken responsibility.
(Sat, 05 Apr 2014 07:53:10 GMT)
Full text and
rfc822 format available.
Notification sent
to
Mattia Ziulu <mziulu <at> gmail.com>:
bug acknowledged by developer.
(Sat, 05 Apr 2014 07:53:11 GMT)
Full text and
rfc822 format available.
Reply sent
to
Eli Zaretskii <eliz <at> gnu.org>:
You have taken responsibility.
(Sat, 05 Apr 2014 07:53:12 GMT)
Full text and
rfc822 format available.
Notification sent
to
Matthias Dahl <ml_emacs-lists <at> binary-island.eu>:
bug acknowledged by developer.
(Sat, 05 Apr 2014 07:53:13 GMT)
Full text and
rfc822 format available.
Reply sent
to
Eli Zaretskii <eliz <at> gnu.org>:
You have taken responsibility.
(Sat, 05 Apr 2014 07:53:13 GMT)
Full text and
rfc822 format available.
Notification sent
to
Clemens Schüller <cs.mlists+bug-gnu-emacs <at> mailbox.org>:
bug acknowledged by developer.
(Sat, 05 Apr 2014 07:53:14 GMT)
Full text and
rfc822 format available.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#16434; Package
emacs.
(Sat, 05 Apr 2014 15:49:03 GMT)
Full text and
rfc822 format available.
Message #62 received at 16434-done <at> debbugs.gnu.org (full text, mbox):
> Backwards incompatible change: make-face previously accepted
> no-init-from-resources as an optional parameter which has now
> been removed. There were no other users within Emacs itself. And this
> parameter shouldn't have been there in the first place, imho.
It's kind of late for 24.4, but it looks like a good API cleanup, so
I think it's OK, tho in the 24.4 branch, please keep the optional
parameter and use it to signal a warning.
Stefan
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#16434; Package
emacs.
(Sat, 05 Apr 2014 16:17:03 GMT)
Full text and
rfc822 format available.
Message #65 received at 16434-done <at> debbugs.gnu.org (full text, mbox):
> From: Stefan Monnier <monnier <at> IRO.UMontreal.CA>
> Cc: Matthias Dahl <ml_emacs-lists <at> binary-island.eu>, gundaetiapo <at> gmail.com,
> cs.mlists+bug-gnu-emacs <at> mailbox.org, 16694-done <at> debbugs.gnu.org,
> 16434-done <at> debbugs.gnu.org, 16378-done <at> debbugs.gnu.org
> Date: Sat, 05 Apr 2014 11:48:50 -0400
>
> > Backwards incompatible change: make-face previously accepted
> > no-init-from-resources as an optional parameter which has now
> > been removed. There were no other users within Emacs itself. And this
> > parameter shouldn't have been there in the first place, imho.
>
> It's kind of late for 24.4, but it looks like a good API cleanup, so
> I think it's OK, tho in the 24.4 branch, please keep the optional
> parameter and use it to signal a warning.
Mathias, could you please prepare 2 patches along these lines, one for
the trunk, the other for the emacs-24 branch? Thanks.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#16434; Package
emacs.
(Mon, 07 Apr 2014 09:59:03 GMT)
Full text and
rfc822 format available.
Message #68 received at 16434-done <at> debbugs.gnu.org (full text, mbox):
Hi...
> Mathias, could you please prepare 2 patches along these lines, one for
> the trunk, the other for the emacs-24 branch? Thanks.
>
Sure. I'll prepare something tomorrow or Wednesday at the latest. Thanks
for applying the patches, btw. :)
So long,
Matthias
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#16434; Package
emacs.
(Wed, 09 Apr 2014 09:50:03 GMT)
Full text and
rfc822 format available.
Message #71 received at 16434-done <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Hello...
Attached the promised patches. Deprecation goes to emacs-24, removal to
master. And the ChangeLog fix, if nobody minds, to both. :)
Hope everything is ok.
Thanks for the patience. :) If there is anything else, please let me know.
So long,
Matthias
[0001-lisp-ChangeLog-Fix-mail-address-for-entry.patch (text/x-patch, attachment)]
[0001-lisp-faces.el-Deprecate-optional-argument-of-make-fa.patch (text/x-patch, attachment)]
[0001-lisp-faces.el-Remove-deprecated-optional-argument-of.patch (text/x-patch, attachment)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#16434; Package
emacs.
(Sat, 12 Apr 2014 14:33:03 GMT)
Full text and
rfc822 format available.
Message #74 received at 16434-done <at> debbugs.gnu.org (full text, mbox):
> Date: Sat, 12 Apr 2014 13:37:11 +0200
> From: Matthias Dahl <matthias.dahl <at> binary-island.eu>
> CC: gundaetiapo <at> gmail.com, cs.mlists+bug-gnu-emacs <at> mailbox.org,
> 16694-done <at> debbugs.gnu.org, 16434-done <at> debbugs.gnu.org,
> 16378-done <at> debbugs.gnu.org
>
> Hello Eli...
>
> Since the pretest is due today, if you get the time, could you apply
> those pending patches? Thanks a lot in advance.
Done.
(Your changes to the trunk didn't compile, so I fixed them.)
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#16434; Package
emacs.
(Sat, 12 Apr 2014 18:33:04 GMT)
Full text and
rfc822 format available.
Message #77 received at 16434-done <at> debbugs.gnu.org (full text, mbox):
Hello Eli...
Since the pretest is due today, if you get the time, could you apply
those pending patches? Thanks a lot in advance.
Have a nice weekend,
Matthias
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#16434; Package
emacs.
(Sat, 12 Apr 2014 18:33:05 GMT)
Full text and
rfc822 format available.
Message #80 received at 16434-done <at> debbugs.gnu.org (full text, mbox):
Hello Eli...
> Done.
Thanks.
> (Your changes to the trunk didn't compile, so I fixed them.)
Sorry about that. I missed a parenthesis. I see that now. I should have
made the compile test before sending it but it was such a simple patch
which was exactly alike what I used locally against 24.4 and I
double-checked it... yeah. :( I appreciate it. And sorry for the
screw-up. It won't happen again.
Have a nice Sunday,
Matthias
--
Dipl.-Inf. (FH) Matthias Dahl | Software Engineer | binary-island.eu
services: custom software [desktop, mobile, web], server administration
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#16434; Package
emacs.
(Wed, 23 Apr 2014 15:52:02 GMT)
Full text and
rfc822 format available.
Message #83 received at 16434 <at> debbugs.gnu.org (full text, mbox):
> Date: Sat, 12 Apr 2014 16:57:46 +0200
> From: Matthias Dahl <matthias.dahl <at> binary-island.eu>
> Cc: 16434-done <at> debbugs.gnu.org, gundaetiapo <at> gmail.com, monnier <at> IRO.UMontreal.CA,
> 16378-done <at> debbugs.gnu.org, cs.mlists+bug-gnu-emacs <at> mailbox.org,
> 16694-done <at> debbugs.gnu.org
>
> Hello Eli...
>
> > Done.
>
> Thanks.
I'm sorry, but now I see that the fix of this bug caused an adverse
side effect: face attributes that are defined in the X resources are
now overridden by the face defaults. At least that's what happens on
MS-Windows, where we simulate the X resources in w32reg.c.
Specifically, in Emacs 24.3, the tool bar has its foreground and
background colors set to SystemButtonText and SystemButtonFace,
accordingly, as specified in SYSTEM_DEFAULT_RESOURCES defined by
w32reg.c. By contrast, in the current pretest, this face has the
default foreground and background colors defined by faces.el:
(defface tool-bar
'((default
:box (:line-width 1 :style released-button)
:foreground "black") <<<<<<<<<<<<<<<<<<<<<<<<<<
(((type x w32 ns) (class color))
:background "grey75") <<<<<<<<<<<<<<<<<<<<<<<<<<
This is clearly seen if one tries to customize this face in an Emacs
that was started without -Q.
I looked at the code, and it seems that the problem is in
face-spec-recalc, and the doc string explicitly says that it is the
intended behavior:
(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)
|
(theme and user customization)
or, if nonexistent or does not match the current frame,
(defface default spec)
|
defface override spec"
The code indeed follows the doc string: it first resets the face, then
applies the X resources, and then applies either the theme or the
default face spec:
(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))
(no-match-found 0)
spec theme-face-applied)
(if theme-faces
(dolist (elt (reverse theme-faces))
(setq spec (face-spec-choose (cadr elt) frame no-match-found))
(unless (eq spec no-match-found)
(face-spec-set-2 face frame spec)
(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.
(unless theme-face-applied
(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)))
What happens on Windows is that make-face-x-resource-internal indeed
picks up the colors specified bu w32reg.c, but then face-spec-set-2
resets the colors to what the defface spec says.
Can you or someone else see if setting the colors of the tool-bar face
in the X resources on Unix is similarly overridden?
I don't understand this logic: resources are a kind of customization,
so they should override the default face spec, not the other way
around. Am I missing something?
This change was done because --reverse-video didn't work, but what
does --reverse-video have to do with X resources and their priority in
determining the face attributes?
Thanks.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#16434; Package
emacs.
(Wed, 23 Apr 2014 18:12:01 GMT)
Full text and
rfc822 format available.
Message #86 received at 16434 <at> debbugs.gnu.org (full text, mbox):
Hello Eli...
If no one else has taken care of this by the weekend, I'll happily take
a look then. Right now, I'm really short on time. I'm very sorry. :(
I apologize for the short answer in advance...
You are right. I looked over the 24.3 sources and also based on what you
described, the priority order needs fixing. If I remember correctly
though, this is very delicate and simply switching positions will cause
other bad side effects. :(
> I don't understand this logic: resources are a kind of customization,
> so they should override the default face spec, not the other way
> around. Am I missing something?
When I initially set out to fix those bugs, I researched and, if I do
remember correctly, found an old thread where this was discussed and I
believe it was settled that Emacs defaults should always prevail. But in
hindsight, this makes no sense and I might have gotten things wrong.
I would like to remind you that the original and fundamental changes to
those functions where done by someone else. IMHO, they cleared a few
things up in contrast to 24.3... but introduced several bugs-- like the
ones I fixed and the one you are seeing now... unfortunately.
> This change was done because --reverse-video didn't work,
If you refer to my patches and explicitly moving the X resources stuff
at the beginning of face-spec-recalc: Those changes were done because
themes were broken. :) The changes I made for the reverse-video stuff
should not have caused this side effects and actually brought everything
more in line w/ 24.3... if anything else.
Like I said, if no one has taken care of this by the weekend, I will
gladly have a look and try to fix this as well. Sorry I couldn't be of
more help at this time, though.
So long,
Matthias
--
Dipl.-Inf. (FH) Matthias Dahl | Software Engineer | binary-island.eu
services: custom software [desktop, mobile, web], server administration
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#16434; Package
emacs.
(Thu, 24 Apr 2014 00:38:02 GMT)
Full text and
rfc822 format available.
Message #89 received at 16434 <at> debbugs.gnu.org (full text, mbox):
> If you refer to my patches and explicitly moving the X resources stuff
> at the beginning of face-spec-recalc: Those changes were done because
> themes were broken. :) The changes I made for the reverse-video stuff
> should not have caused this side effects and actually brought everything
> more in line w/ 24.3... if anything else.
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
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#16434; Package
emacs.
(Sun, 27 Apr 2014 08:23:02 GMT)
Full text and
rfc822 format available.
Message #92 received at 16434 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
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
[emacs24-0001-PATCH-lisp-faces.el-Apply-X-resources-after-defface-.patch (text/x-patch, attachment)]
[master-0001-lisp-faces.el-Apply-X-resources-after-defface-spec.patch (text/x-patch, attachment)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#16434; Package
emacs.
(Mon, 28 Apr 2014 16:39:02 GMT)
Full text and
rfc822 format available.
Message #95 received at 16434 <at> debbugs.gnu.org (full text, mbox):
> Date: Sun, 27 Apr 2014 10:22:27 +0200
> From: Matthias Dahl <matthias.dahl <at> binary-island.eu>
> CC: Eli Zaretskii <eliz <at> gnu.org>, 16434 <at> debbugs.gnu.org,
> gundaetiapo <at> gmail.com
>
> 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.
Unfortunately, it doesn't fix the problem. It looks like the problem
is that when make-face-x-resource-internal is called near the end of
face-spec-recalc, inhibit-x-resources is already set non-nil, and so
make-face-x-resource-internal does nothing.
Don't you see the same problem on X if you set
emacs.tool-bar.attributeBackground in the X resources? That would
allow you to try the change on your system.
> 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.
Btw, in the future, you don't need to submit 2 identical patches, just
one to the release branch is enough: it will get merged onto the trunk
soon enough after being committed to the branch.
Thanks.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#16434; Package
emacs.
(Mon, 28 Apr 2014 18:37:01 GMT)
Full text and
rfc822 format available.
Message #98 received at 16434 <at> debbugs.gnu.org (full text, mbox):
On 28/04/14 18:38, Eli Zaretskii wrote:
> Unfortunately, it doesn't fix the problem. It looks like the problem
> is that when make-face-x-resource-internal is called near the end of
> face-spec-recalc, inhibit-x-resources is already set non-nil, and so
> make-face-x-resource-internal does nothing.
Ah, that little bit of information I missed. In that case, everything's
actually working as intended. If you start Emacs and force it to ignore
X resources, it does so entirely and as expected... even more so in 24.4
now. Programmatically "setting" an X resource behind the scenes and
expecting it to work even though that very system has been asked to do
nothing, is bound to cause trouble.
Don't get me wrong, it is still a bug, nevertheless... only it is imho
debatable what the root cause really is in this case. The fact that the
X resource is not being applied (despite inhibit-x-resources == t) or
that the toolbar coloring is changed this way and expected to work at
all times, even if inhibit-x-resources == t.
> Don't you see the same problem on X if you set
> emacs.tool-bar.attributeBackground in the X resources? That would
> allow you to try the change on your system.
If that would work, I would consider it a bug. If an X resource was
applied even though I started Emacs w/ -Q or otherwise set inhibit-...
that would very much be a bug. At least imho that is.
> Btw, in the future, you don't need to submit 2 identical patches, just
> one to the release branch is enough: it will get merged onto the trunk
> soon enough after being committed to the branch.
Ah, Ok. I'll keep that in mind. Thanks...
I'll have a look at this issue sometime later this week or weekend and
see what is going on exactly and if there is a way to fix this without
actually implementing any exceptions into make-face-x-... or anything
alike. Naturally, if someone else wants to take a stab at it who knows
more about all those tightly intervened and sneaky parts of the code
than I do (that one shouldn't be hard :P), I absolutely won't mind. :-)
So long,
Matthias
--
Dipl.-Inf. (FH) Matthias Dahl | Software Engineer | binary-island.eu
services: custom software [desktop, mobile, web], server administration
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#16434; Package
emacs.
(Mon, 28 Apr 2014 19:19:01 GMT)
Full text and
rfc822 format available.
Message #101 received at 16434 <at> debbugs.gnu.org (full text, mbox):
> Date: Mon, 28 Apr 2014 20:36:26 +0200
> From: Matthias Dahl <matthias.dahl <at> binary-island.eu>
> CC: monnier <at> IRO.UMontreal.CA, 16434 <at> debbugs.gnu.org,
> gundaetiapo <at> gmail.com
>
> On 28/04/14 18:38, Eli Zaretskii wrote:
>
> > Unfortunately, it doesn't fix the problem. It looks like the problem
> > is that when make-face-x-resource-internal is called near the end of
> > face-spec-recalc, inhibit-x-resources is already set non-nil, and so
> > make-face-x-resource-internal does nothing.
>
> Ah, that little bit of information I missed. In that case, everything's
> actually working as intended. If you start Emacs and force it to ignore
> X resources, it does so entirely and as expected... even more so in 24.4
> now. Programmatically "setting" an X resource behind the scenes and
> expecting it to work even though that very system has been asked to do
> nothing, is bound to cause trouble.
I'm terribly sorry, it turns out I tested your change incorrectly. I
did it correctly this time, and of course the problem is solved.
I committed your changes to the emacs-24 branch. Thanks.
> I'll have a look at this issue sometime later this week or weekend and
> see what is going on exactly and if there is a way to fix this without
> actually implementing any exceptions into make-face-x-... or anything
> alike.
No need, the problem is solved. It was my fault.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#16434; Package
emacs.
(Wed, 30 Apr 2014 18:35:01 GMT)
Full text and
rfc822 format available.
Message #104 received at 16434 <at> debbugs.gnu.org (full text, mbox):
Hi Eli...
> I'm terribly sorry, it turns out I tested your change incorrectly.
Absolutely no need to feel sorry. :) The most important part is...
> the problem is solved.
... which is a relieve since I won't have to dig deeper into this. ;-)
> I committed your changes to the emacs-24 branch.
Thanks.
Hopefully we finally caught all the fallout from those changes that went
in earlier this year. Keeping my fingers crossed.
Have a nice evening,
Matthias
bug archived.
Request was from
Debbugs Internal Request <help-debbugs <at> gnu.org>
to
internal_control <at> debbugs.gnu.org.
(Thu, 29 May 2014 11:24:04 GMT)
Full text and
rfc822 format available.
This bug report was last modified 9 years and 341 days ago.
Previous Next
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.