vim-patch:9.0.0900: cursor moves too far with 'smoothscroll'

Problem:    Cursor moves too far with 'smoothscroll'.
Solution:   Only move as far as really needed. (Yee Cheng Chin, closes vim/vim#11504)

81ba26e9de

Co-authored-by: Yee Cheng Chin <ychin.git@gmail.com>

src/nvim/move.c

@@ -164,6 +164,22 @@ static void redraw_for_cursorcolumn(win_T *wp)
   }
 }
 
+/// Calculates how much overlap the smoothscroll marker "<<<" overlaps with
+/// buffer text for curwin.
+/// Parameter "extra2" should be the padding on the 2nd line, not the first
+/// line.
+/// Returns the number of columns of overlap with buffer text, excluding the
+/// extra padding on the ledge.
+static int smoothscroll_marker_overlap(win_T *wp, int extra2)
+{
+  // We don't draw the <<< marker when in showbreak mode, thus no need to
+  // account for it.  See grid_put_linebuf().
+  if (*get_showbreak_value(wp) != NUL) {
+    return 0;
+  }
+  return extra2 > 3 ? 0 : 3 - extra2;
+}
+
 /// Set wp->s_skipcol to zero and redraw later if needed.
 static void reset_skipcol(win_T *wp)
 {
@@ -239,10 +255,12 @@ void update_topline(win_T *wp)
       } else if (wp->w_skipcol > 0 && wp->w_cursor.lnum == wp->w_topline) {
         colnr_T vcol;
 
-        // check the cursor position is visible.  Add 3 for the ">>>"
-        // displayed in the top-left.
+        // Check that the cursor position is visible.  Add columns for the
+        // smoothscroll marker "<<<" displayed in the top-left if needed.
         getvvcol(wp, &wp->w_cursor, &vcol, NULL, NULL);
-        if (wp->w_skipcol + 3 >= vcol) {
+        int smoothscroll_overlap = smoothscroll_marker_overlap(wp,
+                                                               win_col_off(wp) - win_col_off2(wp));
+        if (wp->w_skipcol + smoothscroll_overlap > vcol) {
           check_topline = true;
         }
       }
@@ -1387,12 +1405,21 @@ bool scrollup(long line_count, int byfold)
   }
 
   if (curwin->w_cursor.lnum == curwin->w_topline && do_sms && curwin->w_skipcol > 0) {
-    int width1 = curwin->w_width - curwin_col_off();
-    int width2 = width1 + curwin_col_off2();
+    int col_off = curwin_col_off();
+    int col_off2 = curwin_col_off2();
+
+    int width1 = curwin->w_width - col_off;
+    int width2 = width1 + col_off2;
+    int extra2 = col_off - col_off2;
     long so = get_scrolloff_value(curwin);
     long scrolloff_cols = so == 0 ? 0 : width1 + (so - 1) * width2;
     int space_cols = (curwin->w_height - 1) * width2;
 
+    // If we have non-zero scrolloff, just ignore the <<< marker as we are
+    // going past it anyway.
+    int smoothscroll_overlap = scrolloff_cols != 0 ? 0 :
+                               smoothscroll_marker_overlap(curwin, extra2);
+
     // Make sure the cursor is in a visible part of the line, taking
     // 'scrolloff' into account, but using screen lines.
     // If there are not enough screen lines put the cursor in the middle.
@@ -1400,13 +1427,13 @@ bool scrollup(long line_count, int byfold)
       scrolloff_cols = space_cols / 2;
     }
     validate_virtcol();
-    if (curwin->w_virtcol < curwin->w_skipcol + 3 + scrolloff_cols) {
+    if (curwin->w_virtcol < curwin->w_skipcol + smoothscroll_overlap + scrolloff_cols) {
       colnr_T col = curwin->w_virtcol;
 
       if (col < width1) {
         col += width1;
       }
-      while (col < curwin->w_skipcol + 3 + scrolloff_cols) {
+      while (col < curwin->w_skipcol + smoothscroll_overlap + scrolloff_cols) {
         col += width2;
       }
       curwin->w_curswant = col;
@@ -2180,9 +2207,9 @@ void cursor_correct(void)
   curwin->w_viewport_invalid = true;
 }
 
-// move screen 'count' pages up or down and update screen
+// Move screen "count" pages up or down and update screen.
 //
-// return FAIL for failure, OK otherwise
+// Return FAIL for failure, OK otherwise.
 int onepage(Direction dir, long count)
 {
   long n;

test/functional/legacy/scroll_opt_spec.lua

@@ -279,8 +279,8 @@ describe('smoothscroll', function()
     ]])
     exec('call DoRel()')
     screen:expect([[
-      2<<<ong text very long text very long te|
-          ^xt very long text very long text ver|
+      2<<<^ong text very long text very long te|
+          xt very long text very long text ver|
           y long text very long text very long|
            text very long text very long text |
         1 three                               |
@@ -380,8 +380,20 @@ describe('smoothscroll', function()
     screen:expect_unchanged()
     feed('G')
     screen:expect_unchanged()
-    -- moving cursor up - whole top line shows
-    feed('2k')
+    -- moving cursor up right after the >>> marker - no need to show whole line
+    feed('2gj3l2k')
+    screen:expect([[
+      <<<^h some text with some text           |
+      Line with some text with some text with |
+      some text with some text with some text |
+      with some text with some text           |
+      Line with some text with some text with |
+      some text with some text with some text |
+      with some text with some text           |
+                                              |
+    ]])
+    -- moving cursor up where the >>> marker is - whole top line shows
+    feed('2j02k')
     screen:expect([[
       ^Line with some text with some text with |
       some text with some text with some text |
@@ -524,11 +536,9 @@ describe('smoothscroll', function()
   -- oldtest: Test_smoothscroll_long_line_showbreak()
   it("cursor is not one screen line too far down", function()
     screen:try_resize(40, 6)
-    exec([[
-      " a line that spans four screen lines
-      call setline(1, 'with lots of text in one line '->repeat(6))
-      set smoothscroll scrolloff=0 showbreak=+++\ 
-    ]])
+    -- a line that spans four screen lines
+    exec("call setline(1, 'with lots of text in one line '->repeat(6))")
+    exec('set smoothscroll scrolloff=0 showbreak=+++\\ ')
     local s1 = [[
       ^with lots of text in one line with lots |
       +++ of text in one line with lots of tex|
@@ -540,8 +550,8 @@ describe('smoothscroll', function()
     screen:expect(s1)
     feed('<C-E>')
     screen:expect([[
-      +++ of text in one line with lots of tex|
-      +++ ^t in one line with lots of text in o|
+      +++ ^of text in one line with lots of tex|
+      +++ t in one line with lots of text in o|
       +++ ne line with lots of text in one lin|
       +++ e with lots of text in one line     |
       ~                                       |

test/old/testdir/test_scroll_opt.vim

@@ -247,10 +247,14 @@ func Test_smoothscroll_wrap_scrolloff_zero()
   call term_sendkeys(buf, "G")
   call VerifyScreenDump(buf, 'Test_smooth_wrap_4', {})
 
-  " moving cursor up - whole top line shows
-  call term_sendkeys(buf, "2k")
+  " moving cursor up right after the >>> marker - no need to show whole line
+  call term_sendkeys(buf, "2gj3l2k")
   call VerifyScreenDump(buf, 'Test_smooth_wrap_5', {})
 
+  " moving cursor up where the >>> marker is - whole top line shows
+  call term_sendkeys(buf, "2j02k")
+  call VerifyScreenDump(buf, 'Test_smooth_wrap_6', {})
+
   call StopVimInTerminal(buf)
 endfunc
 
@@ -338,7 +342,6 @@ func Test_smoothscroll_long_line_showbreak()
   let buf = RunVimInTerminal('-S XSmoothLongShowbreak', #{rows: 6, cols: 40})
   call VerifyScreenDump(buf, 'Test_smooth_long_showbreak_1', {})
   
-  " FIXME: this currently has the cursor in screen line 2, should be one up.
   call term_sendkeys(buf, "\<C-E>")
   call VerifyScreenDump(buf, 'Test_smooth_long_showbreak_2', {})
 
@@ -370,10 +373,16 @@ func Test_smoothscroll_cursor_position()
   call s:check_col_calc(1, 2, 41)
   exe "normal \<C-Y>"
   call s:check_col_calc(1, 3, 41)
-  normal ggg$
+
+  normal gg3l
   exe "normal \<C-E>"
 
   " Move down only 1 line when we are out of the range of the <<< display
+  call s:check_col_calc(4, 1, 24)
+  exe "normal \<C-Y>"
+  call s:check_col_calc(4, 2, 24)
+  normal ggg$
+  exe "normal \<C-E>"
   call s:check_col_calc(20, 1, 40)
   exe "normal \<C-Y>"
   call s:check_col_calc(20, 2, 40)
@@ -383,9 +392,11 @@ func Test_smoothscroll_cursor_position()
   setl number
   call s:check_col_calc(5, 1, 1)
   exe "normal \<C-E>"
-  call s:check_col_calc(5, 2, 33)
+
+  " Move down only 1 line when the <<< display is on the number column
+  call s:check_col_calc(5, 1, 17)
   exe "normal \<C-Y>"
-  call s:check_col_calc(5, 3, 33)
+  call s:check_col_calc(5, 2, 17)
   normal ggg$
   exe "normal \<C-E>"
   call s:check_col_calc(20, 1, 32)
@@ -393,13 +404,33 @@ func Test_smoothscroll_cursor_position()
   call s:check_col_calc(20, 2, 32)
   normal gg
 
+  setl numberwidth=1
+
+  " Move down another line when numberwidth is too short to cover the whole
+  " <<< display
+  call s:check_col_calc(3, 1, 1)
+  exe "normal \<C-E>"
+  call s:check_col_calc(3, 2, 37)
+  exe "normal \<C-Y>"
+  call s:check_col_calc(3, 3, 37)
+  normal ggl
+
+  " Only move 1 line down when we are just past the <<< display
+  call s:check_col_calc(4, 1, 2)
+  exe "normal \<C-E>"
+  call s:check_col_calc(4, 1, 20)
+  exe "normal \<C-Y>"
+  call s:check_col_calc(4, 2, 20)
+  normal gg
+  setl numberwidth&
+
   " Test number + showbreak, so test that the additional indentation works
   setl number showbreak=+++
   call s:check_col_calc(5, 1, 1)
   exe "normal \<C-E>"
-  call s:check_col_calc(8, 2, 30)
+  call s:check_col_calc(8, 1, 17)
   exe "normal \<C-Y>"
-  call s:check_col_calc(8, 3, 30)
+  call s:check_col_calc(8, 2, 17)
   normal gg
 
   " Test number + cpo+=n mode, where wrapped lines aren't indented