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

From: Eli Zaretskii <[email-masked]>
Date: Tue, 26 Nov 2019 19:43:01 +0200

> Cc: [email-masked], [email-masked]
> From: Dmitry Gutov <[email-masked]>
> Date: Tue, 26 Nov 2019 01:50:23 +0200
> 
> On 25.11.2019 18:00, Eli Zaretskii wrote:
> 
> > Feel free to make that change (although when did you last see a
> > context diff?).
> 
> It's not for context diffs, it's for context around the changes in 
> unified diffs as well. Notice the gray background on the screenshot.

Ah, okay.  Still, feel free to change its :extend attribute.

> > We need to modify all the themes we provide to specify :extend for
> > faces where we do that by default.  It seems there's no way around
> > that, since the semantics of custom-theme-set-faces is clearly to
> > reset all face attributes to 'unspecified' before applying the face
> > spec, so keeping some attributes from the default face spec is out of
> > the question, unfortunately.  It's clear that the faces stuff was not
> > designed to accommodate addition of attributes easily.
> 
> Seems like it's a consequence of the implementation strategy. There were 
> a couple of others that had been proposed (splitting the attribute in 
> two, with different default values) or using a symbol property.

Splitting into two attributes won't help here (quite the contrary,
since we'd have to deal with 2 new attributes).  As for the symbol
property suggestion, see below.

> You said the latter would complicate the face merging code (which makes 
> sense) and "is extremely unclean". I have to take you at your word here. 

You don't have to take my word for it, the code is there to read and
make up your own mind.

Basically, on the C level each Lisp face is represented by an array of
its attributes, where each array element holds the value of the
corresponding attribute.  Once this array is computed, we can (and do)
manipulate just the array, and for all practical purposes can forget
about the face's symbol.  Using a symbol property would then need to
keep the symbol around at all times, which is inconvenient and would
make the code ugly.

But even if we'd overcome this annoyance, how do you specify this
property for a face like below?

   '(:inherit foo :background "green" :underline "red")

There's no symbol to put the property on.  Do we say that such
anonymous faces cannot support this attribute?  Unclean.