[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
bug#72323: 31.0.50; line-move unconditionally resets vscroll to 0
|
From: |
Steven Allen |
|
Subject: |
bug#72323: 31.0.50; line-move unconditionally resets vscroll to 0 |
|
Date: |
Sat, 27 Jul 2024 13:10:03 -0700 |
Eli Zaretskii <eliz@gnu.org> writes:
>> Cc: "Kim F. Storm" <storm@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@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.
>From 2f028e30b2e5ba3a3cd9fd2ecaeaff7c3d02b62f Mon Sep 17 00:00:00 2001
From: Steven Allen <steven@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