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

[Dmitry Gutov <dgutov@yandex.ru>]

[-- Attachment #1: Type: text/plain, Size: 4131 bytes --]

On 30.07.2020 17:04, Eli Zaretskii wrote:

>> 1. Apply the patch to company.el.
>> 2. Launch 'emacs -Q -L path/to/company -l company'.
>> 3. Turn on company-mode and whitespace-mode.
>> 4. In the scratch:
>> C-u 11 M-x newline
>> space space space
>> C-u 11 M-x previous-line
>> Type 'c', then M-x company-complete-common
> 
> I installed a fix on master.  The fix is simple enough, but it is in
> code that is used in all cases where faces are used in overlay
> strings, and so I don't want to install this on emacs-27, since the
> Emacs 27.1 release is imminent, and I don't want to delay it any
> longer.  We could discuss later whether to backport to Emacs 27.2.

Thank you. This sounds like a decent compromise: after we agree on a 
fix, it will have some time to mature on the master branch.

But the installed fix doesn't solve the other scenario, depicted on the 
second screenshot attached to this report: 
https://debbugs.gnu.org/cgi/bugreport.cgi?msg=5;att=3;filename=Screenshot+from+2020-07-26+20-59-25.png;bug=42552

Do you need a step-by-step repro for that?

The reason for that difference seems to be that it's a log-edit buffer, 
and the delimiter line is fontified using an anonymous face '(:height 
0.1 :inverse-video t :extend t), see the end of log-edit-font-lock-keywords.

Still, Emacs 26.3 doesn't exhibit this problem, and in that version the 
contents of that anonymous face was the same (except without :extend t, 
but back then, all faces did "extend"). (*)

Would it be too hard to have the same behavior in 28+?

The log-edit case isn't too important by itself, but the same thing can 
happen if, for example, the end of the completion overlay falls on a 
smerge region, for example. I'll attach a screenshot with this, too.

Furthermore, in Emacs 26.3 I can propertize the newlines in the overlay 
string with '(face region) and see the "extend" effect. Or keep them 
with 'default' face and see no "extend" effect on those lines.

So with some more work, if the behavior becomes more 26.3-compatible, I 
think I can implement even more accurate rendering, where only the lines 
which need "extending" are extended (in the popup's background).

>> The solution I hoped would fix this, which we discussed in
>> https://debbugs.gnu.org/cgi/bugreport.cgi?bug=38563#60, does not work
>> when the popup is rendered with 'after-string' instead of 'display'
>> overlay property (and a similar problem exists for 'before-string' as
>> well). Hence the title of this bug report.
> 
> Setting the 'face' property of an overlay with the intent of affecting
> the display of an overlay string never worked in Emacs, and the
> comments to the code even mention it (note the last sentence):
> 
>    /* Return the face ID at buffer position POS for displaying ASCII
>       characters associated with overlay strings for overlay OVERLAY.
> 
>       Like face_at_buffer_position except for OVERLAY.  Currently it
>       simply disregards the `face' properties of all overlays.  */
> 
>    int
>    face_for_overlay_string (struct window *w, ptrdiff_t pos,

But it works! That's how we closed bug#38563, didn't we?

To see it have an effect, launch Emacs built from the emacs-27 branch 
(to be 100% sure, but it seems this works with the current master too), 
then go through the scenario outlined in 
https://debbugs.gnu.org/cgi/bugreport.cgi?bug=38563#11.

Use the version of company currently in ELPA, with no extra patches.

You will see the bug fail to reproduce.

Now, delete (or comment out) the line

    (overlay-put ov 'face 'default)

from the function company-pseudo-tooltip-unhide, then re-evaluate it. 
Try the scenario again: the bug is back.

Not only does it help with the :extend property, it also helps to avoid 
inheriting other properties, such as :bold. But only if the overlay 
string is applied via 'display', not 'after-string'.

To be clear, I would prefer the fix of the first variety (making 
behavior more compatible with 26.3), rather than just being able to 
override the "underlying face" using the 'face' property. But either is 
better than nothing, and having both would be ideal.

[-- Attachment #2: Screenshot from 2020-08-03 01-31-51.png --]
[-- Type: image/png, Size: 73298 bytes --]