Subject: Re: bug#79241: [PATCH] WIP: Fix incorrect handling of overlays in `vertical-motion'

From: Eli Zaretskii <[email-masked]>
Date: Sat, 23 Aug 2025 15:25:08 +0300

> From: Sergio Pastor Pérez <[email-masked]>
> Cc: [email-masked]
> Date: Wed, 20 Aug 2025 20:57:02 +0200
> 
> Eli Zaretskii <[email-masked]> writes:
> 
> >> > --- a/src/indent.c
> >> > +++ b/src/indent.c
> >> > @@ -2513,15 +2513,16 @@ line (if such column exists on that line, that is).  If the line is
> >> >  	     and then reposition point at the requested X coordinate;
> >> >  	     if we don't, the cursor will be placed just after the
> >> >  	     string, which might not be the requested column.  */
> >> > -	  if (nlines >= 0 && it.area == TEXT_AREA)
> >> > +	  if (nlines != 0 && it.area == TEXT_AREA)
> >> >  	    {
> >> >  	      while (it.method == GET_FROM_STRING
> >> > -		     && !it.string_from_display_prop_p
> >> >  		     && memchr (SSDATA (it.string) + IT_STRING_BYTEPOS (it),
> >> >  				'\n',
> >> >  				SBYTES (it.string) - IT_STRING_BYTEPOS (it)))
> >> >  		{
> >> > -		  move_it_by_lines (&it, 1);
> >> > +		  int direction = (nlines > 0) ? 1 : -1;
> >> > +
> >> > +		  move_it_by_lines (&it, direction);
> >> >  		  move_it_in_display_line (&it, ZV, first_x + to_x, MOVE_TO_X);
> >> >  		}
> >> 
> >> I don't know what primitives are available to break this loop once the
> >> target line has been reached, so what seems to happen is that if there
> >> are consecutive lines with multi-line overlays, one after the other,
> >> this while loop keeps going until it reaches the first one of them. This
> >> is only a problem for the upward motion. Since I did not find a
> >> solution, I'm writing this mail in case there is someone more familiar
> >> with the code, that knows how we could detect that the while loop should
> >> finish in this particular case of consecutive lines with multi-line
> >> overlays. In all other cases I've encountered, the patch seems to work
> >> well.
> >
> > What is the "target line" in this case? how is it defined? and why
> > going all the way till the first line of a multi-line overlay is not
> > TRT in this case?
> 
> I may have not express myself correctly, I will give you an example to
> clarify what I meant. Let's understand as real lines the ones that are
> numbered, and as visual overlays the others:
> --8<---------------cut here---------------start------------->8---
> 43  QStringList getPlasmaStyles(void);
>     └────────────────────────────── ccls [2]: unknown type name 'QStringList'
> 44  QStringList getColorSchemes(void);
>     └────────────────────────────── ccls [2]: unknown type name 'QStringList'
> 45  QStringList getIconThemes(void);
>     └────────────────────────────── ccls [2]: unknown type name 'QStringList'
> 46  QStringList getCursorThemes(void);
>     └────────────────────────────── ccls [2]: unknown type name 'QStringList'
> 47  QStringList getGtkThemes(void);
>     └────────────────────────────── ccls [2]: unknown type name 'QStringList'
> 48  QStringList getKvantumStyles(void);
>     └────────────────────────────── ccls [2]: unknown type name 'QStringList'
>    <|>
> --8<---------------cut here---------------end--------------->8---
> 
> In the previous example, `<|>' represents the point.
> 
> In this situation, with the patch I provided, issuing
> `(vertical-motion '(4 . -1))' will move the point to column 4 of line
> 43, when the point should land on column 4 of line 48.

If you test the conditions of the while-loop between the call to
move_it_by_lines and the following call to move_it_in_display_line,m
wouldn't you be able to stop at line 48?  AFAIU, the overlay string is
placed on the end of the code line whose diagnostic it shows, so the
character at BOL doesn't have the overlay, and you should be able to
break the loop when you want.  Or am I missing something?

> To clarify, in my previous message I did not meant that chained
> multi-line overlays should not be skipped, I meant that when there are
> multiple real lines, having multi-line overlays one after the other, the
> point should not go over them. For that, I think we need the while loop
> to stop iterating somehow. We need a way to know if point is outside of
> the overlay and break the loop. If that's not possible, maybe there is a
> way to know if we are already in the target line?

You know when the iterator is outside of the overlay by examining
it.method and the other stuff.  A real line will should
GET_FROM_BUFFER there, because what you have at BOL of a real line is
buffer text, not an overlay.