Mercurial > vim
changeset 34485:157cf882799f v9.1.0150
patch 9.1.0150: Several minor 'winfixbuf' issues
Commit: https://github.com/vim/vim/commit/4bb505e28cac0389561fff78d8bbe0319c2bcf2f
Author: Sean Dewar <6256228+seandewar@users.noreply.github.com>
Date: Tue Mar 5 20:39:07 2024 +0100
patch 9.1.0150: Several minor 'winfixbuf' issues
Problem: several minor 'winfixbuf' issues exist, mostly relating to the
quickfix list
Solution: address them and adjust tests. Retab and reflow a few things too.
(Sean Dewar)
Things touched include:
- Replace the semsgs with gettext'd emsgs.
- Handle window switching in ex_listdo properly, so curbuf and curwin
are kept in-sync and trigger autocommands; handle those properly.
- Don't change the list entry index in qf_jump_edit_buffer if we fail
due to 'wfb' (achieved by returning FAIL; QF_ABORT should only be used
if the list was changed).
- Make qf_jump_edit_buffer actually switch to prevwin when using `:cXX`
commands **outside** of the list window if 'wfb' is set in curwin.
Handle autocommands properly in case they mess with the list.
NOTE: previously, it seemed to split if 'wfb' was set, but do nothing
and fail if prevwin is *valid*. This behaviour seemed strange, and maybe
unintentional? Now it aligns more with what's described for the `:cXX`
commands in the original PR description when used outside a list window,
I think.
- In both functions, only consider prevwin if 'wfb' isn't set for it;
fallback to splitting otherwise.
- Use win_split to split. Not sure if there was a specific reason for
using ex_splitview. win_split is simpler and respects modifiers like
:vertical that may have been used. Plus, its return value can be checked
for setting opened_window in qf code (technically win_split_ins autocmds
could immediately close it or change windows, in which the qf code might
close some other window on failure; it's already the case elsewhere,
though).
closes: #14142
Signed-off-by: Sean Dewar <6256228+seandewar@users.noreply.github.com>
Signed-off-by: Christian Brabandt <cb@256bit.org>
author | Christian Brabandt <cb@256bit.org> |
---|---|
date | Tue, 05 Mar 2024 20:45:04 +0100 |
parents | 2425b78ab0d7 |
children | 0290a9a4c0e7 |
files | runtime/doc/message.txt src/ex_cmds2.c src/quickfix.c src/testdir/test_winfixbuf.vim src/version.c src/window.c |
diffstat | 6 files changed, 181 insertions(+), 51 deletions(-) [+] |
line wrap: on
line diff
--- a/runtime/doc/message.txt +++ b/runtime/doc/message.txt @@ -1,4 +1,4 @@ -*message.txt* For Vim version 9.1. Last change: 2024 Mar 03 +*message.txt* For Vim version 9.1. Last change: 2024 Mar 05 VIM REFERENCE MANUAL by Bram Moolenaar @@ -122,7 +122,7 @@ wiped out a buffer which contains a mark You cannot have two buffers with exactly the same name. This includes the path leading to the file. - *E1513* > + *E1513* Cannot edit buffer. 'winfixbuf' is enabled ~ If a window has 'winfixbuf' enabled, you cannot change that window's current
--- a/src/ex_cmds2.c +++ b/src/ex_cmds2.c @@ -460,26 +460,33 @@ ex_listdo(exarg_T *eap) if (curwin->w_p_wfb) { - if ((eap->cmdidx == CMD_ldo || eap->cmdidx == CMD_lfdo) && !eap->forceit) - { - // Disallow :ldo if 'winfixbuf' is applied - semsg("%s", e_winfixbuf_cannot_go_to_buffer); - return; - } + if ((eap->cmdidx == CMD_ldo || eap->cmdidx == CMD_lfdo) && + !eap->forceit) + { + // Disallow :ldo if 'winfixbuf' is applied + emsg(_(e_winfixbuf_cannot_go_to_buffer)); + return; + } - if (win_valid(prevwin)) - // Change the current window to another because 'winfixbuf' is enabled - curwin = prevwin; - else - { - // Split the window, which will be 'nowinfixbuf', and set curwin to that - exarg_T new_eap; - CLEAR_FIELD(new_eap); - new_eap.cmdidx = CMD_split; - new_eap.cmd = (char_u *)"split"; - new_eap.arg = (char_u *)""; - ex_splitview(&new_eap); - } + if (win_valid(prevwin) && !prevwin->w_p_wfb) + { + // 'winfixbuf' is set; attempt to change to a window without it. + win_goto(prevwin); + } + if (curwin->w_p_wfb) + { + // Split the window, which will be 'nowinfixbuf', and set curwin to + // that + win_split(0, 0); + + if (curwin->w_p_wfb) + { + // Autocommands set 'winfixbuf' or sent us to another window + // with it set. Give up. + emsg(_(e_winfixbuf_cannot_go_to_buffer)); + return; + } + } } #if defined(FEAT_SYN_HL)
--- a/src/quickfix.c +++ b/src/quickfix.c @@ -3146,7 +3146,8 @@ qf_goto_win_with_qfl_file(int qf_fnum) // Didn't find it, go to the window before the quickfix // window, unless 'switchbuf' contains 'uselast': in this case we // try to jump to the previously used window first. - if ((swb_flags & SWB_USELAST) && win_valid(prevwin) && !prevwin->w_p_wfb) + if ((swb_flags & SWB_USELAST) && win_valid(prevwin) && + !prevwin->w_p_wfb) win = prevwin; else if (altwin != NULL) win = altwin; @@ -3158,7 +3159,8 @@ qf_goto_win_with_qfl_file(int qf_fnum) } // Remember a usable window. - if (altwin == NULL && !win->w_p_pvw && !win->w_p_wfb && bt_normal(win->w_buffer)) + if (altwin == NULL && !win->w_p_pvw && !win->w_p_wfb && + bt_normal(win->w_buffer)) altwin = win; } @@ -3262,30 +3264,48 @@ qf_jump_edit_buffer( } else { - if (!forceit && curwin->w_p_wfb) + int fnum = qf_ptr->qf_fnum; + + if (!forceit && curwin->w_p_wfb && curbuf->b_fnum != fnum) { if (qi->qfl_type == QFLT_LOCATION) { - // Location lists cannot split or reassign their window - // so 'winfixbuf' windows must fail - semsg("%s", e_winfixbuf_cannot_go_to_buffer); - return QF_ABORT; + // Location lists cannot split or reassign their window + // so 'winfixbuf' windows must fail + emsg(_(e_winfixbuf_cannot_go_to_buffer)); + return FAIL; } - if (!win_valid(prevwin)) + if (win_valid(prevwin) && !prevwin->w_p_wfb && + !bt_quickfix(prevwin->w_buffer)) { - // Split the window, which will be 'nowinfixbuf', and set curwin to that - exarg_T new_eap; - CLEAR_FIELD(new_eap); - new_eap.cmdidx = CMD_split; - new_eap.cmd = (char_u *)"split"; - new_eap.arg = (char_u *)""; - ex_splitview(&new_eap); + // 'winfixbuf' is set; attempt to change to a window without it + // that isn't a quickfix/location list window. + win_goto(prevwin); + } + if (curwin->w_p_wfb) + { + // Split the window, which will be 'nowinfixbuf', and set curwin + // to that + if (win_split(0, 0) == OK) + *opened_window = TRUE; + + if (curwin->w_p_wfb) + { + // Autocommands set 'winfixbuf' or sent us to another window + // with it set. Give up, but don't return immediately, as + // they may have messed with the list. + emsg(_(e_winfixbuf_cannot_go_to_buffer)); + retval = FAIL; + } } } - retval = buflist_getfile(qf_ptr->qf_fnum, - (linenr_T)1, GETF_SETMARK | GETF_SWITCH, forceit); + if (retval == OK) + { + retval = buflist_getfile(fnum, + (linenr_T)1, GETF_SETMARK | GETF_SWITCH, forceit); + } } // If a location list, check whether the associated window is still
--- a/src/testdir/test_winfixbuf.vim +++ b/src/testdir/test_winfixbuf.vim @@ -1254,14 +1254,27 @@ func Test_lNext() call s:reset_all_buffers() let [l:first, l:middle, _] = s:make_simple_location_list() + call assert_equal(1, getloclist(0, #{idx: 0}).idx) + lnext! + call assert_equal(2, getloclist(0, #{idx: 0}).idx) + call assert_equal(l:middle, bufnr()) call assert_fails("lNext", "E1513:") + " Ensure the entry didn't change. + call assert_equal(2, getloclist(0, #{idx: 0}).idx) call assert_equal(l:middle, bufnr()) - lnext! " Reset for the next test + lnext! + call assert_equal(3, getloclist(0, #{idx: 0}).idx) + call assert_equal(l:middle, bufnr()) lNext! + call assert_equal(2, getloclist(0, #{idx: 0}).idx) + call assert_equal(l:middle, bufnr()) + + lNext! + call assert_equal(1, getloclist(0, #{idx: 0}).idx) call assert_equal(l:first, bufnr()) endfunc @@ -1271,14 +1284,23 @@ func Test_lNfile() call s:reset_all_buffers() let [l:first, l:current, _] = s:make_simple_location_list() + call assert_equal(1, getloclist(0, #{idx: 0}).idx) + lnext! + call assert_equal(2, getloclist(0, #{idx: 0}).idx) + call assert_equal(l:current, bufnr()) call assert_fails("lNfile", "E1513:") + " Ensure the entry didn't change. + call assert_equal(2, getloclist(0, #{idx: 0}).idx) call assert_equal(l:current, bufnr()) - lnext! " Reset for the next test + lnext! + call assert_equal(3, getloclist(0, #{idx: 0}).idx) + call assert_equal(l:current, bufnr()) lNfile! + call assert_equal(1, getloclist(0, #{idx: 0}).idx) call assert_equal(l:first, bufnr()) endfunc @@ -1485,14 +1507,18 @@ func Test_lnfile() call s:reset_all_buffers() let [_, l:current, l:last] = s:make_simple_location_list() + call assert_equal(1, getloclist(0, #{idx: 0}).idx) + lnext! + call assert_equal(2, getloclist(0, #{idx: 0}).idx) + call assert_equal(l:current, bufnr()) call assert_fails("lnfile", "E1513:") + call assert_equal(2, getloclist(0, #{idx: 0}).idx) call assert_equal(l:current, bufnr()) - lprevious! " Reset for the next test call - lnfile! + call assert_equal(4, getloclist(0, #{idx: 0}).idx) call assert_equal(l:last, bufnr()) endfunc @@ -1519,14 +1545,19 @@ func Test_lprevious() call s:reset_all_buffers() let [l:first, l:middle, _] = s:make_simple_location_list() + call assert_equal(1, getloclist(0, #{idx: 0}).idx) + lnext! + call assert_equal(2, getloclist(0, #{idx: 0}).idx) + call assert_equal(l:middle, bufnr()) call assert_fails("lprevious", "E1513:") + " Ensure the entry didn't change. + call assert_equal(2, getloclist(0, #{idx: 0}).idx) call assert_equal(l:middle, bufnr()) - lnext! " Reset for the next test call - lprevious! + call assert_equal(1, getloclist(0, #{idx: 0}).idx) call assert_equal(l:first, bufnr()) endfunc @@ -3134,17 +3165,87 @@ endfunc func Test_quickfix_switchbuf_invalid_prevwin() call s:reset_all_buffers() - let [l:first, _] = s:make_simple_quickfix() - call assert_notequal(l:first, bufnr()) - call assert_equal(1, winnr('$')) + call s:make_simple_quickfix() + call assert_equal(1, getqflist(#{idx: 0}).idx) set switchbuf=uselast split copen execute winnr('#') 'quit' - - call assert_fails('cfirst', 'E1513:') + call assert_equal(2, winnr('$')) + + cnext " Would've triggered a null pointer member access + call assert_equal(2, getqflist(#{idx: 0}).idx) + set switchbuf& endfunc +func Test_listdo_goto_prevwin() + call s:reset_all_buffers() + call s:make_buffers_list() + + new + call assert_equal(0, &winfixbuf) + wincmd p + call assert_equal(1, &winfixbuf) + call assert_notequal(bufnr(), bufnr('#')) + + augroup ListDoGotoPrevwin + au! + au BufLeave * let s:triggered = 1 + \| call assert_equal(bufnr(), winbufnr(winnr())) + augroup END + " Should correctly switch to the window without 'winfixbuf', and curbuf should + " be consistent with curwin->w_buffer for autocommands. + bufdo " + call assert_equal(0, &winfixbuf) + call assert_equal(1, s:triggered) + unlet! s:triggered + au! ListDoGotoPrevwin + + set winfixbuf + wincmd p + call assert_equal(2, winnr('$')) + " Both curwin and prevwin have 'winfixbuf' set, so should split a new window + " without it set. + bufdo " + call assert_equal(0, &winfixbuf) + call assert_equal(3, winnr('$')) + + quit + call assert_equal(2, winnr('$')) + call assert_equal(1, &winfixbuf) + augroup ListDoGotoPrevwin + au! + au WinEnter * ++once set winfixbuf + augroup END + " Same as before, but naughty autocommands set 'winfixbuf' for the new window. + " :bufdo should give up in this case. + call assert_fails('bufdo "', 'E1513:') + + au! ListDoGotoPrevwin + augroup! ListDoGotoPrevwin +endfunc + +func Test_quickfix_changed_split_failed() + call s:reset_all_buffers() + + call s:make_simple_quickfix() + call assert_equal(1, winnr('$')) + + " Quickfix code will open a split in an attempt to get a 'nowinfixbuf' window + " to switch buffers in. Interfere with things by setting 'winfixbuf' in it. + augroup QfChanged + au! + au WinEnter * ++once call assert_equal(2, winnr('$')) + \| set winfixbuf | call setqflist([], 'f') + augroup END + call assert_fails('cnext', ['E1513:', 'E925:']) + " Check that the split was automatically closed. + call assert_equal(1, winnr('$')) + + au! QfChanged + augroup! QfChanged +endfunc + " vim: shiftwidth=2 sts=2 expandtab
--- 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 */ /**/ + 150, +/**/ 149, /**/ 148,
--- a/src/window.c +++ b/src/window.c @@ -167,7 +167,7 @@ check_can_set_curbuf_disabled(void) { if (curwin->w_p_wfb) { - semsg("%s", e_winfixbuf_cannot_go_to_buffer); + emsg(_(e_winfixbuf_cannot_go_to_buffer)); return FALSE; } return TRUE; @@ -183,7 +183,7 @@ check_can_set_curbuf_forceit(int forceit { if (!forceit && curwin->w_p_wfb) { - semsg("%s", e_winfixbuf_cannot_go_to_buffer); + emsg(_(e_winfixbuf_cannot_go_to_buffer)); return FALSE; } return TRUE;