bug#37774: 27.0.50; new :extend attribute broke visuals of all themes and other packages

[Dmitry Gutov]

On 05.12.2019 17:47, Eli Zaretskii wrote:
> > Cc: 37774@debbugs.gnu.org, juri@linkov.net
> > From: Dmitry Gutov <dgutov@yandex.ru>
> > Date: Thu, 5 Dec 2019 03:44:22 +0200
> >
> > The new definition for diff-added would look like:
> >
> >     (defface diff-added
> >       '((default
> >          :inherit diff-changed)
> >         (((class color) (min-colors 257) (background light))
> >          :background "#eeffee")
> >         (((class color) (min-colors 88) (background light))
> >          :background "#ddffdd")
> >         (((class color) (min-colors 88) (background dark))
> >          :background "#335533")
> >         (((class color))
> >          :foreground "green"))
> >       "`diff-mode' face used to highlight added lines.")
> >
> >     (put 'diff-added 'face-transient-spec '((t :extend t)))
>
> OK, and how will this work to countermand the problem with themes?
>
> custom-theme-set-faces calls face-spec-set, which calls
> face-spec-recalc, which starts by resetting all face attributes to
> 'unspecified'.  And the last 2 functions are general-purpose, not
> specific to themes.

Well, the idea was to use a different structure to store the "transient" 
attributes. That could be an extra symbol property, or an additional 
structure for storing faces attributes, in addition to 
default-frame-alist. But looking at the code now, it seems fairly clunky 
and crossing abstraction levels.

It's great that you mentioned face-spec-recalc. It looks just like the 
place to change, since both defface and theme definitions and 
customizations go through it.

We can implement in there a new kind of "face spec" along the lines of 
my previous description, or simply special-case the :extend attribute, 
and take it from the default spec. The latter option is implemented in 
the attached patch, which seems to work in my limited testing.

inherit-face-extend-spec.diff
Description: Text Data