vim-patch:9.1.0171: Small split-move related improvements

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

Some of these changes were already applied to Nvim.
Here are the ones which were missing:

- 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 vim/vim#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 vim/vim#14109, which I hope to get around to
  finishing at some point).

- Apply the winframe_restore comment changes, and remove win_comp_pos from after
  winframe_restore in win_splitmove, as it shouldn't be necessary (no need to
  remove it from nvim_win_set_config too, as it was already omitted).
  Move win_append after winframe_restore in win_splitmove to match Vim.

closes: vim/vim#14185

5cac1a9bee

src/nvim/eval/window.c

@@ -701,8 +701,8 @@ void f_win_splitmove(typval_T *argvars, typval_T *rettv, EvalFuncData fptr)
     size = (int)tv_dict_get_number(d, "size");
   }
 
-  // Check if we can split the target before we bother switching windows.
-  if (is_aucmd_win(wp) || text_or_buf_locked() || check_split_disallowed(targetwin) == FAIL) {
+  // Check if we're allowed to continue before we bother switching windows.
+  if (is_aucmd_win(wp) || text_or_buf_locked() || check_split_disallowed(wp) == FAIL) {
     return;
   }
 

src/nvim/window.c

@@ -939,7 +939,7 @@ void ui_ext_win_viewport(win_T *wp)
   }
 }
 
-/// If "split_disallowed" is set or "wp"s buffer is closing, give an error and return FAIL.
+/// If "split_disallowed" is set, or "wp"'s buffer is closing, give an error and return FAIL.
 /// Otherwise return OK.
 int check_split_disallowed(const win_T *wp)
   FUNC_ATTR_NONNULL_ALL
@@ -1966,13 +1966,12 @@ int win_splitmove(win_T *wp, int size, int flags)
 
   // Split a window on the desired side and put "wp" there.
   if (win_split_ins(size, flags, wp, dir, unflat_altfr) == NULL) {
-    win_append(wp->w_prev, wp, NULL);
     if (!wp->w_floating) {
       // win_split_ins doesn't change sizes or layout if it fails to insert an
       // existing window, so just undo winframe_remove.
       winframe_restore(wp, dir, unflat_altfr);
-      win_comp_pos();  // recompute window positions
     }
+    win_append(wp->w_prev, wp, NULL);
     return FAIL;
   }
 
@@ -3271,7 +3270,6 @@ win_T *winframe_find_altwin(win_T *win, int *dirp, tabpage_T *tp, frame_T **altf
 /// 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)
   FUNC_ATTR_NONNULL_ALL
 {
@@ -3359,7 +3357,8 @@ void winframe_restore(win_T *wp, int dir, frame_T *unflat_altfr)
   int row = wp->w_winrow;
   int col = wp->w_wincol;
 
-  // 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);

test/old/testdir/test_window_cmd.vim

@@ -200,6 +200,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
@@ -216,12 +230,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:
@@ -238,18 +250,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
@@ -257,11 +269,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))
@@ -1076,6 +1086,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
@@ -1086,7 +1097,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)
@@ -2088,13 +2099,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