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 #12480)

Author: luukvbaal

src/edit.c

@@ -2755,17 +2755,12 @@ 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.
  */
-    linenr_T
+    void
 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 or the count is
-    // larger than the line number and '-' is in 'cpoptions'
-    if (lnum <= 1 || (n >= lnum && vim_strchr(p_cpo, CPO_MINUS) != NULL))
-	return 0;
     if (n >= lnum)
 	lnum = 1;
     else
@@ -2798,16 +2793,20 @@ cursor_up_inner(win_T *wp, long n)
 	lnum -= n;
 
     wp->w_cursor.lnum = lnum;
-    return lnum;
 }
 
     int
 cursor_up(
     long	n,
     int		upd_topline)	    // When TRUE: update topline
 {
-    if (n > 0 && cursor_up_inner(curwin, n) == 0)
+    // This fails if the cursor is already in the first line or the count is
+    // larger than the line number and '-' is in 'cpoptions'
+    linenr_T lnum = curwin->w_cursor.lnum;
+    if (n > 0 && (lnum <= 1
+		       || (n >= lnum && vim_strchr(p_cpo, CPO_MINUS) != NULL)))
 	return FAIL;
+    cursor_up_inner(curwin, n);
 
     // try to advance to the column we want to be at
     coladvance(curwin->w_curswant);
@@ -2821,23 +2820,13 @@ cursor_up(
 /*
  * Move the cursor down "n" lines in window "wp".
  * Takes care of closed folds.
- * Returns the new cursor line or zero for failure.
  */
-    linenr_T
+    void
 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;
 
-#ifdef FEAT_FOLDING
-    // Move to last line of fold, will fail if it's the end-of-file.
-    (void)hasFoldingWin(wp, lnum, NULL, &lnum, TRUE, NULL);
-#endif
-    // This fails if the cursor is already in the last line or would move
-    // beyond the last line and '-' is in 'cpoptions'
-    if (lnum >= line_count
-	    || (lnum + n > line_count && vim_strchr(p_cpo, CPO_MINUS) != NULL))
-	return FAIL;
     if (lnum + n >= line_count)
 	lnum = line_count;
     else
@@ -2849,6 +2838,7 @@ 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
@@ -2864,7 +2854,6 @@ cursor_down_inner(win_T *wp, long n)
 	lnum += n;
 
     wp->w_cursor.lnum = lnum;
-    return lnum;
 }
 
 /*
@@ -2875,8 +2864,16 @@ cursor_down(
     long	n,
     int		upd_topline)	    // When TRUE: update topline
 {
-    if (n > 0 &&  cursor_down_inner(curwin, n) == 0)
+    linenr_T	lnum = curwin->w_cursor.lnum;
+    linenr_T	line_count = curwin->w_buffer->b_ml.ml_line_count;
+    // This fails if the cursor is already in the last line or would move
+    // beyond the last line and '-' is in 'cpoptions'
+    if (n > 0
+	    && (lnum >= line_count
+		|| (lnum + n > line_count
+				     && vim_strchr(p_cpo, CPO_MINUS) != NULL)))
 	return FAIL;
+    cursor_down_inner(curwin, n);
 
     // try to advance to the column we want to be at
     coladvance(curwin->w_curswant);

src/normal.c

@@ -2359,11 +2359,13 @@ nv_screengo(oparg_T *oap, int dir, long dist)
 	    else
 	    {
 		// to previous line
-		if (!cursor_up_inner(curwin, 1))
+		if (curwin->w_cursor.lnum <= 1)
 		{
 		    retval = FAIL;
 		    break;
 		}
+		cursor_up_inner(curwin, 1);
+
 		linelen = linetabsize_str(ml_get_curline());
 		if (linelen > width1)
 		    curwin->w_curswant += (((linelen - width1 - 1) / width2)
@@ -2386,12 +2388,15 @@ nv_screengo(oparg_T *oap, int dir, long dist)
 	    else
 	    {
 		// to next line
-		if (!cursor_down_inner(curwin, 1))
+		if (curwin->w_cursor.lnum
+				       >= curwin->w_buffer->b_ml.ml_line_count)
 		{
 		    retval = FAIL;
 		    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/proto/edit.pro

@@ -19,9 +19,9 @@ char_u *add_char2buf(int c, char_u *s);
 void beginline(int flags);
 int oneright(void);
 int oneleft(void);
-linenr_T cursor_up_inner(win_T *wp, long n);
+void cursor_up_inner(win_T *wp, long n);
 int cursor_up(long n, int upd_topline);
-linenr_T cursor_down_inner(win_T *wp, long n);
+void cursor_down_inner(win_T *wp, long n);
 int cursor_down(long n, int upd_topline);
 int stuff_inserted(int c, long count, int no_esc);
 char_u *get_last_insert(void);

src/testdir/test_window_cmd.vim

@@ -1819,9 +1819,20 @@ endfunc
 
 func 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

src/version.c

@@ -695,6 +695,8 @@ static char *(features[]) =
 
 static int included_patches[] =
 {   /* Add new patch number below this line */
+/**/
+    1599,
 /**/
     1598,
 /**/

src/window.c

@@ -1406,7 +1406,7 @@ win_split_ins(
 	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);
 
     // Don't change the window height/width to 'winheight' / 'winwidth' if a
@@ -2012,7 +2012,7 @@ win_equal(
     win_equal_rec(next_curwin == NULL ? curwin : next_curwin, current,
 		      topframe, dir, 0, tabline_height(),
 					   (int)Columns, topframe->fr_height);
-    if (*p_spk != 'c' && !is_aucmd_win(next_curwin))
+    if (!is_aucmd_win(next_curwin))
 	win_fix_scroll(TRUE);
 }
 
@@ -2822,8 +2822,7 @@ win_close(win_T *win, int free_buf)
     else
     {
 	win_comp_pos();
-	if (*p_spk != 'c')
-	    win_fix_scroll(FALSE);
+	win_fix_scroll(FALSE);
     }
     if (close_curwin)
     {
@@ -5906,7 +5905,7 @@ shell_new_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);
 
 #if 0
@@ -6111,8 +6110,7 @@ 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);
 }
@@ -6642,8 +6640,7 @@ win_drag_status_line(win_T *dragwin, int offset)
     p_ch = MAX(Rows - cmdline_row, 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();
@@ -6772,21 +6769,22 @@ set_fraction(win_T *wp)
 }
 
 /*
- * Handle scroll position for 'splitkeep'.  Replaces scroll_to_fraction()
- * call from win_new_height().  Instead we iterate over all windows in a
- * tabpage and calculate the new scroll position.
+ * Handle scroll position, depending on 'splitkeep'.  Replaces the
+ * scroll_to_fraction() call from win_new_height() if 'splitkeep' is "screen"
+ * or "topline".  Instead we iterate over all windows in a tabpage and
+ * calculate the new scroll position.
  * TODO: Ensure this also works with wrapped lines.
- * Requires topline to be able to be set to a bufferline with some
- * 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.
  */
     static void
 win_fix_scroll(int resize)
 {
-    int		diff;
-    win_T	*wp;
-    linenr_T	lnum;
+    if (*p_spk == 'c')
+	return;  // 'splitkeep' is "cursor"
 
     skip_update_topline = TRUE;
+    win_T	*wp;
     FOR_ALL_WINDOWS(wp)
     {
 	// Skip when window height has not changed.
@@ -6796,18 +6794,22 @@ win_fix_scroll(int resize)
 	    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;
-		diff = (wp->w_winrow - wp->w_prev_winrow)
-		     + (wp->w_height - wp->w_prev_height);
+		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;
 	    }
 	    else if (wp == curwin)
@@ -6835,32 +6837,33 @@ win_fix_scroll(int resize)
     static void
 win_fix_cursor(int normal)
 {
-    long	so = get_scrolloff_value();
     win_T	*wp = curwin;
-    linenr_T	nlnum = 0;
-    linenr_T	lnum = wp->w_cursor.lnum;
-    linenr_T	bot;
-    linenr_T	top;
 
-    if (wp->w_buffer->b_ml.ml_line_count < wp->w_height)
-	return;
-    if (skip_win_fix_cursor)
+    if (skip_win_fix_cursor || wp->w_buffer->b_ml.ml_line_count < wp->w_height)
 	return;
 
     // Determine valid cursor range.
-    so = MIN(wp->w_height / 2, so);
+    long so = MIN(wp->w_height / 2, get_scrolloff_value());
+    linenr_T lnum = wp->w_cursor.lnum;
+
     wp->w_cursor.lnum = wp->w_topline;
-    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;
-    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)  // Save to jumplist and set cursor to avoid scrolling.
 	{