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

From: Sergio Pastor Pérez <[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.

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?