Package: emacs;
Reported by: Steven Allen <steven <at> stebalien.com>
Date: Sat, 27 Jul 2024 17:59:02 UTC
Severity: normal
Found in version 31.0.50
Done: Stefan Kangas <stefankangas <at> gmail.com>
Bug is archived. No further changes may be made.
Message #11 received at 72323 <at> debbugs.gnu.org (full text, mbox):
From: Steven Allen <steven <at> stebalien.com> To: Eli Zaretskii <eliz <at> gnu.org> Cc: 72323 <at> debbugs.gnu.org, storm <at> cua.dk Subject: Re: bug#72323: 31.0.50; line-move unconditionally resets vscroll to 0 Date: Sat, 27 Jul 2024 13:10:03 -0700
[Message part 1 (text/plain, inline)]
Eli Zaretskii <eliz <at> gnu.org> writes: >> Cc: "Kim F. Storm" <storm <at> cua.dk> >> Date: Sat, 27 Jul 2024 10:57:44 -0700 >> From: Steven Allen via "Bug reports for GNU Emacs, >> the Swiss army knife of text editors" <bug-gnu-emacs <at> gnu.org> >> >> Fixing home/end (beginning of line, end of line) case seems trivial: >> don't call `line-move' in these cases (or have `line-move' short-circuit >> when `arg' is 0). However, even after reading all the comments about >> scrolling images, I'm still not sure why it's necessary to reset vscroll >> to 0. > > Because otherwise what line-move does cannot be described in sensible > terms. If vscroll is not reset, then what would be the reference for > such a "line move"? By its very definition, line-move moves N screen > lines wrt the line which was its starting point, but with vscroll > non-zero that starting point could be anywhere. I'm a bit confused because vscroll is about scrolling the window and `line-move' is about moving point (only incidentally scrolling the window if the point leaves it). Clearly `set-window-start' needs to reset `vscroll', but I'm not sure I understand why `line-move' does. Is this about detecting the correct column? > In addition, when we move to another screen line, the value of vscroll > cannot be meaningfully kept, because its meaning is tightly coupled to > the screen line where it was applied. In essence, vscroll is a trick > to allow display of a tall display element (image, text using a large > fonts, etc.) in a window whose height is too small to show all of the > display element at once. > >> After commenting this line out, I can't tell a difference, even >> when scrolling images with and without `auto-window-vscroll' and >> `try-vscroll'. > > line-move is not just for scrolling across images, it is also about > scrolling across tall text lines and other display elements. In any > case, asking for removal of that reset is a non-starter, for the > reasons explained above, so it isn't going to happen. The solution > for any Lisp program that doesn't want vscroll to be rest is not to > call line-move. Now that I do more testing, I can see how removing that line breaks `line-move' although I'm still not sure why. Would it be acceptable to restore the vertical scroll position as long as `line-move' hasn't otherwise scrolled the screen? See attached patch. >> I was hoping Kim could shed some light on this. > > I'd be thrilled to hear from Kim, but I'm as guilty of the code in > line-move as he is, so "blame" me if you want. Ah, sorry, should have gone back further in the blame history.
[0001-Restore-vertical-scroll-offset-after-line-move.patch (text/x-patch, inline)]
From 2f028e30b2e5ba3a3cd9fd2ecaeaff7c3d02b62f Mon Sep 17 00:00:00 2001
From: Steven Allen <steven <at> stebalien.com>
Date: Sat, 27 Jul 2024 12:54:31 -0700
Subject: [PATCH] Restore vertical-scroll offset after line-move
Restore the vertical-scroll offset after moving lines unless the window
was otherwise scrolled and/or altering the vertical scroll would move
the point off-screen.
* lisp/simple.el (line-move): restore `window-vscroll' when
appropriate (Bug#72323).
---
lisp/simple.el | 85 ++++++++++++++++++++++++++++----------------------
1 file changed, 48 insertions(+), 37 deletions(-)
diff --git a/lisp/simple.el b/lisp/simple.el
index a9f8b5845d8..71ae175d198 100644
--- a/lisp/simple.el
+++ b/lisp/simple.el
@@ -7930,43 +7930,54 @@ line-move
;; doesn't have very long lines.
(long-line-optimizations-p)))
(line-move-partial arg noerror))
- (set-window-vscroll nil 0 t)
- (if (and line-move-visual
- ;; Display-based column are incompatible with goal-column.
- (not goal-column)
- ;; Lines aren't truncated.
- (not
- (and
- (or truncate-lines (truncated-partial-width-window-p))
- (long-line-optimizations-p)))
- ;; When the text in the window is scrolled to the left,
- ;; display-based motion doesn't make sense (because each
- ;; logical line occupies exactly one screen line).
- (not (> (window-hscroll) 0))
- ;; Likewise when the text _was_ scrolled to the left
- ;; when the current run of vertical motion commands
- ;; started.
- (not (and (memq last-command
- `(next-line previous-line ,this-command))
- auto-hscroll-mode
- (numberp temporary-goal-column)
- (>= temporary-goal-column
- (- (window-width) hscroll-margin)))))
- (prog1 (line-move-visual arg noerror)
- ;; If we moved into a tall line, set vscroll to make
- ;; scrolling through tall images more smooth.
- (let ((lh (line-pixel-height))
- (edges (window-inside-pixel-edges))
- (dlh (default-line-height))
- winh)
- (setq winh (- (nth 3 edges) (nth 1 edges) 1))
- (if (and (< arg 0)
- (< (point) (window-start))
- (> lh winh))
- (set-window-vscroll
- nil
- (- lh dlh) t))))
- (line-move-1 arg noerror)))))
+ (let ((initial-vscroll (window-vscroll nil t))
+ (initial-window-start (window-start)))
+ (set-window-vscroll nil 0 t)
+ (prog1
+ (if (and line-move-visual
+ ;; Display-based column are incompatible with goal-column.
+ (not goal-column)
+ ;; Lines aren't truncated.
+ (not
+ (and
+ (or truncate-lines (truncated-partial-width-window-p))
+ (long-line-optimizations-p)))
+ ;; When the text in the window is scrolled to the left,
+ ;; display-based motion doesn't make sense (because each
+ ;; logical line occupies exactly one screen line).
+ (not (> (window-hscroll) 0))
+ ;; Likewise when the text _was_ scrolled to the left
+ ;; when the current run of vertical motion commands
+ ;; started.
+ (not (and (memq last-command
+ `(next-line previous-line ,this-command))
+ auto-hscroll-mode
+ (numberp temporary-goal-column)
+ (>= temporary-goal-column
+ (- (window-width) hscroll-margin)))))
+ (prog1 (line-move-visual arg noerror)
+ ;; If we moved into a tall line, set vscroll to make
+ ;; scrolling through tall images more smooth.
+ (let ((lh (line-pixel-height))
+ (edges (window-inside-pixel-edges))
+ (dlh (default-line-height))
+ winh)
+ (setq winh (- (nth 3 edges) (nth 1 edges) 1))
+ (if (and (< arg 0)
+ (< (point) (window-start))
+ (> lh winh))
+ (set-window-vscroll
+ nil
+ (- lh dlh) t))))
+ (line-move-1 arg noerror))
+ ;; Restore the vscroll position, if any.
+ (when (and (not (zerop initial-vscroll))
+ ;; But not if scrolling would hide the point.
+ (< initial-vscroll (cdr (posn-x-y (posn-at-point))))
+ ;; Or if line-move otherwise scrolled the window.
+ (= initial-window-start (window-start))
+ (zerop (window-vscroll nil t)))
+ (set-window-vscroll nil initial-vscroll t)))))))
;; Display-based alternative to line-move-1.
;; Arg says how many lines to move. The value is t if we can move the
--
2.45.2
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.