vim-patch:9.0.1599: Cursor not adjusted when 'splitkeep' is not "cursor"

Problem:    Cursor not adjusted when near top or bottom of window and
            'splitkeep' is not "cursor".
Solution:   Move boundary checks to outer cursor move functions, inner
            functions should only return valid cursor positions. (Luuk van
            Baal, closes vim/vim#12480)

a109f39ef5

src/nvim/edit.c

@@ -2549,15 +2549,10 @@ int oneleft(void)
 
 /// Move the cursor up "n" lines in window "wp".
 /// Takes care of closed folds.
-/// Returns the new cursor line or zero for failure.
+void cursor_up_inner(win_T *wp, long n)
-linenr_T cursor_up_inner(win_T *wp, long n)
 {
   linenr_T lnum = wp->w_cursor.lnum;
 
-  // This fails if the cursor is already in the first line.
-  if (lnum <= 1) {
-    return 0;
-  }
   if (n >= lnum) {
     lnum = 1;
   } else if (hasAnyFolding(wp)) {
@@ -2587,15 +2582,16 @@ linenr_T cursor_up_inner(win_T *wp, long n)
   }
 
   wp->w_cursor.lnum = lnum;
-  return lnum;
 }
 
 /// @param upd_topline  When true: update topline
 int cursor_up(long n, int upd_topline)
 {
-  if (n > 0 && cursor_up_inner(curwin, n) == 0) {
+  // This fails if the cursor is already in the first line.
+  if (n > 0 && curwin->w_cursor.lnum <= 1) {
     return FAIL;
   }
+  cursor_up_inner(curwin, n);
 
   // try to advance to the column we want to be at
   coladvance(curwin->w_curswant);
@@ -2609,18 +2605,11 @@ int cursor_up(long n, int upd_topline)
 
 /// Move the cursor down "n" lines in window "wp".
 /// Takes care of closed folds.
-/// Returns the new cursor line or zero for failure.
+void cursor_down_inner(win_T *wp, long n)
-linenr_T cursor_down_inner(win_T *wp, long n)
 {
   linenr_T lnum = wp->w_cursor.lnum;
   linenr_T line_count = wp->w_buffer->b_ml.ml_line_count;
 
-  // Move to last line of fold, will fail if it's the end-of-file.
-  (void)hasFoldingWin(wp, lnum, NULL, &lnum, true, NULL);
-  // This fails if the cursor is already in the last line.
-  if (lnum >= line_count) {
-    return FAIL;
-  }
   if (lnum + n >= line_count) {
     lnum = line_count;
   } else if (hasAnyFolding(wp)) {
@@ -2628,6 +2617,7 @@ linenr_T cursor_down_inner(win_T *wp, long n)
 
     // count each sequence of folded lines as one logical line
     while (n--) {
+      // Move to last line of fold, will fail if it's the end-of-file.
       if (hasFoldingWin(wp, lnum, NULL, &last, true, NULL)) {
         lnum = last + 1;
       } else {
@@ -2645,15 +2635,16 @@ linenr_T cursor_down_inner(win_T *wp, long n)
   }
 
   wp->w_cursor.lnum = lnum;
-  return lnum;
 }
 
 /// @param upd_topline  When true: update topline
 int cursor_down(long n, int upd_topline)
 {
-  if (n > 0 && cursor_down_inner(curwin, n) == 0) {
+  // This fails if the cursor is already in the last line.
+  if (n > 0 && curwin->w_cursor.lnum >= curwin->w_buffer->b_ml.ml_line_count) {
     return FAIL;
   }
+  cursor_down_inner(curwin, n);
 
   // try to advance to the column we want to be at
   coladvance(curwin->w_curswant);

src/nvim/normal.c

@@ -2498,10 +2498,12 @@ static bool nv_screengo(oparg_T *oap, int dir, long dist)
           curwin->w_curswant -= width2;
         } else {
           // to previous line
-          if (!cursor_up_inner(curwin, 1)) {
+          if (curwin->w_cursor.lnum <= 1) {
             retval = false;
             break;
           }
+          cursor_up_inner(curwin, 1);
+
           linelen = linetabsize(get_cursor_line_ptr());
           if (linelen > width1) {
             int w = (((linelen - width1 - 1) / width2) + 1) * width2;
@@ -2521,11 +2523,13 @@ static bool nv_screengo(oparg_T *oap, int dir, long dist)
           curwin->w_curswant += width2;
         } else {
           // to next line
-          if (!cursor_down_inner(curwin, 1)) {
+          if (curwin->w_cursor.lnum >= curwin->w_buffer->b_ml.ml_line_count) {
             retval = false;
             break;
           }
+          cursor_down_inner(curwin, 1);
           curwin->w_curswant %= width2;
+
           // Check if the cursor has moved below the number display
           // when width1 < width2 (with cpoptions+=n). Subtract width2
           // to get a negative value for w_curswant, which will get

src/nvim/window.c

@@ -1573,7 +1573,7 @@ int win_split_ins(int size, int flags, win_T *new_wp, int dir)
   // equalize the window sizes.
   if (do_equal || dir != 0) {
     win_equal(wp, true, (flags & WSP_VERT) ? (dir == 'v' ? 'b' : 'h') : (dir == 'h' ? 'b' : 'v'));
-  } else if (*p_spk != 'c' && !is_aucmd_win(wp)) {
+  } else if (!is_aucmd_win(wp)) {
     win_fix_scroll(false);
   }
 
@@ -2163,7 +2163,7 @@ void win_equal(win_T *next_curwin, bool current, int dir)
   win_equal_rec(next_curwin == NULL ? curwin : next_curwin, current,
                 topframe, dir, 0, tabline_height(),
                 Columns, topframe->fr_height);
-  if (*p_spk != 'c' && !is_aucmd_win(next_curwin)) {
+  if (!is_aucmd_win(next_curwin)) {
     win_fix_scroll(true);
   }
 }
@@ -2956,9 +2956,7 @@ int win_close(win_T *win, bool free_buf, bool force)
       win_equal(curwin, curwin->w_frame->fr_parent == win_frame, dir);
     } else {
       (void)win_comp_pos();
-      if (*p_spk != 'c') {
+      win_fix_scroll(false);
-        win_fix_scroll(false);
-      }
     }
   }
 
@@ -5350,7 +5348,7 @@ void win_new_screen_rows(void)
   compute_cmdrow();
   curtab->tp_ch_used = p_ch;
 
-  if (*p_spk != 'c' && !skip_win_fix_scroll) {
+  if (!skip_win_fix_scroll) {
     win_fix_scroll(true);
   }
 }
@@ -5797,9 +5795,7 @@ void win_setheight_win(int height, win_T *win)
     msg_row = row;
     msg_col = 0;
 
-    if (*p_spk != 'c') {
+    win_fix_scroll(true);
-      win_fix_scroll(true);
-    }
 
     redraw_all_later(UPD_NOT_VALID);
     redraw_cmdline = true;
@@ -6277,9 +6273,7 @@ void win_drag_status_line(win_T *dragwin, int offset)
   p_ch = MAX(Rows - cmdline_row, p_ch_was_zero ? 0 : 1);
   curtab->tp_ch_used = p_ch;
 
-  if (*p_spk != 'c') {
+  win_fix_scroll(true);
-    win_fix_scroll(true);
-  }
 
   redraw_all_later(UPD_SOME_VALID);
   showmode();
@@ -6398,15 +6392,18 @@ void set_fraction(win_T *wp)
   }
 }
 
-/// Handle scroll position for 'splitkeep'.  Replaces scroll_to_fraction()
+/// Handle scroll position, depending on 'splitkeep'.  Replaces the
-/// call from win_set_inner_size().  Instead we iterate over all windows in a
+/// scroll_to_fraction() call from win_new_height() if 'splitkeep' is "screen"
-/// tabpage and calculate the new scroll position.
+/// or "topline".  Instead we iterate over all windows in a tabpage and
-/// TODO(luukvbaal): Ensure this also works with wrapped lines.
+/// calculate the new scroll position.
-/// Requires topline to be able to be set to a bufferline with some
+/// TODO(vim): Ensure this also works with wrapped lines.
-/// offset(row-wise scrolling/smoothscroll).
+/// Requires a not fully visible cursor line to be allowed at the bottom of
+/// a window("zb"), probably only when 'smoothscroll' is also set.
 void win_fix_scroll(int resize)
 {
-  linenr_T lnum;
+  if (*p_spk == 'c') {
+    return;  // 'splitkeep' is "cursor"
+  }
 
   skip_update_topline = true;
   FOR_ALL_WINDOWS_IN_TAB(wp, curtab) {
@@ -6415,17 +6412,20 @@ void win_fix_scroll(int resize)
       // If window has moved update botline to keep the same screenlines.
       if (*p_spk == 's' && wp->w_winrow != wp->w_prev_winrow
           && wp->w_botline - 1 <= wp->w_buffer->b_ml.ml_line_count) {
-        lnum = wp->w_cursor.lnum;
         int diff = (wp->w_winrow - wp->w_prev_winrow)
                    + (wp->w_height - wp->w_prev_height);
+        linenr_T lnum = wp->w_cursor.lnum;
         wp->w_cursor.lnum = wp->w_botline - 1;
+
         // Add difference in height and row to botline.
         if (diff > 0) {
           cursor_down_inner(wp, diff);
         } else {
           cursor_up_inner(wp, -diff);
         }
-        // Bring the new cursor position to the bottom of the screen.
+
+        // Scroll to put the new cursor position at the bottom of the
+        // screen.
         wp->w_fraction = FRACTION_MULT;
         scroll_to_fraction(wp, wp->w_prev_height);
         wp->w_cursor.lnum = lnum;
@@ -6454,36 +6454,37 @@ void win_fix_scroll(int resize)
 static void win_fix_cursor(int normal)
 {
   win_T *wp = curwin;
-  long so = get_scrolloff_value(wp);
-  linenr_T nlnum = 0;
-  linenr_T lnum = wp->w_cursor.lnum;
 
-  if (wp->w_buffer->b_ml.ml_line_count < wp->w_height
+  if (skip_win_fix_cursor || wp->w_buffer->b_ml.ml_line_count < wp->w_height) {
-      || skip_win_fix_cursor) {
     return;
   }
 
   // Determine valid cursor range.
-  so = MIN(wp->w_height_inner / 2, so);
+  long so = MIN(wp->w_height_inner / 2, get_scrolloff_value(wp));
+  linenr_T lnum = wp->w_cursor.lnum;
+
   wp->w_cursor.lnum = wp->w_topline;
-  linenr_T top = cursor_down_inner(wp, so);
+  cursor_down_inner(wp, so);
+  linenr_T top = wp->w_cursor.lnum;
+
   wp->w_cursor.lnum = wp->w_botline - 1;
-  linenr_T bot = cursor_up_inner(wp, so);
+  cursor_up_inner(wp, so);
+  linenr_T bot = wp->w_cursor.lnum;
+
   wp->w_cursor.lnum = lnum;
   // Check if cursor position is above or below valid cursor range.
+  linenr_T nlnum = 0;
   if (lnum > bot && (wp->w_botline - wp->w_buffer->b_ml.ml_line_count) != 1) {
     nlnum = bot;
   } else if (lnum < top && wp->w_topline != 1) {
     nlnum = (so == wp->w_height / 2) ? bot : top;
   }
 
-  if (nlnum) {  // Cursor is invalid for current scroll position.
+  if (nlnum != 0) {  // Cursor is invalid for current scroll position.
-    if (normal) {
+    if (normal) {    // Save to jumplist and set cursor to avoid scrolling.
-      // Save to jumplist and set cursor to avoid scrolling.
       setmark('\'');
       wp->w_cursor.lnum = nlnum;
-    } else {
+    } else {         // Scroll instead when not in normal mode.
-      // Scroll instead when not in normal mode.
       wp->w_fraction = (nlnum == bot) ? FRACTION_MULT : 0;
       scroll_to_fraction(wp, wp->w_prev_height);
       validate_botline(curwin);

test/old/testdir/test_window_cmd.vim

@@ -1765,9 +1765,20 @@ endfunction
 
 function Test_splitkeep_misc()
   set splitkeep=screen
-  set splitbelow
 
   call setline(1, range(1, &lines))
+  " Cursor is adjusted to start and end of buffer
+  norm M
+  wincmd s
+  resize 1
+  call assert_equal(1, line('.'))
+  wincmd j
+  norm GM
+  resize 1
+  call assert_equal(&lines, line('.'))
+  only!
+
+  set splitbelow
   norm Gzz
   let top = line('w0')
   " No scroll when aucmd_win is opened