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 19:06:08 +0200
> 
> > I meant to avoid such consequences by making sure those other callers
> > can never trigger this new processing of :extend.
> 
> Eli, what you're asking for would be actively harmful.
> 
> To make an analogy, when we're changing a core Emacs behavior, we 
> shouldn't try to make it work only on Tuesdays. Even if the original bug 
> reporter observed the problem on a Tuesday.

Sorry, I don't see the analogy.

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

> Can we please focus on the more pressing question: whether the proposed 
> patch actually works, and does that reliably, or are there 
> scenarios/configurations where it would do something unexpected.

We were considering only one scenario: that of a theme defining its
own face specs.  face-spec-recalc is called in other scenarios, but I
don't think we should consider them, because we don't want to change
the behavior in any of those other scenarios.

> >    . the theme doesn't specify :extend
> >    . the default spec for a face specifies ':extend nil'
> > 
> > In this case, after applying the theme, the face should have
> > ':extend nil', implicitly "inherited" from the default spec.  Do you
> > agree?
> 
> Ok, I think I understand the distinction: it's for when a character has 
> several faces specified for it.

Yes, it's important when merging with those other faces that are in
effect for displaying a character.

> +    (when (and theme-face-applied
> +               (eq 'unspecified (face-attribute face :extend frame t)))
> +      (let ((tail (plist-member default-attrs :extend)))
> +        (and tail (face-spec-set-2 face frame
> +                                   (list :extend (cadr tail))))))

This is OK, but why say

    (list :extend (cadr tail))

instead of just

    tail

?  Unless I'm missing something here, the value of 'tail' here should
be (:extend VAL), where VAL is either t or nil.  Right?