patch 9.1.0117: Stop split-moving from firing WinNew and WinNewPre autocommands
Commit: https://github.com/vim/vim/commit/96cc4aef3d47d0fd70e68908af3d48a0dce8ea70 Author: Sean Dewar <6256228+seande...@users.noreply.github.com> Date: Tue Feb 20 21:52:31 2024 +0100 patch 9.1.0117: Stop split-moving from firing WinNew and WinNewPre autocommands Problem: win_splitmove fires WinNewPre and possibly WinNew when moving windows, even though no new windows are created. Solution: don't fire WinNew and WinNewPre when inserting an existing window, even if it isn't the current window. Improve the accuracy of related documentation. (Sean Dewar) Likewise, before this patch, WinClosed was not fired anyway (even for :wincmd H/J/K/L, which also didn't fire WinNew, but did still fire WinNewPre), despite documentation saying windows are "closed". Note that :wincmd T actually indeed works by creating a new window (and closing the old one), unlike the others. This also fixes issues where WinNewPre is fired when split-moving while curwin doesn't yet have a frame or entry in the window list, causing many things to not work (it's not considered valid at that point). This was guaranteed when using :wincmd H/J/K/L. Because WinNewPre is no longer fired when split-moving, this makes restoring the previous window layout on failure easier, as we can be sure that frames are not resized from WinNewPre autocommands if win_split_ins fails. This allows us to use a different strategy in the following commit. -- In my opinion, this leaves questions about the current usefulness of WinNewPre. A motivation described in #10635 states how creating a new window can steal room from other windows, and how WinNewPre will be useful for detecting that, but this is also true when inserting an existing window, which now doesn't fire it. Maybe the autocommand should be changed to have a better name? There are also other issues I found with the current implementation of WinNewPre that need addressing: - it allows switching windows and tabpages, which can cause incorrect windows to be split/moved, and big problems when switching tabpages. - it fires before win_split_ins checks for room, before it makes any changes to window sizes or before it considers allocating a new window. This should be changed or documented. I hope to address some of this stuff in a different PR, if possible. related: #14038 Signed-off-by: Sean Dewar <6256228+seande...@users.noreply.github.com> Signed-off-by: Christian Brabandt <c...@256bit.org> diff --git a/runtime/doc/builtin.txt b/runtime/doc/builtin.txt index 0b3d59e96..0cddeffa7 100644 --- a/runtime/doc/builtin.txt +++ b/runtime/doc/builtin.txt @@ -1,4 +1,4 @@ -*builtin.txt* For Vim version 9.1. Last change: 2024 Feb 12 +*builtin.txt* For Vim version 9.1. Last change: 2024 Feb 20 VIM REFERENCE MANUAL by Bram Moolenaar @@ -10762,10 +10762,10 @@ win_screenpos({nr}) *win_screenpos()* GetWinid()->win_screenpos() < win_splitmove({nr}, {target} [, {options}]) *win_splitmove()* - Move the window {nr} to a new split of the window {target}. - This is similar to moving to {target}, creating a new window - using |:split| but having the same contents as window {nr}, and - then closing {nr}. + Temporarily switch to window {target}, then move window {nr} + to a new split adjacent to {target}. + Unlike commands such as |:split|, no new windows are created + (the |window-ID| of window {nr} is unchanged after the move). Both {nr} and {target} can be window numbers or |window-ID|s. Both must be in the current tab page. diff --git a/runtime/doc/windows.txt b/runtime/doc/windows.txt index 806474875..e264e5117 100644 --- a/runtime/doc/windows.txt +++ b/runtime/doc/windows.txt @@ -1,4 +1,4 @@ -*windows.txt* For Vim version 9.1. Last change: 2022 Nov 27 +*windows.txt* For Vim version 9.1. Last change: 2024 Feb 20 VIM REFERENCE MANUAL by Bram Moolenaar @@ -519,35 +519,33 @@ horizontally split windows. CTRL-W H does it the other way around. *CTRL-W_K* CTRL-W K Move the current window to be at the very top, using the full - width of the screen. This works like closing the current - window and then creating another one with ":topleft split", - except that the current window contents is used for the new - window. + width of the screen. This works like `:topleft split`, except + it is applied to the current window and no new window is + created. *CTRL-W_J* CTRL-W J Move the current window to be at the very bottom, using the - full width of the screen. This works like closing the current - window and then creating another one with ":botright split", - except that the current window contents is used for the new - window. + full width of the screen. This works like `:botright split`, + except it is applied to the current window and no new window + is created. *CTRL-W_H* CTRL-W H Move the current window to be at the far left, using the - full height of the screen. This works like closing the - current window and then creating another one with - `:vert topleft split`, except that the current window contents - is used for the new window. + full height of the screen. This works like + `:vert topleft split`, except it is applied to the current + window and no new window is created. *CTRL-W_L* CTRL-W L Move the current window to be at the far right, using the full - height of the screen. This works like closing the - current window and then creating another one with - `:vert botright split`, except that the current window - contents is used for the new window. + height of the screen. This works like `:vert botright split`, + except it is applied to the current window and no new window + is created. *CTRL-W_T* CTRL-W T Move the current window to a new tab page. This fails if there is only one window in the current tab page. + This works like `:tab split`, except the previous window is + closed. When a count is specified the new tab page will be opened before the tab page with this index. Otherwise it comes after the current tab page. diff --git a/src/testdir/test_window_cmd.vim b/src/testdir/test_window_cmd.vim index c898a233a..5b50f9dc5 100644 --- a/src/testdir/test_window_cmd.vim +++ b/src/testdir/test_window_cmd.vim @@ -1104,6 +1104,18 @@ func Test_win_splitmove() leftabove split b leftabove vsplit c leftabove split d + + " win_splitmove doesn't actually create or close any windows, so expect an + " unchanged winid and no WinNew/WinClosed events, like :wincmd H/J/K/L. + let s:triggered = [] + augroup WinSplitMove + au! + au WinNewPre * let s:triggered += ['WinNewPre'] + au WinNew * let s:triggered += ['WinNew', win_getid()] + au WinClosed * let s:triggered += ['WinClosed', str2nr(expand('<afile>'))] + augroup END + let winid = win_getid() + call assert_equal(0, win_splitmove(winnr(), winnr('l'))) call assert_equal(bufname(winbufnr(1)), 'c') call assert_equal(bufname(winbufnr(2)), 'd') @@ -1126,6 +1138,11 @@ func Test_win_splitmove() call assert_equal(bufname(winbufnr(3)), 'a') call assert_equal(bufname(winbufnr(4)), 'd') call assert_fails('call win_splitmove(winnr(), winnr("k"), test_null_dict())', 'E1297:') + call assert_equal([], s:triggered) + call assert_equal(winid, win_getid()) + + unlet! s:triggered + au! WinSplitMove only | bd call assert_fails('call win_splitmove(winnr(), 123)', 'E957:') diff --git a/src/version.c b/src/version.c index cf1c9da78..9ceb6cebe 100644 --- a/src/version.c +++ b/src/version.c @@ -704,6 +704,8 @@ static char *(features[]) = static int included_patches[] = { /* Add new patch number below this line */ +/**/ + 117, /**/ 116, /**/ diff --git a/src/window.c b/src/window.c index de4347665..1ae7b27d8 100644 --- a/src/window.c +++ b/src/window.c @@ -935,6 +935,8 @@ win_split(int size, int flags) * When "new_wp" is NULL: split the current window in two. * When "new_wp" is not NULL: insert this window at the far * top/left/right/bottom. + * On failure, if "new_wp" was not NULL, no changes will have been made to the + * window layout or sizes. * Return FAIL for failure, OK otherwise. */ int @@ -964,7 +966,8 @@ win_split_ins( // Do not redraw here, curwin->w_buffer may be invalid. ++RedrawingDisabled; - trigger_winnewpre(); + if (new_wp == NULL) + trigger_winnewpre(); if (flags & WSP_TOP) oldwin = firstwin; @@ -1444,7 +1447,7 @@ win_split_ins( /* * make the new window the current window */ - (void)win_enter_ext(wp, WEE_TRIGGER_NEW_AUTOCMDS + (void)win_enter_ext(wp, (new_wp == NULL ? WEE_TRIGGER_NEW_AUTOCMDS : 0) | WEE_TRIGGER_ENTER_AUTOCMDS | WEE_TRIGGER_LEAVE_AUTOCMDS); if (flags & WSP_VERT) p_wiw = i; -- -- You received this message from the "vim_dev" maillist. Do not top-post! Type your reply below the text you are replying to. For more information, visit http://www.vim.org/maillist.php --- You received this message because you are subscribed to the Google Groups "vim_dev" group. To unsubscribe from this group and stop receiving emails from it, send an email to vim_dev+unsubscr...@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/vim_dev/E1rcXgk-0099iS-1p%40256bit.org.
