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: Tue, 3 Dec 2019 02:01:10 +0200
> 
> > As for the other aspect of this: the idea was that almost all faces do
> > not need to be extended, something that no default will succeed to do
> > silently.
> 
> Whose idea was it?

It's the main idea behind the feature.  Without it, the feature makes
no sense at all.

> I think we can all agree about not extending foreground and related 
> attributes, but extending background is a long-standing behavior. So it 
> would make sense to introduce non-extending backgrounds only in select 
> faces and gradually. I think that's the general expectation in the Emacs 
> community. But we don't have to do this, we can go another way.

I don't think we should restart this discussion, we already had it.

> > The array is computed only once, whereas merging happens many times
> > and in different places.  So we cannot compute the value of an
> > attribute only once, because its value depends on what other faces are
> > being merged, on whether their :extend attribute is set. and on
> > whether the particular merging process cares about :extend.
> 
> I'm not talking about face merging (*).

This whole issue, and in fact the feature itself, is about face
merging, and only about it.  Emacs displays faces by merging
attributes from all of the possible sources of face information that
are in effect at a given buffer position.

> > I don't think I understand your proposal in concrete terms, and you
> > didn't read the code to check your proposal against the actual
> > implementation, so this is a sure way to misunderstandings and talking
> > past each other.
> 
> You haven't pointed at any code to read. So if this email doesn't help 
> reach clarity, could you give some pointers?

I suggest to start with the large comment at the beginning of
xfaces.c, and then proceed to read these functions:

  get_lface_attributes
  merge_face_vectors
  merge_named_face
  merge_face_ref
  internal-make-lisp-face
  internal-set-lisp-face-attribute

The last two should make it clear how defface makes a face with all of
the attributes.

> Like I said, the new property I suggested is a way to go around having 
> to change custom-theme-set-faces in an incompatible way. We would create 
> a second "namespace" for face attributes that wouldn't be overwritten.
> 
> >>      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.
> >>
> >> See above. Just add ':extend t' (or nil) to the end of the value.
> > 
> > So you propose to have both symbol property _and_ a face attribute?
> 
> Yes. Sorry for not making this clear. Since you outlined the "unnamed 
> face value" situation, I've had to amend the idea a little (**). 
> Further, since the :extend attribute would still be used, this wouldn't 
> require too many changes on the C level.
> 
> The new symbol property could be used for different attributes, but it 
> seems like :extend needs it the most.

Sorry, still not clear.  Maybe providing examples of defining a face
to be extended, and then repeating the above in more detail with
references to the examples, would help in clearing the picture.