GNU bug report logs - #16378
Empty face settings ignored

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 16378 in the body.
You can then email your comments to 16378 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: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: bug-gnu-emacs <at> gnu.org
Subject: Empty face settings ignored
Date: Mon, 06 Jan 2014 20:14:23 -0500

Package: Emacs
Version: 24.3.50


Some recent change made Emacs ignore "empty" face settings:

   src/emacs -Q lisp/minibuffer.el
   M-x customize-face RET RET
   click on the "Inherit" button so as to disable inheritance: the result
   should be a face that does not specify any particular appearance.
   Middle click "State => Set for current session"
   and notice that the color is still the same.


-- Stefan

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

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Matthias Dahl <ml_emacs-lists <at> binary-island.eu>
Cc: 16378 <at> debbugs.gnu.org
Subject: Re: Empty face settings ignored (was bug#16694: Regression by commit
 115663 (bzr): Theme face attributes no longer take precedence over X
 resources)
Date: Wed, 26 Feb 2014 10:49:58 -0500

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

The desired behavior is that after middle clicking "State => Set for
current session", the "sample" (and all code highlighted with that
font-lock-comment-delimiter-face) should be displayed with the default
color used for all non-highlighted text (i.e. a black foreground,
typically), whereas it's still displayed using the reddish color of
font-lock-comment-face.
IOW, even though I specifically asked to remove the inheritance between 
font-lock-comment-delimiter-face and font-lock-comment-face, this
inheritance is still present.

Note that this has nothing to do with inheritance.  The same recipe
works for other properties:

   src/emacs -Q lisp/minibuffer.el
   C-e
   M-x customize-face RET RET
   [ So this time we get font-lock-comment-face. ]
   click on the "Foreground" button so as to disable the reddish color:
   the result should be a face that does not specify any particular
   appearance.  Middle click "State => Set for current session" and
   notice that the face is still using the same reddish foreground.


-- Stefan

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

From: Matthias Dahl <ml_emacs-lists <at> binary-island.eu>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 16378 <at> debbugs.gnu.org
Subject: Re: Empty face settings ignored (was bug#16694: Regression by commit
 115663 (bzr): Theme face attributes no longer take precedence over X
 resources)
Date: Thu, 27 Feb 2014 20:05:46 +0100

Thanks Stefan for the explanation, that was quite helpful. I'll have a
look and see if I can find the cause. I'll keep you posted.

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

From: Matthias Dahl <ml_emacs-lists <at> binary-island.eu>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 16378 <at> debbugs.gnu.org
Subject: Re: Empty face settings ignored (was bug#16694: Regression by commit
 115663 (bzr): Theme face attributes no longer take precedence over X
 resources)
Date: Sun, 02 Mar 2014 15:26:18 +0100

FYI: I found the cause and I've a patch in the pipeline to correct this
which needs further work at this point. Since I'm quite pressed for time
today, it might take another day or two.

Message #17 received at 16378 <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, 16378 <at> debbugs.gnu.org
Subject: Re: bug#16378: Empty face settings ignored (was bug#16694: Regression
 by	commit 115663 (bzr): Theme face attributes no longer take	precedence over
 X resources)
Date: Sun, 02 Mar 2014 18:56:34 +0200

> Date: Sun, 02 Mar 2014 15:26:18 +0100
> From: Matthias Dahl <ml_emacs-lists <at> binary-island.eu>
> Cc: 16378 <at> debbugs.gnu.org
> 
> FYI: I found the cause and I've a patch in the pipeline to correct this
> which needs further work at this point. Since I'm quite pressed for time
> today, it might take another day or two.

Take your time, and thanks.

Message #20 received at 16378 <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 #23 received at 16378 <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 #26 received at 16378 <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 #31 received at 16378-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 #34 received at 16378-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 #37 received at 16378-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 #40 received at 16378-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 #43 received at 16378-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 #46 received at 16378-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 #49 received at 16378-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 #52 received at 16378-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.