mirror of
https://github.com/neovim/neovim.git
synced 2026-03-28 03:12:00 +00:00
fix(api): merge split window config only on success
Problem: nvim_win_set_config may merge configs despite failing to configure a split, and without applying necessary side-effects (like setting style=minimal options). Plus, autocommands may apply a different config after the merge, causing side-effects to apply for an outdated config. Solution: merge configs last, only on success. Include fields only relevant to splits. Properly set _cmdline_offset for splits. Maybe better to disallow _cmdline_offset for splits instead, as the pum is relative to cmdline_row anyway? (I didn't want to change behaviour too much) Also use expect_unchanged in an unrelated test to quash a warning.
This commit is contained in:
@@ -431,7 +431,6 @@ static bool win_config_split(win_T *win, Dict(win_config) *config, WinConfig *fc
|
||||
fconfig->split = (old_split == kWinSplitBelow || p_sb) ? kWinSplitBelow : kWinSplitAbove;
|
||||
}
|
||||
}
|
||||
merge_win_config(&win->w_config, *fconfig);
|
||||
|
||||
// If there's no "vertical" or "split" set, or if "split" is unchanged, then we can just change
|
||||
// the size of the window.
|
||||
@@ -605,6 +604,14 @@ resize:
|
||||
if (HAS_KEY_X(config, height)) {
|
||||
win_setheight_win(fconfig->height, win);
|
||||
}
|
||||
|
||||
// Merge configs now. If previously a float, clear fields irrelevant to splits that `fconfig` may
|
||||
// have shallowly copied; don't free them as win_split_ins handled that. If already a split,
|
||||
// clearing isn't needed, as parse_win_config shouldn't allow setting irrelevant fields.
|
||||
if (!was_split) {
|
||||
clear_float_config(fconfig, false);
|
||||
}
|
||||
merge_win_config(&win->w_config, *fconfig);
|
||||
return true;
|
||||
#undef HAS_KEY_X
|
||||
}
|
||||
@@ -662,11 +669,9 @@ void nvim_win_set_config(Window window, Dict(win_config) *config, Error *err)
|
||||
win_config_float(win, fconfig);
|
||||
}
|
||||
|
||||
if (HAS_KEY_X(config, style)) {
|
||||
if (fconfig.style == kWinStyleMinimal) {
|
||||
win_set_minimal_style(win);
|
||||
didset_window_options(win, true);
|
||||
}
|
||||
if (HAS_KEY_X(config, style) && fconfig.style == kWinStyleMinimal) {
|
||||
win_set_minimal_style(win);
|
||||
didset_window_options(win, true);
|
||||
}
|
||||
if (fconfig._cmdline_offset < INT_MAX) {
|
||||
cmdline_win = win;
|
||||
|
||||
@@ -864,6 +864,22 @@ void merge_win_config(WinConfig *dst, const WinConfig src)
|
||||
*dst = src;
|
||||
}
|
||||
|
||||
/// Clear fields in `fconfig` that are only used for floating windows.
|
||||
/// Also clears fields unused after configure time, like width/height.
|
||||
void clear_float_config(WinConfig *fconfig, bool free_fields)
|
||||
FUNC_ATTR_NONNULL_ALL
|
||||
{
|
||||
WinStyle saved_style = fconfig->style;
|
||||
int saved_cmdline_offset = fconfig->_cmdline_offset;
|
||||
if (free_fields) {
|
||||
merge_win_config(fconfig, WIN_CONFIG_INIT);
|
||||
} else {
|
||||
*fconfig = WIN_CONFIG_INIT;
|
||||
}
|
||||
fconfig->style = saved_style;
|
||||
fconfig->_cmdline_offset = saved_cmdline_offset;
|
||||
}
|
||||
|
||||
void ui_ext_win_position(win_T *wp, bool validate)
|
||||
{
|
||||
wp->w_pos_changed = false;
|
||||
@@ -1364,7 +1380,6 @@ win_T *win_split_ins(int size, int flags, win_T *new_wp, int dir, frame_T *to_fl
|
||||
// make the contents of the new window the same as the current one
|
||||
win_init(wp, curwin, flags);
|
||||
} else if (wp->w_floating) {
|
||||
WinStyle saved_style = wp->w_config.style;
|
||||
ui_comp_remove_grid(&wp->w_grid_alloc);
|
||||
if (ui_has(kUIMultigrid)) {
|
||||
wp->w_pos_changed = true;
|
||||
@@ -1387,10 +1402,8 @@ win_T *win_split_ins(int size, int flags, win_T *new_wp, int dir, frame_T *to_fl
|
||||
new_frame(wp);
|
||||
|
||||
// non-floating window doesn't store float config or have a border.
|
||||
merge_win_config(&wp->w_config, WIN_CONFIG_INIT);
|
||||
clear_float_config(&wp->w_config, true);
|
||||
CLEAR_FIELD(wp->w_border_adj);
|
||||
// Restore WinConfig style. #37067
|
||||
wp->w_config.style = saved_style;
|
||||
}
|
||||
|
||||
// Going to reorganize frames now, make sure they're flat.
|
||||
|
||||
@@ -3553,5 +3553,41 @@ describe('API/win', function()
|
||||
eq('', api.nvim_get_option_value('colorcolumn', { win = win }))
|
||||
eq('', api.nvim_get_option_value('statuscolumn', { win = win }))
|
||||
end)
|
||||
|
||||
it('merges configs only after successfully configuring split', function()
|
||||
local win = api.nvim_open_win(0, true, {
|
||||
relative = 'editor',
|
||||
width = 10,
|
||||
height = 10,
|
||||
row = 5,
|
||||
col = 5,
|
||||
})
|
||||
local cfg = api.nvim_win_get_config(win)
|
||||
eq('', cfg.style)
|
||||
command('set cursorline | tabnew')
|
||||
local tp2_win = api.nvim_get_current_win()
|
||||
command('tabfirst | autocmd WinEnter * ++once wincmd p')
|
||||
eq(
|
||||
'Failed to switch away from window 1001',
|
||||
pcall_err(
|
||||
api.nvim_win_set_config,
|
||||
win,
|
||||
{ split = 'below', win = tp2_win, style = 'minimal' }
|
||||
)
|
||||
)
|
||||
eq(cfg, api.nvim_win_get_config(win))
|
||||
eq(true, api.nvim_get_option_value('cursorline', { win = win }))
|
||||
|
||||
exec([[
|
||||
autocmd WinLeave * ++once let g:style_before = nvim_win_get_config(0).style
|
||||
\| let g:cul_before = &cursorline
|
||||
\| call nvim_win_set_config(0, #{style: ""})
|
||||
]])
|
||||
api.nvim_win_set_config(win, { split = 'below', win = tp2_win, style = 'minimal' })
|
||||
eq('', eval('g:style_before'))
|
||||
eq(1, eval('g:cul_before'))
|
||||
eq('minimal', api.nvim_win_get_config(win).style)
|
||||
eq(false, api.nvim_get_option_value('cursorline', { win = win }))
|
||||
end)
|
||||
end)
|
||||
end)
|
||||
|
||||
@@ -390,6 +390,12 @@ describe('vim.ui_attach', function()
|
||||
9 bufname( {12: } |
|
||||
Excommand:call bufadd^( |
|
||||
]])
|
||||
-- _cmdline_offset remains set after being turned into a split.
|
||||
exec_lua(function()
|
||||
vim.fn.win_execute(_G.win, 'wincmd J')
|
||||
end)
|
||||
feed('<Tab>') -- Was a signed int overflow; offset was INT_MAX despite cmdline_win being set.
|
||||
eq(9, exec_lua('return vim.api.nvim_win_get_config(_G.win)._cmdline_offset'))
|
||||
-- No crash after _cmdline_offset window is closed #35584.
|
||||
exec_lua(function()
|
||||
vim.ui_detach(_G.ns)
|
||||
|
||||
@@ -8942,17 +8942,7 @@ describe('float window', function()
|
||||
unchanged = true,
|
||||
}
|
||||
else
|
||||
screen:expect([[
|
||||
Ut enim ad minim veniam, quis nostrud |
|
||||
exercitation ullamco laboris nisi ut aliquip ex |
|
||||
ea co{2:test}{3:o consequat}. Duis aute irure dolor in |
|
||||
repre{3:henderit in vol}uptate velit esse cillum |
|
||||
dolor{2:popup}{3:fugi}{2:text}{3:ul}la pariatur. Excepteur sint |
|
||||
occaecat cupidatat non proident, sunt in culpa |
|
||||
qui officia deserunt mollit anim id est |
|
||||
laborum^. |
|
||||
|
|
||||
]])
|
||||
screen:expect_unchanged()
|
||||
end
|
||||
api.nvim_buf_set_lines(buf, 0, -1, true, test_data)
|
||||
|
||||
|
||||
Reference in New Issue
Block a user