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

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 16694 in the body.
You can then email your comments to 16694 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Matthias Dahl <ml_emacs-lists <at> binary-island.eu>
To: bug-gnu-emacs <at> gnu.org
Subject: Regression by commit 115663 (bzr): Theme face attributes no longer
 take precedence over X resources
Date: Sat, 08 Feb 2014 11:06:38 +0100

Hello @all...

If a theme changes attributes from the default face for example (like
:background), those will shortly flash and then revert back to the
default of the current GTK theme, thus leaving the user with a mixed
state of the Emacs theme and GTK theme, which is most certainly not
intended.

This regression was introduced by bzr commit 115663: imho in function
face-spec-recalc from faces.el, make-face-x-resource-internal should not
be called last, which overrides parts of the theme again with the
defaults from the GTK theme.

Either commenting that line, or starting emacs with inhibit-x-resources
set to true, will work again. With the later though, you naturally loose
some other options as well, which is not desirable.

(As a side note: It is not properly documented either in the command
line help, nor the man pages, that starting emacs with -Q also sets
inhibit-x-resources to true.)

If more information or help is required, please let me know and I will
happily lend a hand or two. :-)

INFO from a stripped down Emacs checked out and built moments ago, which
(as expected) still has the problem:

In GNU Emacs 24.3.50.1 (x86_64-pc-linux-gnu, GTK+ Version 3.10.7)
 of 2014-02-08 on dreamgate
Repository revision: larsi <at> gnus.org-20140208065815-ie7d8pg17yeaagla
Windowing system distributor `The X.Org Foundation', version 11.0.11500000
System Description:	NAME=Gentoo

Configured using:
 `configure --prefix=/usr --build=x86_64-pc-linux-gnu
 --host=x86_64-pc-linux-gnu --mandir=/usr/share/man
 --infodir=/usr/share/info --datadir=/usr/share --sysconfdir=/etc
 --localstatedir=/var/lib --libdir=/usr/lib64 --disable-silent-rules
 --disable-dependency-tracking --program-suffix=-emacs-24-vcs
 --infodir=/usr/share/info/emacs-24-vcs
 --enable-locallisppath=/etc/emacs:/usr/share/emacs/site-lisp
 --with-gameuser=games --without-compress-install
 --with-file-notification=inotify --enable-acl --with-dbus
 --without-gnutls --with-gpm --without-hesiod --without-kerberos
 --without-kerberos5 --with-xml2 --without-selinux --with-wide-int
 --with-zlib --with-sound=alsa --with-x --without-ns --without-gconf
 --without-gsettings --without-toolkit-scroll-bars --with-gif
 --with-jpeg --with-png --with-rsvg --with-tiff --with-xpm
 --with-imagemagick --with-xft --without-libotf --without-m17n-flt
 --with-x-toolkit=gtk3 GENTOO_PACKAGE=app-editors/emacs-vcs-24.3.9999
 EBZR_BRANCH=trunk EBZR_REVNO=116339 'CFLAGS=-march=native -O2 -pipe'
 CPPFLAGS= 'LDFLAGS=-Wl,-O1 -Wl,--as-needed -Wl,-O1 -Wl,-z,combreloc
 -Wl,-z,now''

Important settings:
  value of $LC_COLLATE: de_DE.utf8
  value of $LC_CTYPE: de_DE.utf8
  value of $LC_MESSAGES: en_US.utf8
  value of $LC_MONETARY: de_DE.utf8
  value of $LC_NUMERIC: de_DE.utf8
  value of $LC_TIME: en_GB.utf8
  value of $LANG: en_US.utf8
  locale-coding-system: utf-8-unix

Major mode: Fundamental

Minor modes in effect:
  tooltip-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  tool-bar-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  blink-cursor-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  buffer-read-only: t
  line-number-mode: t
  transient-mark-mode: t

Recent input:
M-x s e n d - b u g <tab> <backspace> <backspace> <backspace>
<backspace> <backspace> <backspace> <backspace> <backspace>
r e p o <tab> r t - e <tab> <return>

Recent messages:
For information about GNU Emacs and the GNU system, type C-h C-a.
Making completion list...
call-interactively: End of buffer [6 times]

Load-path shadows:
None found.

Features:
(shadow sort gnus-util mail-extr emacsbug message idna format-spec
rfc822 mml mml-sec mm-decode mm-bodies mm-encode mail-parse rfc2231
mailabbrev gmm-utils mailheader sendmail rfc2047 rfc2045 ietf-drums
mm-util help-fns mail-prsvr mail-utils help-mode easymenu package
site-gentoo time-date tooltip electric uniquify ediff-hook vc-hooks
lisp-float-type mwheel x-win x-dnd tool-bar dnd fontset image regexp-opt
fringe tabulated-list newcomment lisp-mode prog-mode register page
menu-bar rfn-eshadow timer select scroll-bar mouse jit-lock font-lock
syntax facemenu font-core frame cham georgian utf-8-lang misc-lang
vietnamese tibetan thai tai-viet lao korean japanese hebrew greek
romanian slovak czech european ethiopic indian cyrillic chinese
case-table epa-hook jka-cmpr-hook help simple abbrev minibuffer nadvice
loaddefs button faces cus-face macroexp files text-properties overlay
sha1 md5 base64 format env code-pages mule custom widget
hashtable-print-readable backquote make-network-process dbusbind inotify
dynamic-setting font-render-setting move-toolbar gtk x-toolkit x
multi-tty emacs)

Message #8 received at 16694 <at> debbugs.gnu.org (full text, mbox):

From: Glenn Morris <rgm <at> gnu.org>
To: Matthias Dahl <ml_emacs-lists <at> binary-island.eu>
Cc: 16694 <at> debbugs.gnu.org
Subject: Re: bug#16694: Regression by commit 115663 (bzr): Theme face
 attributes no longer take precedence over X resources
Date: Wed, 12 Feb 2014 17:21:34 -0500

Matthias Dahl wrote:

> This regression was introduced by bzr commit 115663:

Then this is probably the same root issue as
http://debbugs.gnu.org/16434 (and 16440, and 16443, and maybe 16378).

There is clearly something going on there that needs to be investigated
before 24.4.

Message #13 received at 16694 <at> debbugs.gnu.org (full text, mbox):

From: Matthias Dahl <ml_emacs-lists <at> binary-island.eu>
To: Glenn Morris <rgm <at> gnu.org>
Cc: 16694 <at> debbugs.gnu.org
Subject: Re: bug#16694: Regression by commit 115663 (bzr): Theme face
 attributes no longer take precedence over X resources
Date: Fri, 14 Feb 2014 20:17:42 +0100

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

On 12/02/14 23:21, Glenn Morris wrote:

> Then this is probably the same root issue as
> http://debbugs.gnu.org/16434 (and 16440, and 16443, and maybe 16378).

More than likely.

> There is clearly something going on there that needs to be investigated
> before 24.4.

The attached patch is what I am using locally and it fixes all problems
for me. I also tested bug #16434 and it works fine w/ the patch applied.
Basically it puts everything from X resources back to be applied first
and everything else on-top. Thus everything else is taking priority.

Consider the patch a raw version and a RFC-- missing proper changes to
the comments and I'd probably pull the conditional
(no-init-from-x-resources) up into make-face-x-resource-internal and
change the usage throughout the rest of the file.

But I'm not too well versed with Emacs' deep inner workings, and even
though it makes sense to me :), I might be completely off and just
fixing the symptoms. If so, I'd gladly help in finding the proper root
cause and learning a bit more in the process...

Hope that helps...
Matthias

[faces.el.diff (text/plain, attachment)]

Message #16 received at 16694 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Matthias Dahl <ml_emacs-lists <at> binary-island.eu>
Cc: rgm <at> gnu.org, 16694 <at> debbugs.gnu.org
Subject: Re: bug#16694: Regression by commit 115663 (bzr): Theme face
 attributes no longer take precedence over X resources
Date: Thu, 20 Feb 2014 20:27:08 +0200

> Date: Fri, 14 Feb 2014 20:17:42 +0100
> From: Matthias Dahl <ml_emacs-lists <at> binary-island.eu>
> Cc: 16694 <at> debbugs.gnu.org
> 
> The attached patch is what I am using locally and it fixes all problems
> for me. I also tested bug #16434 and it works fine w/ the patch applied.
> Basically it puts everything from X resources back to be applied first
> and everything else on-top. Thus everything else is taking priority.
> 
> Consider the patch a raw version and a RFC-- missing proper changes to
> the comments and I'd probably pull the conditional
> (no-init-from-x-resources) up into make-face-x-resource-internal and
> change the usage throughout the rest of the file.
> 
> But I'm not too well versed with Emacs' deep inner workings, and even
> though it makes sense to me :), I might be completely off and just
> fixing the symptoms. If so, I'd gladly help in finding the proper root
> cause and learning a bit more in the process...

Any reasons not to install this?  Anybody?

Message #19 received at 16694 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: Matthias Dahl <ml_emacs-lists <at> binary-island.eu>, 16694 <at> debbugs.gnu.org
Subject: Re: bug#16694: Regression by commit 115663 (bzr): Theme face
 attributes no longer take precedence over X resources
Date: Thu, 20 Feb 2014 16:47:07 -0500

> Any reasons not to install this?  Anybody?

I don't understand this code nor the change that seems to have broken
it, so I neither support nor oppose the change.
Does it fix bug#16378 as well?


        Stefan

Message #22 received at 16694 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: ml_emacs-lists <at> binary-island.eu, 16694 <at> debbugs.gnu.org
Subject: Re: bug#16694: Regression by commit 115663 (bzr): Theme face
 attributes no longer take precedence over X resources
Date: Fri, 21 Feb 2014 11:07:48 +0200

> From: Stefan Monnier <monnier <at> iro.umontreal.ca>
> Cc: Matthias Dahl <ml_emacs-lists <at> binary-island.eu>,  16694 <at> debbugs.gnu.org
> Date: Thu, 20 Feb 2014 16:47:07 -0500
> 
> > Any reasons not to install this?  Anybody?
> 
> I don't understand this code nor the change that seems to have broken
> it, so I neither support nor oppose the change.

If no one objects in a couple of days, I think we should install it.

> Does it fix bug#16378 as well?

No.

Message #25 received at 16694 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: ml_emacs-lists <at> binary-island.eu, 16694 <at> debbugs.gnu.org
Subject: Re: bug#16694: Regression by commit 115663 (bzr): Theme face
 attributes no longer take precedence over X resources
Date: Fri, 21 Feb 2014 12:36:26 -0500

>> Does it fix bug#16378 as well?
> No.

Bummer!


        Stefan "who experiences bug#16378 all the time"

Message #28 received at 16694 <at> debbugs.gnu.org (full text, mbox):

From: Matthias Dahl <ml_emacs-lists <at> binary-island.eu>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: Stefan Monnier <monnier <at> iro.umontreal.ca>, 16694 <at> debbugs.gnu.org
Subject: Re: bug#16694: Regression by commit 115663 (bzr): Theme face
 attributes no longer take precedence over X resources
Date: Sun, 23 Feb 2014 17:46:39 +0100

Hello...

Sorry for my late reply. :(

> If no one objects in a couple of days, I think we should install it.

Good to hear. If you don't mind, I'd like to take a closer look again
and probably revise the original comments to match the new behavior.
Also, maybe add a second patch to pull the check up into the utility
function itself and refactor the few bits and pieces where it is used
accordingly. I'll try to get this done within the next few days.

>        Stefan "who experiences bug#16378 all the time"

I had a look at the description of bug #16378 but unfortunately I
couldn't make head or tail of it. If you don't mind filling me in, I'd
try to track this one down as well.

So long,
Matthias

Message #31 received at 16694 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Matthias Dahl <ml_emacs-lists <at> binary-island.eu>
Cc: monnier <at> iro.umontreal.ca, 16694 <at> debbugs.gnu.org
Subject: Re: bug#16694: Regression by commit 115663 (bzr): Theme face
 attributes no longer take precedence over X resources
Date: Sun, 23 Feb 2014 19:18:35 +0200

> Date: Sun, 23 Feb 2014 17:46:39 +0100
> From: Matthias Dahl <ml_emacs-lists <at> binary-island.eu>
> CC: Stefan Monnier <monnier <at> iro.umontreal.ca>, 16694 <at> debbugs.gnu.org
> 
> > If no one objects in a couple of days, I think we should install it.
> 
> Good to hear. If you don't mind, I'd like to take a closer look again
> and probably revise the original comments to match the new behavior.
> Also, maybe add a second patch to pull the check up into the utility
> function itself and refactor the few bits and pieces where it is used
> accordingly. I'll try to get this done within the next few days.

Sure, please do, and thanks.

> >        Stefan "who experiences bug#16378 all the time"
> 
> I had a look at the description of bug #16378 but unfortunately I
> couldn't make head or tail of it. If you don't mind filling me in, I'd
> try to track this one down as well.

You mean, you couldn't reproduce the problem?  Or do you mean
something else?

Message #34 received at 16694 <at> debbugs.gnu.org (full text, mbox):

From: Matthias Dahl <ml_emacs-lists <at> binary-island.eu>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: monnier <at> iro.umontreal.ca, 16694 <at> debbugs.gnu.org
Subject: Re: bug#16694: Regression by commit 115663 (bzr): Theme face
 attributes no longer take precedence over X resources
Date: Mon, 24 Feb 2014 20:29:03 +0100

> You mean, you couldn't reproduce the problem?  Or do you mean
> something else?

Both, actually. :) What I meant was: Since I am still "new" to emacs, I
don't know all of its dark corners and functionality. Thus, when I read
the bug report done by Stefan, I tried to figure out what was the
desired and what was the buggy behavior, all in the context of some
functionality that was rather foggy to me. When I tried to reproduce it
with my current installation (which has the patch applied), I could
not... but I suspect that I just didn't know what to look for or
messed up along the road.

To make a long story short: If someone could give me a bit more details
and background, I'd be more than happy to look into that bug and try to
track it down.

Message #37 received at 16694 <at> debbugs.gnu.org (full text, mbox):

From: Matthias Dahl <ml_emacs-lists <at> binary-island.eu>
To: 16694 <at> debbugs.gnu.org
Cc: Eli Zaretskii <eliz <at> gnu.org>, Stefan Monnier <monnier <at> IRO.UMontreal.CA>,
 16378 <at> debbugs.gnu.org
Subject: bugs #16694/#16378: Patches
Date: Wed, 05 Mar 2014 17:14:34 +0100

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

Hello @all.

Since bugs #16694 and #16378 are somewhat loosely connected, I am
posting this to both.

The attached patches have been tested quite a bit locally and especially
the fix for #16378 has gone through several revisions to cover all
corner cases and side-effects.

The first two patches should be quite safe to apply. Naturally I would
appreciate any testing and feedback. :)

The last patch is not required to fix any bugs (though it might), it
just cleans up a bit but introduces a backwards incompatible change on
the way which should (imho) not be a problem. That one is more a RFC but
works just fine on my system as well and I think applying it to the tree
shouldn't be trouble.

If there are any questions or problems, please let me know.

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)]

Message #40 received at 16694 <at> debbugs.gnu.org (full text, mbox):

From: Matthias Dahl <ml_emacs-lists <at> binary-island.eu>
To: 16694 <at> debbugs.gnu.org
Cc: Eli Zaretskii <eliz <at> gnu.org>, Stefan Monnier <monnier <at> IRO.UMontreal.CA>,
 16378 <at> debbugs.gnu.org
Subject: Re: bugs #16694/#16378: Patches
Date: Sun, 16 Mar 2014 20:13:53 +0100

Hello @all.

It's been almost two weeks since I sent the patches with absolute and
rather discouraging silence in-between. :-) So I quickly wanted to ask
if there is any feedback (whether good or very bad) or something that
needs improving? And if there is a chance of getting those fixes applied
to the tree?

Thanks in advance,
Matthias

Message #43 received at 16694 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Matthias Dahl <ml_emacs-lists <at> binary-island.eu>
Cc: 16694 <at> debbugs.gnu.org, 16378 <at> debbugs.gnu.org, monnier <at> IRO.UMontreal.CA
Subject: Re: bugs #16694/#16378: Patches
Date: Mon, 17 Mar 2014 16:33:39 +0200

> Date: Sun, 16 Mar 2014 20:13:53 +0100
> From: Matthias Dahl <ml_emacs-lists <at> binary-island.eu>
> CC: 16378 <at> debbugs.gnu.org, Stefan Monnier <monnier <at> IRO.UMontreal.CA>, 
>  Eli Zaretskii <eliz <at> gnu.org>
> 
> Hello @all.
> 
> It's been almost two weeks since I sent the patches with absolute and
> rather discouraging silence in-between. :-) So I quickly wanted to ask
> if there is any feedback (whether good or very bad) or something that
> needs improving? And if there is a chance of getting those fixes applied
> to the tree?

Barring any objections, I will commit this in a few days.

Thanks.

Message #46 received at 16694 <at> debbugs.gnu.org (full text, mbox):

From: Barry OReilly <gundaetiapo <at> gmail.com>
To: ml_emacs-lists <at> binary-island.eu, 16694 <at> debbugs.gnu.org
Subject: bug#16694: bugs #16694/#16378: Patches
Date: Fri, 21 Mar 2014 14:05:22 -0400

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

I tried 0001-lisp-faces.el-Fix-application-of-X-resource-attribut.patch and
got:

Loading /home/boreilly/l/emacs/x-resources-16434/lisp/faces.el (source)...
Symbol's value as variable is void: no-init-from-resources
make[2]: *** [bootstrap-emacs] Error 1

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

Message #49 received at 16694 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Barry OReilly <gundaetiapo <at> gmail.com>
Cc: ml_emacs-lists <at> binary-island.eu, 16694 <at> debbugs.gnu.org
Subject: Re: bug#16694: bugs #16694/#16378: Patches
Date: Sat, 22 Mar 2014 10:30:08 +0200

> Date: Fri, 21 Mar 2014 14:05:22 -0400
> From: Barry OReilly <gundaetiapo <at> gmail.com>
> 
> I tried 0001-lisp-faces.el-Fix-application-of-X-resource-attribut.patch and
> got:
> 
> Loading /home/boreilly/l/emacs/x-resources-16434/lisp/faces.el (source)...
> Symbol's value as variable is void: no-init-from-resources
> make[2]: *** [bootstrap-emacs] Error 1

Mathias, it looks something is indeed missing in your patches.  Should
face-spec-recalc somehow accept an additional argument?

Message #52 received at 16694 <at> debbugs.gnu.org (full text, mbox):

From: Matthias Dahl <ml_emacs-lists <at> binary-island.eu>
To: Eli Zaretskii <eliz <at> gnu.org>, Barry OReilly <gundaetiapo <at> gmail.com>
Cc: 16694 <at> debbugs.gnu.org
Subject: Re: bug#16694: bugs #16694/#16378: Patches
Date: Sun, 23 Mar 2014 18:04:47 +0100

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

Hello Eli and Barry,

sorry for my brain fart on this one. It was actually a rather late edit
which, even though I tested it and deemed harmless but useful, slipped
through my fingers.

By the way: no-init-from-resources is a global variable set by Emacs
during startup if (and only if) "-Q" is given, thus X resources should
not be applied. In patch 3 I accounted for that but simply forgot to do
the same in patch 1. Oh well.

Attached are the updated patches which should all work just fine now.
Thanks for the feedback and testing btw. Very much appreciated.

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)]

Message #57 received at 16694 <at> debbugs.gnu.org (full text, mbox):

From: Barry OReilly <gundaetiapo <at> gmail.com>
To: ml_emacs-lists <at> binary-island.eu, 16694 <at> debbugs.gnu.org
Subject: bug#16694: bugs #16694/#16378: Patches
Date: Mon, 24 Mar 2014 19:42:34 -0400

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

> By the way: no-init-from-resources is a global variable set by Emacs
> during startup if (and only if) "-Q" is given, thus X resources should
> not be applied.

I built fine with the
0001-lisp-faces.el-Fix-application-of-X-resource-attribut.patch .

  ./src/emacs -Q -r

works as expected. However, commenting out my entire init.el,

  ./src/emacs -r

still starts Emacs with a white background, contrary to expectation.
So the patch doesn't fix 16434.

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

Message #60 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Clemens Schüller <cs.mlists+bug-gnu-emacs <at> mailbox.org>
To: emacs bugs <bug-gnu-emacs <at> gnu.org>
Cc: gundaetiapo <at> gmail.com, ml_emacs-lists <at> binary-island.eu
Subject: Re: bug#16694: bugs #16694/#16378: Patches
Date: Tue, 25 Mar 2014 00:49:57 +0100

Hello!

>> By the way: no-init-from-resources is a global variable set by Emacs
>> during startup if (and only if) "-Q" is given, thus X resources should
>> not be applied.
>
> I built fine with the
> 0001-lisp-faces.el-Fix-application-of-X-resource-attribut.patch .
>
> ./Src/emacs -Q -r
>
> Works as expected. However, commenting out my entire init.el,
>
> ./Src/emacs -r
>
> Still starts Emacs with a white background, contrary to expectation.
> So the patch doesn't fix 16434.

I had the same problem with the white background:

After the integration of _all_ three patches the problem was gone.

-- 
Best Regards, Clemens Schüller

Message #63 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Barry OReilly <gundaetiapo <at> gmail.com>
To: Clemens Schüller <cs.mlists+bug-gnu-emacs <at> mailbox.org>
Cc: emacs bugs <bug-gnu-emacs <at> gnu.org>, ml_emacs-lists <at> binary-island.eu
Subject: Re: bug#16694: bugs #16694/#16378: Patches
Date: Tue, 25 Mar 2014 10:17:58 -0400

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

> After the integration of _all_ three patches the problem was gone.

I applied all three patches, but './src/emacs -r' with commented out
init.el still starts Emacs with unexpected white background.

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

Message #66 received at 16694 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Barry OReilly <gundaetiapo <at> gmail.com>
Cc: ml_emacs-lists <at> binary-island.eu, cs.mlists+bug-gnu-emacs <at> mailbox.org,
 16694 <at> debbugs.gnu.org
Subject: Re: bug#16694: bugs #16694/#16378: Patches
Date: Tue, 25 Mar 2014 17:51:52 +0200

> Date: Tue, 25 Mar 2014 10:17:58 -0400
> From: Barry OReilly <gundaetiapo <at> gmail.com>
> Cc: ml_emacs-lists <at> binary-island.eu, 16694 <at> debbugs.gnu.org
> 
> > After the integration of _all_ three patches the problem was gone.
> 
> I applied all three patches, but './src/emacs -r' with commented out
> init.el still starts Emacs with unexpected white background.

The reverse-video option was misbehaving for a long time; are you sure
that issue is at all related to the bugs discussed here?  When did you
last see -r DTRT in this scenario?

Message #69 received at 16694 <at> debbugs.gnu.org (full text, mbox):

From: Barry OReilly <gundaetiapo <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: ml_emacs-lists <at> binary-island.eu,
 Clemens Schüller <cs.mlists+bug-gnu-emacs <at> mailbox.org>,
 16694 <at> debbugs.gnu.org
Subject: Re: bug#16694: bugs #16694/#16378: Patches
Date: Tue, 25 Mar 2014 12:17:45 -0400

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

> When did you last see -r DTRT in this scenario?

I Git bisected it in bug 16434.

http://debbugs.gnu.org/cgi/bugreport.cgi?bug=16434#5

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

Message #72 received at 16694 <at> debbugs.gnu.org (full text, mbox):

From: Matthias Dahl <ml_emacs-lists <at> binary-island.eu>
To: Barry OReilly <gundaetiapo <at> gmail.com>, Eli Zaretskii <eliz <at> gnu.org>
Cc: Clemens Schüller <cs.mlists+bug-gnu-emacs <at> mailbox.org>,
 16694 <at> debbugs.gnu.org
Subject: Re: bug#16694: bugs #16694/#16378: Patches
Date: Tue, 25 Mar 2014 20:09:15 +0100

Hello all.

I'll have a look at everything later this week and report back. I am
rather busy at the moment, unfortunately. :(

So long,
Matthias

Message #75 received at 16694 <at> debbugs.gnu.org (full text, mbox):

From: Barry OReilly <gundaetiapo <at> gmail.com>
To: joakim.tjernlund <at> transmode.se, cyd <at> gnu.org, 16694 <at> debbugs.gnu.org, 
 ml_emacs-lists <at> binary-island.eu
Subject: Re: Strange background color problem in gentoo Linux
Date: Wed, 26 Mar 2014 09:39:51 -0400

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

> I am using Gnome2, KDE and MATE on a fresh Gentoo system. In my
> .emacs I have:
>
> (add-to-list 'default-frame-alist '(background-color . "#333333"))
> (add-to-list 'default-frame-alist '(foreground-color . "White"))
> (add-to-list 'default-frame-alist '(cursor-color . "White"))
>
> Whenever I use MATE or KDE, emacs fails to set the background color
> to Green(#333333), it becomes white instead and I end up with White
> one White on White :( If I open a new Frame I get my Green
> background though.
>
> This happens on 24.3.50_pre20140228 and 24.3.9999-r1 (top of repo
> tree) Tried both gtk2 and gtk3 USE flags

Sounds similar to several other bug reports since the regression was
introduced in December. Most discussion is at
http://debbugs.gnu.org/cgi/bugreport.cgi?bug=16694 .

Chong, it would be nice if you could opine about the problem and
Matthias's patches. The offending commit was:

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)]

Message #78 received at 16694 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Barry OReilly <gundaetiapo <at> gmail.com>
Cc: ml_emacs-lists <at> binary-island.eu, cs.mlists+bug-gnu-emacs <at> mailbox.org,
 16694 <at> debbugs.gnu.org
Subject: Re: bug#16694: bugs #16694/#16378: Patches
Date: Wed, 26 Mar 2014 17:30:13 +0200

> Date: Tue, 25 Mar 2014 12:17:45 -0400
> From: Barry OReilly <gundaetiapo <at> gmail.com>
> Cc: Clemens Schüller <cs.mlists+bug-gnu-emacs <at> mailbox.org>, 
> 	ml_emacs-lists <at> binary-island.eu, 16694 <at> debbugs.gnu.org
> 
> > When did you last see -r DTRT in this scenario?
> 
> I Git bisected it in bug 16434.
> 
> http://debbugs.gnu.org/cgi/bugreport.cgi?bug=16434#5

Thanks.  So this is yet another bug to take care of.

Message #81 received at 16694 <at> debbugs.gnu.org (full text, mbox):

From: Joakim Tjernlund <joakim.tjernlund <at> transmode.se>
To: Barry OReilly <gundaetiapo <at> gmail.com>
Cc: ml_emacs-lists <at> binary-island.eu, cyd <at> gnu.org, 16694 <at> debbugs.gnu.org
Subject: Re: Strange background color problem in gentoo Linux
Date: Wed, 26 Mar 2014 16:11:29 +0100

Barry OReilly <gundaetiapo <at> gmail.com> wrote on 2014/03/26 14:39:51:
> 
> > I am using Gnome2, KDE and MATE on a fresh Gentoo system. In my
> > .emacs I have:
> >
> > (add-to-list 'default-frame-alist '(background-color . "#333333"))
> > (add-to-list 'default-frame-alist '(foreground-color . "White"))
> > (add-to-list 'default-frame-alist '(cursor-color . "White"))
> >
> > Whenever I use MATE or KDE, emacs fails to set the background color
> > to Green(#333333), it becomes white instead and I end up with White
> > one White on White :( If I open a new Frame I get my Green
> > background though.
> >
> > This happens on 24.3.50_pre20140228 and 24.3.9999-r1 (top of repo
> > tree) Tried both gtk2 and gtk3 USE flags
> 
> Sounds similar to several other bug reports since the regression was
> introduced in December. Most discussion is at
> http://debbugs.gnu.org/cgi/bugreport.cgi?bug=16694 .

Sure looks similar, thank you

One more data point, if I add 
  (setq initial-frame-alist default-frame-alist)
as in
  (add-to-list 'default-frame-alist '(background-color . "#333333"))
  (add-to-list 'default-frame-alist '(foreground-color . "White"))
  (add-to-list 'default-frame-alist '(cursor-color . "White")) 
  (setq initial-frame-alist default-frame-alist)
I get the correct green background in my initial Frame.

May I also ask, in another Gentoo GNU/Linux system running Gnome 2 I get
this after updating that install:

jocke <at> gentoo64 ~ $ emacs

(emacs:2436): GLib-GObject-WARNING **: Attempt to add property 
GtkSettings::gtk-button-images after class was initialised

(emacs:2436): GLib-GObject-WARNING **: Attempt to add property 
GtkSettings::gtk-label-select-on-focus after class was initialised

(emacs:2436): GLib-GObject-WARNING **: Attempt to add property 
GtkSettings::gtk-menu-bar-popup-delay after class was initialised

(emacs:2436): GLib-GObject-WARNING **: Attempt to add property 
GtkSettings::gtk-can-change-accels after class was initialised

(emacs:2436): GLib-GObject-WARNING **: Attempt to add property 
GtkSettings::gtk-menu-popup-delay after class was initialised

(emacs:2436): GLib-GObject-WARNING **: Attempt to add property 
GtkSettings::gtk-menu-popdown-delay after class was initialised

and I cannot figure out where the GLib warnings come from, any ideas?

 Jocke

Message #84 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Clemens Schüller <cs.mlists+bug-gnu-emacs <at> mailbox.org>
To: emacs bugs <bug-gnu-emacs <at> gnu.org>
Cc: ml_emacs-lists <at> binary-island.eu, cyd <at> gnu.org, joakim.tjernlund <at> transmode.se,
 16694 <at> debbugs.gnu.org
Subject: Re: bug#16694: Strange background color problem in gentoo Linux
Date: Wed, 26 Mar 2014 16:58:11 +0100

Hello!


> Sounds similar to several other bug reports since the regression was
> introduced in December. Most discussion is at
> http://debbugs.gnu.org/cgi/bugreport.cgi?bug=16694 .
>
> Chong, it would be nice if you could opine about the problem and
> Matthias's patches. The offending commit was:
>
> 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.

Here is my patch, after I copied the ebuild file in my local overlay:

--- /usr/portage/app-editors/emacs-vcs/emacs-vcs-24.4.9999.ebuild       2014-03-21 15:22:30.000000000 +0100
+++ emacs-vcs-24.4.9999.ebuild  2014-03-24 21:20:49.947795118 +0100
@@ -115,6 +115,9 @@
        fi
 
        epatch_user
+       epatch "${FILESDIR}/0001-lisp-faces.el-Fix-application-of-X-resource-attribut.patch"
+       epatch "${FILESDIR}/0002-lisp-faces.el-Fix-empty-face-handling.patch"
+       epatch "${FILESDIR}/0003-lisp-faces.el-Centralize-no-init-from-x-resources-ch.patch"
 
        # Fix filename reference in redirected man page
        sed -i -e "/^\\.so/s/etags/&-${EMACS_SUFFIX}/" doc/man/ctags.1 \


-- 
Best Regards, Clemens Schüller

Message #90 received at 16694 <at> debbugs.gnu.org (full text, mbox):

From: Glenn Morris <rgm <at> gnu.org>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: Barry OReilly <gundaetiapo <at> gmail.com>, 16694 <at> debbugs.gnu.org
Subject: Re: bug#16694: bugs #16694/#16378: Patches
Date: Wed, 26 Mar 2014 12:03:35 -0400

Eli Zaretskii wrote:

>> http://debbugs.gnu.org/cgi/bugreport.cgi?bug=16434#5
>
> Thanks.  So this is yet another bug to take care of.

Perhaps I merged too many bugs there. It seemed like the same commit
caused all the issues. Hopefully someone has time to review the various
reports and see if any should be unmerged.

Message #93 received at 16694 <at> debbugs.gnu.org (full text, mbox):

From: Barry OReilly <gundaetiapo <at> gmail.com>
To: Joakim Tjernlund <joakim.tjernlund <at> transmode.se>
Cc: 16694 <at> debbugs.gnu.org
Subject: Re: Strange background color problem in gentoo Linux
Date: Wed, 26 Mar 2014 12:49:39 -0400

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

> (emacs:2436): GLib-GObject-WARNING **: Attempt to add property
> GtkSettings::gtk-menu-popdown-delay after class was initialised

Maybe http://debbugs.gnu.org/cgi/bugreport.cgi?bug=16522 , though the
warning is a little different.

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

Message #96 received at 16694 <at> debbugs.gnu.org (full text, mbox):

From: Joakim Tjernlund <joakim.tjernlund <at> transmode.se>
To: Barry OReilly <gundaetiapo <at> gmail.com>
Cc: 16694 <at> debbugs.gnu.org
Subject: Re: Strange background color problem in gentoo GNU/Linux
Date: Wed, 26 Mar 2014 19:12:58 +0100

Barry OReilly <gundaetiapo <at> gmail.com> wrote on 2014/03/26 17:49:39:
> 
> > (emacs:2436): GLib-GObject-WARNING **: Attempt to add property
> > GtkSettings::gtk-menu-popdown-delay after class was initialised
> 
> Maybe http://debbugs.gnu.org/cgi/bugreport.cgi?bug=16522 , though the 
warning is a little different.

Ahh, that seems to be it. On this system gtk+ was at an older version(
2.24.17).
I am rebuilding with 2.24.22 now, thanks :)

 Jocke

Message #99 received at 16694 <at> debbugs.gnu.org (full text, mbox):

From: Joakim Tjernlund <joakim.tjernlund <at> transmode.se>
To: Clemens Schüller <cs.mlists+bug-gnu-emacs <at> mailbox.org>
Cc: ml_emacs-lists <at> binary-island.eu, emacs bugs <bug-gnu-emacs <at> gnu.org>,
 cyd <at> gnu.org, 16694 <at> debbugs.gnu.org
Subject: Re: bug#16694: Strange background color problem in gentoo Linux
Date: Wed, 26 Mar 2014 19:18:41 +0100

Clemens Schüller <cs.mlists+bug-gnu-emacs <at> mailbox.org> wrote on 2014/03/26 
16:58:11:
> 
> Hello!
> 
> 
> > Sounds similar to several other bug reports since the regression was
> > introduced in December. Most discussion is at
> > http://debbugs.gnu.org/cgi/bugreport.cgi?bug=16694 .
> >
> > Chong, it would be nice if you could opine about the problem and
> > Matthias's patches. The offending commit was:
> >
> > 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.
> 
> Here is my patch, after I copied the ebuild file in my local overlay:
> 
> --- /usr/portage/app-editors/emacs-vcs/emacs-vcs-24.4.9999.ebuild 
2014-03-21 15:22:30.000000000 +0100
> +++ emacs-vcs-24.4.9999.ebuild  2014-03-24 21:20:49.947795118 +0100
> @@ -115,6 +115,9 @@
>         fi
> 
>         epatch_user
> +       epatch 
"${FILESDIR}/0001-lisp-faces.el-Fix-application-of-X-resource-attribut.patch"
> +       epatch 
"${FILESDIR}/0002-lisp-faces.el-Fix-empty-face-handling.patch"
> +       epatch 
"${FILESDIR}/0003-lisp-faces.el-Centralize-no-init-from-x-resources-ch.patch"
> 
>         # Fix filename reference in redirected man page
>         sed -i -e "/^\\.so/s/etags/&-${EMACS_SUFFIX}/" doc/man/ctags.1 \

If this is a local fix you are better off using epatch_user than modifying 
the ebuild.
You could also try the easier:
 (setq initial-frame-alist default-frame-alist)

 Jocke

Message #105 received at 16694 <at> debbugs.gnu.org (full text, mbox):

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: Re: 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)]

Message #108 received at 16694 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> IRO.UMontreal.CA>
To: Barry OReilly <gundaetiapo <at> gmail.com>
Cc: Matthias Dahl <ml_emacs-lists <at> binary-island.eu>,
 Clemens Schüller
 <cs.mlists+bug-gnu-emacs <at> mailbox.org>, 16694 <at> debbugs.gnu.org
Subject: Re: bug#16694: bugs #16694/#16378: Patches
Date: Thu, 27 Mar 2014 10:22:57 -0400

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

Not sure if such a person exists, and she does, whether she is reading
this list :-(


        Stefan

Message #111 received at 16694 <at> debbugs.gnu.org (full text, mbox):

From: Barry OReilly <gundaetiapo <at> gmail.com>
To: 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
Subject: Re: bug#16694: bugs #16694/#16378: Patches
Date: Fri, 28 Mar 2014 10:59:22 -0400

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

The last patch I indicated is my best guess safe patch that solves the
--reverse-video symptom I care about. To summarize my rationale: an
unconditional call to make-face-x-resource-internal was moved down into
face-spec-recalc, but at least one other caller of face-spec-recalc clearly
doesn't want an unconditional call to make-face-x-resource-internal. Moving
the make-face-x-resource-internal call back up one call level to the caller
which the offending patch touched thus seems right.

Matthias, in your patch, that same make-face-x-resource-internal call was
moved to an earlier line in the same face-spec-recalc function, so it's not
obvious how to reconcile your patch with mine. The concern I raised applies
equally to your patch. Maybe you could give my patch a go to see if it has
any effect on your ill symptom. If not, then perhaps you have an idea about
reconciling the patches?

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

Message #114 received at 16694 <at> debbugs.gnu.org (full text, mbox):

From: Matthias Dahl <ml_emacs-lists <at> binary-island.eu>
To: Barry OReilly <gundaetiapo <at> gmail.com>
Cc: 16434 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>,
 Clemens Schüller <cs.mlists+bug-gnu-emacs <at> mailbox.org>,
 Stefan Monnier <monnier <at> IRO.UMontreal.CA>, 16694 <at> debbugs.gnu.org
Subject: Re: bug#16694: bugs #16694/#16378: Patches
Date: Fri, 28 Mar 2014 16:15:27 +0100

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

Message #117 received at 16694 <at> debbugs.gnu.org (full text, mbox):

From: Barry OReilly <gundaetiapo <at> gmail.com>
To: Matthias Dahl <ml_emacs-lists <at> binary-island.eu>
Cc: 16434 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>,
 Clemens Schüller <cs.mlists+bug-gnu-emacs <at> mailbox.org>,
 Stefan Monnier <monnier <at> iro.umontreal.ca>, 16694 <at> debbugs.gnu.org
Subject: Re: bug#16694: bugs #16694/#16378: Patches
Date: Tue, 1 Apr 2014 13:15:23 -0400

[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)]

Message #120 received at 16694 <at> debbugs.gnu.org (full text, mbox):

From: Matthias Dahl <ml_emacs-lists <at> binary-island.eu>
To: Barry OReilly <gundaetiapo <at> gmail.com>
Cc: 16434 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>,
 Clemens Schüller <cs.mlists+bug-gnu-emacs <at> mailbox.org>,
 Stefan Monnier <monnier <at> iro.umontreal.ca>, 16694 <at> debbugs.gnu.org
Subject: Re: bug#16694: bugs #16694/#16378: Patches
Date: Tue, 01 Apr 2014 20:17:52 +0200

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

Message #123 received at 16694 <at> debbugs.gnu.org (full text, mbox):

From: Matthias Dahl <ml_emacs-lists <at> binary-island.eu>
To: Barry OReilly <gundaetiapo <at> gmail.com>
Cc: 16434 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>,
 Clemens Schüller <cs.mlists+bug-gnu-emacs <at> mailbox.org>,
 Stefan Monnier <monnier <at> iro.umontreal.ca>, 16694 <at> debbugs.gnu.org
Subject: Re: bug#16694: bugs #16694/#16378: Patches
Date: Wed, 02 Apr 2014 17:04:19 +0200

[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)]

Message #126 received at 16694 <at> debbugs.gnu.org (full text, mbox):

From: Barry OReilly <gundaetiapo <at> gmail.com>
To: Matthias Dahl <ml_emacs-lists <at> binary-island.eu>
Cc: 16434 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>,
 Clemens Schüller <cs.mlists+bug-gnu-emacs <at> mailbox.org>,
 Stefan Monnier <monnier <at> iro.umontreal.ca>, 16694 <at> debbugs.gnu.org
Subject: Re: bug#16694: bugs #16694/#16378: Patches
Date: Wed, 2 Apr 2014 12:47:55 -0400

[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)]

Message #129 received at 16694 <at> debbugs.gnu.org (full text, mbox):

From: Matthias Dahl <ml_emacs-lists <at> binary-island.eu>
To: Barry OReilly <gundaetiapo <at> gmail.com>
Cc: 16434 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>,
 Clemens Schüller <cs.mlists+bug-gnu-emacs <at> mailbox.org>,
 Stefan Monnier <monnier <at> iro.umontreal.ca>, 16694 <at> debbugs.gnu.org
Subject: Re: bug#16694: bugs #16694/#16378: Patches
Date: Wed, 02 Apr 2014 20:36:08 +0200

[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)]

Message #132 received at 16694 <at> debbugs.gnu.org (full text, mbox):

From: Barry OReilly <gundaetiapo <at> gmail.com>
To: Matthias Dahl <ml_emacs-lists <at> binary-island.eu>
Cc: 16434 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>,
 Clemens Schüller <cs.mlists+bug-gnu-emacs <at> mailbox.org>,
 Stefan Monnier <monnier <at> iro.umontreal.ca>, 16694 <at> debbugs.gnu.org
Subject: Re: bug#16694: bugs #16694/#16378: Patches
Date: Wed, 2 Apr 2014 15:34:59 -0400

>> 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.

Message #135 received at 16694-done <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Matthias Dahl <ml_emacs-lists <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
Subject: Re: bug#16694: bugs #16694/#16378: Patches
Date: Sat, 05 Apr 2014 10:52:54 +0300

> 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.

Message #138 received at 16694-done <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> IRO.UMontreal.CA>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: Matthias Dahl <ml_emacs-lists <at> binary-island.eu>, 16434-done <at> debbugs.gnu.org,
 gundaetiapo <at> gmail.com, 16378-done <at> debbugs.gnu.org,
 cs.mlists+bug-gnu-emacs <at> mailbox.org, 16694-done <at> debbugs.gnu.org
Subject: Re: bug#16694: bugs #16694/#16378: Patches
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.


        Stefan

Message #141 received at 16694-done <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Stefan Monnier <monnier <at> IRO.UMontreal.CA>
Cc: ml_emacs-lists <at> binary-island.eu, 16434-done <at> debbugs.gnu.org,
 gundaetiapo <at> gmail.com, 16378-done <at> debbugs.gnu.org,
 cs.mlists+bug-gnu-emacs <at> mailbox.org, 16694-done <at> debbugs.gnu.org
Subject: Re: bug#16694: bugs #16694/#16378: Patches
Date: Sat, 05 Apr 2014 19:15:53 +0300

> 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.

Message #144 received at 16694-done <at> debbugs.gnu.org (full text, mbox):

From: Matthias Dahl <ml_emacs-lists <at> binary-island.eu>
To: Eli Zaretskii <eliz <at> gnu.org>, 
 Stefan Monnier <monnier <at> IRO.UMontreal.CA>
Cc: gundaetiapo <at> gmail.com, 16434-done <at> debbugs.gnu.org,
 cs.mlists+bug-gnu-emacs <at> mailbox.org, 16378-done <at> debbugs.gnu.org,
 16694-done <at> debbugs.gnu.org
Subject: Re: bug#16694: bugs #16694/#16378: Patches
Date: Mon, 07 Apr 2014 11:58:02 +0200

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

Message #147 received at 16694-done <at> debbugs.gnu.org (full text, mbox):

From: Matthias Dahl <ml_emacs-lists <at> binary-island.eu>
To: Eli Zaretskii <eliz <at> gnu.org>, 
 Stefan Monnier <monnier <at> IRO.UMontreal.CA>
Cc: gundaetiapo <at> gmail.com, 16434-done <at> debbugs.gnu.org,
 cs.mlists+bug-gnu-emacs <at> mailbox.org, 16378-done <at> debbugs.gnu.org,
 16694-done <at> debbugs.gnu.org
Subject: Re: bug#16694: bugs #16694/#16378: Patches
Date: Wed, 09 Apr 2014 11:49:25 +0200

[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)]

Message #150 received at 16694-done <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: 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
Subject: Re: bug#16694: bugs #16694/#16378: Patches
Date: Sat, 12 Apr 2014 17:32:44 +0300

> 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.)

Message #153 received at 16694-done <at> debbugs.gnu.org (full text, mbox):

From: Matthias Dahl <matthias.dahl <at> binary-island.eu>
To: Eli Zaretskii <eliz <at> gnu.org>, 
 Stefan Monnier <monnier <at> IRO.UMontreal.CA>
Cc: gundaetiapo <at> gmail.com, 16434-done <at> debbugs.gnu.org,
 cs.mlists+bug-gnu-emacs <at> mailbox.org, 16378-done <at> debbugs.gnu.org,
 16694-done <at> debbugs.gnu.org
Subject: Re: bug#16694: bugs #16694/#16378: Patches
Date: Sat, 12 Apr 2014 13:37:11 +0200

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

Message #156 received at 16694-done <at> debbugs.gnu.org (full text, mbox):

From: Matthias Dahl <matthias.dahl <at> binary-island.eu>
To: Eli Zaretskii <eliz <at> gnu.org>
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
Subject: Re: bug#16694: bugs #16694/#16378: Patches
Date: Sat, 12 Apr 2014 16:57:46 +0200

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

Previous Next

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