From 7be4ae796faa2066044b6cafe266ebe6354c55fe Mon Sep 17 00:00:00 2001 From: Sean Dewar <6256228+seandewar@users.noreply.github.com> Date: Tue, 10 Mar 2026 20:59:48 +0000 Subject: [PATCH] fix(api): relax config validation for "win" Problem: only possible to move floats between tabpages if relative=win, which has the restrictive effect of also anchoring it to the target window. Solution: allow "win" without "relative" or "split"/"vertical". Only assume missing "win" is 0 if relative=win is given to maintain that behaviour. (or when configuring a new window) Also add an error when attempting to change a split into a float that's in another tabpage, as this isn't actually supported yet. (until the next commit) Maybe this could do with some bikeshedding. Unclear if "win" should require "relative" to be given, like with "row"/"col"; this can be annoying though as specifying "relative" requires other fields to be given too. --- runtime/doc/api.txt | 8 ++-- runtime/lua/vim/_meta/api.lua | 5 ++- src/nvim/api/win_config.c | 59 +++++++++++++++++------------ test/functional/api/window_spec.lua | 49 +++++++++--------------- test/functional/ui/float_spec.lua | 26 +++++++++---- 5 files changed, 78 insertions(+), 69 deletions(-) diff --git a/runtime/doc/api.txt b/runtime/doc/api.txt index 318ba96605..3813792418 100644 --- a/runtime/doc/api.txt +++ b/runtime/doc/api.txt @@ -3921,9 +3921,11 @@ nvim_open_win({buffer}, {enter}, {config}) *nvim_open_win()* Default is `"left"`. • vertical: Split vertically |:vertical|. • width: Window width (in character cells). Minimum of 1. - • win: |window-ID| window to split, or relative window when - creating a float (relative="win"). When splitting, - negative value works like |:topleft|, |:botright|. + • win: |window-ID| target window. Can be in a different tab + page. Determines the window to split (negative values act + like |:topleft|, |:botright|), the relative window for a + `relative="win"` float, or just the target tab page + (inferred from the window) for others. • zindex: Stacking order. floats with higher `zindex` go on top on floats with lower indices. Must be larger than zero. The following screen elements have hard-coded diff --git a/runtime/lua/vim/_meta/api.lua b/runtime/lua/vim/_meta/api.lua index 975db4ac3b..0510e2bef9 100644 --- a/runtime/lua/vim/_meta/api.lua +++ b/runtime/lua/vim/_meta/api.lua @@ -1848,8 +1848,9 @@ function vim.api.nvim_open_term(buffer, opts) end --- Default is `"left"`. --- - vertical: Split vertically `:vertical`. --- - width: Window width (in character cells). Minimum of 1. ---- - win: `window-ID` window to split, or relative window when creating a float (relative="win"). ---- When splitting, negative value works like `:topleft`, `:botright`. +--- - win: `window-ID` target window. Can be in a different tab page. Determines the window to +--- split (negative values act like `:topleft`, `:botright`), the relative window for a +--- `relative="win"` float, or just the target tab page (inferred from the window) for others. --- - zindex: Stacking order. floats with higher `zindex` go on top on --- floats with lower indices. Must be larger than zero. The --- following screen elements have hard-coded z-indices: diff --git a/src/nvim/api/win_config.c b/src/nvim/api/win_config.c index 5c7f3a7d32..cf10883d81 100644 --- a/src/nvim/api/win_config.c +++ b/src/nvim/api/win_config.c @@ -180,8 +180,9 @@ /// Default is `"left"`. /// - vertical: Split vertically |:vertical|. /// - width: Window width (in character cells). Minimum of 1. -/// - win: |window-ID| window to split, or relative window when creating a float (relative="win"). -/// When splitting, negative value works like |:topleft|, |:botright|. +/// - win: |window-ID| target window. Can be in a different tab page. Determines the window to +/// split (negative values act like |:topleft|, |:botright|), the relative window for a +/// `relative="win"` float, or just the target tab page (inferred from the window) for others. /// - zindex: Stacking order. floats with higher `zindex` go on top on /// floats with lower indices. Must be larger than zero. The /// following screen elements have hard-coded z-indices: @@ -745,6 +746,16 @@ void nvim_win_set_config(Window window, Dict(win_config) *config, Error *err) } if (was_split && !to_split) { + win_T *parent = find_window_by_handle(fconfig.window, err); + if (!parent) { + return; + } + // TODO(seandewar): support this, preferably via win_config_float_tp. + if (!win_valid(parent)) { + api_set_error(err, kErrorTypeValidation, + "Cannot configure split into float in another tabpage"); + return; + } if (!win_new_float(win, false, fconfig, err)) { return; } @@ -1351,33 +1362,31 @@ static bool parse_win_config(win_T *wp, Dict(win_config) *config, WinConfig *fco goto fail; } - if (relative_is_win || is_split) { - if (wp && relative_is_win) { - win_T *target_win = find_window_by_handle(config->win, err); - if (!target_win) { - goto fail; - } - - if (target_win == wp) { - api_set_error(err, kErrorTypeException, "floating window cannot be relative to itself"); - 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. + win_T *target_win = find_window_by_handle(config->win, err); + if (!target_win) { + goto fail; } - fconfig->window = curwin->handle; + if (target_win == wp) { + api_set_error(err, kErrorTypeException, "floating window cannot be relative to itself"); + goto fail; + } + fconfig->window = target_win->handle; + } else { + // Handle is not validated here, as win_config_split can accept negative values. if (HAS_KEY_X(config, win)) { - if (config->win > 0) { - fconfig->window = config->win; + if (!is_split && !has_relative && (!wp || !wp->w_floating)) { + api_set_error(err, kErrorTypeValidation, + "non-float with 'win' requires at least 'split' or 'vertical'"); + goto fail; } + fconfig->window = config->win; } - } else if (HAS_KEY_X(config, win)) { - if (has_relative) { - api_set_error(err, kErrorTypeValidation, - "'win' key is only valid with relative='win' and relative=''"); - goto fail; - } else if (!is_split) { - api_set_error(err, kErrorTypeValidation, - "non-float with 'win' requires at least 'split' or 'vertical'"); - goto fail; + // Resolve, but skip validating. E.g: win_config_split accepts negative "win". + if (fconfig->window == 0) { + fconfig->window = curwin->handle; } } diff --git a/test/functional/api/window_spec.lua b/test/functional/api/window_spec.lua index 1c48c51921..579eb88663 100644 --- a/test/functional/api/window_spec.lua +++ b/test/functional/api/window_spec.lua @@ -2585,19 +2585,26 @@ describe('API/win', function() }, }, layout) + -- converting split into a float for a different tabpage is not yet supported + eq( + 'Cannot configure split into float in another tabpage', + pcall_err( + api.nvim_win_set_config, + win, + { relative = 'editor', row = 0, col = 0, width = 1, height = 1, win = first_win } + ) + ) + -- convert new win to float in new tabpage api.nvim_win_set_config(win, { relative = 'editor', row = 2, col = 2, height = 2, width = 2 }) api.nvim_set_current_tabpage(first_tab) -- move to other tabpage - api.nvim_win_set_config(win, { relative = 'win', win = first_win, row = 2, col = 2 }) + api.nvim_win_set_config(win, { win = first_win }) eq(first_tab, api.nvim_win_get_tabpage(win)) eq({ first_win, win }, api.nvim_tabpage_list_wins(first_tab)) eq({ tab2_win }, api.nvim_tabpage_list_wins(new_tab)) -- unlike splits, negative win is invalid - eq( - 'Invalid window id: -1', - pcall_err(api.nvim_win_set_config, win, { relative = 'win', win = -1, row = 2, col = 2 }) - ) + eq('Invalid window id: -1', pcall_err(api.nvim_win_set_config, win, { win = -1 })) end) it('correctly moves curwin when moving curwin to a different tabpage', function() @@ -2875,7 +2882,7 @@ describe('API/win', function() }) api.nvim_set_current_tabpage(tab1) api.nvim_set_var('result', {}) - api.nvim_win_set_config(fwin, { relative = 'win', win = tab1_win1, row = 4, col = 4 }) + api.nvim_win_set_config(fwin, { win = tab1_win1 }) eq({}, eval('g:result')) end) @@ -3584,11 +3591,7 @@ describe('API/win', function() split_ok, split_err = pcall(vim.api.nvim_win_set_config, 0, { win = other_tp_win, split = 'right' }) - float_ok, float_err = pcall( - vim.api.nvim_win_set_config, - 0, - { relative = 'win', win = other_tp_win, row = 0, col = 0 } - ) + float_ok, float_err = pcall(vim.api.nvim_win_set_config, 0, { win = other_tp_win }) end) return win_type, split_ok, split_err, float_ok, float_err end) @@ -3714,11 +3717,7 @@ describe('API/win', function() command('autocmd WinLeave * ++once call nvim_win_close(' .. tab2_win .. ', v:true)') eq( 'Target windows were closed', - pcall_err( - api.nvim_win_set_config, - float_win, - { relative = 'win', win = tab2_win, row = 0, col = 0 } - ) + pcall_err(api.nvim_win_set_config, float_win, { win = tab2_win }) ) eq(float_win, api.nvim_get_current_win()) @@ -3727,11 +3726,7 @@ describe('API/win', function() command('tabprev | autocmd WinEnter * ++once wincmd p') eq( ('Failed to switch away from window %d'):format(float_win), - pcall_err( - api.nvim_win_set_config, - float_win, - { relative = 'win', win = tab3_win, row = 0, col = 0 } - ) + pcall_err(api.nvim_win_set_config, float_win, { win = tab3_win }) ) eq(float_win, api.nvim_get_current_win()) @@ -3743,11 +3738,7 @@ describe('API/win', function() ) eq( ('Window %d was made non-floating'):format(float_win), - pcall_err( - api.nvim_win_set_config, - float_win, - { relative = 'win', win = tab3_win, row = 0, col = 0 } - ) + pcall_err(api.nvim_win_set_config, float_win, { win = tab3_win }) ) eq(float_win, api.nvim_get_current_win()) @@ -3776,11 +3767,7 @@ describe('API/win', function() ) 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 } - ) + pcall_err(api.nvim_win_set_config, float_win, { win = tab3_win }) ) eq(float_win, api.nvim_get_current_win()) end) diff --git a/test/functional/ui/float_spec.lua b/test/functional/ui/float_spec.lua index 1c7f309627..a3d4289520 100644 --- a/test/functional/ui/float_spec.lua +++ b/test/functional/ui/float_spec.lua @@ -3783,8 +3783,8 @@ describe('float window', function() local buf = api.nvim_create_buf(false, false) eq("Invalid key: 'bork'", pcall_err(api.nvim_open_win, buf, false, { width = 20, height = 2, bork = true })) eq( - "'win' key is only valid with relative='win' and relative=''", - pcall_err(api.nvim_open_win, buf, false, { width = 20, height = 2, relative = 'editor', row = 0, col = 0, win = 0 }) + "Must specify 'relative' or 'external' when creating a float", + pcall_err(api.nvim_open_win, buf, false, { win = 0 }) ) eq( "floating windows cannot have 'vertical'", @@ -11191,7 +11191,17 @@ describe('float window', function() local winid = api.nvim_open_win(buf, false, config) api.nvim_set_current_win(winid) eq('floating window cannot be relative to itself', pcall_err(api.nvim_win_set_config, winid, config)) - -- Also when configuring split into float. + eq('floating window cannot be relative to itself', pcall_err(api.nvim_win_set_config, winid, { win = winid })) + -- Don't assume win=0 if no win given for existing relative=win float; so no error. + api.nvim_win_set_config(winid, { width = 7 }) + eq(7, api.nvim_win_get_config(winid).width) + -- Don't expect the error when configuring to something other than relative=win, as win=self + -- is fine in those cases. (though maybe pointless) Other errors might be expected, though. + eq('Cannot split a floating window', pcall_err(api.nvim_win_set_config, winid, { split = 'above', win = winid })) + eq('win', api.nvim_win_get_config(winid).relative) + api.nvim_win_set_config(winid, { relative = 'editor', win = winid, row = 3, col = 3 }) + eq('editor', api.nvim_win_get_config(winid).relative) + -- An error when configuring split into relative=win float. command('split') eq('floating window cannot be relative to itself', pcall_err(api.nvim_win_set_config, 0, config)) end) @@ -11881,9 +11891,9 @@ describe('float window', function() -- Schedule an UPD_NOT_VALID redraw, but in one event move the float out of curtab before it's -- handled. Do not flush before then. local float = exec_lua(function() - local float = vim.api.nvim_open_win(0, true, { relative = 'editor', width = 10, height = 5, row = 0, col = 0 }) + local float = vim.api.nvim_open_win(0, true, { relative = 'editor', width = 10, height = 5, row = 1, col = 1 }) vim.api.nvim__redraw({ valid = false, flush = false }) - vim.api.nvim_win_set_config(float, { relative = 'win', win = tab1_win, row = 1, col = 1 }) + vim.api.nvim_win_set_config(float, { win = tab1_win }) return float end) @@ -11914,7 +11924,7 @@ describe('float window', function() end -- Importantly, want tabline redrawn and float's hl attribs to be correct here. - api.nvim_win_set_config(float, { relative = 'win', win = 0, row = 1, col = 1 }) + api.nvim_win_set_config(float, { win = 0 }) if multigrid then screen:expect({ grid = [[ @@ -11935,7 +11945,7 @@ describe('float window', function() {2:~ }|*4 ]], float_pos = { - [5] = { 1002, 'NW', 4, 1, 1, true, 50, 1, 1, 1 }, + [5] = { 1002, 'NW', 1, 1, 1, true, 50, 1, 1, 1 }, }, }) else @@ -12053,7 +12063,7 @@ describe('float window', function() ]]) end - api.nvim_win_set_config(float, { relative = 'win', win = tab2_win, row = 1, col = 1 }) + api.nvim_win_set_config(float, { win = tab2_win }) if multigrid then screen:expect({ grid = [[