patch 9.1.0171: Small split-move related improvementsv9.1.0171

Problem:  small improvements can be made to split-move related
          functions.
Solution: apply them (Sean Dewar):

- Improve some doc comments (frame_flatten should still work for non-current
  tabpages, despite the topframe check, which looks benign, though I'm unsure if
  it's still needed; see #2467).

- f_win_splitmove should check_split_disallowed on wp, not targetwin, as that's
  what win_splitmove checks (though it's probably unnecessary to check
  b_locked_split at all; see #14109, which I hope to get around to finishing at
  some point).

- Make winframe_restore restore window positions for the altframe, which
  winframe_remove changes. This doesn't affect the prior behaviour, as we called
  win_comp_pos after, but as win_comp_pos only works for curtab, and
  winframe_remove supports non-current tabpages, we should undo it. Regardless,
  this should mean we don't need win_comp_pos anymore; adjust tests to check
  that window positions remain unchanged.

  I'm not sure win_comp_pos is needed after last_status anyway if it doesn't
  steal rows from another frame to make room for a new statusline, which
  shouldn't be the case after winframe_remove? To be safe, I'll leave it as is.

closes: #14185

Signed-off-by: Sean Dewar <6256228+seandewar@users.noreply.github.com>
Signed-off-by: Christian Brabandt <cb@256bit.org>

4 files changed, 44 insertions, 24 deletions

diff --git a/src/evalwindow.c b/src/evalwindow.c
index cb813a6f43..ceca2c9ff1 100644
--- a/src/evalwindow.c
+++ b/src/evalwindow.c
@@ -1001,8 +1001,8 @@ f_win_splitmove(typval_T *argvars, typval_T *rettv)
 	size = (int)dict_get_number(d, "size");
     }
 
-    // Check if we can split the target before we bother switching windows.
-    if (text_or_buf_locked() || check_split_disallowed(targetwin) == FAIL)
+    // Check if we're allowed to continue before we bother switching windows.
+    if (text_or_buf_locked() || check_split_disallowed(wp) == FAIL)
 	return;
 
     if (curwin != targetwin)
diff --git a/src/testdir/test_window_cmd.vim b/src/testdir/test_window_cmd.vim
index fc4786d4ac..332a7f3470 100644
--- a/src/testdir/test_window_cmd.vim
+++ b/src/testdir/test_window_cmd.vim
@@ -218,6 +218,20 @@ func Test_window_split_edit_bufnr()
   %bw!
 endfunc
 
+func s:win_layout_info() abort
+  return #{
+        \ layout: winlayout(),
+        \ pos_sizes: range(1, winnr('$'))
+        \            ->map({_, nr -> win_getid(nr)->getwininfo()[0]})
+        \            ->map({_, wininfo -> #{id: wininfo.winid,
+        \                                   row: wininfo.winrow,
+        \                                   col: wininfo.wincol,
+        \                                   width: wininfo.width,
+        \                                   height: wininfo.height}})
+        \            ->sort({a, b -> a.id - b.id})
+        \ }
+endfunc
+
 func Test_window_split_no_room()
   " N horizontal windows need >= 2*N + 1 lines:
   " - 1 line + 1 status line in each window
@@ -234,12 +248,10 @@ func Test_window_split_no_room()
 
   botright vsplit
   wincmd |
-  let layout = winlayout()
-  let restcmd = winrestcmd()
+  let info = s:win_layout_info()
   call assert_fails('wincmd J', 'E36:')
   call assert_fails('wincmd K', 'E36:')
-  call assert_equal(layout, winlayout())
-  call assert_equal(restcmd, winrestcmd())
+  call assert_equal(info, s:win_layout_info())
   only
 
   " N vertical windows need >= 2*(N - 1) + 1 columns:
@@ -256,18 +268,18 @@ func Test_window_split_no_room()
 
   split
   wincmd |
-  let layout = winlayout()
-  let restcmd = winrestcmd()
+  let info = s:win_layout_info()
   call assert_fails('wincmd H', 'E36:')
   call assert_fails('wincmd L', 'E36:')
-  call assert_equal(layout, winlayout())
-  call assert_equal(restcmd, winrestcmd())
+  call assert_equal(info, s:win_layout_info())
 
   " Check that the last statusline isn't lost.
   " Set its window's width to 2 for the test.
   wincmd j
   set laststatus=0 winminwidth=0
   vertical resize 2
+  " Update expected positions/sizes after the resize.  Layout is unchanged.
+  let info.pos_sizes = s:win_layout_info().pos_sizes
   set winminwidth&
   call setwinvar(winnr('k'), '&statusline', '@#')
   let last_stl_row = win_screenpos(0)[0] - 1
@@ -275,11 +287,9 @@ func Test_window_split_no_room()
   call assert_equal('@#|', GetScreenStr(last_stl_row))
   call assert_equal('~ |', GetScreenStr(&lines - &cmdheight))
 
-  let restcmd = winrestcmd()
   call assert_fails('wincmd H', 'E36:')
   call assert_fails('wincmd L', 'E36:')
-  call assert_equal(layout, winlayout())
-  call assert_equal(restcmd, winrestcmd())
+  call assert_equal(info, s:win_layout_info())
   call setwinvar(winnr('k'), '&statusline', '=-')
   redraw
   call assert_equal('=-|', GetScreenStr(last_stl_row))
@@ -1117,6 +1127,7 @@ func Test_win_splitmove()
   augroup WinSplitMove
     au!
     au WinEnter * ++once let s:triggered = v:true
+          \| call assert_fails('call win_splitmove(winnr(), winnr("$"))', 'E242:')
           \| call assert_fails('call win_splitmove(winnr("$"), winnr())', 'E242:')
   augroup END
   quit
@@ -1127,7 +1138,7 @@ func Test_win_splitmove()
   augroup WinSplitMove
     au!
     au BufHidden * ++once let s:triggered = v:true
-          \| call assert_fails('call win_splitmove(winnr("#"), winnr())', 'E1159:')
+          \| call assert_fails('call win_splitmove(winnr(), winnr("#"))', 'E1159:')
   augroup END
   hide
   call assert_equal(v:true, s:triggered)
@@ -2153,13 +2164,11 @@ func Test_autocmd_window_force_room()
     au BufEnter * ++once let s:triggered = v:true
                       \| call assert_equal('autocmd', win_gettype())
   augroup END
-  let layout = winlayout()
-  let restcmd = winrestcmd()
+  let info = s:win_layout_info()
   " bufload opening the autocommand window shouldn't give E36.
   call bufload('unload me')
   call assert_equal(v:true, s:triggered)
-  call assert_equal(winlayout(), layout)
-  call assert_equal(winrestcmd(), restcmd)
+  call assert_equal(info, s:win_layout_info())
 
   unlet! s:triggered
   au! AucmdWinForceRoom
diff --git a/src/version.c b/src/version.c
index 73cdfa274e..8485362385 100644
--- a/src/version.c
+++ b/src/version.c
@@ -705,6 +705,8 @@ static char *(features[]) =
 static int included_patches[] =
 {   /* Add new patch number below this line */
 /**/
+    171,
+/**/
     170,
 /**/
     169,
diff --git a/src/window.c b/src/window.c
index 00b35bbbbf..fbdad9c2a2 100644
--- a/src/window.c
+++ b/src/window.c
@@ -896,8 +896,8 @@ cmd_with_count(
 }
 
 /*
- * If "split_disallowed" is set for "wp", give an error and return FAIL.
- * Otherwise return OK.
+ * If "split_disallowed" is set, or "wp"'s buffer is closing, give an error and
+ * return FAIL.  Otherwise return OK.
  */
     int
 check_split_disallowed(win_T *wp)
@@ -1980,7 +1980,6 @@ win_splitmove(win_T *wp, int size, int flags)
 	// existing window, so just undo winframe_remove.
 	winframe_restore(wp, dir, unflat_altfr);
 	win_append(wp->w_prev, wp);
-	(void)win_comp_pos();   // recompute window positions
 	return FAIL;
     }
 
@@ -3607,7 +3606,6 @@ winframe_remove(
  * Flatten "frp" into its parent frame if it's the only child, also merging its
  * list with the grandparent if they share the same layout.
  * Frees "frp" if flattened; also "frp->fr_parent" if it has the same layout.
- * "frp" must be valid in the current tabpage.
  */
     static void
 frame_flatten(frame_T *frp)
@@ -3660,13 +3658,16 @@ frame_flatten(frame_T *frp)
 
 /*
  * Undo changes from a prior call to winframe_remove, also restoring lost
- * vertical separators and statuslines.
+ * vertical separators and statuslines, and changed window positions for
+ * windows within "unflat_altfr".
  * Caller must ensure no other changes were made to the layout or window sizes!
  */
     static void
 winframe_restore(win_T *wp, int dir, frame_T *unflat_altfr)
 {
     frame_T	*frp = wp->w_frame;
+    int		row = wp->w_winrow;
+    int		col = wp->w_wincol;
 
     // Put "wp"'s frame back where it was.
     if (frp->fr_prev != NULL)
@@ -3684,17 +3685,25 @@ winframe_restore(win_T *wp, int dir, frame_T *unflat_altfr)
 	    && frp->fr_parent->fr_layout == FR_COL && frp->fr_prev != NULL)
 	frame_add_statusline(frp->fr_prev);
 
-    // Restore the lost room that was redistributed to the altframe.
+    // Restore the lost room that was redistributed to the altframe.  Also
+    // adjusts window sizes to fit restored statuslines/separators, if needed.
     if (dir == 'v')
     {
 	frame_new_height(unflat_altfr, unflat_altfr->fr_height - frp->fr_height,
 		unflat_altfr == frp->fr_next, FALSE);
+	row += frp->fr_height;
     }
     else if (dir == 'h')
     {
 	frame_new_width(unflat_altfr, unflat_altfr->fr_width - frp->fr_width,
 		unflat_altfr == frp->fr_next, FALSE);
+	col += frp->fr_width;
     }
+
+    // If rows/columns went to a window below/right, its positions need to be
+    // restored.  Can only be done after the sizes have been updated.
+    if (unflat_altfr == frp->fr_next)
+	frame_comp_pos(unflat_altfr, &row, &col);
 }
 
 /*