bug#16789: 24.3; load-theme ignores some faces the value of a attribute is nil

bug#16789: 24.3; load-theme ignores some faces the value of a attribute is nil

[kuanyui]

load-theme is invalid if a value of a attribute in defface is nil.
Or more accurately, it seems to ignore some faces under some situation.


Define a theme, for example, OUR-THEME.el

Magit inherit its diff faces from diff.el, like this:

    (defface magit-diff-add
      '((t :inherit diff-added))
      "Face for lines in a diff that have been added."
      :group 'magit-faces)

If you've defined a face for `diff-added` like this:

`(diff-added ((((class color) (min-colors 89))
           (:foreground "#008700" :background "#d7ff5f" :bold t))))

But now you want to *remove* the :background of `magit-diff-add`,
so add a nil on :background :

    `(magit-diff-add ((((class color) (min-colors 89))
            (:foreground "#008700" :background nil :bold t))))

Then, restart Emacs (to ensure Emacs would be "clean"), `require 'magit`
and `load-theme OUR-THEME`; you will found load-theme IGNORES the whole 
line defination of magit-diff-add, And magit-diff-add still only
inherits diff-added.

* Most strange is, if you `find-file` OUR-THEME.el and M-x eval-buffer,
the :background nil TAKE EFFECT. *

I ask this issue because this strange behavior is an obstacle on 
defining theme.

I've tested on my system and write some test, and reproducible.

-----

kuanyui 20140218

In GNU Emacs 24.3.1 (x86_64-suse-linux-gnu, GTK+ Version 3.10.4)
 of 2014-01-23 on cloud107
Windowing system distributor `The X.Org Foundation', version 11.0.11403901
System Description:	openSUSE 13.1 (Bottle) (x86_64)

Configured using:
 `configure '--with-pop' '--without-hesiod' '--with-kerberos'
 '--with-kerberos5' '--with-xim' '--with-wide-int' '--enable-autodepend'
 '--prefix=/usr' '--mandir=/usr/share/man' '--infodir=/usr/share/info'
 '--datadir=/usr/share' '--localstatedir=/var'
 '--sharedstatedir=/var/lib' '--libexecdir=/usr/lib' '--with-x'
 '--with-sound' '--with-xpm' '--with-jpeg' '--with-tiff' '--with-gif'
 '--with-png' '--with-rsvg' '--with-dbus' '--without-gpm'
 '--with-x-toolkit=gtk3' '--x-includes=/usr/include'
 '--x-libraries=/usr/lib64' '--with-xft' '--with-libotf'
 '--with-m17n-flt' '--build=x86_64-suse-linux'
 'build_alias=x86_64-suse-linux' 'CFLAGS=-fmessage-length=0
 -grecord-gcc-switches -fstack-protector -O2 -Wall -D_FORTIFY_SOURCE=2
 -funwind-tables -fasynchronous-unwind-tables -g -D_GNU_SOURCE
 -std=gnu89 -pipe -Wno-pointer-sign -Wno-unused-variable
 -Wno-unused-label -Wno-unprototyped-calls -fno-optimize-sibling-calls
 -DSYSTEM_PURESIZE_EXTRA=55000 -DSITELOAD_PURESIZE_EXTRA=10000 '
 'LDFLAGS=-Wl,-O2 -Wl,--hash-size=65521''

bug#16789: 24.3; load-theme ignores some faces the value of a attribute is nil

[Eli Zaretskii]

> From: kuanyui <azazabc123@gmail.com>
> Date: Tue, 18 Feb 2014 13:40:20 +0800
> 
> 
> load-theme is invalid if a value of a attribute in defface is nil.
> Or more accurately, it seems to ignore some faces under some situation.
> 
> 
> Define a theme, for example, OUR-THEME.el
> 
> Magit inherit its diff faces from diff.el, like this:
> 
>     (defface magit-diff-add
>       '((t :inherit diff-added))
>       "Face for lines in a diff that have been added."
>       :group 'magit-faces)
> 
> If you've defined a face for `diff-added` like this:
> 
> `(diff-added ((((class color) (min-colors 89))
>            (:foreground "#008700" :background "#d7ff5f" :bold t))))
> 
> But now you want to *remove* the :background of `magit-diff-add`,
> so add a nil on :background :
> 
>     `(magit-diff-add ((((class color) (min-colors 89))
>             (:foreground "#008700" :background nil :bold t))))
> 
> Then, restart Emacs (to ensure Emacs would be "clean"), `require 'magit`
> and `load-theme OUR-THEME`; you will found load-theme IGNORES the whole 
> line defination of magit-diff-add, And magit-diff-add still only
> inherits diff-added.
> 
> * Most strange is, if you `find-file` OUR-THEME.el and M-x eval-buffer,
> the :background nil TAKE EFFECT. *
> 
> I ask this issue because this strange behavior is an obstacle on 
> defining theme.

Isn't this the consequence of this change:

  *** Face specs set via Custom themes now replace the `defface' spec
  rather than inheriting from it (as do face specs set via Customize).

bug#16789: 24.3; load-theme ignores some faces the value of a attribute is nil

[Alex Kost]

kuanyui (2014-02-18 09:40 +0400) wrote:

> load-theme is invalid if a value of a attribute in defface is nil.
> Or more accurately, it seems to ignore some faces under some situation.
>
>
> Define a theme, for example, OUR-THEME.el
>
> Magit inherit its diff faces from diff.el, like this:
>
>     (defface magit-diff-add
>       '((t :inherit diff-added))
>       "Face for lines in a diff that have been added."
>       :group 'magit-faces)
>
> If you've defined a face for `diff-added` like this:
>
> `(diff-added ((((class color) (min-colors 89))
>            (:foreground "#008700" :background "#d7ff5f" :bold t))))
>
> But now you want to *remove* the :background of `magit-diff-add`,
> so add a nil on :background :
>
>     `(magit-diff-add ((((class color) (min-colors 89))
>             (:foreground "#008700" :background nil :bold t))))
>
> Then, restart Emacs (to ensure Emacs would be "clean"), `require 'magit`
> and `load-theme OUR-THEME`; you will found load-theme IGNORES the whole 
> line defination of magit-diff-add, And magit-diff-add still only
> inherits diff-added.
>
> * Most strange is, if you `find-file` OUR-THEME.el and M-x eval-buffer,
> the :background nil TAKE EFFECT. *
>
> I ask this issue because this strange behavior is an obstacle on 
> defining theme.

It is so in 24.3.1, but it works OK in 24.3.50.

You may look at <http://debbugs.gnu.org/cgi/bugreport.cgi?bug=16658> for details.