bug#9782: 24.0.90; move-to-window-line not taking header line into account

bug#9782: 24.0.90; move-to-window-line not taking header line into account

[David Engster]

Recipe:

* emacs -Q

* Enter in scratch buffer:

(move-to-window-line (cdr (posn-actual-col-row (posn-at-point))))

and enter an additional newline so this is not the last line in the buffer.

* Move behind last bracket an hit C-x C-e

* Cursor will move to beginning of line, as expected.

* Now do M-: (setq header-line-format "") RET

* Evaluate the above again. You'll see that cursor now will move to the
  beginning of the next line, which is wrong.


This behavior occurs since rev. 106022, which fixed posn-actual-col-row
when a header-line is active, but it seems move-to-window-line now has
to be fixed as well.


Cheers,
David

bug#9782: 24.0.90; move-to-window-line not taking header line into account

[Eli Zaretskii]

> From: David Engster <deng@randomsample.de>
> Date: Tue, 18 Oct 2011 14:03:13 +0200
> 
> Recipe:
> 
> * emacs -Q
> 
> * Enter in scratch buffer:
> 
> (move-to-window-line (cdr (posn-actual-col-row (posn-at-point))))
> 
> and enter an additional newline so this is not the last line in the buffer.
> 
> * Move behind last bracket an hit C-x C-e
> 
> * Cursor will move to beginning of line, as expected.
> 
> * Now do M-: (setq header-line-format "") RET
> 
> * Evaluate the above again. You'll see that cursor now will move to the
>   beginning of the next line, which is wrong.
> 
> 
> This behavior occurs since rev. 106022, which fixed posn-actual-col-row
> when a header-line is active, but it seems move-to-window-line now has
> to be fixed as well.

Please provide some arguments as to why the current behavior is wrong.

posn-actual-col-row returns a _row_ derived from a pixel position,
while move-to-window-line accepts a _line_number_ starting from the
beginning of the text displayed in the window.  These two are not the
same.  Unless I'm mistaken, I see many users of move-to-window-line
that would break if we make the change you suggest.  E.g., what will
happen to code that does this:

  (move-to-window-line 0)

when there's a header line in the buffer, if your suggestion is
implemented?

Put it another way, the posn-* family of function deals with mouse
events, which are inherently oblivious to where text is displayed and
where we have window decorations.  By contrast, move-to-window-line
belongs to a different family of functions, one that deals with lines
of text.

Please show where this reasoning is wrong.

bug#9782: 24.0.90; move-to-window-line not taking header line into account

[David Engster]

Eli Zaretskii writes:
>> From: David Engster <deng@randomsample.de>
>> Date: Tue, 18 Oct 2011 14:03:13 +0200
>> 
>
>> Recipe:
>> 
>> * emacs -Q
>> 
>> * Enter in scratch buffer:
>> 
>> (move-to-window-line (cdr (posn-actual-col-row (posn-at-point))))
>> 
>> and enter an additional newline so this is not the last line in the buffer.
>> 
>> * Move behind last bracket an hit C-x C-e
>> 
>> * Cursor will move to beginning of line, as expected.
>> 
>> * Now do M-: (setq header-line-format "") RET
>> 
>> * Evaluate the above again. You'll see that cursor now will move to the
>>   beginning of the next line, which is wrong.
>> 
>> 
>> This behavior occurs since rev. 106022, which fixed posn-actual-col-row
>> when a header-line is active, but it seems move-to-window-line now has
>> to be fixed as well.
>
> Please provide some arguments as to why the current behavior is wrong.
>
> posn-actual-col-row returns a _row_ derived from a pixel position,
> while move-to-window-line accepts a _line_number_ starting from the
> beginning of the text displayed in the window.  These two are not the
> same.  Unless I'm mistaken, I see many users of move-to-window-line
> that would break if we make the change you suggest.  E.g., what will
> happen to code that does this:
>
>   (move-to-window-line 0)
>
> when there's a header line in the buffer, if your suggestion is
> implemented?

Yes, this would be wrong, obviously.

> Put it another way, the posn-* family of function deals with mouse
> events, which are inherently oblivious to where text is displayed and
> where we have window decorations.  By contrast, move-to-window-line
> belongs to a different family of functions, one that deals with lines
> of text.
>
> Please show where this reasoning is wrong.

I can't. My report was motivated by seeing that the tooltip display of
company-mode (currently in ELPA) broke due to the change in
posn-actual-col-row. Company-mode happens to depend on the above test
case, that means it first determines the actual row at point, moves to
(1+ row) through move-to-window-line and then displays the overlay
there. I don't know why it chooses this way to do this, but it has
worked for years, so I figured move-to-window-line has to be fixed.  If
this new behavior is the correct one, then please close this report and
I will send a bug report to the author of company-mode.

Thanks,
David

bug#9782: 24.0.90; move-to-window-line not taking header line into account

[Eli Zaretskii]

> From: David Engster <deng@randomsample.de>
> Cc: 9782@debbugs.gnu.org
> Date: Tue, 18 Oct 2011 16:23:37 +0200
> 
> My report was motivated by seeing that the tooltip display of
> company-mode (currently in ELPA) broke due to the change in
> posn-actual-col-row. Company-mode happens to depend on the above test
> case, that means it first determines the actual row at point, moves to
> (1+ row) through move-to-window-line and then displays the overlay
> there. I don't know why it chooses this way to do this, but it has
> worked for years, so I figured move-to-window-line has to be fixed.  If
> this new behavior is the correct one, then please close this report and
> I will send a bug report to the author of company-mode.

I'm closing the bug report.  My opinion is that company-mode should
either change the way it computes the line to move to, or account for
the header-line itself (e.g., by looking at the value of
header-line-format).  If the author of that package disagrees, I
suggest that he or she starts a discussion on emacs-devel.

Thanks.

Re: bug#9782: 24.0.90; move-to-window-line not taking header line into account

[Dmitry Gutov]

Eli Zaretskii <eliz@gnu.org> writes:
>> From: David Engster <deng@randomsample.de>
>> Cc: 9782@debbugs.gnu.org
>> Date: Tue, 18 Oct 2011 16:23:37 +0200
>> 
>> My report was motivated by seeing that the tooltip display of
>> company-mode (currently in ELPA) broke due to the change in
>> posn-actual-col-row. Company-mode happens to depend on the above test
>> case, that means it first determines the actual row at point, moves to
>> (1+ row) through move-to-window-line and then displays the overlay
>> there. I don't know why it chooses this way to do this, but it has
>> worked for years, so I figured move-to-window-line has to be fixed.  If
>> this new behavior is the correct one, then please close this report and
>> I will send a bug report to the author of company-mode.
>
> I'm closing the bug report.  My opinion is that company-mode should
> either change the way it computes the line to move to, or account for
> the header-line itself (e.g., by looking at the value of
> header-line-format).  If the author of that package disagrees, I
> suggest that he or she starts a discussion on emacs-devel.

There you go, I'm starting the discussion.

What do you suggest would be a better way to get the window row of a
buffer position, save that value, and then move to that row later?

Currently, company calls (cdr (posn-actual-col-row posn)) to retrieve
the row, and `move-to-window-line' to get to it.

Simply adjusting the row number for presence of `header-line-format'
won't do, because company strives to support Emacs 23 (and maybe even
22), and `post-actual-col-row' behavior was different before the
revision 106022.

--Dmitry

Re: bug#9782: 24.0.90; move-to-window-line not taking header line into account

[Eli Zaretskii]

> From: Dmitry Gutov <dgutov@yandex.ru>
> Cc: eliz@gnu.org
> Date: Sun, 05 May 2013 01:12:52 +0400
> 
> What do you suggest would be a better way to get the window row of a
> buffer position, save that value, and then move to that row later?
> 
> Currently, company calls (cdr (posn-actual-col-row posn)) to retrieve
> the row, and `move-to-window-line' to get to it.
> 
> Simply adjusting the row number for presence of `header-line-format'
> won't do, because company strives to support Emacs 23 (and maybe even
> 22), and `post-actual-col-row' behavior was different before the
> revision 106022.

Why is that a problem to have special code for specific Emacs
versions?

Re: bug#9782: 24.0.90; move-to-window-line not taking header line into account

[Dmitry Gutov]

On 05.05.2013 6:47, Eli Zaretskii wrote:
>> From: Dmitry Gutov <dgutov@yandex.ru>
>> Cc: eliz@gnu.org
>> Date: Sun, 05 May 2013 01:12:52 +0400
>>
>> What do you suggest would be a better way to get the window row of a
>> buffer position, save that value, and then move to that row later?
>>
>> Currently, company calls (cdr (posn-actual-col-row posn)) to retrieve
>> the row, and `move-to-window-line' to get to it.
>>
>> Simply adjusting the row number for presence of `header-line-format'
>> won't do, because company strives to support Emacs 23 (and maybe even
>> 22), and `post-actual-col-row' behavior was different before the
>> revision 106022.
>
> Why is that a problem to have special code for specific Emacs
> versions?

It's ugly, for one thing. If you don't have any better suggestions, I'll 
do that, I guess.

But it's a seemingly reasonable piece of code, it worked, and your 
change made it break in some peculiar special case important to some 
users. That might imply that the code doesn't make sense on some higher 
level.

So I thought you could suggest how that code could be written 
better/faster/stronger instead.

Re: bug#9782: 24.0.90; move-to-window-line not taking header line into account

[Eli Zaretskii]

> Date: Sun, 05 May 2013 06:59:48 +0400
> From: Dmitry Gutov <dgutov@yandex.ru>
> CC: emacs-devel@gnu.org
> 
> >> Simply adjusting the row number for presence of `header-line-format'
> >> won't do, because company strives to support Emacs 23 (and maybe even
> >> 22), and `post-actual-col-row' behavior was different before the
> >> revision 106022.
> >
> > Why is that a problem to have special code for specific Emacs
> > versions?
> 
> It's ugly, for one thing.

Working around a bug is frequently not pretty.

> But it's a seemingly reasonable piece of code, it worked, and your 
> change made it break in some peculiar special case important to some 
> users.

That's not true.  It was not a peculiar special case. The function did
not behave according to its contract, and my change fixed that.

> So I thought you could suggest how that code could be written 
> better/faster/stronger instead.

Well, how about telling more about what you need to do, and in
particular why do you need to go to the place you go by row and
column?  Why not use posn-point, for example?  For that matter, why
not use buffer positions directly?  It seems to me you take a buffer
position, convert it to row and column, then use that to get to the
buffer position, is that right?  If you need the beginning and end of
the line that includes point, that is easy to get in any number of
ways, without ever calculating the row number.

What am I missing?

Re: bug#9782: 24.0.90; move-to-window-line not taking header line into account

[Dmitry Gutov]

On 05.05.2013 19:53, Eli Zaretskii wrote:
>> Date: Sun, 05 May 2013 06:59:48 +0400
>> From: Dmitry Gutov <dgutov@yandex.ru>
>> CC: emacs-devel@gnu.org
>>
>>>> Simply adjusting the row number for presence of `header-line-format'
>>>> won't do, because company strives to support Emacs 23 (and maybe even
>>>> 22), and `post-actual-col-row' behavior was different before the
>>>> revision 106022.

>> But it's a seemingly reasonable piece of code, it worked, and your
>> change made it break in some peculiar special case important to some
>> users.
>
> That's not true.  It was not a peculiar special case. The function did
> not behave according to its contract, and my change fixed that.

Comparing the descriptions of `move-to-window-line' and 
`posn-actual-col-row', it doesn't seem obvious that one considers the 
header line part of the window, and another doesn't. The words "row" and 
"line" are often synonymous in English.

>> So I thought you could suggest how that code could be written
>> better/faster/stronger instead.
>
> Well, how about telling more about what you need to do, and in
> particular why do you need to go to the place you go by row and
> column?  Why not use posn-point, for example?  For that matter, why
> not use buffer positions directly?

In short, we're painting a rectangle near point, using an overlay. So 
columns and rows are a natural way to think about the problem.

Historically, the overlay rectangle is called "pseudo-tooltip" here.

https://raw.github.com/company-mode/company-mode/master/company.el

`company-pseudo-tooltip-show-at-point' passes the current row and column 
to `company-pseudo-tooltip-show', which collects are required lines and 
prepares the `before-string' for the overlay.

These two would be the easiest to convert to direct point manipulation, 
except for the detail that they calculate, use and store the beginning 
column in the overlay property, for use later.

`company--pseudo-tooltip-height' calculates the appropriate rectangle 
height, based on the current row and the inner height of the window.

`company--electric-do' (somewhat unrelated) calls `recenter' using the 
value returned by `posn-actual-col-row'.

`company-select-mouse' and `company--inside-tooltip-p' handle mouse 
interaction with the rectangle, comparing the coordinates of the mouse 
click with the coordinates and dimensions of the rectangle (some of them 
saved previously; the current row is again retrieved using 
`posn-actual-col-row').

To conclude, I see two alternative solutions, both require 
re-implementing `company-row':

1) Using `move-to-window-line', `vertical-motion' and a counter.

2) Call `posn-at-point' and `posn-actual-col-row' in `company--row' 
twice, for the current point, and for the point at the top of the window 
(use `move-to-window-line' here), then subtract.

`posn-at-point' is relatively slow already, so the second option seems 
to be the worst one.

Re: bug#9782: 24.0.90; move-to-window-line not taking header line into account

[Eli Zaretskii]

> Date: Mon, 06 May 2013 07:38:43 +0400
> From: Dmitry Gutov <dgutov@yandex.ru>
> CC: emacs-devel@gnu.org
> 
> >> But it's a seemingly reasonable piece of code, it worked, and your
> >> change made it break in some peculiar special case important to some
> >> users.
> >
> > That's not true.  It was not a peculiar special case. The function did
> > not behave according to its contract, and my change fixed that.
> 
> Comparing the descriptions of `move-to-window-line' and 
> `posn-actual-col-row', it doesn't seem obvious that one considers the 
> header line part of the window, and another doesn't. The words "row" and 
> "line" are often synonymous in English.

Perhaps we need to improve the documentation of the posn-* family of
functions, then.  It is important to keep in mind that this family
serves mainly the mouse events, so its coordinate system is
window-relative.  The use of columns and rows there is just to measure
in character units, rather than in pixels.  But it is still
window-relative and agnostic to the header line, like the mouse is.

> `company-pseudo-tooltip-show-at-point' passes the current row and column 
> to `company-pseudo-tooltip-show', which collects are required lines and 
> prepares the `before-string' for the overlay.
> 
> These two would be the easiest to convert to direct point manipulation, 
> except for the detail that they calculate, use and store the beginning 
> column in the overlay property, for use later.
> 
> `company--pseudo-tooltip-height' calculates the appropriate rectangle 
> height, based on the current row and the inner height of the window.
> 
> `company--electric-do' (somewhat unrelated) calls `recenter' using the 
> value returned by `posn-actual-col-row'.
> 
> `company-select-mouse' and `company--inside-tooltip-p' handle mouse 
> interaction with the rectangle, comparing the coordinates of the mouse 
> click with the coordinates and dimensions of the rectangle (some of them 
> saved previously; the current row is again retrieved using 
> `posn-actual-col-row').

I don't see company--inside-tooltip-p anywhere in today's ELPA company
package, but anyway, why do you need to use coordinates to determine
whether the mouse clicked on the pseudo-tooltip?  The pseudo-tooltip
is a display or overlay string, right?  So the OBJECT part of the
event's POSITION member will tell you whether you clicked on the
string or not: if it's nil, the click is on some buffer text, but if
it's the display string, the click was on that string.

> 1) Using `move-to-window-line', `vertical-motion' and a counter.

Actually, I think you want something like

  (count-screen-lines (window-start) (point))

And for the mouse, see the suggestion above.

Re: bug#9782: 24.0.90; move-to-window-line not taking header line into account

[Dmitry Gutov]

On 06.05.2013 20:38, Eli Zaretskii wrote:
> Perhaps we need to improve the documentation of the posn-* family of
> functions, then.  It is important to keep in mind that this family
> serves mainly the mouse events, so its coordinate system is
> window-relative.  The use of columns and rows there is just to measure
> in character units, rather than in pixels.  But it is still
> window-relative and agnostic to the header line, like the mouse is.

One might think that `move-to-window-line' is also window-relative and 
thus agnostic to the header line (that naturally follows if we consider 
the header line a part of the window).

To answer your old question, `(move-to-window-line 0)' would then do 
pretty much the same as what calling it with any other unreachable line 
number does - bring point as close as possible, to line 1 in this case.

> I don't see company--inside-tooltip-p anywhere in today's ELPA company
> package,

Naturally. I only added it recently to fix an old bug.

> but anyway, why do you need to use coordinates to determine
> whether the mouse clicked on the pseudo-tooltip?  The pseudo-tooltip
> is a display or overlay string, right?  So the OBJECT part of the
> event's POSITION member will tell you whether you clicked on the
> string or not: if it's nil, the click is on some buffer text, but if
> it's the display string, the click was on that string.

Ah, good, thank you. This takes care of the rows check.

We still need to compare the column values to see if the click happened 
exactly inside the rectangle, not to the right or left of it.

And in `company-select-mouse', we need the row values to find out which 
rectangle line was clicked (which candidate to select), so 
`company--row' has to agree with mouse coordinates.

>> 1) Using `move-to-window-line', `vertical-motion' and a counter.
>
> Actually, I think you want something like
>
>    (count-screen-lines (window-start) (point))
>
> And for the mouse, see the suggestion above.

Taking the above into account, I think having a header-line-aware 
version of `move-to-window-line' instead would be best.

Like suggested before, it would adjust the argument by 1 if 
emacs-version >= "24", and the header line is present.
It will break if/when the window can include something else on the top.

Re: bug#9782: 24.0.90; move-to-window-line not taking header line into account

[Eli Zaretskii]

> Date: Tue, 07 May 2013 05:26:43 +0400
> From: Dmitry Gutov <dgutov@yandex.ru>
> CC: emacs-devel@gnu.org
> 
> On 06.05.2013 20:38, Eli Zaretskii wrote:
> > Perhaps we need to improve the documentation of the posn-* family of
> > functions, then.  It is important to keep in mind that this family
> > serves mainly the mouse events, so its coordinate system is
> > window-relative.  The use of columns and rows there is just to measure
> > in character units, rather than in pixels.  But it is still
> > window-relative and agnostic to the header line, like the mouse is.
> 
> One might think that `move-to-window-line' is also window-relative and 
> thus agnostic to the header line (that naturally follows if we consider 
> the header line a part of the window).

"Line" is a line of text, and move-to-window-line was written to go to
a line of text.

> > but anyway, why do you need to use coordinates to determine
> > whether the mouse clicked on the pseudo-tooltip?  The pseudo-tooltip
> > is a display or overlay string, right?  So the OBJECT part of the
> > event's POSITION member will tell you whether you clicked on the
> > string or not: if it's nil, the click is on some buffer text, but if
> > it's the display string, the click was on that string.
> 
> Ah, good, thank you. This takes care of the rows check.
> 
> We still need to compare the column values to see if the click happened 
> exactly inside the rectangle, not to the right or left of it.

Doesn't the overlay cover the entire rectangle?

> And in `company-select-mouse', we need the row values to find out which 
> rectangle line was clicked (which candidate to select)

Isn't each rectangle a different string?

> > Actually, I think you want something like
> >
> >    (count-screen-lines (window-start) (point))
> >
> > And for the mouse, see the suggestion above.
> 
> Taking the above into account, I think having a header-line-aware 
> version of `move-to-window-line' instead would be best.

??? But you don't want to _move_ anywhere, AFAIU.  You want to compute
a window-relative screen line number of a given position (I used point
in the above example, but that can be replaced by any buffer
position).  So how does move-to-window-line fit that bill?  Its
return value is undocumented, so you cannot really rely on that.

> Like suggested before, it would adjust the argument by 1 if 
> emacs-version >= "24", and the header line is present.

The function move-to-window-line is implemented in C, so it is
pointless to make it sensitive to Emacs version.

Re: bug#9782: 24.0.90; move-to-window-line not taking header line into account

[Dmitry Gutov]

On 07.05.2013 6:54, Eli Zaretskii wrote:
>> One might think that `move-to-window-line' is also window-relative and
>> thus agnostic to the header line (that naturally follows if we consider
>> the header line a part of the window).
>
> "Line" is a line of text, and move-to-window-line was written to go to
> a line of text.

The question is how they're numbered. One might imagine that the 
"zeroth" line is covered by the header, so the first one visible is 
number one.

Note I'm not really arguing in favor in that change, since it won't help 
me get rid of the version check for 24.1-24.3 anyway.

>> We still need to compare the column values to see if the click happened
>> exactly inside the rectangle, not to the right or left of it.
>
> Doesn't the overlay cover the entire rectangle?
>
>> And in `company-select-mouse', we need the row values to find out which
>> rectangle line was clicked (which candidate to select)
>
> Isn't each rectangle a different string?
>

There's just one overlay, and it covers all of them (plus all text on 
the sides). Maybe what you're suggesting would be an improvement (I see 
dropdown-list.el also does that), but the current approach works fast 
enough, and it would have the advantage in a hypothetical situation when 
some of the text we need to "draw on" is already rendered via `display' 
property.

>> Taking the above into account, I think having a header-line-aware
>> version of `move-to-window-line' instead would be best.
>
> ??? But you don't want to _move_ anywhere, AFAIU.  You want to compute
> a window-relative screen line number of a given position (I used point
> in the above example, but that can be replaced by any buffer
> position).  So how does move-to-window-line fit that bill?  Its
> return value is undocumented, so you cannot really rely on that.

I mean fixing the row number <-> line number discrepancy from the other 
side, by making a wrapper for `move-to-window-line', the only function 
of the bunch that deals with line numbers. It's used in 
`company-pseudo-tooltip-show'.

>> Like suggested before, it would adjust the argument by 1 if
>> emacs-version >= "24", and the header line is present.
>
> The function move-to-window-line is implemented in C, so it is
> pointless to make it sensitive to Emacs version.

The wrapper will need to check the version because Emacs 23 and earlier 
don't need the adjustment.

Re: bug#9782: 24.0.90; move-to-window-line not taking header line into account

[Eli Zaretskii]

> Date: Tue, 07 May 2013 11:41:00 +0400
> From: Dmitry Gutov <dgutov@yandex.ru>
> CC: emacs-devel@gnu.org
> 
> On 07.05.2013 6:54, Eli Zaretskii wrote:
> >> One might think that `move-to-window-line' is also window-relative and
> >> thus agnostic to the header line (that naturally follows if we consider
> >> the header line a part of the window).
> >
> > "Line" is a line of text, and move-to-window-line was written to go to
> > a line of text.
> 
> The question is how they're numbered. One might imagine that the 
> "zeroth" line is covered by the header, so the first one visible is 
> number one.

But you cannot move point to the header line.  And argument of zero to
move-to-window-line already has a different meaning.

> >> We still need to compare the column values to see if the click happened
> >> exactly inside the rectangle, not to the right or left of it.
> >
> > Doesn't the overlay cover the entire rectangle?
> >
> >> And in `company-select-mouse', we need the row values to find out which
> >> rectangle line was clicked (which candidate to select)
> >
> > Isn't each rectangle a different string?
> >
> 
> There's just one overlay, and it covers all of them (plus all text on 
> the sides).

Sorry, I don't understand.  Perhaps we are talking about two different
things.  Are you talking about the menu-like display of possible
completions, shown to the user to let her select one of the
candidates?  If so, how can they be a single overlay string, when they
are shown in different screen lines?  Are you also copying the buffer
text into the overlay string or something?  Otherwise I don't
understand how can you show a multi-line overlay string and still have
buffer text be seen.  What am I missing?

> Maybe what you're suggesting would be an improvement (I see 
> dropdown-list.el also does that), but the current approach works fast 
> enough, and it would have the advantage in a hypothetical situation when 
> some of the text we need to "draw on" is already rendered via `display' 
> property.

I don't see any advantages even in that situation, but maybe I'm
misunderstanding what you mean.

My suggestion is to make each completion candidate a separate display
string, then the event position list will tell you directly which
string was clicked.

> I mean fixing the row number <-> line number discrepancy from the other 
> side, by making a wrapper for `move-to-window-line', the only function 
> of the bunch that deals with line numbers. It's used in 
> `company-pseudo-tooltip-show'.

count-screen-lines also deals with line numbers.

Anyway, I think you now have the information needed to fix company.el,
and can select the implementation that you like best.

Re: bug#9782: 24.0.90; move-to-window-line not taking header line into account

[Dmitry Gutov]

On 07.05.2013 20:50, Eli Zaretskii wrote:
>> There's just one overlay, and it covers all of them (plus all text on
>> the sides).
>
> Sorry, I don't understand.  Perhaps we are talking about two different
> things.  Are you talking about the menu-like display of possible
> completions, shown to the user to let her select one of the
> candidates?  If so, how can they be a single overlay string, when they
> are shown in different screen lines?  Are you also copying the buffer
> text into the overlay string or something?  Otherwise I don't
> understand how can you show a multi-line overlay string and still have
> buffer text be seen.  What am I missing?

Yes. Yes, the whole lines are copied, modified in the right place and 
then shown using `before-string' overlay property (the original text is 
hidden with `invisible' -> t).

>> Maybe what you're suggesting would be an improvement (I see
>> dropdown-list.el also does that), but the current approach works fast
>> enough, and it would have the advantage in a hypothetical situation when
>> some of the text we need to "draw on" is already rendered via `display'
>> property.
>
> I don't see any advantages even in that situation, but maybe I'm
> misunderstanding what you mean.

If some of the lines in the middle of the modified region are doing 
freaky things with `display', `before-string', etc, it complicates our 
ability to position each overlay on the right column visually 
(`current-column' only considers physical characters, the "right" column 
may turn out to be inside a `before-string' string, stuff like that).

Making one overlay spread over all those lines means we can pre-process 
all lines, only take the physical text (plus composed chars), and mostly 
disregard third-party overlays.

> My suggestion is to make each completion candidate a separate display
> string, then the event position list will tell you directly which
> string was clicked.

I understood that, thank you. It would indeed allow us to skirt the row 
<-> line issue, at the cost of pervasive changes in the code.

>> I mean fixing the row number <-> line number discrepancy from the other
>> side, by making a wrapper for `move-to-window-line', the only function
>> of the bunch that deals with line numbers. It's used in
>> `company-pseudo-tooltip-show'.
>
> count-screen-lines also deals with line numbers.

Yes, but that would be fixing the discrepancy from the other side, and 
then `company--row', reimplemented using `count-screen-lines', would 
conflict with row values from mouse events, so this will only work when 
we don't need to compare them (i.e. when using multiple overlays).

> Anyway, I think you now have the information needed to fix company.el,
> and can select the implementation that you like best.

I guess so. Thanks for the discussion!

Re: bug#9782: 24.0.90; move-to-window-line not taking header line into account

[Dmitry Gutov]

Hi again,

If you don't mind, I'd like to renew this discussion for a bit.

On 05/07/2013 09:19 PM, Dmitry Gutov wrote:
> On 07.05.2013 20:50, Eli Zaretskii wrote:
>>> I mean fixing the row number <-> line number discrepancy from the other
>>> side, by making a wrapper for `move-to-window-line', the only function
>>> of the bunch that deals with line numbers. It's used in
>>> `company-pseudo-tooltip-show'.
>>
>> count-screen-lines also deals with line numbers.
>
> Yes, but that would be fixing the discrepancy from the other side, and
> then `company--row', reimplemented using `count-screen-lines', would
> conflict with row values from mouse events, so this will only work when
> we don't need to compare them (i.e. when using multiple overlays).

Now we've used the following function to get the current line number, 
for a while:

(defun company--row (&optional pos)
   (save-excursion
     (when pos (goto-char pos))
     (count-screen-lines (window-start)
                         (progn (vertical-motion 0) (point)))))

It has a fundamental problem: it doesn't deal well with `display'; this 
is most apparent in the `M-x report-emacs-bug' buffer, leading to bugs 
like this: https://github.com/company-mode/company-mode/issues/136

The actual behavior is a bit different in my Emacs compared to the 
screenshot, but the problem is there.

Since `vertical-motion' doesn't jump into the "virtual" text inside the 
multiline `display' property, `company--row' returns lower values than 
it should. `move-to-window-line' takes multiline `display' into account 
just fine, so (move-to-window-line (company--row)) might move to a 
different line than the point is at.

Do you have any further suggestions? Should we revert to using 
`posn-col-row'?

Maybe I could do an alternative implementation of `move-to-window-line' 
that disregards multiline `display' the same way, using 
`vertical-motion', but this approach would probably cause buffer text 
jumping when the popup is displayed in more cases than the 
`posn-col-row' one.

Re: bug#9782: 24.0.90; move-to-window-line not taking header line into account

[Eli Zaretskii]

> Date: Sat, 16 Aug 2014 03:33:44 +0400
> From: Dmitry Gutov <raaahh@gmail.com>
> CC: emacs-devel@gnu.org
> 
> On 05/07/2013 09:19 PM, Dmitry Gutov wrote:
> > On 07.05.2013 20:50, Eli Zaretskii wrote:
> >>> I mean fixing the row number <-> line number discrepancy from the other
> >>> side, by making a wrapper for `move-to-window-line', the only function
> >>> of the bunch that deals with line numbers. It's used in
> >>> `company-pseudo-tooltip-show'.
> >>
> >> count-screen-lines also deals with line numbers.

I don't see this part in the bug discussion.  Where did it take place?

> Now we've used the following function to get the current line number, 
> for a while:
> 
> (defun company--row (&optional pos)
>    (save-excursion
>      (when pos (goto-char pos))
>      (count-screen-lines (window-start)
>                          (progn (vertical-motion 0) (point)))))
> 
> It has a fundamental problem: it doesn't deal well with `display'; this 
> is most apparent in the `M-x report-emacs-bug' buffer, leading to bugs 
> like this: https://github.com/company-mode/company-mode/issues/136
> 
> The actual behavior is a bit different in my Emacs compared to the 
> screenshot, but the problem is there.
> 
> Since `vertical-motion' doesn't jump into the "virtual" text inside the 
> multiline `display' property, `company--row' returns lower values than 
> it should. `move-to-window-line' takes multiline `display' into account 
> just fine, so (move-to-window-line (company--row)) might move to a 
> different line than the point is at.
> 
> Do you have any further suggestions? Should we revert to using 
> `posn-col-row'?

If the only problem with posn-col-row is that it errs by one when
there's a header line, adding one in that case sounds like an easy way
out.

Re: bug#9782: 24.0.90; move-to-window-line not taking header line into account

[Dmitry Gutov]

On 08/16/2014 12:03 PM, Eli Zaretskii wrote:

> I don't see this part in the bug discussion.  Where did it take place?

It had already moved to emacs-devel at that point.

> If the only problem with posn-col-row is that it errs by one when
> there's a header line, adding one in that case sounds like an easy way
> out.

Hmm, I guess so. This is now simplified by our droppped support for 
Emacs <24. Thanks.

Re: bug#9782: 24.0.90; move-to-window-line not taking header line into account

[Eli Zaretskii]

> Date: Sat, 16 Aug 2014 12:27:32 +0400
> From: Dmitry Gutov <dgutov@yandex.ru>
> CC: emacs-devel@gnu.org
> 
> On 08/16/2014 12:03 PM, Eli Zaretskii wrote:
> 
> > I don't see this part in the bug discussion.  Where did it take place?
> 
> It had already moved to emacs-devel at that point.

Yes, but I don't seem to find it there as well.

> > If the only problem with posn-col-row is that it errs by one when
> > there's a header line, adding one in that case sounds like an easy way
> > out.
> 
> Hmm, I guess so. This is now simplified by our droppped support for 
> Emacs <24. Thanks.

You are welcome.

Re: bug#9782: 24.0.90; move-to-window-line not taking header line into account

[Dmitry Gutov]

On 08/16/2014 03:12 PM, Eli Zaretskii wrote:

>>> I don't see this part in the bug discussion.  Where did it take place?
>>
>> It had already moved to emacs-devel at that point.
>
> Yes, but I don't seem to find it there as well.

Here it is:

http://lists.gnu.org/archive/html/emacs-devel/2013-05/msg00167.html