GNU bug report logs - #38563
27.0.50; Company popup renders with newlines (?) inheriting the bg properties of the character at next line's bol

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 38563 in the body.
You can then email your comments to 38563 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: bug-gnu-emacs <at> gnu.org
Subject: 27.0.50; Company popup renders with newlines (?) inheriting the bg
 properties of the character at next line's bol
Date: Wed, 11 Dec 2019 03:13:59 +0200

[Message part 1 (text/plain, inline)]

Here's an unfortunate new behavior which I didn't have time to report
or properly describe before.

When the next line starts on a character that has a distinct background,
that background is inherited when displaying newlines inside the display
string that renders the popup.

Here are two ways I usually trigger it:

1. Initiate completion on the "Author:" line of a LogEdit buffer (the
next line is an inverse-video line).

2. Have at least one space at the beginning of a line, and have
whitespace-mode on to highlight it in red. Then initiate completion on
the preceding line.

Screenshots are attached.

Needless to say, the latest Emacs 26 doesn't exhibit this problem.

[Screenshot from 2019-12-11 03-00-35.png (image/png, attachment)]

[Screenshot from 2019-12-11 03-00-41.png (image/png, attachment)]

Message #8 received at 38563 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Dmitry Gutov <dgutov <at> yandex.ru>
Cc: 38563 <at> debbugs.gnu.org
Subject: Re: bug#38563: 27.0.50;
 Company popup renders with newlines (?) inheriting the bg properties
 of the character at next line's bol
Date: Wed, 11 Dec 2019 19:20:17 +0200

> From: Dmitry Gutov <dgutov <at> yandex.ru>
> Date: Wed, 11 Dec 2019 03:13:59 +0200
> 
> Here are two ways I usually trigger it:
> 
> 1. Initiate completion on the "Author:" line of a LogEdit buffer (the
> next line is an inverse-video line).
> 
> 2. Have at least one space at the beginning of a line, and have
> whitespace-mode on to highlight it in red. Then initiate completion on
> the preceding line.

Thanks, but I don't think I understand how to reproduce this, based on
your descriptions.  Can you show a complete recipe for the second
scenario, starting from "emacs -Q", then loading whitespace-mode and
company-mode, and finally initiating the completion so as to show the
problem?

Message #11 received at 38563 <at> debbugs.gnu.org (full text, mbox):

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 38563 <at> debbugs.gnu.org
Subject: Re: bug#38563: 27.0.50; Company popup renders with newlines (?)
 inheriting the bg properties of the character at next line's bol
Date: Wed, 11 Dec 2019 23:47:30 +0200

[Message part 1 (text/plain, inline)]

On 11.12.2019 19:20, Eli Zaretskii wrote:
>> From: Dmitry Gutov <dgutov <at> yandex.ru>
>> Date: Wed, 11 Dec 2019 03:13:59 +0200
>>
>> Here are two ways I usually trigger it:
>>
>> 1. Initiate completion on the "Author:" line of a LogEdit buffer (the
>> next line is an inverse-video line).
>>
>> 2. Have at least one space at the beginning of a line, and have
>> whitespace-mode on to highlight it in red. Then initiate completion on
>> the preceding line.
> 
> Thanks, but I don't think I understand how to reproduce this, based on
> your descriptions.  Can you show a complete recipe for the second
> scenario, starting from "emacs -Q", then loading whitespace-mode and
> company-mode, and finally initiating the completion so as to show the

1. Launch 'emacs -Q -L path/to/company -l company'.
2. Turn on company-mode and whitespace-mode.
3. In the scratch:
newline
newline
space space space
previous-line
Type 'c', then M-x company-complete-common

Then wait ~3 seconds because of that "you can run this command ..." 
nonsense which is implemented using sit-for. The popup will appear.

Observe how the space to the right of the popup is all yellow (the color 
whitespace-mode assigns to the spaces on the next line by default).

Going back to the original screenshots, it shows two windows, and in 
each example there's a hollow cursor in the "other" window which denotes 
the position where you'd have to initiate completion at to see the other 
example.

[Screenshot from 2019-12-11 23-34-27.png (image/png, attachment)]

Message #14 received at 38563 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Dmitry Gutov <dgutov <at> yandex.ru>
Cc: 38563 <at> debbugs.gnu.org
Subject: Re: bug#38563: 27.0.50; Company popup renders with newlines (?)
 inheriting the bg properties of the character at next line's bol
Date: Thu, 12 Dec 2019 13:32:41 +0200

> Cc: 38563 <at> debbugs.gnu.org
> From: Dmitry Gutov <dgutov <at> yandex.ru>
> Date: Wed, 11 Dec 2019 23:47:30 +0200
> 
> 1. Launch 'emacs -Q -L path/to/company -l company'.
> 2. Turn on company-mode and whitespace-mode.
> 3. In the scratch:
> newline
> newline
> space space space
> previous-line
> Type 'c', then M-x company-complete-common
> 
> Then wait ~3 seconds because of that "you can run this command ..." 
> nonsense which is implemented using sit-for. The popup will appear.
> 
> Observe how the space to the right of the popup is all yellow (the color 
> whitespace-mode assigns to the spaces on the next line by default).

Thanks.

The "character at next bol" sounds strange, since the display engine
has no look-ahead -- it never examines characters on the next line
while displaying the current line.  But it all starts making sense
when you recall that Company mode puts its overlay on that next line.
So the "inherited" face is not on the next line, it is at the position
where the Company overlay is set.  IOW, it's the "underlying face" for
the overlay string.

Should be fixed now, please test.

Btw, the bug is triggered because Company mode uses a weird '(default)
face, a list, instead of just 'default.  This is valid, but it wastes
a slot in the frame's face cache, so perhaps there's a good reason to
avoid that and simplify '(default) to 'default when you propertize the
tooltip text.

Message #17 received at 38563 <at> debbugs.gnu.org (full text, mbox):

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 38563 <at> debbugs.gnu.org
Subject: Re: bug#38563: 27.0.50; Company popup renders with newlines (?)
 inheriting the bg properties of the character at next line's bol
Date: Fri, 13 Dec 2019 00:13:37 +0200

On 12.12.2019 13:32, Eli Zaretskii wrote:
> The "character at next bol" sounds strange, since the display engine
> has no look-ahead -- it never examines characters on the next line
> while displaying the current line.  But it all starts making sense
> when you recall that Company mode puts its overlay on that next line.
> So the "inherited" face is not on the next line, it is at the position
> where the Company overlay is set.  IOW, it's the "underlying face" for
> the overlay string.

Yeah, OK. Now that you mentioned the 'default' face, I remembered: it's 
used there exactly so that we don't inherit the background from the 
"underlying face".

> Should be fixed now, please test.

It looks fixed in the whitespace-mode example, but not in the other one.

Just call M-x company-complete-common on the "Author:" line in a LogEdit 
buffer to reproduce. (I've tested common d7efe98951).

By the way, I kind of wonder why the fix added more lines than it 
deleted. Before, this feature just worked. Was that simply by accident? 
Or were the changes brought in by :extend major enough?

> Btw, the bug is triggered because Company mode uses a weird '(default)
> face, a list, instead of just 'default.  This is valid, but it wastes
> a slot in the frame's face cache, so perhaps there's a good reason to
> avoid that and simplify '(default) to 'default when you propertize the
> tooltip text.

Ultimately, the reason it's there like that is because 
font-lock-append-text-property coerces all values to lists. We could 
change that and maybe save some memory in the process. It would be an 
(minor) incompatible change, however.

Message #20 received at 38563 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Dmitry Gutov <dgutov <at> yandex.ru>
Cc: 38563 <at> debbugs.gnu.org
Subject: Re: bug#38563: 27.0.50; Company popup renders with newlines (?)
 inheriting the bg properties of the character at next line's bol
Date: Fri, 13 Dec 2019 10:22:55 +0200

> Cc: 38563 <at> debbugs.gnu.org
> From: Dmitry Gutov <dgutov <at> yandex.ru>
> Date: Fri, 13 Dec 2019 00:13:37 +0200
> 
> On 12.12.2019 13:32, Eli Zaretskii wrote:
> > The "character at next bol" sounds strange, since the display engine
> > has no look-ahead -- it never examines characters on the next line
> > while displaying the current line.  But it all starts making sense
> > when you recall that Company mode puts its overlay on that next line.
> > So the "inherited" face is not on the next line, it is at the position
> > where the Company overlay is set.  IOW, it's the "underlying face" for
> > the overlay string.
> 
> Yeah, OK. Now that you mentioned the 'default' face, I remembered: it's 
> used there exactly so that we don't inherit the background from the 
> "underlying face".

That's no longer "good enough", see below.

> > Should be fixed now, please test.
> 
> It looks fixed in the whitespace-mode example, but not in the other one.
> 
> Just call M-x company-complete-common on the "Author:" line in a LogEdit 
> buffer to reproduce. (I've tested common d7efe98951).

That's not a bug: the face on that thin line on whose first character
you put the tooltip overlay has a non-nil :extend attribute, so
Company will have to explicitly say ':extend nil' in its face(s) to
countermand that.  Recall that a string from a display property merges
into its face all the attributes from the "underlying" face, so with
the current :extend machinery it is no longer enough just to specify a
background color in the display string's face, as you did before.

Btw, if you used 'default instead of '(default), I think that would
have avoided the issue as well, because the default face gets a
special treatment in this context.

> By the way, I kind of wonder why the fix added more lines than it 
> deleted.

??? I added a condition under which not to merge a face, so how can I
avoid adding a few lines?  The addition is 7 lines of code, including
a small refactoring, all the rest is comments.

> Before, this feature just worked. Was that simply by accident? 
> Or were the changes brought in by :extend major enough?

Previously, whether a face's background was extended to EOL was
determined only by the background color of the newline; now the
:extend attribute determines that independently of the background
color.

Message #23 received at 38563 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: dgutov <at> yandex.ru
Cc: 38563 <at> debbugs.gnu.org
Subject: Re: bug#38563: 27.0.50;
 Company popup renders with newlines (?) inheriting the bg properties
 of the character at next line's bol
Date: Fri, 13 Dec 2019 11:30:50 +0200

> Date: Fri, 13 Dec 2019 10:22:55 +0200
> From: Eli Zaretskii <eliz <at> gnu.org>
> Cc: 38563 <at> debbugs.gnu.org
> 
> > It looks fixed in the whitespace-mode example, but not in the other one.
> > 
> > Just call M-x company-complete-common on the "Author:" line in a LogEdit 
> > buffer to reproduce. (I've tested common d7efe98951).
> 
> That's not a bug: the face on that thin line on whose first character
> you put the tooltip overlay has a non-nil :extend attribute, so
> Company will have to explicitly say ':extend nil' in its face(s) to
> countermand that.

Actually, this won't work.  Let me think some more about this
situation.

Message #26 received at 38563 <at> debbugs.gnu.org (full text, mbox):

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 38563 <at> debbugs.gnu.org
Subject: Re: bug#38563: 27.0.50; Company popup renders with newlines (?)
 inheriting the bg properties of the character at next line's bol
Date: Fri, 13 Dec 2019 12:17:06 +0200

On 13.12.2019 10:22, Eli Zaretskii wrote:

>> It looks fixed in the whitespace-mode example, but not in the other one.
>>
>> Just call M-x company-complete-common on the "Author:" line in a LogEdit
>> buffer to reproduce. (I've tested common d7efe98951).
> 
> That's not a bug: the face on that thin line on whose first character
> you put the tooltip overlay has a non-nil :extend attribute, so
> Company will have to explicitly say ':extend nil' in its face(s) to
> countermand that.  Recall that a string from a display property merges
> into its face all the attributes from the "underlying" face, so with
> the current :extend machinery it is no longer enough just to specify a
> background color in the display string's face, as you did before.

I see. But isn't the issue that the background from the underlying face 
is used at all, rather that it's extended?

But I suppose I could add a face with ':extend nil' and use it in place 
of 'default' there.

> Btw, if you used 'default instead of '(default), I think that would
> have avoided the issue as well, because the default face gets a
> special treatment in this context.

I can bump the requirement to Emacs 24.4 and use add-face-text-property 
there (too bad for setting mouse-face only the slower function is 
available), but as you noted in the follow-up email, it doesn't help 
with the remaining example.

>> By the way, I kind of wonder why the fix added more lines than it
>> deleted.
> 
> ??? I added a condition under which not to merge a face, so how can I
> avoid adding a few lines?  The addition is 7 lines of code, including
> a small refactoring, all the rest is comments.

If the issue is "whether to extend", then indeed, it makes sense, thanks.

>> Before, this feature just worked. Was that simply by accident?
>> Or were the changes brought in by :extend major enough?
> 
> Previously, whether a face's background was extended to EOL was
> determined only by the background color of the newline; now the
> :extend attribute determines that independently of the background
> color.

...but the background color is taken specifically from the face that 
specified :extend?

Message #29 received at 38563 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: dgutov <at> yandex.ru
Cc: 38563 <at> debbugs.gnu.org
Subject: Re: bug#38563: 27.0.50;
 Company popup renders with newlines (?) inheriting the bg properties
 of the character at next line's bol
Date: Fri, 13 Dec 2019 12:31:55 +0200

> Date: Fri, 13 Dec 2019 11:30:50 +0200
> From: Eli Zaretskii <eliz <at> gnu.org>
> Cc: 38563 <at> debbugs.gnu.org
> 
> > That's not a bug: the face on that thin line on whose first character
> > you put the tooltip overlay has a non-nil :extend attribute, so
> > Company will have to explicitly say ':extend nil' in its face(s) to
> > countermand that.
> 
> Actually, this won't work.  Let me think some more about this
> situation.

I think I found a better solution, which should "just work" for you.
Please try the latest master.

Message #34 received at 38563-done <at> debbugs.gnu.org (full text, mbox):

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 38563-done <at> debbugs.gnu.org
Subject: Re: bug#38563: 27.0.50; Company popup renders with newlines (?)
 inheriting the bg properties of the character at next line's bol
Date: Fri, 13 Dec 2019 14:00:26 +0200

On 13.12.2019 12:31, Eli Zaretskii wrote:

> I think I found a better solution, which should "just work" for you.
> Please try the latest master.

It does! Thank you Eli.

Message #37 received at 38563 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Dmitry Gutov <dgutov <at> yandex.ru>
Cc: 38563 <at> debbugs.gnu.org
Subject: Re: bug#38563: 27.0.50; Company popup renders with newlines (?)
 inheriting the bg properties of the character at next line's bol
Date: Fri, 13 Dec 2019 16:12:02 +0200

> Cc: 38563 <at> debbugs.gnu.org
> From: Dmitry Gutov <dgutov <at> yandex.ru>
> Date: Fri, 13 Dec 2019 12:17:06 +0200
> 
> > That's not a bug: the face on that thin line on whose first character
> > you put the tooltip overlay has a non-nil :extend attribute, so
> > Company will have to explicitly say ':extend nil' in its face(s) to
> > countermand that.  Recall that a string from a display property merges
> > into its face all the attributes from the "underlying" face, so with
> > the current :extend machinery it is no longer enough just to specify a
> > background color in the display string's face, as you did before.
> 
> I see. But isn't the issue that the background from the underlying face 
> is used at all, rather that it's extended?

Yes and no.  See below.

> But I suppose I could add a face with ':extend nil' and use it in place 
> of 'default' there.

It wouldn't have worked, because ':extend nil' means the face which
says this is ineligible for face merging when face extension is
considered.  IOW, ':extend nil' cannot countermand some other face
that's being merged which says ':extend t'.

> > Previously, whether a face's background was extended to EOL was
> > determined only by the background color of the newline; now the
> > :extend attribute determines that independently of the background
> > color.
> 
> ...but the background color is taken specifically from the face that 
> specified :extend?

The problem is that the company's tooltip faces don't say ':extend t',
so they are ineligible for merging when the face beyond EOL is
considered.  The only face which was eligible was the face of the
first character of the line where you place your overlay.  Even if it
doesn't have ':extend t', we treat the base face (the first one being
merged) specially: we always treat it as eligible (ever wondered how
come 'default', 'tool-bar', and other "basic" faces get extended
although they don't specify :extend?).

The latest changes simply reset the :extend attribute of the "basic"
face used in the merge process, since I believe this is expected in
all the use cases (fingers crossed that no one comes with a valid use
case where it's not TRT).

Message #40 received at 38563 <at> debbugs.gnu.org (full text, mbox):

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 38563 <at> debbugs.gnu.org
Subject: Re: bug#38563: 27.0.50; Company popup renders with newlines (?)
 inheriting the bg properties of the character at next line's bol
Date: Fri, 13 Dec 2019 17:04:21 +0200

On 13.12.2019 16:12, Eli Zaretskii wrote:

>> But I suppose I could add a face with ':extend nil' and use it in place
>> of 'default' there.
> 
> It wouldn't have worked, because ':extend nil' means the face which
> says this is ineligible for face merging when face extension is
> considered.  IOW, ':extend nil' cannot countermand some other face
> that's being merged which says ':extend t'.

Hmm, that's counter to my intuition how this should work (meaning, 
:extend nil should be used during merging, like it's used during 
inheritance), but maybe this way enables functionality that wouldn't be 
possible otherwise.

>> ...but the background color is taken specifically from the face that
>> specified :extend?
> 
> The problem is that the company's tooltip faces don't say ':extend t',
> so they are ineligible for merging when the face beyond EOL is
> considered.

And we definitely wouldn't want the tooptil faces to say ':extend t'. Or 
else it would not be rectangular.

> The only face which was eligible was the face of the
> first character of the line where you place your overlay.  Even if it
> doesn't have ':extend t', we treat the base face (the first one being
> merged) specially: we always treat it as eligible (ever wondered how
> come 'default', 'tool-bar', and other "basic" faces get extended
> although they don't specify :extend?).

Um, okay. I suppose they have to be extended because we need to draw 
*something* at the place "occupied by" newline.

> The latest changes simply reset the :extend attribute of the "basic"
> face used in the merge process, since I believe this is expected in
> all the use cases (fingers crossed that no one comes with a valid use
> case where it's not TRT).

Fingers crossed.

Thanks.

Message #43 received at 38563 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Dmitry Gutov <dgutov <at> yandex.ru>
Cc: 38563 <at> debbugs.gnu.org
Subject: Re: bug#38563: 27.0.50; Company popup renders with newlines (?)
 inheriting the bg properties of the character at next line's bol
Date: Fri, 13 Dec 2019 17:32:46 +0200

> Cc: 38563 <at> debbugs.gnu.org
> From: Dmitry Gutov <dgutov <at> yandex.ru>
> Date: Fri, 13 Dec 2019 17:04:21 +0200
> 
> On 13.12.2019 16:12, Eli Zaretskii wrote:
> 
> > It wouldn't have worked, because ':extend nil' means the face which
> > says this is ineligible for face merging when face extension is
> > considered.  IOW, ':extend nil' cannot countermand some other face
> > that's being merged which says ':extend t'.
> 
> Hmm, that's counter to my intuition how this should work (meaning, 
> :extend nil should be used during merging, like it's used during 
> inheritance), but maybe this way enables functionality that wouldn't be 
> possible otherwise.

':extend nil' is "used" during merging in the sense that such a face
is skipped when we want a face for extending past EOL.  How else could
we implement that?  Setting :extend to nil means that _none_ of the
other attributes of the face are to be taken into account for merging.

Inheritance just makes the inheriting face implicitly behave as if its
:extend attribute is the same as of the parent face, when the
inheriting face doesn't itself specify :extend, i.e. has it set to
'unspecified'.

> > The problem is that the company's tooltip faces don't say ':extend t',
> > so they are ineligible for merging when the face beyond EOL is
> > considered.
> 
> And we definitely wouldn't want the tooptil faces to say ':extend t'. Or 
> else it would not be rectangular.

Sure.

> > The only face which was eligible was the face of the
> > first character of the line where you place your overlay.  Even if it
> > doesn't have ':extend t', we treat the base face (the first one being
> > merged) specially: we always treat it as eligible (ever wondered how
> > come 'default', 'tool-bar', and other "basic" faces get extended
> > although they don't specify :extend?).
> 
> Um, okay. I suppose they have to be extended because we need to draw 
> *something* at the place "occupied by" newline.

Yes.

Message #46 received at 38563 <at> debbugs.gnu.org (full text, mbox):

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 38563 <at> debbugs.gnu.org
Subject: Re: bug#38563: 27.0.50; Company popup renders with newlines (?)
 inheriting the bg properties of the character at next line's bol
Date: Sat, 14 Dec 2019 01:10:13 +0200

On 13.12.2019 17:32, Eli Zaretskii wrote:

>>> It wouldn't have worked, because ':extend nil' means the face which
>>> says this is ineligible for face merging when face extension is
>>> considered.  IOW, ':extend nil' cannot countermand some other face
>>> that's being merged which says ':extend t'.
>>
>> Hmm, that's counter to my intuition how this should work (meaning,
>> :extend nil should be used during merging, like it's used during
>> inheritance), but maybe this way enables functionality that wouldn't be
>> possible otherwise.
> 
> ':extend nil' is "used" during merging in the sense that such a face
> is skipped when we want a face for extending past EOL.  How else could
> we implement that?  Setting :extend to nil means that _none_ of the
> other attributes of the face are to be taken into account for merging.

We are talking about "merging" a list of faces applied to a 'face' text 
property on a char, right? That kind of merging?

If so, I would expect seeing ':extend nil' would mean not using any of 
the face attributes on that char for extending past EOL. If it's the 
last character on the line, using the default face's attributes instead.

And if we see ':extend t', then we would use the background from the 
first face in the list that has the :background attribute set. Is that 
not how merging faces in a list value usually works?

> Inheritance just makes the inheriting face implicitly behave as if its
> :extend attribute is the same as of the parent face, when the
> inheriting face doesn't itself specify :extend, i.e. has it set to
> 'unspecified'.

I think that's how inheritance for most attributes works, right?

Message #49 received at 38563 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Dmitry Gutov <dgutov <at> yandex.ru>
Cc: 38563 <at> debbugs.gnu.org
Subject: Re: bug#38563: 27.0.50; Company popup renders with newlines (?)
 inheriting the bg properties of the character at next line's bol
Date: Sat, 14 Dec 2019 10:13:55 +0200

> Cc: 38563 <at> debbugs.gnu.org
> From: Dmitry Gutov <dgutov <at> yandex.ru>
> Date: Sat, 14 Dec 2019 01:10:13 +0200
> 
> On 13.12.2019 17:32, Eli Zaretskii wrote:
> 
> > ':extend nil' is "used" during merging in the sense that such a face
> > is skipped when we want a face for extending past EOL.  How else could
> > we implement that?  Setting :extend to nil means that _none_ of the
> > other attributes of the face are to be taken into account for merging.
> 
> We are talking about "merging" a list of faces applied to a 'face' text 
> property on a char, right? That kind of merging?

We are talking about face merging for displaying a character, yes.
That process merges face information from all the sources that are in
effect for that character.  See the description at the beginning of
the "Displaying Faces" node in the ELisp manual.

When we merge faces for display past EOL, we modify the face-merging
process such that faces whose :extend attribute is not t are not
merged.  "Not merged" means here that these faces are bypassed by the
merging process, so their attributes (not just the background color)
do not contribute anything to the result of the merge.

> If so, I would expect seeing ':extend nil' would mean not using any of 
> the face attributes on that char for extending past EOL.

Indeed, but only for the face whose :extend is not t.  Other faces do
contribute the attributes to the merge.

> If it's the last character on the line, using the default face's
> attributes instead.

Why default?  There could be several faces involved (e.g., some
font-lock face, plus region, plus the face from the overlay string at
that position).  Any of these faces whose :extend is t will get
merged for character past EOL, the rest of the faces will not.

> And if we see ':extend t', then we would use the background from the 
> first face in the list that has the :background attribute set. Is that 
> not how merging faces in a list value usually works?

No.  The :extend attribute affects only the face where that attribute
is set.  It doesn't affect other faces, and also it affects all the
attributes, not just the background color.

> > Inheritance just makes the inheriting face implicitly behave as if its
> > :extend attribute is the same as of the parent face, when the
> > inheriting face doesn't itself specify :extend, i.e. has it set to
> > 'unspecified'.
> 
> I think that's how inheritance for most attributes works, right?

Yes.

Message #52 received at 38563 <at> debbugs.gnu.org (full text, mbox):

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 38563 <at> debbugs.gnu.org
Subject: Re: bug#38563: 27.0.50; Company popup renders with newlines (?)
 inheriting the bg properties of the character at next line's bol
Date: Mon, 16 Dec 2019 00:26:21 +0200

On 14.12.2019 10:13, Eli Zaretskii wrote:
> When we merge faces for display past EOL, we modify the face-merging
> process such that faces whose :extend attribute is not t are not
> merged.  "Not merged" means here that these faces are bypassed by the
> merging process, so their attributes (not just the background color)
> do not contribute anything to the result of the merge.

Ok, I've considered it a little and now I understand the primary reason 
for this mechanic: so that the region face extends over EOL (by 
default), and that extension doesn't take soak up the properties of the 
last character on the line (which is how I expected it to work).

Thanks!

Message #55 received at 38563 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Dmitry Gutov <dgutov <at> yandex.ru>
Cc: 38563 <at> debbugs.gnu.org
Subject: Re: bug#38563: 27.0.50; Company popup renders with newlines (?)
 inheriting the bg properties of the character at next line's bol
Date: Mon, 16 Dec 2019 17:49:17 +0200

reopen 38563
thanks

> Cc: 38563 <at> debbugs.gnu.org
> From: Dmitry Gutov <dgutov <at> yandex.ru>
> Date: Mon, 16 Dec 2019 00:26:21 +0200
> 
> Ok, I've considered it a little and now I understand the primary reason 
> for this mechanic: so that the region face extends over EOL (by 
> default), and that extension doesn't take soak up the properties of the 
> last character on the line (which is how I expected it to work).

Unfortunately, in bug#38633 in turned out my "fix" was not really a
fix, but a stupid typo, which just happened to stop the problem in
this bug from happening, but caused serious breakage elsewhere.

So I'm reopening this bug, and will have to think of a better
solution.

Message #60 received at 38563 <at> debbugs.gnu.org (full text, mbox):

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 38563 <at> debbugs.gnu.org
Subject: Re: bug#38563: 27.0.50; Company popup renders with newlines (?)
 inheriting the bg properties of the character at next line's bol
Date: Wed, 18 Dec 2019 22:37:49 +0200

On 16.12.2019 17:49, Eli Zaretskii wrote:

> Unfortunately, in bug#38633 in turned out my "fix" was not really a
> fix, but a stupid typo, which just happened to stop the problem in
> this bug from happening, but caused serious breakage elsewhere.
> 
> So I'm reopening this bug, and will have to think of a better
> solution.

In case you don't find it, how about we put 'default' as the 'face' 
property of the overlay? That seems to work.

And with that we can stop appending 'default' to all 'face' values in 
the overlay string.

WDYT?


diff --git a/company.el b/company.el
index 80398a3..399e9ea 100644
--- a/company.el
+++ b/company.el
@@ -2939,6 +2939,7 @@ Returns a negative number if the tooltip should be 
displayed above point."
           (overlay-put ov 'display disp)
         (overlay-put ov 'after-string disp)
         (overlay-put ov 'invisible t))
+      (overlay-put ov 'face 'default)
       (overlay-put ov 'window (selected-window)))))

 (defun company-pseudo-tooltip-guard ()

Message #63 received at 38563 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Dmitry Gutov <dgutov <at> yandex.ru>
Cc: 38563 <at> debbugs.gnu.org
Subject: Re: bug#38563: 27.0.50; Company popup renders with newlines (?)
 inheriting the bg properties of the character at next line's bol
Date: Sat, 21 Dec 2019 09:53:26 +0200

> Cc: 38563 <at> debbugs.gnu.org
> From: Dmitry Gutov <dgutov <at> yandex.ru>
> Date: Wed, 18 Dec 2019 22:37:49 +0200
> 
> On 16.12.2019 17:49, Eli Zaretskii wrote:
> 
> > So I'm reopening this bug, and will have to think of a better
> > solution.
> 
> In case you don't find it, how about we put 'default' as the 'face' 
> property of the overlay? That seems to work.
> 
> And with that we can stop appending 'default' to all 'face' values in 
> the overlay string.
> 
> WDYT?

Sorry for a long delay in responding.

Yes, this looks like a good solution to me.  In fact, I was about to
ask you to make a similar (less elegant) change, because I couldn't
think of a way to resolve this in core: when a face explicitly asks to
extend itself past EOL, we cannot in good faith refrain from doing so
in some use cases.

Thanks.

Message #68 received at 38563-done <at> debbugs.gnu.org (full text, mbox):

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 38563-done <at> debbugs.gnu.org
Subject: Re: bug#38563: 27.0.50; Company popup renders with newlines (?)
 inheriting the bg properties of the character at next line's bol
Date: Sat, 21 Dec 2019 15:22:46 +0200

On 21.12.2019 9:53, Eli Zaretskii wrote:

> Yes, this looks like a good solution to me.

Thank you for verifying. Then I'm closing this report.

> In fact, I was about to
> ask you to make a similar (less elegant) change,

Like temporarily changing the face of the underlying character? Yeah, I 
would have paused at that idea.

> because I couldn't
> think of a way to resolve this in core: when a face explicitly asks to
> extend itself past EOL, we cannot in good faith refrain from doing so
> in some use cases.

Let's see if other third-party issues come up, then.

Message #71 received at 38563 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Dmitry Gutov <dgutov <at> yandex.ru>
Cc: 38563 <at> debbugs.gnu.org
Subject: Re: bug#38563: 27.0.50; Company popup renders with newlines (?)
 inheriting the bg properties of the character at next line's bol
Date: Sat, 21 Dec 2019 15:31:13 +0200

> Cc: 38563-done <at> debbugs.gnu.org
> From: Dmitry Gutov <dgutov <at> yandex.ru>
> Date: Sat, 21 Dec 2019 15:22:46 +0200
> 
> > In fact, I was about to
> > ask you to make a similar (less elegant) change,
> 
> Like temporarily changing the face of the underlying character? Yeah, I 
> would have paused at that idea.

No, a different change in the face you put on the newline.

> > because I couldn't
> > think of a way to resolve this in core: when a face explicitly asks to
> > extend itself past EOL, we cannot in good faith refrain from doing so
> > in some use cases.
> 
> Let's see if other third-party issues come up, then.

Right.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.