To: vim_dev@googlegroups.com
Subject: Patch 9.0.1599
Fcc: outbox
From: Bram Moolenaar <Bram@moolenaar.net>
Mime-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
------------

Patch 9.0.1599
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)
Files:      src/edit.c, src/proto/edit.pro, src/normal.c, src/window.c,
            src/testdir/test_window_cmd.vim


*** ../vim-9.0.1598/src/edit.c	2023-05-20 14:06:56.669542805 +0100
--- src/edit.c	2023-06-02 13:50:44.181628850 +0100
***************
*** 2755,2771 ****
  /*
   * 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
  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
--- 2755,2766 ----
  /*
   * Move the cursor up "n" lines in window "wp".
   * Takes care of closed folds.
   */
!     void
  cursor_up_inner(win_T *wp, long n)
  {
      linenr_T	lnum = wp->w_cursor.lnum;
  
      if (n >= lnum)
  	lnum = 1;
      else
***************
*** 2798,2804 ****
  	lnum -= n;
  
      wp->w_cursor.lnum = lnum;
-     return lnum;
  }
  
      int
--- 2793,2798 ----
***************
*** 2806,2813 ****
      long	n,
      int		upd_topline)	    // When TRUE: update topline
  {
!     if (n > 0 && cursor_up_inner(curwin, n) == 0)
  	return FAIL;
  
      // try to advance to the column we want to be at
      coladvance(curwin->w_curswant);
--- 2800,2812 ----
      long	n,
      int		upd_topline)	    // When TRUE: update topline
  {
!     // 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,2843 ****
  /*
   * 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
  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
--- 2820,2832 ----
  /*
   * Move the cursor down "n" lines in window "wp".
   * Takes care of closed folds.
   */
!     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;
  
      if (lnum + n >= line_count)
  	lnum = line_count;
      else
***************
*** 2849,2854 ****
--- 2838,2844 ----
  	// 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,2870 ****
  	lnum += n;
  
      wp->w_cursor.lnum = lnum;
-     return lnum;
  }
  
  /*
--- 2854,2859 ----
***************
*** 2875,2882 ****
      long	n,
      int		upd_topline)	    // When TRUE: update topline
  {
!     if (n > 0 &&  cursor_down_inner(curwin, n) == 0)
  	return FAIL;
  
      // try to advance to the column we want to be at
      coladvance(curwin->w_curswant);
--- 2864,2879 ----
      long	n,
      int		upd_topline)	    // When TRUE: update topline
  {
!     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);
*** ../vim-9.0.1598/src/proto/edit.pro	2022-09-27 12:30:53.222180148 +0100
--- src/proto/edit.pro	2023-06-02 13:48:13.629873068 +0100
***************
*** 19,27 ****
  void beginline(int flags);
  int oneright(void);
  int oneleft(void);
! linenr_T 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);
  int cursor_down(long n, int upd_topline);
  int stuff_inserted(int c, long count, int no_esc);
  char_u *get_last_insert(void);
--- 19,27 ----
  void beginline(int flags);
  int oneright(void);
  int oneleft(void);
! void cursor_up_inner(win_T *wp, long n);
  int cursor_up(long n, int upd_topline);
! 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);
*** ../vim-9.0.1598/src/normal.c	2023-05-12 15:47:21.856773279 +0100
--- src/normal.c	2023-06-02 13:48:43.721823978 +0100
***************
*** 2359,2369 ****
  	    else
  	    {
  		// to previous line
! 		if (!cursor_up_inner(curwin, 1))
  		{
  		    retval = FAIL;
  		    break;
  		}
  		linelen = linetabsize_str(ml_get_curline());
  		if (linelen > width1)
  		    curwin->w_curswant += (((linelen - width1 - 1) / width2)
--- 2359,2371 ----
  	    else
  	    {
  		// to previous line
! 		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,2397 ****
  	    else
  	    {
  		// to next line
! 		if (!cursor_down_inner(curwin, 1))
  		{
  		    retval = FAIL;
  		    break;
  		}
  		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
--- 2388,2402 ----
  	    else
  	    {
  		// to next line
! 		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
*** ../vim-9.0.1598/src/window.c	2023-05-24 21:02:20.489162125 +0100
--- src/window.c	2023-06-02 14:07:16.016018650 +0100
***************
*** 1406,1412 ****
  	win_equal(wp, TRUE,
  		(flags & WSP_VERT) ? (dir == 'v' ? 'b' : 'h')
  		: dir == 'h' ? 'b' : 'v');
!     else if (*p_spk != 'c' && !is_aucmd_win(wp))
  	win_fix_scroll(FALSE);
  
      // Don't change the window height/width to 'winheight' / 'winwidth' if a
--- 1406,1412 ----
  	win_equal(wp, TRUE,
  		(flags & WSP_VERT) ? (dir == 'v' ? 'b' : 'h')
  		: dir == 'h' ? 'b' : 'v');
!     else if (!is_aucmd_win(wp))
  	win_fix_scroll(FALSE);
  
      // Don't change the window height/width to 'winheight' / 'winwidth' if a
***************
*** 2012,2018 ****
      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))
  	win_fix_scroll(TRUE);
  }
  
--- 2012,2018 ----
      win_equal_rec(next_curwin == NULL ? curwin : next_curwin, current,
  		      topframe, dir, 0, tabline_height(),
  					   (int)Columns, topframe->fr_height);
!     if (!is_aucmd_win(next_curwin))
  	win_fix_scroll(TRUE);
  }
  
***************
*** 2822,2829 ****
      else
      {
  	win_comp_pos();
! 	if (*p_spk != 'c')
! 	    win_fix_scroll(FALSE);
      }
      if (close_curwin)
      {
--- 2822,2828 ----
      else
      {
  	win_comp_pos();
! 	win_fix_scroll(FALSE);
      }
      if (close_curwin)
      {
***************
*** 5906,5912 ****
      compute_cmdrow();
      curtab->tp_ch_used = p_ch;
  
!     if (*p_spk != 'c' && !skip_win_fix_scroll)
  	win_fix_scroll(TRUE);
  
  #if 0
--- 5905,5911 ----
      compute_cmdrow();
      curtab->tp_ch_used = p_ch;
  
!     if (!skip_win_fix_scroll)
  	win_fix_scroll(TRUE);
  
  #if 0
***************
*** 6111,6118 ****
      msg_row = row;
      msg_col = 0;
  
!     if (*p_spk != 'c')
! 	win_fix_scroll(TRUE);
  
      redraw_all_later(UPD_NOT_VALID);
  }
--- 6110,6116 ----
      msg_row = row;
      msg_col = 0;
  
!     win_fix_scroll(TRUE);
  
      redraw_all_later(UPD_NOT_VALID);
  }
***************
*** 6642,6649 ****
      p_ch = MAX(Rows - cmdline_row, 1);
      curtab->tp_ch_used = p_ch;
  
!     if (*p_spk != 'c')
! 	win_fix_scroll(TRUE);
  
      redraw_all_later(UPD_SOME_VALID);
      showmode();
--- 6640,6646 ----
      p_ch = MAX(Rows - cmdline_row, 1);
      curtab->tp_ch_used = p_ch;
  
!     win_fix_scroll(TRUE);
  
      redraw_all_later(UPD_SOME_VALID);
      showmode();
***************
*** 6772,6792 ****
  }
  
  /*
!  * 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.
   * 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).
   */
      static void
  win_fix_scroll(int resize)
  {
!     int		diff;
!     win_T	*wp;
!     linenr_T	lnum;
  
      skip_update_topline = TRUE;
      FOR_ALL_WINDOWS(wp)
      {
  	// Skip when window height has not changed.
--- 6769,6790 ----
  }
  
  /*
!  * 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 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)
  {
!     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,6813 ****
  	    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);
  		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.
  		wp->w_fraction = FRACTION_MULT;
  		scroll_to_fraction(wp, wp->w_prev_height);
  		wp->w_cursor.lnum = lnum;
  	    }
  	    else if (wp == curwin)
--- 6794,6815 ----
  	    if (*p_spk == 's' && wp->w_winrow != wp->w_prev_winrow
  		      && wp->w_botline - 1 <= wp->w_buffer->b_ml.ml_line_count)
  	    {
! 		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);
! 
! 		// 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,6866 ****
      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)
  	return;
  
      // Determine valid cursor range.
!     so = MIN(wp->w_height / 2, so);
      wp->w_cursor.lnum = wp->w_topline;
!     top = cursor_down_inner(wp, so);
      wp->w_cursor.lnum = wp->w_botline - 1;
!     bot = cursor_up_inner(wp, so);
      wp->w_cursor.lnum = lnum;
      // Check if cursor position is above or below valid cursor range.
      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 (normal)  // Save to jumplist and set cursor to avoid scrolling.
  	{
--- 6837,6869 ----
      static void
  win_fix_cursor(int normal)
  {
      win_T	*wp = curwin;
  
!     if (skip_win_fix_cursor || wp->w_buffer->b_ml.ml_line_count < wp->w_height)
  	return;
  
      // Determine valid cursor range.
!     long so = MIN(wp->w_height / 2, get_scrolloff_value());
!     linenr_T lnum = wp->w_cursor.lnum;
! 
      wp->w_cursor.lnum = wp->w_topline;
!     cursor_down_inner(wp, so);
!     linenr_T top = wp->w_cursor.lnum;
! 
      wp->w_cursor.lnum = wp->w_botline - 1;
!     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 != 0)  // Cursor is invalid for current scroll position.
      {
  	if (normal)  // Save to jumplist and set cursor to avoid scrolling.
  	{
*** ../vim-9.0.1598/src/testdir/test_window_cmd.vim	2023-05-06 12:53:33.711345001 +0100
--- src/testdir/test_window_cmd.vim	2023-06-02 14:16:08.579164110 +0100
***************
*** 1819,1827 ****
  
  func Test_splitkeep_misc()
    set splitkeep=screen
-   set splitbelow
  
    call setline(1, range(1, &lines))
    norm Gzz
    let top = line('w0')
    " No scroll when aucmd_win is opened
--- 1819,1838 ----
  
  func Test_splitkeep_misc()
    set splitkeep=screen
  
    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
*** ../vim-9.0.1598/src/version.c	2023-06-01 20:26:52.064180242 +0100
--- src/version.c	2023-06-02 14:14:29.471321762 +0100
***************
*** 697,698 ****
--- 697,700 ----
  {   /* Add new patch number below this line */
+ /**/
+     1599,
  /**/

-- 
hundred-and-one symptoms of being an internet addict:
93. New mail alarm on your palmtop annoys other churchgoers.

 /// Bram Moolenaar -- Bram@Moolenaar.net -- http://www.Moolenaar.net   \\\
///                                                                      \\\
\\\        sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ ///
 \\\            help me help AIDS victims -- http://ICCF-Holland.org    ///