fix(api): win_set_config fires unnecessary autocmds

Problem: win_set_config should have the observable effect of moving an existing
window to another place, but instead fires autocommands as if a new window was
created and entered (and does not fire autocommands reflecting a "return" to the
original window).

Solution: do not fire win_enter-related autocommands when splitting the window,
but continue to fire them when entering the window that fills the new space when
moving a window to a different tabpage, as the new curwin changes.

Also, remove "++once" from the WinEnter autocmd in the other test, as omitting
it also crashed Nvim before this fix.
This commit is contained in:
Sean Dewar 2024-02-04 01:50:49 +00:00 committed by Sean Dewar
parent bcb70eeac4
commit 233649bc75
No known key found for this signature in database
GPG Key ID: 08CC2C83AD41B581
2 changed files with 41 additions and 21 deletions

View File

@ -505,7 +505,7 @@ void nvim_win_set_config(Window window, Dict(win_config) *config, Error *err)
win->w_pos_changed = true; win->w_pos_changed = true;
} }
int flags = win_split_flags(fconfig.split, parent == NULL); int flags = win_split_flags(fconfig.split, parent == NULL) | WSP_NOENTER;
if (parent == NULL) { if (parent == NULL) {
if (!win_split_ins(0, flags, win, 0)) { if (!win_split_ins(0, flags, win, 0)) {
@ -514,24 +514,13 @@ void nvim_win_set_config(Window window, Dict(win_config) *config, Error *err)
return; return;
} }
} else { } else {
win_execute_T args; switchwin_T switchwin;
// `parent` is valid in its tabpage, so switch_win should not fail.
tabpage_T *tp = win_find_tabpage(parent); const int result = switch_win(&switchwin, parent, win_find_tabpage(parent), true);
if (!win_execute_before(&args, parent, tp)) { (void)result;
// TODO(willothy): how should we handle this / what should the message be? assert(result == OK);
api_set_error(err, kErrorTypeException, "Failed to switch to tabpage %d", tp->handle); win_split_ins(0, flags, win, 0);
win_execute_after(&args); restore_win(&switchwin, true);
return;
}
// This should return the same ptr to `win`, but we check for
// NULL to detect errors.
win_T *res = win_split_ins(0, flags, win, 0);
win_execute_after(&args);
if (!res) {
// TODO(willothy): What should this error message say?
api_set_error(err, kErrorTypeException, "Failed to split window");
return;
}
} }
if (HAS_KEY_X(config, width)) { if (HAS_KEY_X(config, width)) {
win_setwidth_win(fconfig.width, win); win_setwidth_win(fconfig.width, win);
@ -539,7 +528,6 @@ void nvim_win_set_config(Window window, Dict(win_config) *config, Error *err)
if (HAS_KEY_X(config, height)) { if (HAS_KEY_X(config, height)) {
win_setheight_win(fconfig.height, win); win_setheight_win(fconfig.height, win);
} }
redraw_later(win, UPD_NOT_VALID);
return; return;
} else { } else {
win_config_float(win, fconfig); win_config_float(win, fconfig);

View File

@ -1668,7 +1668,7 @@ describe('API/win', function()
command('split | tabnew') command('split | tabnew')
local w = api.nvim_get_current_win() local w = api.nvim_get_current_win()
local t = api.nvim_get_current_tabpage() local t = api.nvim_get_current_tabpage()
command('tabfirst | autocmd WinEnter * ++once quit') command('tabfirst | autocmd WinEnter * quit')
api.nvim_win_set_config(0, { win = w, split = 'left' }) api.nvim_win_set_config(0, { win = w, split = 'left' })
-- New tabpage is now the only one, as WinEnter closed the new curwin in the original. -- New tabpage is now the only one, as WinEnter closed the new curwin in the original.
eq(t, api.nvim_get_current_tabpage()) eq(t, api.nvim_get_current_tabpage())
@ -1684,6 +1684,38 @@ describe('API/win', function()
pcall_err(api.nvim_win_set_config, 0, { win = w, split = 'left' }) pcall_err(api.nvim_win_set_config, 0, { win = w, split = 'left' })
) )
end) end)
it('expected autocmds when moving window to other tabpage', function()
local new_curwin = api.nvim_get_current_win()
command('split')
local win = api.nvim_get_current_win()
command('tabnew')
local parent = api.nvim_get_current_win()
exec([[
tabfirst
let result = []
autocmd WinEnter * let result += ["Enter", win_getid()]
autocmd WinLeave * let result += ["Leave", win_getid()]
autocmd WinNew * let result += ["New", win_getid()]
]])
api.nvim_win_set_config(0, { win = parent, split = 'left' })
-- Shouldn't see WinNew, as we're not creating any new windows, just moving existing ones.
eq({ 'Leave', win, 'Enter', new_curwin }, eval('result'))
end)
it('no autocmds when moving window within same tabpage', function()
local parent = api.nvim_get_current_win()
exec([[
split
let result = []
autocmd WinEnter * let result += ["Enter", win_getid()]
autocmd WinLeave * let result += ["Leave", win_getid()]
autocmd WinNew * let result += ["New", win_getid()]
]])
api.nvim_win_set_config(0, { win = parent, split = 'left' })
-- Shouldn't see any of those events, as we remain in the same window.
eq({}, eval('result'))
end)
end) end)
describe('get_config', function() describe('get_config', function()