deftheme, :inherit & overriding existing values

deftheme, :inherit & overriding existing values

[Dirk-Jan C. Binnema]

Hi all,

I've 'ported' the zenburn color theme
   https://github.com/djcb/elisp/blob/master/themes/zenburn-theme.el
to the new emacs 24+ color-theme system. I'm using the latest emacs 24 from
bzr.

Overall it works well if I start the theme from my .emacs startup, but it
works less well if I load a theme after some other packages. E.g., deftheme
before wanderlust works, but not the other way around.

Let's look at the 'describe-face' output, I've removed all the 'unspecified'
values.

Loading zenburn before Wanderlust (this works):
,----
| Face: wl-highlight-folder-unread-face (sample) (customize this face)
|        Inherit: (quote zenburn-red-3)
`----

Loading Wanderlust before zenburn (this does not work, i.e. the foreground
color will be 'gold', not 'zenburn-red-3'):

,----
| Face: wl-highlight-folder-unread-face (sample) (customize this face)
|     Foreground: gold
|     Inherit: (quote zenburn-red-3)
`----

Where zenburn-red-3 is defined as
       `(zenburn-red-3 ((t (:foreground ,zenburn-red-3))))

Apparently, :inherit does not really work as I expected...
       
Reloading the theme won't help -- but, what *does* help is visiting the theme
file and issuing M-x eval-buffer.

So, I'm not sure this is actually a *bug*, but the current behaviour seems
suboptimal anyhow. The equivalent of load-theme + eval-buffer gives the
expected result - maybe that should be the default behaviour of load-theme?

Best wishes,
Dirk.

-- 
Dirk-Jan C. Binnema                  Helsinki, Finland
e:djcb@djcbsoftware.nl           w:www.djcbsoftware.nl
pgp: D09C E664 897D 7D39 5047 A178 E96A C7A1 017D DA3C

Re: deftheme, :inherit & overriding existing values

[Christoph Scholtes]

On 5/7/2011 3:17 AM, Dirk-Jan C. Binnema wrote:

> Overall it works well if I start the theme from my .emacs startup, but it
> works less well if I load a theme after some other packages. E.g., deftheme
> before wanderlust works, but not the other way around.

[snip]

> Apparently, :inherit does not really work as I expected...

I think this might be similar behavior to what I reported here:

http://debbugs.gnu.org/cgi/bugreport.cgi?bug=8454

Christoph

bug#8454: deftheme, :inherit & overriding existing values

[Chong Yidong]

Christoph Scholtes <cschol2112@googlemail.com> writes:

> On 5/7/2011 3:17 AM, Dirk-Jan C. Binnema wrote:
>
>> Overall it works well if I start the theme from my .emacs startup,
>> but it works less well if I load a theme after some other
>> packages. E.g., deftheme before wanderlust works, but not the other
>> way around.
>
> [snip]
>
>> Apparently, :inherit does not really work as I expected...
>
> I think this might be similar behavior to what I reported here:
>
> http://debbugs.gnu.org/cgi/bugreport.cgi?bug=8454

I'm afraid both bug reports are too vague for me to figure out exactly
what is supposed to be going wrong.  Could someone provide a precise,
step-by-step recipe for reproducing the problem?

In the case of Christoph's report (Bug#8454), the statement "none of the
custom faces for erc are actually applied" isn't specific, and when I
tried to reproduce this, the buffer produced by M-x erc RET RET RET
seems to display erc-default-face using the Zenburn default.  A more
precise description of the problem is needed.

As for Dirk-Jan's report, it is very inconvenient to reference external
packages like Wanderlust.  Please provide a recipe using something in
Emacs itself.

bug#8454: deftheme, :inherit & overriding existing values

[Chong Yidong]

Chong Yidong <cyd@stupidchicken.com> writes:

> I'm afraid both bug reports are too vague for me to figure out exactly
> what is supposed to be going wrong.  Could someone provide a precise,
> step-by-step recipe for reproducing the problem?

OK, I found a bug in the case where defface is called after loading a
theme with settings for that face.  The defface settings were not being
applied; I've checked a fix into the trunk.  However, I'm not sure this
is the issue you are talking about, since this bug was not specific to
the :inherit property.  Could you check if your problem is fixed?

One possible source of confusion, which may apply to Dirk-Jan's report,
is that :inherit only tells Emacs to consult the parent face if a face
attribute is unspecified.  If the attribute is specified, it overrides
the inheritance.  So if your Custom theme want to "cancel out", say, any
existing foreground attribute setting, you need something like

 '(froob ((t (:foreground unspecified :inherit parent-face))))

bug#8454: deftheme, :inherit & overriding existing values

[Dirk-Jan C. Binnema]

Hi,

>>>>> On Sun, 08 May 2011 16:14:53 -0400, Chong Yidong ("CYD") wrote:

  CYD> Chong Yidong <cyd@stupidchicken.com> writes:
  >> I'm afraid both bug reports are too vague for me to figure out exactly
  >> what is supposed to be going wrong.  Could someone provide a precise,
  >> step-by-step recipe for reproducing the problem?

Sorry about the 'vagueness' - but glad you found it out anyhow.
  
  CYD> OK, I found a bug in the case where defface is called after loading a
  CYD> theme with settings for that face.  The defface settings were not being
  CYD> applied; I've checked a fix into the trunk.  However, I'm not sure this
  CYD> is the issue you are talking about, since this bug was not specific to
  CYD> the :inherit property.  Could you check if your problem is fixed?

No, it does solve the problem, but I guess below confirms my suspicion..
  
  CYD> One possible source of confusion, which may apply to Dirk-Jan's report,
  CYD> is that :inherit only tells Emacs to consult the parent face if a face
  CYD> attribute is unspecified.  If the attribute is specified, it overrides
  CYD> the inheritance.  So if your Custom theme want to "cancel out", say, any
  CYD> existing foreground attribute setting, you need something like

  CYD>  '(froob ((t (:foreground unspecified :inherit parent-face))))

Ok, yes that is what happens. I guess this could work, but in practice it
makes :inherit rather suboptimal -- imagine switching from one theme to
another, you'd have to explicitly cancel out every attribute the other theme
may have set.

Best wishes,
Dirk.

-- 
Dirk-Jan C. Binnema                  Helsinki, Finland
e:djcb@djcbsoftware.nl           w:www.djcbsoftware.nl
pgp: D09C E664 897D 7D39 5047 A178 E96A C7A1 017D DA3C

bug#8454: deftheme, :inherit & overriding existing values

[Christoph Scholtes]

On 5/8/2011 2:14 PM, Chong Yidong wrote:

> OK, I found a bug in the case where defface is called after loading a
> theme with settings for that face.  The defface settings were not being
> applied; I've checked a fix into the trunk.  However, I'm not sure this
> is the issue you are talking about, since this bug was not specific to
> the :inherit property.  Could you check if your problem is fixed?
>
> One possible source of confusion, which may apply to Dirk-Jan's report,
> is that :inherit only tells Emacs to consult the parent face if a face
> attribute is unspecified.  If the attribute is specified, it overrides
> the inheritance.  So if your Custom theme want to "cancel out", say, any
> existing foreground attribute setting, you need something like
>
>   '(froob ((t (:foreground unspecified :inherit parent-face))))

This did not fix it for me.

I think Dirk-Jan and I have the same issue, though, which is the 
behavior of :inherit.

One example from my custom theme:

  '(erc-default-face ((t (:inherit default))))

This is supposed to inherit from

  '(default ((t (:background "#3f3f3f" :foreground "#dcdccc"))))

but it does not work.

This is probably related to what you describe above, but I don't really 
understand it. foregound in erc-default-face is unspecified, right? 
Should the inherit get the attributes from default or not?

I figured out a way to make my custom theme work, though. Loading it 
through customize does not work, but if I open the theme file and eval 
the buffer, it works and erc shows the my custom faces. Maybe this helps 
to troubleshoot the problem?

Christoph

bug#8454: deftheme, :inherit & overriding existing values

[Dirk-Jan C. Binnema]

Hi,

>>>>> On Mon, 09 May 2011 21:18:25 -0600, Christoph Scholtes ("CS") wrote:

  CS> On 5/8/2011 2:14 PM, Chong Yidong wrote:
  >> OK, I found a bug in the case where defface is called after loading a
  >> theme with settings for that face.  The defface settings were not being
  >> applied; I've checked a fix into the trunk.  However, I'm not sure this
  >> is the issue you are talking about, since this bug was not specific to
  >> the :inherit property.  Could you check if your problem is fixed?
  >> 
  >> One possible source of confusion, which may apply to Dirk-Jan's report,
  >> is that :inherit only tells Emacs to consult the parent face if a face
  >> attribute is unspecified.  If the attribute is specified, it overrides
  >> the inheritance.  So if your Custom theme want to "cancel out", say, any
  >> existing foreground attribute setting, you need something like
  >> 
  >> '(froob ((t (:foreground unspecified :inherit parent-face))))

  CS> This did not fix it for me.

  CS> I think Dirk-Jan and I have the same issue, though, which is the
  CS> behavior of :inherit.

Yes -- in fact, before this change, the theme would look as expected if I
loaded it before - say - Wanderlust. However, with this change, that does not
work either anymore.
  
  CS> One example from my custom theme:

  CS>  '(erc-default-face ((t (:inherit default))))

  CS> This is supposed to inherit from

  CS>  '(default ((t (:background "#3f3f3f" :foreground "#dcdccc"))))

  CS> but it does not work.

  CS> This is probably related to what you describe above, but I don't
  CS> really understand it. foregound in erc-default-face is unspecified,
  CS> right? Should the inherit get the attributes from default or not?

  CS> I figured out a way to make my custom theme work, though. Loading it
  CS> through customize does not work, but if I open the theme file and eval
  CS> the buffer, it works and erc shows the my custom faces. Maybe this
  CS> helps to troubleshoot the problem?

Yes, indeed, that is what I do too now:
  
(defun zenburn()
  (interactive)
  (load-theme 'zenburn)
  (load-library "~/.emacs.d/elisp/themes/zenburn-theme.el"))


In general, it might make sense to first go back to 'factory settings' before
changing the theme, as it's quite unlikely that theme2 always overrides
everything from theme1 -- e.g. zenburn redefines hundreds of faces.
  
Best wishes,
Dirk.

-- 
Dirk-Jan C. Binnema                  Helsinki, Finland
e:djcb@djcbsoftware.nl           w:www.djcbsoftware.nl
pgp: D09C E664 897D 7D39 5047 A178 E96A C7A1 017D DA3C