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

[Eli Zaretskii]

> Cc: 37774@debbugs.gnu.org, juri@linkov.net
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Sat, 7 Dec 2019 20:55:31 +0200
> 
> > We are not trying to change the core behavior, we are trying to change
> > how themes define faces, in a way that makes the :extend attribute be
> > implicitly inherited from the default spec of the face to the face
> > after theme's customizations.
> 
> We're really trying to change how a face's attributes are calculated 
> based on its default spec, as well as enabled themes. There are 
> different callers to face-spec-recalc because different user features 
> require that re-calculation.
> 
> > All I want is to make sure no other
> > caller of face-spec-recalc, one that has nothing to do with themes
> > defining faces, picks up the change, because that would be unintended.
> > If you consider this incorrect or unjustified, please explain why.
> 
> Here's some examples:
> 
> enable-theme needs that recalculation because a different set of themes 
> is now in effect, and face attributes need to be updated.
> 
> face-set-after-frame-default needs that because a frame's parameters can 
> affect how faces look as well.
> 
> frame-set-background-mode needs that to update how face specs are 
> interpreted given the changed background mode.
> 
> All of these affect how a face spec is evaluated, which affects how 
> theme specs and user specs apply to the face. Which should be able to 
> change which spec the value of :extend is taken from.
> 
> Or, to look at it from another direction: if we create a special 
> different version of face-spec-recalc purely for custom-theme-set-faces, 
> and face-set-after-frame-default wouldn't use it, whatever changed logic 
> we implement wouldn't apply to new frames.

Our goal is to allow themes "inherit" the :extend attribute without
having to specify it in their face specs, unlike with other
attributes.  That's the only goal; we don't want :extend to behave
differently from other face attributes in any other context.

If you are saying that we cannot make this change apply only to face
definitions by themes, then it means we don't really understand what
we could break here, and then I don't think I want this change in
Emacs 27.  Sorry, it's too risky.

(I thought cus-face.el stores information in symbol properties that
enables it to apply the face attributes in a special way.  But I don't
consider myself an expert on these matters, so if you say we cannot
differentiate between general face definition and what themes do, so
be it.)

> I'm not sure :extend is always the last pair in the list.
> 
> ELISP> (plist-member '(:a 1 :b 2 :c 3) :b)
> (:b 2 :c 3)

You are right, your code is more correct.