bug#42552: 28.0.50; Overlay 'face' property doesn't set the "underlying face" for 'after-string'

[Dmitry Gutov <dgutov@yandex.ru>]

On 05.08.2020 17:58, Eli Zaretskii wrote:

>> Should the "overlay string" obey the underlying face, though? It doesn't
>> obey the 'face' property, like you explained. Seems inconsistent.
> 
> Emacs always worked this way, so changing it now is probably a big
> deal.  AFAIU, the reason for this behavior is so that overlay strings
> which specify no faces use the same face as the surrounding text.
> Which sounds reasonable.

Do you imagine creating a better consistency the other way (by having 
the 'face' property affect overlay strings) would be as likely to create 
problems?

>>> Emacs 26 and before simply extended the face of the last character of
>>> the line.  Emacs 27 doesn't do that, it examines the faces in effect
>>> anew, filtering out those which don't have the :extend attribute set,
>>> so the result is different.  This is exactly the change in behavior
>>> that was intended, not a bug.
>>
>> But it should obey :extend set to nil, shouldn't it?
> 
> It does, but :extend nil doesn't override :extend t, it just says that
> the face with a nil :extend attribute should not be considered when
> computing the face for the empty space past EOL.

BTW, are there other attributes with a similar property? For example, I 
find that I have to add (:inverse-video nil) explicitly to the computed 
face which would be appended to the overlay string's 'face' properties.

Otherwise, the newlines are still "extended" with the "inverse video" 
effect. Try this for an example:

1. Eval:

(with-silent-modifications
   (insert (propertize "abc"
                       'font-lock-face
                       '((:background "green" :extend t)
                         default
                         ( :inverse-video t
                           :foreground "yellow"
                           :extend t)))))

2. C-j

The "extended" newline is yellow.

>> That seems to work. It also has the significant benefit on working in
>> Emacs 27, so if that's the approach you recommend in the end, the fix
>> you already pushed to master might be unnecessary.
> 
> The fix I pushed might not be necessary for company's sake, but it is
> necessary for other use cases, because without it we don't behave as
> documented when :extend attribute is not present: we extend using the
> wrong background color.

All right, thank you.

I'll try to implement this all as suggested, and will come back after.