mirror of
https://github.com/neovim/neovim.git
synced 2026-03-29 03:42:11 +00:00
fix(api): improve external window validation
Problem: "win" is allowed in external window configs in some cases. External window converted to normal float can't move tabpages in one nvim_win_set_config call. External window can't be turned into a normal split. Solution: disallow setting "win" for external windows. Allow external window to move tabpages, which turns it non-external. Allow external window to be turned into a (non-external) split. parse_win_config has more validation issues from not considering the window's existing config enough (not from this PR). For example, zindex can be set for an existing split if "split"/"vertical" isn't given, despite intending for that to be an error. Plus the logic is confusing. It could do with a refactor at some point...
This commit is contained in:
@@ -416,10 +416,6 @@ static bool win_can_move_tp(win_T *wp, tabpage_T *tp, Error *err)
|
||||
api_set_error(err, kErrorTypeException, "%s", e_cmdwin);
|
||||
return false;
|
||||
}
|
||||
if (wp->w_config.external) {
|
||||
api_set_error(err, kErrorTypeException, "Cannot move external window to another tabpage");
|
||||
return false;
|
||||
}
|
||||
return true;
|
||||
}
|
||||
|
||||
@@ -772,7 +768,7 @@ void nvim_win_set_config(Window window, Dict(win_config) *config, Error *err)
|
||||
WinConfig fconfig = win->w_config;
|
||||
|
||||
bool to_split = config->relative.size == 0
|
||||
&& !(HAS_KEY_X(config, external) ? config->external : fconfig.external)
|
||||
&& !(HAS_KEY_X(config, external) && config->external)
|
||||
&& (has_split || has_vertical || was_split);
|
||||
|
||||
if (!parse_win_config(win, config, &fconfig, !was_split || to_split, err)) {
|
||||
@@ -1289,6 +1285,7 @@ static bool parse_win_config(win_T *wp, Dict(win_config) *config, WinConfig *fco
|
||||
} else if (!config->external) {
|
||||
if (HAS_KEY_X(config, vertical) || HAS_KEY_X(config, split)) {
|
||||
is_split = true;
|
||||
fconfig->external = false;
|
||||
} else if (wp == NULL) { // new win
|
||||
api_set_error(err, kErrorTypeValidation,
|
||||
"Must specify 'relative' or 'external' when creating a float");
|
||||
@@ -1380,6 +1377,23 @@ static bool parse_win_config(win_T *wp, Dict(win_config) *config, WinConfig *fco
|
||||
goto fail;
|
||||
}
|
||||
|
||||
if (HAS_KEY_X(config, external)) {
|
||||
fconfig->external = config->external;
|
||||
if (has_relative && fconfig->external) {
|
||||
api_set_error(err, kErrorTypeValidation,
|
||||
"Only one of 'relative' and 'external' must be used");
|
||||
goto fail;
|
||||
}
|
||||
if (fconfig->external && !ui_has(kUIMultigrid)) {
|
||||
api_set_error(err, kErrorTypeValidation, "UI doesn't support external windows");
|
||||
goto fail;
|
||||
}
|
||||
}
|
||||
|
||||
if (HAS_KEY_X(config, win) && fconfig->external) {
|
||||
api_set_error(err, kErrorTypeValidation, "external window cannot have 'win'");
|
||||
goto fail;
|
||||
}
|
||||
if (relative_is_win || (HAS_KEY_X(config, win) && !is_split && wp && wp->w_floating
|
||||
&& fconfig->relative == kFloatRelativeWindow)) {
|
||||
// When relative=win is given, missing win field means win=0.
|
||||
@@ -1408,19 +1422,6 @@ static bool parse_win_config(win_T *wp, Dict(win_config) *config, WinConfig *fco
|
||||
}
|
||||
}
|
||||
|
||||
if (HAS_KEY_X(config, external)) {
|
||||
fconfig->external = config->external;
|
||||
if (has_relative && fconfig->external) {
|
||||
api_set_error(err, kErrorTypeValidation,
|
||||
"Only one of 'relative' and 'external' must be used");
|
||||
goto fail;
|
||||
}
|
||||
if (fconfig->external && !ui_has(kUIMultigrid)) {
|
||||
api_set_error(err, kErrorTypeValidation, "UI doesn't support external windows");
|
||||
goto fail;
|
||||
}
|
||||
}
|
||||
|
||||
if (HAS_KEY_X(config, focusable)) {
|
||||
fconfig->focusable = config->focusable;
|
||||
fconfig->mouse = config->focusable;
|
||||
|
||||
@@ -3354,6 +3354,24 @@ describe('API/win', function()
|
||||
|
|
||||
]])
|
||||
end)
|
||||
|
||||
it('can convert external window to non-external', function()
|
||||
Screen.new(20, 7, { ext_multigrid = true }) -- multigrid needed for external windows
|
||||
api.nvim_open_win(0, true, { external = true, width = 5, height = 5 })
|
||||
eq(true, api.nvim_win_get_config(0).external)
|
||||
api.nvim_win_set_config(0, { split = 'below', win = fn.win_getid(1) })
|
||||
eq(false, api.nvim_win_get_config(0).external)
|
||||
|
||||
api.nvim_win_set_config(0, { external = true, width = 5, height = 5 })
|
||||
eq(true, api.nvim_win_get_config(0).external)
|
||||
api.nvim_win_set_config(0, { relative = 'editor', row = 3, col = 3 })
|
||||
eq(false, api.nvim_win_get_config(0).external)
|
||||
|
||||
api.nvim_win_set_config(0, { external = true, width = 5, height = 5 })
|
||||
eq(true, api.nvim_win_get_config(0).external)
|
||||
api.nvim_win_set_config(0, { external = false })
|
||||
eq(false, api.nvim_win_get_config(0).external)
|
||||
end)
|
||||
end)
|
||||
|
||||
describe('get_config', function()
|
||||
@@ -3841,35 +3859,6 @@ describe('API/win', function()
|
||||
pcall_err(api.nvim_win_set_config, float_win, { win = tab3_win })
|
||||
)
|
||||
eq(float_win, api.nvim_get_current_win())
|
||||
|
||||
-- Need multigrid for external windows.
|
||||
Screen.new(20, 9, { ext_multigrid = true })
|
||||
api.nvim_win_set_config(float_win, { external = true, width = 5, height = 5 })
|
||||
eq(true, api.nvim_win_get_config(float_win).external)
|
||||
eq(
|
||||
('Cannot move external window to another tabpage'):format(float_win),
|
||||
pcall_err(
|
||||
api.nvim_win_set_config,
|
||||
float_win,
|
||||
{ relative = 'win', win = tab3_win, row = 0, col = 0 }
|
||||
)
|
||||
)
|
||||
eq(float_win, api.nvim_get_current_win())
|
||||
|
||||
-- Error if made external by autocommand when attempting to move.
|
||||
api.nvim_win_set_config(
|
||||
float_win,
|
||||
{ relative = 'editor', row = 0, col = 0, width = 5, height = 5 }
|
||||
)
|
||||
eq(false, api.nvim_win_get_config(float_win).external)
|
||||
command(
|
||||
('autocmd WinLeave * ++once call nvim_win_set_config(%d, #{external: 1})'):format(float_win)
|
||||
)
|
||||
eq(
|
||||
('Cannot move external window to another tabpage'):format(float_win),
|
||||
pcall_err(api.nvim_win_set_config, float_win, { win = tab3_win })
|
||||
)
|
||||
eq(float_win, api.nvim_get_current_win())
|
||||
end)
|
||||
end)
|
||||
end)
|
||||
|
||||
@@ -3821,6 +3821,18 @@ describe('float window', function()
|
||||
)
|
||||
eq("Must specify 'width'", pcall_err(api.nvim_open_win, buf, false, { relative = 'editor', row = 0, col = 0 }))
|
||||
eq("Must specify 'height'", pcall_err(api.nvim_open_win, buf, false, { relative = 'editor', row = 0, col = 0, width = 2 }))
|
||||
|
||||
if multigrid then
|
||||
eq(
|
||||
"external window cannot have 'win'",
|
||||
pcall_err(api.nvim_open_win, buf, false, { external = true, win = 0, width = 10, height = 10 })
|
||||
)
|
||||
api.nvim_open_win(buf, true, { external = true, width = 10, height = 10 })
|
||||
eq("external window cannot have 'win'", pcall_err(api.nvim_win_set_config, 0, { win = 0 }))
|
||||
-- OK to include "win" if external window is also reconfigured to a normal float.
|
||||
api.nvim_win_set_config(0, { relative = 'editor', win = 0, row = 0, col = 0, width = 5, height = 5 })
|
||||
eq('editor', api.nvim_win_get_config(0).relative)
|
||||
end
|
||||
end)
|
||||
|
||||
it('can be placed relative window or cursor', function()
|
||||
|
||||
Reference in New Issue
Block a user