mirror of
https://github.com/neovim/neovim.git
synced 2025-10-14 13:56:05 +00:00
fix(api): do not allow opening float to closing buffer
Problem: no check for nvim_open_win opening a new floating window into a closing buffer, which can lead to crashes. Solution: call check_split_disallowed; opening a new float semantically splits from a window, so the same problems as regular splits apply. Also restore the error if switch_win_noblock in win_set_buf fails (may not be possible to hit this, but win_set_buf can silently succeed there since #31595). As the lock check applies to curbuf (not the target buffer) this may feel a bit restrictive, though this isn't different to how things like ":split" or nvim_open_win with "split = true" works when targeting a different buffer. Only checking the target buffer's lock will cause issues if win_set_buf doesn't end up in the target buffer for whatever reason. Maybe we could consider checking the lock of the buffer after win_set_buf and close the window if it's locked (maybe a bit fiddly, especially as closing a window can fail...), or make the open + switch operation more atomic, like how Vim does for its popup windows..? It also used to be the case that win_set_buf would set an error if autocommands sent us to a different buffer than what was requested, but #31595 appears to have also changed that... I haven't touched that here.
This commit is contained in:
@@ -281,6 +281,9 @@ Window nvim_open_win(Buffer buffer, Boolean enter, Dict(win_config) *config, Err
|
|||||||
wp->w_config = fconfig;
|
wp->w_config = fconfig;
|
||||||
}
|
}
|
||||||
} else {
|
} else {
|
||||||
|
if (!check_split_disallowed_err(curwin, err)) {
|
||||||
|
goto cleanup; // error already set
|
||||||
|
}
|
||||||
wp = win_new_float(NULL, false, fconfig, err);
|
wp = win_new_float(NULL, false, fconfig, err);
|
||||||
}
|
}
|
||||||
if (!wp) {
|
if (!wp) {
|
||||||
|
@@ -753,15 +753,17 @@ static void cmd_with_count(char *cmd, char *bufp, size_t bufsize, int64_t Prenum
|
|||||||
void win_set_buf(win_T *win, buf_T *buf, Error *err)
|
void win_set_buf(win_T *win, buf_T *buf, Error *err)
|
||||||
FUNC_ATTR_NONNULL_ALL
|
FUNC_ATTR_NONNULL_ALL
|
||||||
{
|
{
|
||||||
|
const handle_T win_handle = win->handle;
|
||||||
tabpage_T *tab = win_find_tabpage(win);
|
tabpage_T *tab = win_find_tabpage(win);
|
||||||
|
|
||||||
// no redrawing and don't set the window title
|
// no redrawing and don't set the window title
|
||||||
RedrawingDisabled++;
|
RedrawingDisabled++;
|
||||||
|
|
||||||
switchwin_T switchwin;
|
switchwin_T switchwin;
|
||||||
|
int win_result;
|
||||||
|
|
||||||
TRY_WRAP(err, {
|
TRY_WRAP(err, {
|
||||||
int win_result = switch_win_noblock(&switchwin, win, tab, true);
|
win_result = switch_win_noblock(&switchwin, win, tab, true);
|
||||||
if (win_result != FAIL) {
|
if (win_result != FAIL) {
|
||||||
const int save_acd = p_acd;
|
const int save_acd = p_acd;
|
||||||
if (!switchwin.sw_same_win) {
|
if (!switchwin.sw_same_win) {
|
||||||
@@ -776,6 +778,9 @@ void win_set_buf(win_T *win, buf_T *buf, Error *err)
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
});
|
});
|
||||||
|
if (win_result == FAIL && !ERROR_SET(err)) {
|
||||||
|
api_set_error(err, kErrorTypeException, "Failed to switch to window %d", win_handle);
|
||||||
|
}
|
||||||
|
|
||||||
// If window is not current, state logic will not validate its cursor. So do it now.
|
// If window is not current, state logic will not validate its cursor. So do it now.
|
||||||
// Still needed if do_buffer returns FAIL (e.g: autocmds abort script after buffer was set).
|
// Still needed if do_buffer returns FAIL (e.g: autocmds abort script after buffer was set).
|
||||||
|
@@ -2026,6 +2026,38 @@ describe('API/win', function()
|
|||||||
.. '})'
|
.. '})'
|
||||||
)
|
)
|
||||||
command('new | quit')
|
command('new | quit')
|
||||||
|
|
||||||
|
-- Apply to opening floats too, as that can similarly create new views into a closing buffer.
|
||||||
|
-- For example, the following would open a float into an unloaded buffer:
|
||||||
|
exec([[
|
||||||
|
only
|
||||||
|
new
|
||||||
|
let g:buf = bufnr()
|
||||||
|
autocmd BufUnload * ++once let g:win = nvim_open_win(g:buf, 0, #{relative: "editor", width: 5, height: 5, row: 1, col: 1})
|
||||||
|
setlocal bufhidden=unload
|
||||||
|
]])
|
||||||
|
matches('E1159: Cannot split a window when closing the buffer$', pcall_err(command, 'quit'))
|
||||||
|
eq(false, eval('nvim_buf_is_loaded(g:buf)'))
|
||||||
|
eq(0, eval('win_findbuf(g:buf)->len()'))
|
||||||
|
|
||||||
|
-- Only checking b_locked_split for the target buffer is insufficient, as naughty autocommands
|
||||||
|
-- can cause win_set_buf to remain in a closing curbuf:
|
||||||
|
exec([[
|
||||||
|
only
|
||||||
|
new
|
||||||
|
let g:buf = bufnr()
|
||||||
|
autocmd BufWipeout * ++once ++nested let g:buf2 = nvim_create_buf(1, 0)
|
||||||
|
\| execute 'autocmd BufLeave * ++once call nvim_buf_delete(g:buf2, #{force: 1})'
|
||||||
|
\| setlocal bufhidden=
|
||||||
|
\| call nvim_open_win(g:buf2, 1, #{relative: 'editor', width: 5, height: 5, col: 5, row: 5})
|
||||||
|
setlocal bufhidden=wipe
|
||||||
|
]])
|
||||||
|
matches('E1159: Cannot split a window when closing the buffer$', pcall_err(command, 'quit'))
|
||||||
|
eq(false, eval('nvim_buf_is_loaded(g:buf)'))
|
||||||
|
eq(0, eval('win_findbuf(g:buf)->len()'))
|
||||||
|
-- BufLeave shouldn't run here (buf2 isn't deleted and remains hidden)
|
||||||
|
eq(true, eval('nvim_buf_is_loaded(g:buf2)'))
|
||||||
|
eq(0, eval('win_findbuf(g:buf2)->len()'))
|
||||||
end)
|
end)
|
||||||
|
|
||||||
it('restores last known cursor position if BufWinEnter did not move it', function()
|
it('restores last known cursor position if BufWinEnter did not move it', function()
|
||||||
|
Reference in New Issue
Block a user