[vim/vim] patch 9.2.XXXX: CTRL-D moves cursor up with 'scrolloffpad' (PR #21188)
ksh368-bit
Problem:
With 'scrolloff=999' and 'scrolloffpad=1', pressing CTRL-D on the last line invokes cursor correction after a no-op page scroll and moves the cursor upward. This contradicts the documented behavior that nothing happens and a beep is produced.
Solution:
Skip cursor correction only when 'scrolloffpad' is active and neither the viewport nor the cursor moved. Existing correction is preserved when 'scrolloffpad' is disabled or when the page command makes progress. Add a regression test covering both 'scrolloffpad=0' and 'scrolloffpad=1', the unchanged view, and the beep.
Tests:
- Focused Test_scrolloffpad_ctrl_d_at_eof test
- test_scroll_opt: 69 tests passed
- test_normal: 121 tests passed
- codestyle: passed
- Full test suite: 7600 executed, 951 skipped, 0 failed
Fixes #21096
You can view, comment on, or merge this pull request online at:
https://github.com/vim/vim/pull/21188
Commit Summary
- e074e10 patch 9.2.XXXX: CTRL-D moves cursor up with 'scrolloffpad'
File Changes
(2 files)
- M src/move.c (8)
- M src/testdir/test_scroll_opt.vim (18)
Patch Links:
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
Christian Brabandt
chrisbra left a comment (vim/vim#21188)Thanks, I looked at the code for a while and wondered if there are similar bugs with window scrolling. Initially I thought Ctrl-E needs to be handled as well, but that would be wrong. The current behaviour is to allow to scroll the window even with scrolloff value set to 999, so handling this like Ctrl-D would be wrong. There is one strange thing already with Ctrl-E behaviour, that is it suddenly moves the cursor to the middle of the window when scrolloffpad=1. That is probably also wrong.
So I think we may need to change cursor_correct() instead of special casing pagescroll().
Something like this?
diff --git a/src/move.c b/src/move.c index 635e63eb2..db6462ce6 100644 --- a/src/move.c +++ b/src/move.c @@ -3090,6 +3090,7 @@ cursor_correct(void) linenr_T cln; // Cursor Line Number int max_off; long so = get_scrolloff_value(); + int pad = 0; /* * How many lines we would like to have above/below the cursor depends on @@ -3113,9 +3114,14 @@ cursor_correct(void) if (curwin->w_botline == curbuf->b_ml.ml_line_count + 1 && mouse_dragging == 0) { - if (!use_scrolloffpad()) - below_wanted = 0; max_off = (curwin->w_height - 1) / 2; + if (use_scrolloffpad()) + { + pad = curwin->w_empty_rows; + below_wanted = MIN(max_off, below_wanted); + } + else + below_wanted = 0; if (above_wanted > max_off) above_wanted = max_off; } @@ -3126,7 +3132,7 @@ cursor_correct(void) */ cln = curwin->w_cursor.lnum; if (cln >= curwin->w_topline + above_wanted - && cln < curwin->w_botline - below_wanted + && cln < curwin->w_botline + pad - below_wanted #ifdef FEAT_FOLDING && !hasAnyFolding(curwin) #endif @@ -3144,7 +3150,9 @@ cursor_correct(void) #ifdef FEAT_DIFF // count filler lines as context above = curwin->w_topfill; - below = curwin->w_filler_rows; + below = curwin->w_filler_rows + pad; +#else + below = pad; #endif while ((above < above_wanted || below < below_wanted) && topline < botline) { @@ -3331,6 +3339,10 @@ pagescroll(int dir, long count, int half) count = MIN(curwin->w_height, curwin->w_p_scr); long curscount = count; + int pad = 0; + if (use_scrolloffpad()) + pad = (curwin->w_height - 1) / 2; + // Adjust count so as to not reveal end of buffer lines. if (dir == FORWARD && (curwin->w_topline + curwin->w_height + count > buflen @@ -3343,8 +3355,8 @@ pagescroll(int dir, long count, int half) if (n - count < curwin->w_height && curwin->w_topline < buflen) n += plines_m_win(curwin, curwin->w_topline + 1, buflen, curwin->w_height + count); - if (n < curwin->w_height + count) - count = n - curwin->w_height; + if (n < curwin->w_height + count - pad) + count = n - curwin->w_height + pad; } // (Try to) scroll the window unless already at the end of the buffer.
(Note: I just copy-pasted the diff, it will probably break patch, since it breaks tabs).
Plus a test for the CTRL-E behaviour under scrolloffpad=1 and zero
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
ksh368-bit
ksh368-bit left a comment (vim/vim#21188)Thanks for digging into this, @chrisbra. I pushed 5770751, moving the fix into cursor_correct() and removing the pagescroll() no-op guard. CTRL-E still scrolls the window, but no longer moves the cursor upward unexpectedly at EOF.
While testing the padding approach, I found another corner case: after Gzb at EOF, CTRL-D can scroll the viewport, and CTRL-E can still move the cursor upward in an odd-height window. Restoring below_wanted = 0 when EOF is visible avoids those cases while leaving scrolloffpad's automatic viewport centering and the existing half-page boundaries intact.
The regression tests now cover CTRL-D and CTRL-E with scrolloffpad=0 and 1, small and large scrolloff values, odd and even window heights, and both G and Gzb positioning. The focused scrolling, normal-mode, cursor, scrollbind, diff-mode, and codestyle suites pass.
The full script run had eight environment-related failures; all passed on rerun with a shorter socket path or TERM=xterm-256color. The full make test run remains blocked at libvterm by a missing glibtool dependency on this machine.
Could you please take another look at this smaller cursor_correct() change?
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
Christian Brabandt
chrisbra left a comment (vim/vim#21188)thanks this is a very simple change now. CI looks good, so I am not worried about your glibtool issue.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.