GNU bug report logs - #79241
31.0.50; `vertical-motion' not respecting goal column when there is an overlay that spans multiple virtual lines

Previous Next

Full log

View this message in rfc822 format

From: Sergio Pastor Pérez <sergio.pastorperez <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 79241 <at> debbugs.gnu.org
Subject: bug#79241: [PATCH v2] Fix incorrect handling of overlays in `vertical-motion'
Date: Sun, 24 Aug 2025 19:58:47 +0200

Eli Zaretskii <eliz <at> gnu.org> writes:

>> Cc: Sergio Pastor Pérez <sergio.pastorperez <at> gmail.com>
>> From: Sergio Pastor Pérez <sergio.pastorperez <at> gmail.com>
>> Date: Sun, 24 Aug 2025 13:13:01 +0200
>> 
>> * src/indent.c (vertical-motion): If point is inside an overlay, reset
>> it to the beginning of line before trying to reach goal column. This
>> prevents point from being stuck at the beginning of overlay strings
>> during upward motions.
>> ---
>>  src/indent.c | 3 +++
>>  1 file changed, 3 insertions(+)
>> 
>> diff --git a/src/indent.c b/src/indent.c
>> index b4f3c349dc5..01865c1494f 100644
>> --- a/src/indent.c
>> +++ b/src/indent.c
>> @@ -2506,6 +2506,9 @@ line (if such column exists on that line, that is).  If the line is
>>  	 an addition to the hscroll amount.  */
>>        if (!NILP (lcols))
>>  	{
>> +	  if (it.method == GET_FROM_STRING)
>> +	    reseat_at_previous_visible_line_start(&it);
>> +
>>  	  move_it_in_display_line (&it, ZV, first_x + to_x, MOVE_TO_X);
>>  	  /* If we find ourselves in the middle of an overlay string
>>  	     which includes a newline after current string position,
>
> Is this needed only for overlays?  What about 'display' text
> properties?  (There are also other cases when it.method could be
> GET_FROM_STRING.)  If this is only for overlay strings, we need to use
>
>   if (it.method == GET_FROM_STRING && !NILP (it.from_overlay))
>
> instead.  Please test this at least with 'display' property strings
> instead of overlay strings, and see if the same problem happens in
> that case.  Then we will be able to decide how to write the condition
> in this case.

So I've tested this behavior with this display property:
--8<---------------cut here---------------start------------->8---
(let* ((start (point))
       (end (1+ (point)))
       (ov (make-overlay start end)))
  (overlay-put ov 'display "--->HELLO\nAnother message\nDONE<---"))
--8<---------------cut here---------------end--------------->8---

I've noticed that *without* applying the patch, there is some unexpected
behavior in the vertical line movement. With the v2 patch applied this
is partially corrected so it seems some fix is needed for 'display' text
properties as well.

Example test:
--8<---------------cut here---------------start------------->8---
8         Permission is granted to copy, --->HELLO
     Another message
     DONE<---istribute and/or modify this
9         document<|> under the terms of the GNU Free Documentation License,
10        Version 1.3 or any later version published by the Free Software
--8<---------------cut here---------------end--------------->8---

Without the patch going up 1 line goes to:
--8<---------------cut here---------------start------------->8---
8         Permission is granted to copy, --->HELLO
     Another message
     DONE<---istri<|>bute and/or modify this
9         document under the terms of the GNU Free Documentation License,
10        Version 1.3 or any later version published by the Free Software
--8<---------------cut here---------------end--------------->8---

And the next up goes somewhere to line 7 which is very unexpected since
going downwards from line 7 passes through the 'Permissions' and
'distribute' words.

*With* the patch, the behavior is partially corrected, this is what
happens:
--8<---------------cut here---------------start------------->8---
8         Permission is granted to copy, --->HELLO
     Another message
     DONE<---istribute and/or modify this
9         document<|> under the terms of the GNU Free Documentation License,
10        Version 1.3 or any later version published by the Free Software
--8<---------------cut here---------------end--------------->8---

Going upwards 1 line leads point to:
--8<---------------cut here---------------start------------->8---
8         Permissi<|>on is granted to copy, --->HELLO
     Another message
     DONE<---istribute and/or modify this
9         document under the terms of the GNU Free Documentation License,
10        Version 1.3 or any later version published by the Free Software
--8<---------------cut here---------------end--------------->8---

If point where to be here:
--8<---------------cut here---------------start------------->8---
8         Permission is granted to copy, --->HELLO
     Another message
     DONE<---istri<|>bute and/or modify this
9         document under the terms of the GNU Free Documentation License,
10        Version 1.3 or any later version published by the Free Software
--8<---------------cut here---------------end--------------->8---

Going upwards 1 line leads point here:
--8<---------------cut here---------------start------------->8---
8         Permissi<|>on is granted to copy, --->HELLO
     Another message
     DONE<---istribute and/or modify this
9         document under the terms of the GNU Free Documentation License,
10        Version 1.3 or any later version published by the Free Software
--8<---------------cut here---------------end--------------->8---

This is very cumbersome to explain by text, so I encourage you to try it
out if you have time. I think the ideal behavior would be to not skip
passing through 'distribute' when going upwards, as it does in a
downwards motion. I don't know how to make it not skip that line, but I
can tell you that what's happening is that, when going upwards from line
9 towards 8, point reaches the 'distribute' word in the virtual line,
and then the `reseat_at_previous_visible_line_start(&it)' moves point to
'bol' of line 8, before going to goal column in line 8.

I think what we really want is to detect that there is a multi-line
display text property before point in the virtual line, and not trigger
the `reseat_at_previous_visible_line_start(&it)' when going from line 9
to the 'distribute' word. But, if we are already in the line of the
'distribute' word, keep the behavior of the patch.


Let me know if you have any idea for how to archive this result. I
apologize for the convoluted example, I could not think of a clearer way
to describe the behavior by text!

Previous Next

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