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

From: Eli Zaretskii <[email-masked]>
Date: Fri, 06 Dec 2019 18:18:41 +0200

> Cc: [email-masked], [email-masked]
> From: Dmitry Gutov <[email-masked]>
> Date: Fri, 6 Dec 2019 17:44:33 +0200
> 
> 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.

Thanks, but is it clean enough to do such exemption for :extend?

And if we want to do this, why do it in face-spec-recalc and not in
custom-set-faces itself?  The latter will not risk producing
unintended consequences for callers of face-spec-recalc other than
custom-set-faces.

> -    ;; defface spec entirely (rather than inheriting from it).  If
> -    ;; there was no spec applicable to FRAME, apply the defface spec
> -    ;; as well as any applicable X resources.
> +    ;; defface spec entirely rather than inheriting from it, with the
> +    ;; exception of the :extend attribute (which is inherited).

You slightly modified the syntax of the comment, probably a typo.

> +    (when (and theme-face-applied (not themed-extend-attr))
> +      (let ((extend-p (plist-get default-attrs :extend)))
> +        (and extend-p (face-spec-set-2 face frame '(:extend t)))))
                                                     ^^^^^^^^^^^^
I think this should be extend-p instead, because the face's default
spec could legitimately say ":extend nil".  Right?