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

To reply to this bug, email your comments to 79241 AT debbugs.gnu.org.

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

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

From: Sergio Pastor Pérez <sergio.pastorperez <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: 31.0.50; `vertical-motion' not respecting goal column when there is
 an overlay that spans multiple virtual lines
Date: Fri, 15 Aug 2025 10:31:29 +0200

The issue is that `vertical-motion' does not seem to respect goal column
when moving upwards through an overlay that spans multiple visual lines.

I will give a recipe using the latest version of Flymake, since it's an
easy way to generate an overlay that spawns multiple visual lines, but I
suspect that this issue can be reproduced with any overlay of those
characteristics, and is not a particual issue of Flymake. I've tried to
debug the line movement code but at some point I reached
`vertical-motion', which is a C function, so I didn't debug it further.
Invoking: `(vertical-motion '(0.0 . -1))' is enough to see the behavior
described.

1) Install Flymake v1.4.1. (I used Elpaca)
2) `(setq flymake-show-diagnostics-at-end-of-line 'fancy)`
3) Go to a buffer where diagnostic will show, I used Eglot for this.
4) Put the cursor a the beginning of the line prior to the overlay
generated by Flymake. Notice that when using the `'fancy' setting, the
message will span across multiple visual lines.
5) Try to move upwards, towards the diagnostic. We expect the cursor to
remain at the beggining of the line but instead, it goes to the end of
the line. This behavior does not happen when moving downwards through
the diagnostic message.
The issue is that `vertical-motion' does not seem to respect goal column
when moving upwards through an overlay that spans multiple visual lines.

I will give a recipe using the latest version of Flymake, since it's an
easy way to generate an overlay that spawns multiple visual lines, but I
suspect that this issue can be reproduced with any overlay of those
characteristics and is not a particular issue of Flymake. I've tried to
debug the line movement code but at some point I reached
`vertical-motion' which is a C function so I cannot debug it further.
Invoking: `(vertical-motion '(0.0 . -1))' is enough to see the behavior
described.

1) Install Flymake v1.4.1. (I used Elpaca)
2) `(setq flymake-show-diagnostics-at-end-of-line 'fancy)`
3) Go to a buffer where diagnostic will show, I used Eglot for this.
4) Put the cursor a the beginning of the line prior to the overlay
generated by Flymake. Notice that when using the `'fancy' setting, the
message will span across multiple visual lines.
5) Try to move upwords, towards the diagnostic. We expect the cursor to
remain at the beginning of the line but instead, it goes to the end of
the line. This behavior does not happen when moving downwards through
the diagnostic message.


In GNU Emacs 31.0.50 (build 1, x86_64-pc-linux-gnu, GTK+ Version
3.24.43, cairo version 1.18.4)
System Description: Guix System

Configured using:
 'configure
 CONFIG_SHELL=/gnu/store/nnx8iifrj6jfih4sivivq17cf65aa968-bash-minimal-5.2.37/bin/bash
 SHELL=/gnu/store/nnx8iifrj6jfih4sivivq17cf65aa968-bash-minimal-5.2.37/bin/bash
 --prefix=/gnu/store/g1rsgv58jnqpy6q2rnhy7xzqvpjks2f4-emacs-next-pgtk-31.0.50-1.9663c95
 --enable-fast-install --with-pgtk --with-cairo --with-modules
 --with-native-compilation=aot 'CFLAGS=-g -O2
 -Wno-error=incompatible-pointer-types' --disable-build-details'

Configured features:
ACL CAIRO DBUS FREETYPE GIF GLIB GMP GNUTLS GPM GSETTINGS HARFBUZZ JPEG
LCMS2 LIBOTF LIBSELINUX LIBSYSTEMD LIBXML2 MODULES NATIVE_COMP NOTIFY
INOTIFY PDUMPER PGTK PNG RSVG SECCOMP SOUND SQLITE3 THREADS TIFF
TOOLKIT_SCROLL_BARS TREE_SITTER WEBP XIM GTK3 ZLIB

Important settings:
  value of $EMACSLOADPATH: /home/pastor/.guix-home/profile/share/emacs/site-lisp:/home/pastor/.guix-home/profile/share/emacs/site-lisp:/run/current-system/profile/share/emacs/site-lisp:/gnu/store/g1rsgv58jnqpy6q2rnhy7xzqvpjks2f4-emacs-next-pgtk-31.0.50-1.9663c95/share/emacs/31.0.50/lisp
  value of $EMACSNATIVELOADPATH: /home/pastor/.guix-home/profile/lib/emacs/native-site-lisp:/home/pastor/.guix-home/profile/lib/emacs/native-site-lisp
  value of $LANG: en_US.utf8
  locale-coding-system: utf-8-unix

Major mode: C++//l

Minor modes in effect:
  eglot--managed-mode: t
  flymake-mode: t
  elpaca-use-package-mode: t
  override-global-mode: t
  tooltip-mode: t
  global-eldoc-mode: t
  eldoc-mode: t
  show-paren-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  tool-bar-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  minibuffer-regexp-mode: t
  line-number-mode: t
  indent-tabs-mode: t
  transient-mark-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  abbrev-mode: t

Load-path shadows:
/home/pastor/.guix-home/profile/share/emacs/site-lisp/guix-emacs hides /run/current-system/profile/share/emacs/site-lisp/guix-emacs
/home/pastor/.guix-home/profile/share/emacs/site-lisp/site-start hides /run/current-system/profile/share/emacs/site-lisp/site-start
/home/pastor/.emacs.d/elpaca/builds/flymake/flymake hides /gnu/store/g1rsgv58jnqpy6q2rnhy7xzqvpjks2f4-emacs-next-pgtk-31.0.50-1.9663c95/share/emacs/31.0.50/lisp/progmodes/flymake
/gnu/store/1i3864529l6ckl45kc80b2y72wfigf6m-emacs-compat-30.0.2.0/share/emacs/site-lisp/compat-30.0.2.0/compat hides /gnu/store/g1rsgv58jnqpy6q2rnhy7xzqvpjks2f4-emacs-next-pgtk-31.0.50-1.9663c95/share/emacs/31.0.50/lisp/emacs-lisp/compat

Features:
(shadow sort mail-extr info emacsbug lisp-mnt message yank-media puny
dired dired-loaddefs rfc822 mml mml-sec epa derived epg rfc6068
epg-config gnus-util mm-decode mm-bodies mm-encode mail-parse rfc2231
mailabbrev gmm-utils mailheader sendmail rfc2047 rfc2045 ietf-drums
mm-util mail-prsvr mail-utils rect cl-extra eglot tree-widget wid-edit
external-completion jsonrpc xref flymake thingatpt project diff ert pp
ewoc debug backtrace find-func filenotify compile text-property-search
pcase imenu cc-mode cc-fonts cc-guess cc-menus cc-cmds cc-styles
cc-align cc-engine cc-vars cc-defs misearch multi-isearch
flymake-autoloads tramp-archive tramp-gvfs dbus xml tramp trampver
tramp-integration tramp-message help-mode tramp-compat xdg shell
pcomplete comint ansi-osc ring parse-time iso8601 time-date format-spec
ansi-color tramp-loaddefs elpaca-menu-elpa no-littering compat
no-littering-autoloads elpaca-menu-melpa elpaca-menu-org
elpaca-use-package use-package use-package-ensure use-package-delight
use-package-diminish use-package-bind-key bind-key use-package-core
elpaca-use-package-autoloads hl-line display-line-numbers elpaca-log
elpaca-ui url url-proxy url-privacy url-expand url-methods url-history
url-cookie generate-lisp-file url-domsuf url-util url-parse auth-source
eieio eieio-core cl-macs password-cache json subr-x map byte-opt gv
bytecomp byte-compile url-vars mailcap cl-seq elpaca warnings icons
elpaca-process elpaca-autoloads vc-git diff-mode track-changes
easy-mmode files-x vc-dispatcher cl-loaddefs cl-lib
magit-popup-autoloads geiser-guile-autoloads edit-indirect-autoloads
bui-autoloads guix-autoloads rx stgit-autoloads
page-break-lines-autoloads dashboard-autoloads memoize-autoloads
dash-autoloads s-autoloads f-autoloads all-the-icons-autoloads
log4e-autoloads gntp-autoloads alert-autoloads telega-contrib-autoloads
rainbow-identifiers-autoloads visual-fill-column-autoloads
telega-autoloads consult-autoloads consult-notmuch-autoloads
notmuch-indicator-autoloads notmuch-autoloads compat-autoloads
jinx-autoloads tablist-autoloads pdf-tools-autoloads company-autoloads
geiser-autoloads vterm-git.f76de08-autoloads guix-emacs rmc iso-transl
tooltip cconv eldoc paren electric uniquify ediff-hook vc-hooks
lisp-float-type elisp-mode mwheel term/pgtk-win pgtk-win term/common-win
touch-screen pgtk-dnd tool-bar dnd fontset image regexp-opt fringe
tabulated-list replace newcomment text-mode lisp-mode prog-mode register
page tab-bar menu-bar rfn-eshadow isearch easymenu timer select
scroll-bar mouse jit-lock font-lock syntax font-core term/tty-colors
frame minibuffer nadvice seq simple cl-generic indonesian philippine
cham georgian utf-8-lang misc-lang vietnamese tibetan thai tai-viet lao
korean japanese eucjp-ms cp51932 hebrew greek romanian slovak czech
european ethiopic indian cyrillic chinese composite emoji-zwj charscript
charprop case-table epa-hook jka-cmpr-hook help abbrev obarray oclosure
cl-preloaded button loaddefs theme-loaddefs faces cus-face macroexp
files window text-properties overlay sha1 md5 base64 format env
code-pages mule custom widget keymap hashtable-print-readable backquote
threads dbusbind inotify dynamic-setting system-font-setting
font-render-setting cairo gtk pgtk lcms2 multi-tty move-toolbar
make-network-process tty-child-frames native-compile emacs)

Memory information:
((conses 16 505081 51672) (symbols 48 23934 48)
 (strings 32 216943 3808) (string-bytes 1 4504841) (vectors 16 35040)
 (vector-slots 8 447974 25893) (floats 8 76 879)
 (intervals 56 2253 176) (buffers 992 26))

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Sergio Pastor Pérez <sergio.pastorperez <at> gmail.com>
Cc: 79241 <at> debbugs.gnu.org
Subject: Re: bug#79241: 31.0.50;
 `vertical-motion' not respecting goal column when there is an overlay
 that spans multiple virtual lines
Date: Sat, 16 Aug 2025 09:50:57 +0300

severity 79241 minor
thanks

> From: Sergio Pastor Pérez <sergio.pastorperez <at> gmail.com>
> Date: Fri, 15 Aug 2025 10:31:29 +0200
> 
> The issue is that `vertical-motion' does not seem to respect goal column
> when moving upwards through an overlay that spans multiple visual lines.
> 
> I will give a recipe using the latest version of Flymake, since it's an
> easy way to generate an overlay that spawns multiple visual lines, but I
> suspect that this issue can be reproduced with any overlay of those
> characteristics, and is not a particual issue of Flymake. I've tried to
> debug the line movement code but at some point I reached
> `vertical-motion', which is a C function, so I didn't debug it further.
> Invoking: `(vertical-motion '(0.0 . -1))' is enough to see the behavior
> described.
> 
> 1) Install Flymake v1.4.1. (I used Elpaca)
> 2) `(setq flymake-show-diagnostics-at-end-of-line 'fancy)`
> 3) Go to a buffer where diagnostic will show, I used Eglot for this.
> 4) Put the cursor a the beginning of the line prior to the overlay
> generated by Flymake. Notice that when using the `'fancy' setting, the
> message will span across multiple visual lines.
> 5) Try to move upwards, towards the diagnostic. We expect the cursor to
> remain at the beggining of the line but instead, it goes to the end of
> the line. This behavior does not happen when moving downwards through
> the diagnostic message.

Thanks.

The "beginning of the line" where the Flymake diagnostic is displayed
doesn't really exist in the buffer.  If you place point at the end of
the code line for which Flymake shows a diagnostic and then press C-f,
the cursor will jump over all the diagnostic string and land at the
beginning of the _following_ line.  This is expected, because Emacs
cannot place the cursor inside an overlay, not really.  Flymake
attempts to "fix" that by having the 'cursor' property on the
overlay's string, but that only works up to a point.

Overlay strings like this with embedded newlines cause a lot of
complexity and grief in vertical-motion's implementation, so I'm not
surprised that it behaves a bit strangely in this case.  The basic
requirement from vertical-motion in these cases is not to get stuck
around such lines, so that C-n and C-p work reasonably well and allow
movement through the buffer, and that does work in this case.

So from where I stand, this is a very minor annoyance, nothing more.
If someone wants to work on improving the behavior of vertical-motion
in these cases, and they do a clean job, such changes will be very
welcome, but failing that, I think the current behavior is far from a
catastrophe that needs immediate attention.

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

From: Sergio Pastor Pérez <sergio.pastorperez <at> gmail.com>
To: 79241 <at> debbugs.gnu.org
Cc: Sergio Pastor Pérez <sergio.pastorperez <at> gmail.com>
Subject: [PATCH] WIP: Fix incorrect handling of overlays in `vertical-motion'
Date: Tue, 19 Aug 2025 21:21:58 +0200

FIXME: if multiple lines with multi-line overlays are present, going
upwards jumps to the first one of them, instead of the previous line at
the right column.

* src/indent.c (vertical-motion): Ensure multi-line overlay handling
logic captures upward motions.
---
 src/indent.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/src/indent.c b/src/indent.c
index b4f3c349dc5..c1c32824dad 100644
--- 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);
 		}
 	    }
-- 
2.50.1

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

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

Hello!

I've debugged the `vertical-motion' procedure and found the troublesome
code. This patch solves the issue with the goal column, but it has 1
corner case to that I don't know how to solve.

Sergio Pastor Pérez <sergio.pastorperez <at> gmail.com> writes:

> FIXME: if multiple lines with multi-line overlays are present, going
> upwards jumps to the first one of them, instead of the previous line at
> the right column.
>
> * src/indent.c (vertical-motion): Ensure multi-line overlay handling
> logic captures upward motions.
> ---
>  src/indent.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/src/indent.c b/src/indent.c
> index b4f3c349dc5..c1c32824dad 100644
> --- 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.

>  	    }


Best regards,
Sergio

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Sergio Pastor Pérez <sergio.pastorperez <at> gmail.com>
Cc: 79241 <at> debbugs.gnu.org
Subject: Re: bug#79241: [PATCH] WIP: Fix incorrect handling of overlays in
 `vertical-motion'
Date: Wed, 20 Aug 2025 13:45:49 +0300

> From: Sergio Pastor Pérez <sergio.pastorperez <at> gmail.com>
> Cc: Eli Zaretskii <eliz <at> gnu.org>
> Date: Tue, 19 Aug 2025 21:44:55 +0200
> 
> I've debugged the `vertical-motion' procedure and found the troublesome
> code. This patch solves the issue with the goal column, but it has 1
> corner case to that I don't know how to solve.

Thanks for working on this.

> > --- 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?

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

From: Sergio Pastor Pérez <sergio.pastorperez <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 79241 <at> debbugs.gnu.org
Subject: Re: bug#79241: [PATCH] WIP: Fix incorrect handling of overlays in
 `vertical-motion'
Date: Wed, 20 Aug 2025 20:57:02 +0200

Eli Zaretskii <eliz <at> gnu.org> 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?

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Sergio Pastor Pérez <sergio.pastorperez <at> gmail.com>
Cc: 79241 <at> debbugs.gnu.org
Subject: Re: bug#79241: [PATCH] WIP: Fix incorrect handling of overlays in
 `vertical-motion'
Date: Sat, 23 Aug 2025 15:25:08 +0300

> From: Sergio Pastor Pérez <sergio.pastorperez <at> gmail.com>
> Cc: 79241 <at> debbugs.gnu.org
> Date: Wed, 20 Aug 2025 20:57:02 +0200
> 
> Eli Zaretskii <eliz <at> gnu.org> 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.

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

From: Sergio Pastor Pérez <sergio.pastorperez <at> gmail.com>
To: 79241 <at> debbugs.gnu.org
Cc: Sergio Pastor Pérez <sergio.pastorperez <at> gmail.com>
Subject: [PATCH v2] Fix incorrect handling of overlays in `vertical-motion'
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,
-- 
2.51.0

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

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

Hello, Eli.

I've debugged a bit more this issue, I've found an easy solution (2
lines of code) that solves both issues, reaching the goal column and not
skipping lines. I just sent the v2 of the patch. Please take a look!


Best regards,
Sergio

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

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

> 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.

Thanks.

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

From: Sergio Pastor Pérez <sergio.pastorperez <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 79241 <at> debbugs.gnu.org
Subject: Re: 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!

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

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

> From: Sergio Pastor Pérez <sergio.pastorperez <at> gmail.com>
> Cc: 79241 <at> debbugs.gnu.org
> Date: Sun, 24 Aug 2025 19:58:47 +0200
> 
> Eli Zaretskii <eliz <at> gnu.org> writes:
> 
> >> +	  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---

No, this is not what I meant.  I meant to test 'display' text
property, not overlay property.

> 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.

This is a slightly different situation, so let's leave it alone for
now and focus on the previous use case.  Your patch fixed the case of
an overlay string, let's see whether it is needed with display strings
from 'display' text properties.

Thanks.

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

From: Sergio Pastor Pérez <sergio.pastorperez <at> gmail.com>
To: 79241 <at> debbugs.gnu.org
Cc: Sergio Pastor Pérez <sergio.pastorperez <at> gmail.com>
Subject: [PATCH v3] Fix incorrect handling of overlays in `vertical-motion'
Date: Sun, 24 Aug 2025 20:27:06 +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..95228b26825 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 && !NILP (it.from_overlay))
+	    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,
-- 
2.51.0

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

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

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

>> From: Sergio Pastor Pérez <sergio.pastorperez <at> gmail.com>
>> Cc: 79241 <at> debbugs.gnu.org
>> Date: Sun, 24 Aug 2025 19:58:47 +0200
>> 
>> Eli Zaretskii <eliz <at> gnu.org> writes:
>> 
>> >> +	  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---
>
> No, this is not what I meant.  I meant to test 'display' text
> property, not overlay property.

Oh, understood. I've sent version 3 so it only affects overlays, for
display text properties we have the same convoluted behavior I described
in the previous mail. With v2 of the patch, at least the target column
is corrected but has the line skip issue, what I said in the previous
mail still holds.

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Sergio Pastor Pérez <sergio.pastorperez <at> gmail.com>
Cc: 79241 <at> debbugs.gnu.org
Subject: Re: bug#79241: [PATCH v3] Fix incorrect handling of overlays in
 `vertical-motion'
Date: Sat, 30 Aug 2025 12:44:00 +0300

> 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 20:27:06 +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.

Thanks, installed on the master branch.

This was a very small change, so we could accept it without your
assigning the copyright to the FSF.  However, if you intend to
contribute more changes, I would suggest to start your
copyright-assignment paperwork at this time, so that we could in the
future accept your contributions without any size limitations.

If you agree, I will send you the form to fill and the instructions to
go with it.

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

From: Sergio Pastor Pérez <sergio.pastorperez <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 79241 <at> debbugs.gnu.org
Subject: Re: bug#79241: [PATCH v3] Fix incorrect handling of overlays in
 `vertical-motion'
Date: Mon, 01 Sep 2025 18:21:06 +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 20:27:06 +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.
>
> Thanks, installed on the master branch.
>
> This was a very small change, so we could accept it without your
> assigning the copyright to the FSF.  However, if you intend to
> contribute more changes, I would suggest to start your
> copyright-assignment paperwork at this time, so that we could in the
> future accept your contributions without any size limitations.
>
> If you agree, I will send you the form to fill and the instructions to
> go with it.

Sure forward me the form. I will give a hand if I find anything I can
contribute to.


Thanks,
Sergio

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Sergio Pastor Pérez <sergio.pastorperez <at> gmail.com>
Cc: 79241 <at> debbugs.gnu.org
Subject: Re: bug#79241: [PATCH v3] Fix incorrect handling of overlays in
 `vertical-motion'
Date: Mon, 01 Sep 2025 22:27:29 +0300

> From: Sergio Pastor Pérez <sergio.pastorperez <at> gmail.com>
> Cc: 79241 <at> debbugs.gnu.org
> Date: Mon, 01 Sep 2025 18:21:06 +0200
> 
> Eli Zaretskii <eliz <at> gnu.org> writes:
> >
> > This was a very small change, so we could accept it without your
> > assigning the copyright to the FSF.  However, if you intend to
> > contribute more changes, I would suggest to start your
> > copyright-assignment paperwork at this time, so that we could in the
> > future accept your contributions without any size limitations.
> >
> > If you agree, I will send you the form to fill and the instructions to
> > go with it.
> 
> Sure forward me the form. I will give a hand if I find anything I can
> contribute to.

Form sent off-list.

Previous Next

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