From 29d0ed577c7d283402c84df602a031a25349eb59 Mon Sep 17 00:00:00 2001 From: Sean Dewar <6256228+seandewar@users.noreply.github.com> Date: Tue, 12 Mar 2024 21:02:29 +0000 Subject: [PATCH 1/4] vim-patch:9.1.0169: current window number returned by tabpagewinnr may be outdated Problem: current window number returned by tabpagewinnr may be outdated when called from win_execute for the original tabpage. Solution: update the original tabpage's tp_curwin in switch_win; use {un}use_tabpage instead. Don't do it in restore_win to ensure tp_curwin of the temporarily visited tabpage is unchanged from switch_win visiting it, as before. (Sean Dewar) Maybe restore_win should only restore tp_curwin if `curtab == switchwin->sw_curtab`, in case the user changed tabpages from within win_execute, but not doing that is consistent with the old behaviour. related: vim/vim#14186 https://github.com/vim/vim/commit/e101028a5c896480c61fef7ea16855255925709b --- src/nvim/eval/window.c | 22 ++++++++-------------- test/old/testdir/test_execute_func.vim | 24 ++++++++++++++++++++++++ 2 files changed, 32 insertions(+), 14 deletions(-) diff --git a/src/nvim/eval/window.c b/src/nvim/eval/window.c index e54f46dcc3..26624c8dd7 100644 --- a/src/nvim/eval/window.c +++ b/src/nvim/eval/window.c @@ -956,13 +956,8 @@ int switch_win_noblock(switchwin_T *switchwin, win_T *win, tabpage_T *tp, bool n if (tp != NULL) { switchwin->sw_curtab = curtab; if (no_display) { - curtab->tp_firstwin = firstwin; - curtab->tp_lastwin = lastwin; - curtab->tp_topframe = topframe; - curtab = tp; - firstwin = curtab->tp_firstwin; - lastwin = curtab->tp_lastwin; - topframe = curtab->tp_topframe; + unuse_tabpage(curtab); + use_tabpage(tp); } else { goto_tabpage_tp(tp, false, false); } @@ -989,13 +984,12 @@ void restore_win_noblock(switchwin_T *switchwin, bool no_display) { if (switchwin->sw_curtab != NULL && valid_tabpage(switchwin->sw_curtab)) { if (no_display) { - curtab->tp_firstwin = firstwin; - curtab->tp_lastwin = lastwin; - curtab->tp_topframe = topframe; - curtab = switchwin->sw_curtab; - firstwin = curtab->tp_firstwin; - lastwin = curtab->tp_lastwin; - topframe = curtab->tp_topframe; + win_T *const old_tp_curwin = curtab->tp_curwin; + + unuse_tabpage(curtab); + // Don't change the curwin of the tabpage we temporarily visited. + curtab->tp_curwin = old_tp_curwin; + use_tabpage(switchwin->sw_curtab); } else { goto_tabpage_tp(switchwin->sw_curtab, false, false); } diff --git a/test/old/testdir/test_execute_func.vim b/test/old/testdir/test_execute_func.vim index d9909e92a6..ec8ed160c3 100644 --- a/test/old/testdir/test_execute_func.vim +++ b/test/old/testdir/test_execute_func.vim @@ -212,4 +212,28 @@ func Test_execute_cmd_with_null() endif endfunc +func Test_win_execute_tabpagewinnr() + belowright split + tab split + belowright split + call assert_equal(2, tabpagewinnr(1)) + + tabprevious + wincmd p + call assert_equal(1, tabpagenr()) + call assert_equal(1, tabpagewinnr(1)) + call assert_equal(2, tabpagewinnr(2)) + + call win_execute(win_getid(1, 2), + \ 'call assert_equal(2, tabpagenr())' + \ .. '| call assert_equal(1, tabpagewinnr(1))' + \ .. '| call assert_equal(1, tabpagewinnr(2))') + + call assert_equal(1, tabpagenr()) + call assert_equal(1, tabpagewinnr(1)) + call assert_equal(2, tabpagewinnr(2)) + + %bwipe! +endfunc + " vim: shiftwidth=2 sts=2 expandtab From ca7b603d02ecd1ed4098f487cd01acd470ca6a74 Mon Sep 17 00:00:00 2001 From: Sean Dewar <6256228+seandewar@users.noreply.github.com> Date: Tue, 12 Mar 2024 21:08:50 +0000 Subject: [PATCH 2/4] vim-patch:9.1.0170: Re-allow curwin == prevwin, but document it instead Problem: more places exist where curwin == prevwin, and it may even be expected in some cases. Solution: revert v9.1.0001, but document that it's possible instead. (Sean Dewar) I've had a change of heart for the following reasons: - A quick 'n dirty [GitHub code search](https://github.com/search?q=%2F%28winnr%5C%28%5C%29%5Cs*%3D%3D%5Cs*winnr%5C%28%5B%27%22%5D%23%5B%27%22%5D%5C%29%7Cwinnr%5C%28%5B%27%22%5D%23%5B%27%22%5D%5C%29%5Cs*%3D%3D%5Cs*winnr%5C%28%5C%29%29%2F&type=code) reveals some cases where it's expected in the wild. Particularly, it made me aware `winnr() == winnr('#')` is possible when curwin is changed temporarily during the evaluation of a &statusline expression item (`%{...}`), and is used to show something different on the statusline belonging to the previous window; that behaviour wasn't changed in v9.1.0001, but it means curwin == prevwin makes sense in some cases. - The definition and call sites of back_to_prevwin imply some expectation that prevwin == wp (== curwin) is possible, as it's used to skip entering the prevwin in that case. - Prior to v9.1.0001, `:wincmd p` would not beep in the case that was patched in v9.1.0001, but now does. That resulted in vim/vim#14047 being opened, as it affected the CtrlP plugin. I find it odd that `:wincmd p` had cases where it wouldn't beep despite doing nothing, but it may be preferable to keep things that way (or instead also beep if curwin == prevwin, if that's preferred). - After more digging, I found cases in win_free_mem, enter_tabpage, aucmd_restbuf and qf_open_new_cwindow where curwin == prevwin is possible (many of them from autocommands). Others probably exist too, especially in places where curwin is changed temporarily. fixes: vim/vim#14047 closes: vim/vim#14186 https://github.com/vim/vim/commit/d64801e913314d2e19dbb38f60e6d285238debff --- runtime/doc/builtin.txt | 4 +- runtime/lua/vim/_meta/vimfn.lua | 4 +- src/nvim/eval.lua | 4 +- src/nvim/globals.h | 2 +- src/nvim/window.c | 4 -- test/old/testdir/test_window_cmd.vim | 58 ---------------------------- 6 files changed, 10 insertions(+), 66 deletions(-) diff --git a/runtime/doc/builtin.txt b/runtime/doc/builtin.txt index 4b1ccc0c5c..1a7efc3d79 100644 --- a/runtime/doc/builtin.txt +++ b/runtime/doc/builtin.txt @@ -9007,7 +9007,9 @@ winnr([{arg}]) *winnr()* # the number of the last accessed window (where |CTRL-W_p| goes to). If there is no previous window or it is in another tab page 0 is - returned. + returned. May refer to the current window in + some cases (e.g. when evaluating 'statusline' + expressions). {N}j the number of the Nth window below the current window (where |CTRL-W_j| goes to). {N}k the number of the Nth window above the current diff --git a/runtime/lua/vim/_meta/vimfn.lua b/runtime/lua/vim/_meta/vimfn.lua index ee68f669f8..a51f89227b 100644 --- a/runtime/lua/vim/_meta/vimfn.lua +++ b/runtime/lua/vim/_meta/vimfn.lua @@ -10724,7 +10724,9 @@ function vim.fn.winline() end --- # the number of the last accessed window (where --- |CTRL-W_p| goes to). If there is no previous --- window or it is in another tab page 0 is ---- returned. +--- returned. May refer to the current window in +--- some cases (e.g. when evaluating 'statusline' +--- expressions). --- {N}j the number of the Nth window below the --- current window (where |CTRL-W_j| goes to). --- {N}k the number of the Nth window above the current diff --git a/src/nvim/eval.lua b/src/nvim/eval.lua index febd022254..73715e2631 100644 --- a/src/nvim/eval.lua +++ b/src/nvim/eval.lua @@ -12856,7 +12856,9 @@ M.funcs = { # the number of the last accessed window (where |CTRL-W_p| goes to). If there is no previous window or it is in another tab page 0 is - returned. + returned. May refer to the current window in + some cases (e.g. when evaluating 'statusline' + expressions). {N}j the number of the Nth window below the current window (where |CTRL-W_j| goes to). {N}k the number of the Nth window above the current diff --git a/src/nvim/globals.h b/src/nvim/globals.h index aecb9d1116..06fb95b577 100644 --- a/src/nvim/globals.h +++ b/src/nvim/globals.h @@ -363,7 +363,7 @@ EXTERN bool sys_menu INIT( = false); // currently active window. EXTERN win_T *firstwin; // first window EXTERN win_T *lastwin; // last window -EXTERN win_T *prevwin INIT( = NULL); // previous window +EXTERN win_T *prevwin INIT( = NULL); // previous window (may equal curwin) #define ONE_WINDOW (firstwin == lastwin) #define FOR_ALL_FRAMES(frp, first_frame) \ for ((frp) = first_frame; (frp) != NULL; (frp) = (frp)->fr_next) diff --git a/src/nvim/window.c b/src/nvim/window.c index 521699f2f0..843cf154a7 100644 --- a/src/nvim/window.c +++ b/src/nvim/window.c @@ -4915,14 +4915,10 @@ static void win_enter_ext(win_T *const wp, const int flags) if (wp->w_buffer != curbuf) { buf_copy_options(wp->w_buffer, BCO_ENTER | BCO_NOHELP); } - if (!curwin_invalid) { prevwin = curwin; // remember for CTRL-W p curwin->w_redr_status = true; - } else if (wp == prevwin) { - prevwin = NULL; // don't want it to be the new curwin } - curwin = wp; curbuf = wp->w_buffer; diff --git a/test/old/testdir/test_window_cmd.vim b/test/old/testdir/test_window_cmd.vim index 8898d3a02d..9c34c15ff8 100644 --- a/test/old/testdir/test_window_cmd.vim +++ b/test/old/testdir/test_window_cmd.vim @@ -113,64 +113,6 @@ func Test_window_quit() bw Xa Xb endfunc -func Test_window_curwin_not_prevwin() - botright split - call assert_equal(2, winnr()) - call assert_equal(1, winnr('#')) - quit - call assert_equal(1, winnr()) - call assert_equal(0, winnr('#')) - - botright split - botright split - call assert_equal(3, winnr()) - call assert_equal(2, winnr('#')) - 1quit - call assert_equal(2, winnr()) - call assert_equal(1, winnr('#')) - - botright split - call assert_equal(1, tabpagenr()) - call assert_equal(3, winnr()) - call assert_equal(2, winnr('#')) - wincmd T - call assert_equal(2, tabpagenr()) - call assert_equal(1, winnr()) - call assert_equal(0, winnr('#')) - tabfirst - call assert_equal(1, tabpagenr()) - call assert_equal(2, winnr()) - call assert_equal(0, winnr('#')) - - tabonly - botright split - wincmd t - wincmd p - call assert_equal(3, winnr()) - call assert_equal(1, winnr('#')) - quit - call assert_equal(2, winnr()) - call assert_equal(1, winnr('#')) - - botright split - wincmd t - wincmd p - call assert_equal(1, tabpagenr()) - call assert_equal(3, winnr()) - call assert_equal(1, winnr('#')) - wincmd T - call assert_equal(2, tabpagenr()) - call assert_equal(1, winnr()) - call assert_equal(0, winnr('#')) - tabfirst - call assert_equal(1, tabpagenr()) - call assert_equal(2, winnr()) - call assert_equal(1, winnr('#')) - - tabonly - only -endfunc - func Test_window_horizontal_split() call assert_equal(1, winnr('$')) 3wincmd s From 6bbb02d9ba76551dd4856ad50a237e92c678702d Mon Sep 17 00:00:00 2001 From: Sean Dewar <6256228+seandewar@users.noreply.github.com> Date: Tue, 12 Mar 2024 21:19:18 +0000 Subject: [PATCH 3/4] 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 https://github.com/vim/vim/commit/5cac1a9bee0798d70a7fd80363a1f697759638e8 --- src/nvim/eval/window.c | 4 +-- src/nvim/window.c | 9 +++--- test/old/testdir/test_window_cmd.vim | 41 +++++++++++++++++----------- 3 files changed, 31 insertions(+), 23 deletions(-) diff --git a/src/nvim/eval/window.c b/src/nvim/eval/window.c index 26624c8dd7..3e2f6301ca 100644 --- a/src/nvim/eval/window.c +++ b/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; } diff --git a/src/nvim/window.c b/src/nvim/window.c index 843cf154a7..9468207d41 100644 --- a/src/nvim/window.c +++ b/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); diff --git a/test/old/testdir/test_window_cmd.vim b/test/old/testdir/test_window_cmd.vim index 9c34c15ff8..77de5edf8f 100644 --- a/test/old/testdir/test_window_cmd.vim +++ b/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 From c048beef6c034a46e324fcea7210082d48db32ee Mon Sep 17 00:00:00 2001 From: Sean Dewar <6256228+seandewar@users.noreply.github.com> Date: Tue, 12 Mar 2024 21:41:35 +0000 Subject: [PATCH 4/4] vim-patch:9a660d2883f9 runtime(doc): add reference to matchbufline() at :h search() related: vim/vim#14173 https://github.com/vim/vim/commit/9a660d2883f92b3a3761c964dc14363a8f70c8d8 Co-authored-by: Christian Brabandt --- runtime/doc/builtin.txt | 1 + runtime/lua/vim/_meta/vimfn.lua | 1 + src/nvim/eval.lua | 1 + 3 files changed, 3 insertions(+) diff --git a/runtime/doc/builtin.txt b/runtime/doc/builtin.txt index 1a7efc3d79..3df24a3b5f 100644 --- a/runtime/doc/builtin.txt +++ b/runtime/doc/builtin.txt @@ -6054,6 +6054,7 @@ search({pattern} [, {flags} [, {stopline} [, {timeout} [, {skip}]]]]) *search()* When a match has been found its line number is returned. If there is no match a 0 is returned and the cursor doesn't move. No error message is given. + To get the matched string, use |matchbufline()|. {flags} is a String, which can contain these character flags: 'b' search Backward instead of forward diff --git a/runtime/lua/vim/_meta/vimfn.lua b/runtime/lua/vim/_meta/vimfn.lua index a51f89227b..2b93ea7d4e 100644 --- a/runtime/lua/vim/_meta/vimfn.lua +++ b/runtime/lua/vim/_meta/vimfn.lua @@ -7259,6 +7259,7 @@ function vim.fn.screenstring(row, col) end --- When a match has been found its line number is returned. --- If there is no match a 0 is returned and the cursor doesn't --- move. No error message is given. +--- To get the matched string, use |matchbufline()|. --- --- {flags} is a String, which can contain these character flags: --- 'b' search Backward instead of forward diff --git a/src/nvim/eval.lua b/src/nvim/eval.lua index 73715e2631..96dc32259f 100644 --- a/src/nvim/eval.lua +++ b/src/nvim/eval.lua @@ -8746,6 +8746,7 @@ M.funcs = { When a match has been found its line number is returned. If there is no match a 0 is returned and the cursor doesn't move. No error message is given. + To get the matched string, use |matchbufline()|. {flags} is a String, which can contain these character flags: 'b' search Backward instead of forward