Fix an issue where moving the cursor to end of the buffer would result in no visible text

[jamescherti]

This pull request fixes an issue where using (evil-goto-line nil) to move the cursor to the end of the buffer would result in no visible text because of (goto-char (point-max)).

This issue is difficult to reproduce because it doesn't always occur. I was fortunate enough to encounter it today, and the provided patch fixes the problem. This patch draws inspiration from the built-in (end-of-buffer) function, which does not exhibit the same issue as calling (evil-goto-line nil).

[axelf4]

Thanks for the PR! Could you please try to give reproduction steps? Workarounds that cannot be verified as necessary make me uneasy, as they are impossible to remove later. But this also sounds more like a GNU Emacs bug, and in that case it ought to be fixed there instead?

[jamescherti]

Thanks for your feedback! I understand your concern about unverifiable workarounds and I agree that they should be avoided where possible.

But in this case, the issue is difficult to reproduce consistently. To attempt to trigger it, try opening multiple files and pressing G for each file until no text remains visible. However, I have not been able to reproduce it reliably myself.

I am not sure whether this is a bug in GNU/Emacs. It does not seem to be the responsibility of (goto-char (point-max)) to adjust the window start position.

Even the built-in end-of-buffer command uses overlay-recenter and recenter to adjust the window start:

(defun end-of-buffer (&optional arg)
  "Move point to the end of the buffer.
With numeric arg N, put point N/10 of the way from the end.
If the buffer is narrowed, this command uses the end of the
accessible part of the buffer.

Push mark at previous position, unless either a \\[universal-argument] prefix
is supplied, or Transient Mark mode is enabled and the mark is active."
  (declare (interactive-only "use `(goto-char (point-max))' instead."))
  (interactive "^P")
  (or (consp arg) (region-active-p) (push-mark))
  (let ((size (- (point-max) (point-min))))
    (goto-char (if (and arg (not (consp arg)))
		   (- (point-max)
		      (/ (* size (prefix-numeric-value arg)) 10))
		 (point-max))))
  ;; If we went to a place in the middle of the buffer,
  ;; adjust it to the beginning of a line.
  (cond ((and arg (not (consp arg))) (forward-line 1))
	((and (eq (current-buffer) (window-buffer))
              (> (point) (window-end nil t)))
	 ;; If the end of the buffer is not already on the screen,
	 ;; then scroll specially to put it near, but not at, the bottom.
	 (overlay-recenter (point))
	 ;; FIXME: Arguably if `scroll-conservatively' is set, then
         ;; we should pass -1 to `recenter'.
	 (recenter (if (and scroll-minibuffer-conservatively
	                    (window-minibuffer-p))
	               -1 -3)))))

overlay-recenter has been added to end-of-buffer in lisp/simple.el in 1994:

commit 97dfc68c2ed63b83892aa0acf3ced92ad557a716
Author: Richard M. Stallman <[email protected]>
Date:   Mon Jun 13 23:40:33 1994 +0000
(end-of-buffer): Recenter overlay lists.

recenter has been added to end-of-buffer in lisp/simple.el in 1991:

commit 2076c87c104703a6906f46306e28971c05a18b48
Author: Jim Blandy <[email protected]>
Date:   Sat Dec 21 09:29:41 1991 +0000
Initial revision

The Emacs developers added overlay-recenter and recenter to end-of-buffer to prevent the same issue that occasionally occurs when G is pressed in Evil mode.

[jamescherti]

Hello @axelf4,

I finally found a way to reproduce the issue.

Environment

• Emacs version: 30.0.92
• Evil version:

commit b7ab3840dbfc1da5f9ad56542fc94e3dab4be5f1 (HEAD, origin/master)
Author: Axel Forsman
Date:   Sun Oct 6 19:42:28 2024 +0200

Steps to reproduce the issue

Step 1

• Evaluate the expression: (text-scale-set 5)

Step 2

Repeat the following steps many times until the issue happens:

• Press the gg to move the cursor to the beginning of the buffer
• Run the command: (text-scale-increase)
• Press the G key to move the cursor to the end of the buffer

[axelf4]

I finally found a way to reproduce the issue.

Great!

I am not sure whether this is a bug in GNU/Emacs. It does not seem to be the responsibility of (goto-char (point-max)) to adjust the window start position.

Hmm, I got the impression that redisplay should handle this from the scroll-conservatively docstring:

If point moves off-screen, redisplay will scroll by up to scroll-conservatively lines in order to bring point just barely onto the screen again. If that cannot be done, then redisplay recenters point as usual.

The end-of-buffer code may be there just to tailor the scroll-conservatively behavior, i.e. instead of recentering, placing EOB two lines up from the bottom, and not explicitly fix an issue. (overlay-recenter is an obsolete optimization---I do not think we would need to include it.)

The drawback of the proposed fix is that it changes the behavior of G: Instead of obeying scroll-conservatively, it unconditionally makes EOB the bottommost line.

@tomdl89, any thoughts?

[jamescherti]

Hmm, I got the impression that redisplay should handle this from the scroll-conservatively docstring:

If point moves off-screen, redisplay will scroll by up to scroll-conservatively lines in order to bring point just barely onto the screen again. If that cannot be done, then redisplay recenters point as usual.

Thank you for pointing this out.

The value of scroll-conservatively in my setup is set to 10000, which causes the issue to occur. When I change scroll-conservatively to 10, the issue does not occur.

This pull request addresses an edge case where the user opts to set scroll-conservatively to a high value, for instance, to enable smoother scrolling in specific configurations by reducing the time Emacs spends recentering the screen.

[tomdl89]

I agree with @axelf4 about the drawback mentioned, although I guess this could be addressed by checking the value of scroll-conservatively beforehand.

My main problem with this PR is that I can't replicate it at all, even with a high value of scroll-conservatively and repeated invocations of gg, text-scale-increase and G. I'm not sure I'd be happy to merge anything until I can have repro steps that I can follow successfully. Just to make sure I'm not missing something, what are the repro steps, starting with

1. run make emacs in the evil repo

[jamescherti]

Does this make any difference?

(setq scroll-conservatively most-positive-fixnum)

This is the result of make emacs:

[jamescherti]

You can replicate the issue in Emacs 30 on Linux by following these steps:

1. Clone or copy the minimal-emacs.d repository from https://github.com/jamescherti/minimal-emacs.d and use its contents as your early-init.el and init.el files.
2. After loading Emacs, evaluate the expression (setq scroll-conservatively most-positive-fixnum)
3. Open a file that has more than > 400 lines
4. Press gg, text-scale-increase and G

Note that the text should look big, similar to this:

[tomdl89]

Ah, I think I can replicate. I think you confused me with the phrase "no visible text". Your steps result in a buffer with one line of visible text, at the top of the window. And as the convention for most text files is to leave one blank line at the end of the file, this means there is no visible text. Right?

[jamescherti]

Yes, by "no visible text," I meant that the cursor and window start are at (point-max), with all the text located above window-start, preventing the user from seeing it.

(In my configuration, require-final-newline is set to t, which causes (point-max) to always be an empty line.)

[tomdl89]

Understood. So I think replacing (recenter -1) in your code with

(recenter (and (< 100 scroll-conservatively) -1))

Could be an improvement. That means it will emulate vim's behaviour for people who have set scroll-conservatively to a high value, but will emulate end-of-buffer's behaviour for those who haven't. Seems like a good compromise to me. @axelf4 wdyt?

[jamescherti]

I updated the pull request.

Just out of curiosity, why specifically < 100?

[tomdl89]

According to scroll-conservatively's docstring:

If the value is greater than 100, redisplay will never recenter point,
but will always scroll just enough text to bring point into view, even
if you move far away.

[tomdl89]

(for that reason, I don't think 10000 or most-positive-fixnum or 101 are any different)

[jamescherti]

It makes sense. Thank you for clarifying, @tomdl89.

[jamescherti]

According to scroll-conservatively's docstring:

If the value is greater than 100, redisplay will never recenter point,
but will always scroll just enough text to bring point into view, even
if you move far away.

Would this version be better?

(recenter (and (<= 100 scroll-conservatively) -1))

[tomdl89]

The docstring says

If the value is greater than 100…

not

If the value is greater than or equal to 100…

so, no, I don't think your version would be better.

In case lisp isn't something you're super familiar with, [sorry, I see it is, just an oversight I guess 🙂]

(< 100 x)

is equivalnt to

100 < x

in normal maths. For that reason, I find it easier to read < as strictly-increasing? and <= as monotonically-increasing? when I see them in lisp code.

[jamescherti]

I wrote my response too quickly. Here is what I intended to express:

If the value is greater than 100, redisplay will never recenter point,
but will always scroll just enough text to bring point into view, even
if you move far away.

This means that:

• When scroll-conservatively > 100: Redisplay will never recenter the point.
• When scroll-conservatively <= 100: The window may recenter the point if necessary.

I believe there is a small issue with the change you proposed:

(recenter (and (< 100 scroll-conservatively) -1))

This is what the above will do:

• When scroll-conservatively >= 100: (recenter nil) (This places the point in the middle line of the window.)
• When scroll-conservatively < 100: (recenter -1) (This places the point on screen line -1.)

I have changed my pull request to the following instead:

(recenter (and (> 100 scroll-conservatively) -1))

• When scroll-conservatively <= 100: (recenter nil) (when redisplay never recenter point, recenter places the point in the middle line of the window.)
• When scroll-conservatively > 100: (recenter -1) (when the window may be recentered, then places the recenter on screen line -1.)

[jamescherti]

Also, I just tried Vim, and the change (and (> 100 scroll-conservatively) -1) does not emulate Vim's behavior. I reverted the change back to (recenter -1).

Here is Vim:

And here is Vim after I press G in normal mode:

Vim's behavior is similar to Emacs (recenter -1) (and not (recenter nil)). I experimented with different Emacs scroll-margin values, and Emacs (recenter -1) takes scroll-margin into account.

Let me know your thoughts.

[tomdl89]

OK, your new approach works just as fine for me. But I still think

(>= scroll-conservatively 100)

needs to be

(> scroll-conservatively 100)

because of scroll-conservatively's docstring.

[jamescherti]

While Emacs recenters the screen using (recenter nil) when scroll-conservatively is less than or equal to 100, this behavior does not match Vim's.

To align with Vim's behavior, we should use (recenter -1) regardless of the value of scroll-conservatively.

[tomdl89]

This PR is getting really noisy with force-pushes. Can you let me know when you have stopped so I can take a look at the code and review?

[jamescherti]

I had to add a few additional details to the commit message. I've now finished modifying it.