GNU bug report logs - #45837
28.0.50; incorrect cursor position in visual-line-mode when word-wrap-by-category is t

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 45837 in the body.
You can then email your comments to 45837 AT debbugs.gnu.org in the normal way.

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

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

From: Liu Hui <liuhui1610 <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: 28.0.50; incorrect cursor position in visual-line-mode when
 word-wrap-by-category is t
Date: Wed, 13 Jan 2021 10:27:24 +0800

Steps to reproduce:
1. emacs -Q
2. eval the following code in the *scratch* buffer:

    (with-current-buffer-window "*test*" nil nil
      (dotimes (_ 3)
        (dotimes (i 10)
          (insert "一二三四五六七八九十"))
        (dotimes (i 20)
          (insert "test ")))
      (setq word-wrap-by-category t)
      (visual-line-mode))

3. In the *test* buffer, C-a/C-e don't always move the cursor to the
beginning/end of screen line. There is also an offset between the mouse
click location and the cursor position. If there seems to be no problem,
you can adjust the window width and recheck the cursor movement.


In GNU Emacs 28.0.50 (build 1, x86_64-pc-linux-gnu, GTK+ Version
3.24.20, cairo version 1.16.0)
of 2021-01-12 built on lcy01-amd64-029
Windowing system distributor 'The X.Org Foundation', version 11.0.12009000
System Description: Ubuntu 20.04.1 LTS

Configured using:
'configure --build=x86_64-linux-gnu --prefix=/usr
'--includedir=${prefix}/include' '--mandir=${prefix}/share/man'
'--infodir=${prefix}/share/info' --sysconfdir=/etc --localstatedir=/var
--disable-silent-rules '--libdir=${prefix}/lib/x86_64-linux-gnu'
'--libexecdir=${prefix}/lib/x86_64-linux-gnu' --disable-maintainer-mode
--disable-dependency-tracking --prefix=/usr --sharedstatedir=/var/lib
--program-suffix=-snapshot --with-modules=yes --with-x=yes
--with-x-toolkit=gtk3 --with-xwidgets=yes 'CFLAGS=-g -O2
-fdebug-prefix-map=/build/emacs-snapshot-4HYkJA/emacs-snapshot-103322=.
-fstack-protector-strong
-Wformat -Werror=format-security' 'CPPFLAGS=-Wdate-time
-D_FORTIFY_SOURCE=2' 'LDFLAGS=-Wl,-Bsymbolic-functions -Wl,-z,relro''

Configured features:
ACL CAIRO DBUS FREETYPE GIF GLIB GMP GNUTLS GPM GSETTINGS HARFBUZZ JPEG
JSON LCMS2 LIBOTF LIBSELINUX LIBSYSTEMD LIBXML2 M17N_FLT MODULES NOTIFY
INOTIFY PDUMPER PNG RSVG SOUND THREADS TIFF TOOLKIT_SCROLL_BARS X11 XDBE
XIM XPM XWIDGETS GTK3 ZLIB

Important settings:
value of $LANG: zh_CN.UTF-8
value of $XMODIFIERS: @im=ibus
locale-coding-system: utf-8-unix

Major mode: Lisp Interaction

Minor modes in effect:
tooltip-mode: t
global-eldoc-mode: t
eldoc-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
auto-composition-mode: t
auto-encryption-mode: t
auto-compression-mode: t
line-number-mode: t
transient-mark-mode: t

Load-path shadows:
None found.

Features:
(shadow sort mail-extr emacsbug message rmc puny dired dired-loaddefs
rfc822 mml easymenu mml-sec epa derived epg epg-config gnus-util rmail
rmail-loaddefs auth-source cl-seq eieio eieio-core cl-macs
eieio-loaddefs password-cache json map text-property-search time-date
subr-x seq byte-opt gv bytecomp byte-compile cconv mm-decode mm-bodies
mm-encode mail-parse rfc2231 mailabbrev gmm-utils mailheader cl-loaddefs
cl-lib sendmail rfc2047 rfc2045 ietf-drums mm-util mail-prsvr mail-utils
china-util iso-transl tooltip eldoc electric uniquify ediff-hook
vc-hooks lisp-float-type mwheel term/x-win x-win term/common-win x-dnd
tool-bar dnd fontset image regexp-opt fringe tabulated-list replace
newcomment text-mode elisp-mode lisp-mode prog-mode register page
tab-bar menu-bar rfn-eshadow isearch timer select scroll-bar mouse
jit-lock font-lock syntax facemenu font-core term/tty-colors frame
minibuffer cl-generic 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 charscript charprop case-table epa-hook jka-cmpr-hook help
simple abbrev obarray cl-preloaded nadvice button loaddefs faces
cus-face macroexp files window text-properties overlay sha1 md5 base64
format env code-pages mule custom widget hashtable-print-readable
backquote threads xwidget-internal dbusbind inotify lcms2
dynamic-setting system-font-setting font-render-setting cairo
move-toolbar gtk x-toolkit x multi-tty make-network-process emacs)

Memory information:
((conses 16 51700 5953)
(symbols 48 6834 1)
(strings 32 18505 2017)
(string-bytes 1 607714)
(vectors 16 12701)
(vector-slots 8 222094 12046)
(floats 8 21 45)
(intervals 56 226 0)
(buffers 984 10))

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Liu Hui <liuhui1610 <at> gmail.com>
Cc: 45837 <at> debbugs.gnu.org
Subject: Re: bug#45837: 28.0.50;
 incorrect cursor position in visual-line-mode when
 word-wrap-by-category is t
Date: Wed, 13 Jan 2021 16:47:48 +0200

> From: Liu Hui <liuhui1610 <at> gmail.com>
> Date: Wed, 13 Jan 2021 10:27:24 +0800
> 
> Steps to reproduce:
> 1. emacs -Q
> 2. eval the following code in the *scratch* buffer:
> 
>     (with-current-buffer-window "*test*" nil nil
>       (dotimes (_ 3)
>         (dotimes (i 10)
>           (insert "一二三四五六七八九十"))
>         (dotimes (i 20)
>           (insert "test ")))
>       (setq word-wrap-by-category t)
>       (visual-line-mode))
> 
> 3. In the *test* buffer, C-a/C-e don't always move the cursor to the
> beginning/end of screen line. There is also an offset between the mouse
> click location and the cursor position. If there seems to be no problem,
> you can adjust the window width and recheck the cursor movement.

Thanks, should be fixed now.

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

From: Liu Hui <liuhui1610 <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 45837 <at> debbugs.gnu.org
Subject: Re: bug#45837: 28.0.50; incorrect cursor position in visual-line-mode
 when word-wrap-by-category is t
Date: Thu, 14 Jan 2021 12:51:12 +0800

Eli Zaretskii <eliz <at> gnu.org> 于2021年1月13日周三 下午10:47写道：
>
> > From: Liu Hui <liuhui1610 <at> gmail.com>
> > Date: Wed, 13 Jan 2021 10:27:24 +0800
> >
> > Steps to reproduce:
> > 1. emacs -Q
> > 2. eval the following code in the *scratch* buffer:
> >
> >     (with-current-buffer-window "*test*" nil nil
> >       (dotimes (_ 3)
> >         (dotimes (i 10)
> >           (insert "一二三四五六七八九十"))
> >         (dotimes (i 20)
> >           (insert "test ")))
> >       (setq word-wrap-by-category t)
> >       (visual-line-mode))
> >
> > 3. In the *test* buffer, C-a/C-e don't always move the cursor to the
> > beginning/end of screen line. There is also an offset between the mouse
> > click location and the cursor position. If there seems to be no problem,
> > you can adjust the window width and recheck the cursor movement.
>
> Thanks, should be fixed now.

Thank you, cursor motion is correct now.

BTW, in the case above, if the line wraps after a non-whitespace
character, C-k does not delete this character. How about the following
change to kill-visual-line?

diff --git a/lisp/simple.el b/lisp/simple.el
index 54c35c04be..86e2c41ac2 100644
--- a/lisp/simple.el
+++ b/lisp/simple.el
@@ -7337,6 +7337,9 @@ kill-visual-line
       (end-of-visual-line 1)
       (if (= (point) opoint)
          (vertical-motion 1)
+        (when (and word-wrap-by-category
+                   (elt (char-category-set (following-char)) ?|))
+          (forward-char))
        ;; Skip any trailing whitespace at the end of the visual line.
        ;; We used to do this only if `show-trailing-whitespace' is
        ;; nil, but that's wrong; the correct thing would be to check

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Liu Hui <liuhui1610 <at> gmail.com>
Cc: 45837 <at> debbugs.gnu.org
Subject: Re: bug#45837: 28.0.50; incorrect cursor position in visual-line-mode
 when word-wrap-by-category is t
Date: Thu, 14 Jan 2021 15:51:47 +0200

> From: Liu Hui <liuhui1610 <at> gmail.com>
> Date: Thu, 14 Jan 2021 12:51:12 +0800
> Cc: 45837 <at> debbugs.gnu.org
> 
> BTW, in the case above, if the line wraps after a non-whitespace
> character, C-k does not delete this character. How about the following
> change to kill-visual-line?

Yes, good catch.  However, this is not entirely right, as the code you
suggest should be _instead_ of skipping the whitespace that follows,
not _in_addition_ to it (imagine that the line is wrapped at a
non-whitespace character followed by whitespace).  And while looking
into adapting your patch to avoid removing more than the user
intended, I found more issues with the existing code.  So I'm
proposing the patch below instead, which should correctly handle the
following use cases:

 . visual line that ends in one or more whitespace characters, and the
   following visual line begins with one or more whitespace characters
 . visual line that ends with a non-whitespace character (under
   word-wrap-by-category) that is followed by one or more whitespace
   characters
 . line that is continued on the next visual line (visual-line-mode is
   off)
 . visual line that is wrapped in the middle of a display string or an
   overlay string with embedded whitespace characters
 . visual line that is wrapped in the middle of intangible text

(The patch also fixes some minor issues with the documentation of this
command.)

WDYT?

diff --git a/lisp/simple.el b/lisp/simple.el
index 54c35c04be..e7421c069f 100644
--- a/lisp/simple.el
+++ b/lisp/simple.el
@@ -5606,7 +5606,9 @@ zap-to-char
 ;; kill-line and its subroutines.
 
 (defcustom kill-whole-line nil
-  "If non-nil, `kill-line' with no arg at start of line kills the whole line."
+  "If non-nil, `kill-line' with no arg at start of line kills the whole line.
+This variable also affects `kill-visual-line' in the same way as
+it does `kill-line'."
   :type 'boolean
   :group 'killing)
 
@@ -7319,6 +7321,9 @@ kill-visual-line
 If ARG is zero, kill the text before point on the current visual
 line.
 
+If `kill-whole-line' is non-nil, and this command is invoked at
+start of a line that ands in a newline, kill the newline as well.
+
 If you want to append the killed line to the last killed text,
 use \\[append-next-kill] before \\[kill-line].
 
@@ -7331,18 +7336,26 @@ kill-visual-line
   ;; Like in `kill-line', it's better to move point to the other end
   ;; of the kill before killing.
   (let ((opoint (point))
-	(kill-whole-line (and kill-whole-line (bolp))))
+        (kill-whole-line (and kill-whole-line (bolp)))
+        (orig-y (cdr (nth 2 (posn-at-point)))))
     (if arg
 	(vertical-motion (prefix-numeric-value arg))
       (end-of-visual-line 1)
       (if (= (point) opoint)
 	  (vertical-motion 1)
-	;; Skip any trailing whitespace at the end of the visual line.
-	;; We used to do this only if `show-trailing-whitespace' is
-	;; nil, but that's wrong; the correct thing would be to check
-	;; whether the trailing whitespace is highlighted.  But, it's
-	;; OK to just do this unconditionally.
-	(skip-chars-forward " \t")))
+        ;; The first condition below verifies we are still on the same
+        ;; screen line, i.e. that the line isn't continued, and that
+        ;; end-of-visual-line didn't overshoot due to complications
+        ;; like display or overlay strings, intangible text, etc.:
+        ;; otherwise, we don't want to kill a character that's
+        ;; unrelated to the place where the visual line wrapped.
+        (and (= (cdr (nth 2 (posn-at-point))) orig-y)
+             ;; Make sure we delete the character where the line wraps
+             ;; under visual-line-mode.
+             (or (looking-at-p "[ \t]")
+                 (and word-wrap-by-category
+                      (aref (char-category-set (following-char)) ?\|)))
+             (forward-char))))
     (kill-region opoint (if (and kill-whole-line (= (following-char) ?\n))
 			    (1+ (point))
 			  (point)))))

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

From: Liu Hui <liuhui1610 <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 45837 <at> debbugs.gnu.org
Subject: Re: bug#45837: 28.0.50; incorrect cursor position in visual-line-mode
 when word-wrap-by-category is t
Date: Fri, 15 Jan 2021 15:28:33 +0800

Eli Zaretskii <eliz <at> gnu.org> 于2021年1月14日周四 下午9:51写道：
>
> > From: Liu Hui <liuhui1610 <at> gmail.com>
> > Date: Thu, 14 Jan 2021 12:51:12 +0800
> > Cc: 45837 <at> debbugs.gnu.org
> >
> > BTW, in the case above, if the line wraps after a non-whitespace
> > character, C-k does not delete this character. How about the following
> > change to kill-visual-line?
>
> Yes, good catch.  However, this is not entirely right, as the code you
> suggest should be _instead_ of skipping the whitespace that follows,
> not _in_addition_ to it (imagine that the line is wrapped at a
> non-whitespace character followed by whitespace).  And while looking
> into adapting your patch to avoid removing more than the user
> intended, I found more issues with the existing code.  So I'm
> proposing the patch below instead, which should correctly handle the
> following use cases:
>
>  . visual line that ends in one or more whitespace characters, and the
>    following visual line begins with one or more whitespace characters
>  . visual line that ends with a non-whitespace character (under
>    word-wrap-by-category) that is followed by one or more whitespace
>    characters
>  . line that is continued on the next visual line (visual-line-mode is
>    off)
>  . visual line that is wrapped in the middle of a display string or an
>    overlay string with embedded whitespace characters
>  . visual line that is wrapped in the middle of intangible text
>
> (The patch also fixes some minor issues with the documentation of this
> command.)
>
> WDYT?
>

I have tested the patch and found that the condition `(= (cdr (nth 2
(posn-at-point))) orig-y)` was sometimes too strict. `posn-at-point`
may give slightly different y positions for characters on the same
line when different fonts were used (examples can be found in the
HELLO file). If there are inline graphics (e.g. latex previews), the y
position can also be different.

My suggestion is `(< (abs (- (cdr (nth 2 (posn-at-point))) orig-y))
X)`, where X could be, empirically, `(/ (line-pixel-height) 3)` or
a customizable value.

The patch works well in other cases, thanks!

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Liu Hui <liuhui1610 <at> gmail.com>
Cc: 45837 <at> debbugs.gnu.org
Subject: Re: bug#45837: 28.0.50; incorrect cursor position in visual-line-mode
 when word-wrap-by-category is t
Date: Fri, 15 Jan 2021 10:15:08 +0200

> From: Liu Hui <liuhui1610 <at> gmail.com>
> Date: Fri, 15 Jan 2021 15:28:33 +0800
> Cc: 45837 <at> debbugs.gnu.org
> 
> I have tested the patch and found that the condition `(= (cdr (nth 2
> (posn-at-point))) orig-y)` was sometimes too strict. `posn-at-point`
> may give slightly different y positions for characters on the same
> line when different fonts were used (examples can be found in the
> HELLO file). If there are inline graphics (e.g. latex previews), the y
> position can also be different.

Hmm... you are right.  But that sounds like a bug in posn-at-point, I
will look into fixing it soon.

> My suggestion is `(< (abs (- (cdr (nth 2 (posn-at-point))) orig-y))
> X)`, where X could be, empirically, `(/ (line-pixel-height) 3)` or
> a customizable value.

I went with half the line height, thanks.

> The patch works well in other cases, thanks!

Thanks for testing, I've now installed the change on the master
branch.  I'm not yet closing the bug, because I want to see what can
be done about removing the tolerance in comparing values of
posn-at-point.

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: liuhui1610 <at> gmail.com
Cc: 45837 <at> debbugs.gnu.org
Subject: Re: bug#45837: 28.0.50;
 incorrect cursor position in visual-line-mode when
 word-wrap-by-category is t
Date: Mon, 18 Jan 2021 19:14:10 +0200

> Date: Fri, 15 Jan 2021 10:15:08 +0200
> From: Eli Zaretskii <eliz <at> gnu.org>
> Cc: 45837 <at> debbugs.gnu.org
> 
> > From: Liu Hui <liuhui1610 <at> gmail.com>
> > Date: Fri, 15 Jan 2021 15:28:33 +0800
> > Cc: 45837 <at> debbugs.gnu.org
> > 
> > I have tested the patch and found that the condition `(= (cdr (nth 2
> > (posn-at-point))) orig-y)` was sometimes too strict. `posn-at-point`
> > may give slightly different y positions for characters on the same
> > line when different fonts were used (examples can be found in the
> > HELLO file). If there are inline graphics (e.g. latex previews), the y
> > position can also be different.
> 
> Hmm... you are right.  But that sounds like a bug in posn-at-point, I
> will look into fixing it soon.

Upon looking into this, it isn't a bug: posn-at-point is working as
designed and documented:

  Return nil if POS is not visible in WINDOW.  Otherwise,
  the return value is similar to that returned by ‘event-start’ for
  a mouse click at the upper left corner of the glyph corresponding
  to POS:       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

"At the upper left corner of the glyph".  IOW, if we ask about a glyph
from a small font on an otherwise tall visual line, posn-at-point will
try to adjust the Y coordinate for the fact that the glyph is smaller,
and thus its upper left corner is further down from the window top.

So using the (X . Y) part of what posn-at-point returns is unworkable
for the purpose of this issue: a character about which we are asking
can be arbitrarily small, and thus to work in all the cases, we will
have to allow for a very large tolerance, and that could miss some
situations where we really end up on the next visual line.

However, posn-at-point also returns the coordinates in (COLUMN . ROW)
units, and we can use that instead.  So I propose to fix the current
code on master as below.  WDYT?

diff --git a/lisp/simple.el b/lisp/simple.el
index 37c0885dcc..2c6e3916cd 100644
--- a/lisp/simple.el
+++ b/lisp/simple.el
@@ -7338,10 +7338,7 @@ kill-visual-line
   ;; of the kill before killing.
   (let ((opoint (point))
         (kill-whole-line (and kill-whole-line (bolp)))
-        (orig-y (cdr (nth 2 (posn-at-point))))
-        ;; FIXME: This tolerance should be zero!  It isn't due to a
-        ;; bug in posn-at-point, see bug#45837.
-        (tol (/ (line-pixel-height) 2)))
+        (orig-vlnum (cdr (nth 6 (posn-at-point)))))
     (if arg
 	(vertical-motion (prefix-numeric-value arg))
       (end-of-visual-line 1)
@@ -7352,8 +7349,8 @@ kill-visual-line
         ;; end-of-visual-line didn't overshoot due to complications
         ;; like display or overlay strings, intangible text, etc.:
         ;; otherwise, we don't want to kill a character that's
-        ;; unrelated to the place where the visual line wrapped.
-        (and (< (abs (- (cdr (nth 2 (posn-at-point))) orig-y)) tol)
+        ;; unrelated to the place where the visual line wraps.
+        (and (= (cdr (nth 6 (posn-at-point))) orig-vlnum)
              ;; Make sure we delete the character where the line wraps
              ;; under visual-line-mode, be it whitespace or a
              ;; character whose category set allows to wrap at it.

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

From: Liu Hui <liuhui1610 <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 45837 <at> debbugs.gnu.org
Subject: Re: bug#45837: 28.0.50; incorrect cursor position in visual-line-mode
 when word-wrap-by-category is t
Date: Thu, 21 Jan 2021 11:07:50 +0800

Eli Zaretskii <eliz <at> gnu.org> 于2021年1月19日周二 上午1:14写道：
>
> However, posn-at-point also returns the coordinates in (COLUMN . ROW)
> units, and we can use that instead.  So I propose to fix the current
> code on master as below.  WDYT?

I think it's a better fix. Thanks!

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Liu Hui <liuhui1610 <at> gmail.com>
Cc: 45837-done <at> debbugs.gnu.org
Subject: Re: bug#45837: 28.0.50; incorrect cursor position in visual-line-mode
 when word-wrap-by-category is t
Date: Thu, 21 Jan 2021 16:22:53 +0200

> From: Liu Hui <liuhui1610 <at> gmail.com>
> Date: Thu, 21 Jan 2021 11:07:50 +0800
> Cc: 45837 <at> debbugs.gnu.org
> 
> Eli Zaretskii <eliz <at> gnu.org> 于2021年1月19日周二 上午1:14写道：
> >
> > However, posn-at-point also returns the coordinates in (COLUMN . ROW)
> > units, and we can use that instead.  So I propose to fix the current
> > code on master as below.  WDYT?
> 
> I think it's a better fix. Thanks!

Thanks for the feedback.  I've now installed that change, and I'm
closing this bug.

Previous Next

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