From e2a0748cb28fb44f6ee42f7470ef50a9dda6e320 Mon Sep 17 00:00:00 2001 From: glepnir Date: Fri, 5 Sep 2025 13:35:27 +0800 Subject: [PATCH 01/10] feat(api): nvim_win_set_config can move floatwin to another tabpage Problem: nvim_win_set_config can't move floating windows to different tab pages. Solution: allow it. Co-authored-by: Sean Dewar <6256228+seandewar@users.noreply.github.com> --- src/nvim/api/win_config.c | 111 ++++++++++++++++++++-- test/functional/api/window_spec.lua | 140 +++++++++++++++++++++------- 2 files changed, 209 insertions(+), 42 deletions(-) diff --git a/src/nvim/api/win_config.c b/src/nvim/api/win_config.c index 55e089b133..21b1c98e7f 100644 --- a/src/nvim/api/win_config.c +++ b/src/nvim/api/win_config.c @@ -32,6 +32,7 @@ #include "nvim/syntax.h" #include "nvim/types_defs.h" #include "nvim/ui.h" +#include "nvim/ui_compositor.h" #include "nvim/ui_defs.h" #include "nvim/vim_defs.h" #include "nvim/window.h" @@ -381,6 +382,24 @@ static int win_split_flags(WinSplit split, bool toplevel) return flags; } +static bool can_move(win_T *wp, bool switch_tab, Error *err) +{ + if (is_aucmd_win(wp) && switch_tab) { + api_set_error(err, kErrorTypeException, "Cannot move autocmd window to another tabpage"); + return false; + } + // Can't move the cmdwin or its old curwin to a different tabpage. + if ((wp == cmdwin_win || wp == cmdwin_old_curwin) && switch_tab) { + api_set_error(err, kErrorTypeException, "%s", e_cmdwin); + return false; + } + if (wp->w_floating && wp->w_config.external) { + api_set_error(err, kErrorTypeException, "%s", "Cannot move external floating window"); + return false; + } + return true; +} + static bool win_config_split(win_T *win, Dict(win_config) *config, WinConfig *fconfig, Error *err) FUNC_ATTR_NONNULL_ALL { @@ -423,13 +442,7 @@ static bool win_config_split(win_T *win, Dict(win_config) *config, WinConfig *fc api_set_error(err, kErrorTypeException, "Cannot split a floating window"); return false; } - if (is_aucmd_win(win) && win_tp != parent_tp) { - api_set_error(err, kErrorTypeException, "Cannot move autocmd window to another tabpage"); - return false; - } - // Can't move the cmdwin or its old curwin to a different tabpage. - if ((win == cmdwin_win || win == cmdwin_old_curwin) && win_tp != parent_tp) { - api_set_error(err, kErrorTypeException, "%s", e_cmdwin); + if (!can_move(win, win_tp != parent_tp, err)) { return false; } } @@ -652,6 +665,24 @@ void nvim_win_set_config(Window window, Dict(win_config) *config, Error *err) return; } + win_T *parent = NULL; + tabpage_T *parent_tp = NULL; + tabpage_T *win_tp = win_find_tabpage(win); + bool curwin_moving_tp = false; + if (config->win == 0) { + parent = curwin; + parent_tp = curtab; + } else if (config->win > 0) { + parent = find_window_by_handle(fconfig.window, err); + if (!parent) { + return; + } + parent_tp = win_find_tabpage(parent); + if (!parent_tp) { + return; + } + } + if (was_split && !to_split) { if (!win_new_float(win, false, fconfig, err)) { return; @@ -662,6 +693,64 @@ void nvim_win_set_config(Window window, Dict(win_config) *config, Error *err) return; } } else { + if (win->w_floating && HAS_KEY_X(config, win) && parent && parent_tp != win_tp) { + if (!can_move(win, win_tp != parent_tp, err)) { + return; + } + if (win == curwin) { + curwin_moving_tp = true; + win_goto(win_float_find_altwin(win, NULL)); + + // Autocommands may have been a real nuisance and messed things up... + if (curwin == win) { + api_set_error(err, kErrorTypeException, "Failed to switch away from window %d", + win->handle); + return; + } + win_tp = win_find_tabpage(win); + parent_tp = win_find_tabpage(parent); + if (!win_tp || !parent_tp) { + api_set_error(err, kErrorTypeException, "Target windows were closed"); + goto restore_curwin; + } + if (!win->w_floating) { + api_set_error(err, kErrorTypeException, "Window %d was made non-floating", + win->handle); + goto restore_curwin; + } + if (!can_move(win, win_tp != parent_tp, err)) { + goto restore_curwin; + } + } + + if (win_tp != parent_tp) { + win_remove(win, win_tp == curtab ? NULL : win_tp); + win_T *target_after; + if (parent_tp == curtab) { + target_after = lastwin_nofloating(); + } else { + target_after = parent_tp->tp_lastwin; + while (target_after->w_floating) { + target_after = target_after->w_prev; + } + } + + win_append(target_after, win, parent_tp == curtab ? NULL : parent_tp); + // If `win` was the curwin of its old tabpage, select a new curwin for it. + if (win_tp != curtab && win_tp->tp_curwin == win) { + win_tp->tp_curwin = win_float_find_altwin(win, win_tp); + } + + if (parent_tp != curtab) { + if (ui_has(kUIMultigrid)) { + ui_call_win_hide(win->w_grid_alloc.handle); + } + ui_comp_remove_grid(&win->w_grid_alloc); + } else { + win->w_hl_needs_update = true; + } + } + } win_config_float(win, fconfig); } @@ -675,6 +764,14 @@ void nvim_win_set_config(Window window, Dict(win_config) *config, Error *err) } else if (win == cmdline_win && fconfig._cmdline_offset == INT_MAX) { cmdline_win = NULL; } + return; + +restore_curwin: + // If `win` was the original curwin, and autocommands didn't move it outside of curtab, be a + // good citizen and try to return to it. + if (curwin_moving_tp && win_valid(win)) { + win_goto(win); + } #undef HAS_KEY_X } diff --git a/test/functional/api/window_spec.lua b/test/functional/api/window_spec.lua index 62cc514058..9a8b4db7e1 100644 --- a/test/functional/api/window_spec.lua +++ b/test/functional/api/window_spec.lua @@ -2561,60 +2561,61 @@ describe('API/win', function() eq(win, layout[2][2][2]) end) - it('moves splits to other tabpages', function() - local curtab = api.nvim_get_current_tabpage() + it('moves splits or floats to other tabpages', function() + local first_tab = api.nvim_get_current_tabpage() + local first_win = api.nvim_get_current_win() local win = api.nvim_open_win(0, false, { split = 'left' }) command('tabnew') - local tabnr = api.nvim_get_current_tabpage() - command('tabprev') -- return to the initial tab - - api.nvim_win_set_config(win, { - split = 'right', - win = api.nvim_tabpage_get_win(tabnr), - }) - - eq(tabnr, api.nvim_win_get_tabpage(win)) + local new_tab = api.nvim_get_current_tabpage() + local tab2_win = api.nvim_get_current_win() + api.nvim_set_current_tabpage(first_tab) + -- move new win to new tabpage + api.nvim_win_set_config(win, { split = 'right', win = api.nvim_tabpage_get_win(new_tab) }) + eq(new_tab, api.nvim_win_get_tabpage(win)) -- we are changing the config, the current tabpage should not change - eq(curtab, api.nvim_get_current_tabpage()) + eq(first_tab, api.nvim_get_current_tabpage()) - command('tabnext') -- switch to the new tabpage so we can get the layout + api.nvim_set_current_tabpage(new_tab) local layout = fn.winlayout() - eq({ 'row', { - { 'leaf', api.nvim_tabpage_get_win(tabnr) }, + { 'leaf', api.nvim_tabpage_get_win(new_tab) }, { 'leaf', win }, }, }, layout) + + -- convert new win to float window 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 another tabpage + api.nvim_win_set_config(win, { relative = 'win', win = first_win, row = 2, col = 2 }) + 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)) end) it('correctly moves curwin when moving curwin to a different tabpage', function() - local curtab = api.nvim_get_current_tabpage() + local tab1 = api.nvim_get_current_tabpage() + local tab1_win = api.nvim_get_current_win() command('tabnew') local tab2 = api.nvim_get_current_tabpage() local tab2_win = api.nvim_get_current_win() - - command('tabprev') -- return to the initial tab - - local neighbor = api.nvim_get_current_win() - + api.nvim_set_current_tabpage(tab1) -- return to the initial tab -- create and enter a new split local win = api.nvim_open_win(0, true, { vertical = false, }) - eq(curtab, api.nvim_win_get_tabpage(win)) - - eq({ win, neighbor }, api.nvim_tabpage_list_wins(curtab)) + eq(tab1, api.nvim_win_get_tabpage(win)) + eq({ win, tab1_win }, api.nvim_tabpage_list_wins(tab1)) -- move the current win to a different tabpage api.nvim_win_set_config(win, { split = 'right', win = api.nvim_tabpage_get_win(tab2), }) - - eq(curtab, api.nvim_get_current_tabpage()) + eq(tab1, api.nvim_get_current_tabpage()) -- win should have moved to tab2 eq(tab2, api.nvim_win_get_tabpage(win)) @@ -2622,10 +2623,18 @@ describe('API/win', function() eq(tab2_win, api.nvim_tabpage_get_win(tab2)) -- win lists should be correct eq({ tab2_win, win }, api.nvim_tabpage_list_wins(tab2)) - eq({ neighbor }, api.nvim_tabpage_list_wins(curtab)) - + eq({ tab1_win }, api.nvim_tabpage_list_wins(tab1)) -- current win should have moved to neighboring win - eq(neighbor, api.nvim_tabpage_get_win(curtab)) + eq(tab1_win, api.nvim_tabpage_get_win(tab1)) + + api.nvim_set_current_tabpage(tab2) + -- convert new win to float window + api.nvim_win_set_config(win, { relative = 'editor', row = 2, col = 2, height = 2, width = 2 }) + api.nvim_set_current_win(win) + api.nvim_win_set_config(win, { relative = 'win', win = tab1_win, row = 3, col = 3 }) + eq(tab1, api.nvim_win_get_tabpage(win)) + eq(tab2, api.nvim_get_current_tabpage()) + eq({ tab1_win, win }, api.nvim_tabpage_list_wins(tab1)) end) it('splits windows in non-current tabpage', function() @@ -2834,18 +2843,35 @@ describe('API/win', function() eq({ 'Leave', win, 'Enter', new_curwin }, eval('result')) end) - it('no autocmds when moving window within same tabpage', function() + it('no autocmds when moving window in same or other 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()] + let g:result = [] + autocmd WinEnter * let g:result += ["Enter", win_getid()] + autocmd WinLeave * let g:result += ["Leave", win_getid()] + autocmd WinNew * let g: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')) + eq({}, eval('g:result')) + + -- move float window from tab2 to tab1 + command('tabdo only') + local tab1 = api.nvim_get_current_tabpage() + local tab1_win1 = api.nvim_get_current_win() + command('tabnew') + local fwin = api.nvim_open_win(0, false, { + relative = 'editor', + row = 2, + col = 2, + height = 2, + width = 2, + }) + 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 }) + eq({}, eval('g:result')) end) it('checks if splitting disallowed', function() @@ -3656,5 +3682,49 @@ describe('API/win', function() api.nvim_open_win(0, true, { split = 'below', style = 'minimal' }) command('quit') end) + + it('preserve current floating window when moving fails', function() + local buf = api.nvim_create_buf(false, true) + local tab1_win = api.nvim_get_current_win() + local float_win = api.nvim_open_win(buf, true, { + relative = 'editor', + row = 1, + col = 1, + width = 10, + height = 5, + }) + command('tabnew') + local tab2_win = api.nvim_get_current_win() + command('tabprev') + api.nvim_set_current_win(float_win) + 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 } + ) + ) + eq(float_win, api.nvim_get_current_win()) + + command('tabnew') + local tab3_win = api.nvim_get_current_win() + command('tabprev') + command( + ('autocmd WinLeave * ++once call nvim_win_set_config(%d, {"split": "left", "win": %d})'):format( + float_win, + tab1_win + ) + ) + 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 } + ) + ) + end) end) end) From e5240b35c3b78e34ca16ccc8369d0aeb4e1add8e Mon Sep 17 00:00:00 2001 From: Sean Dewar <6256228+seandewar@users.noreply.github.com> Date: Tue, 10 Mar 2026 09:05:27 +0000 Subject: [PATCH 02/10] refactor(api): cleanup, more comments, more tests, news - Factor out logic to keep nvim_win_set_config clean. - Clean up a few things, remove redundant logic, reflow some lines. - Add some more comments where appropriate. - Don't consider negative "win", as that's only relevant for splits. - Add more test coverage. - Add news.txt entry. --- runtime/doc/news.txt | 1 + src/nvim/api/win_config.c | 206 ++++++++++++++-------------- test/functional/api/window_spec.lua | 80 +++++++++-- 3 files changed, 174 insertions(+), 113 deletions(-) diff --git a/runtime/doc/news.txt b/runtime/doc/news.txt index e8c52a8ca9..9f2a348025 100644 --- a/runtime/doc/news.txt +++ b/runtime/doc/news.txt @@ -167,6 +167,7 @@ API they were so specified in `nvim_create_user_command()`. • |nvim_open_win()| floating windows can show a 'statusline'. Plugins can use `style='minimal'` or `:setlocal statusline=` to hide the statusline. +• |nvim_win_set_config()| can move floating windows to other tab pages. • Added experimental |nvim__exec_lua_fast()| to allow remote API clients to execute code while nvim is blocking for input. • |vim.secure.trust()| accepts `path` for the `allow` action. diff --git a/src/nvim/api/win_config.c b/src/nvim/api/win_config.c index 21b1c98e7f..34c3e56e8b 100644 --- a/src/nvim/api/win_config.c +++ b/src/nvim/api/win_config.c @@ -40,6 +40,8 @@ #include "api/win_config.c.generated.h" +#define HAS_KEY_X(d, key) HAS_KEY(d, win_config, key) + /// Opens a new split window, floating window, or external window. /// /// - Specify `relative` to create a floating window. Floats are drawn over the split layout, @@ -198,7 +200,6 @@ Window nvim_open_win(Buffer buffer, Boolean enter, Dict(win_config) *config, Error *err) FUNC_API_SINCE(6) FUNC_API_TEXTLOCK_ALLOW_CMDWIN { -#define HAS_KEY_X(d, key) HAS_KEY(d, win_config, key) buf_T *buf = find_buffer_by_handle(buffer, err); if (!buf) { return 0; @@ -349,7 +350,6 @@ cleanup: unblock_autocmds(); } return rv; -#undef HAS_KEY_X } static WinSplit win_split_dir(win_T *win) @@ -382,28 +382,31 @@ static int win_split_flags(WinSplit split, bool toplevel) return flags; } -static bool can_move(win_T *wp, bool switch_tab, Error *err) +/// Checks if window `wp` can be moved to tabpage `tp`. +static bool win_can_move_tp(win_T *wp, Error *err) + FUNC_ATTR_NONNULL_ALL { - if (is_aucmd_win(wp) && switch_tab) { + if (is_aucmd_win(wp)) { api_set_error(err, kErrorTypeException, "Cannot move autocmd window to another tabpage"); return false; } // Can't move the cmdwin or its old curwin to a different tabpage. - if ((wp == cmdwin_win || wp == cmdwin_old_curwin) && switch_tab) { + if (wp == cmdwin_win || wp == cmdwin_old_curwin) { api_set_error(err, kErrorTypeException, "%s", e_cmdwin); return false; } - if (wp->w_floating && wp->w_config.external) { - api_set_error(err, kErrorTypeException, "%s", "Cannot move external floating window"); + if (wp->w_config.external) { + api_set_error(err, kErrorTypeException, "Cannot move external window to another tabpage"); return false; } return true; } -static bool win_config_split(win_T *win, Dict(win_config) *config, WinConfig *fconfig, Error *err) +/// Configures `win` into a split, also moving it to another tabpage if requested. +static bool win_config_split(win_T *win, const Dict(win_config) *config, WinConfig *fconfig, + Error *err) FUNC_ATTR_NONNULL_ALL { -#define HAS_KEY_X(d, key) HAS_KEY(d, win_config, key) bool was_split = !win->w_floating; bool has_split = HAS_KEY_X(config, split); bool has_vertical = HAS_KEY_X(config, vertical); @@ -442,8 +445,8 @@ static bool win_config_split(win_T *win, Dict(win_config) *config, WinConfig *fc api_set_error(err, kErrorTypeException, "Cannot split a floating window"); return false; } - if (!can_move(win, win_tp != parent_tp, err)) { - return false; + if (win_tp != parent_tp && !win_can_move_tp(win, err)) { + return false; // error already set } } @@ -472,8 +475,7 @@ static bool win_config_split(win_T *win, Dict(win_config) *config, WinConfig *fc // Autocommands may have been a real nuisance and messed things up... if (curwin == win) { - api_set_error(err, kErrorTypeException, "Failed to switch away from window %d", - win->handle); + api_set_error(err, kErrorTypeException, "Failed to switch away from window %d", win->handle); return false; } win_tp = win_find_tabpage(win); @@ -621,7 +623,92 @@ resize: } merge_win_config(&win->w_config, *fconfig); return true; -#undef HAS_KEY_X +} + +/// Configures `win` into a float, also moving it to another tabpage if requested. +static bool win_config_float_tp(win_T *win, const Dict(win_config) *config, + const WinConfig *fconfig, Error *err) + FUNC_ATTR_NONNULL_ALL +{ + tabpage_T *win_tp = win_find_tabpage(win); + win_T *parent = win; + tabpage_T *parent_tp = win_tp; + if (HAS_KEY_X(config, win)) { + parent = find_window_by_handle(fconfig->window, err); + if (!parent) { + return false; // error already set + } + parent_tp = win_find_tabpage(parent); + } + + if (win_tp != parent_tp) { + if (!win_can_move_tp(win, err)) { + return false; // error already set + } + if (curwin == win) { + win_goto(win_float_find_altwin(win, NULL)); + + // Autocommands may have been a real nuisance and messed things up... + if (curwin == win) { + api_set_error(err, kErrorTypeException, "Failed to switch away from window %d", + win->handle); + return false; + } + win_tp = win_find_tabpage(win); + parent_tp = win_find_tabpage(parent); + + bool restore_curwin = false; + if (!win_tp || !parent_tp) { + api_set_error(err, kErrorTypeException, "Target windows were closed"); + restore_curwin = true; + } else if (!win->w_floating) { + api_set_error(err, kErrorTypeException, "Window %d was made non-floating", win->handle); + restore_curwin = true; + } else if (win_tp != parent_tp && !win_can_move_tp(win, err)) { + restore_curwin = true; // error already set + } + if (restore_curwin) { + // As `win` was the original curwin, and autocommands didn't move it outside of curtab, be + // a good citizen and try to return to it. + if (win_valid(win)) { + win_goto(win); + } + return false; + } + } + + // Check again, in case autocommands above moved windows to the same tabpage. + if (win_tp != parent_tp) { + win_remove(win, win_tp == curtab ? NULL : win_tp); + win_T *after; + if (parent_tp == curtab) { + after = lastwin_nofloating(); + } else { + after = parent_tp->tp_lastwin; + while (after->w_floating) { + after = after->w_prev; + } + } + + win_append(after, win, parent_tp == curtab ? NULL : parent_tp); + // If `win` was the curwin of its old tabpage, select a new curwin for it. + if (win_tp != curtab && win_tp->tp_curwin == win) { + win_tp->tp_curwin = win_float_find_altwin(win, win_tp); + } + + if (parent_tp != curtab) { + if (ui_has(kUIMultigrid)) { + ui_call_win_hide(win->w_grid_alloc.handle); + } + ui_comp_remove_grid(&win->w_grid_alloc); + } else { + win->w_hl_needs_update = true; + } + } + } + + win_config_float(win, *fconfig); + return true; } /// Reconfigures the layout and properties of a window. @@ -644,7 +731,6 @@ resize: void nvim_win_set_config(Window window, Dict(win_config) *config, Error *err) FUNC_API_SINCE(6) { -#define HAS_KEY_X(d, key) HAS_KEY(d, win_config, key) win_T *win = find_window_by_handle(window, err); if (!win) { return; @@ -665,24 +751,6 @@ void nvim_win_set_config(Window window, Dict(win_config) *config, Error *err) return; } - win_T *parent = NULL; - tabpage_T *parent_tp = NULL; - tabpage_T *win_tp = win_find_tabpage(win); - bool curwin_moving_tp = false; - if (config->win == 0) { - parent = curwin; - parent_tp = curtab; - } else if (config->win > 0) { - parent = find_window_by_handle(fconfig.window, err); - if (!parent) { - return; - } - parent_tp = win_find_tabpage(parent); - if (!parent_tp) { - return; - } - } - if (was_split && !to_split) { if (!win_new_float(win, false, fconfig, err)) { return; @@ -693,65 +761,10 @@ void nvim_win_set_config(Window window, Dict(win_config) *config, Error *err) return; } } else { - if (win->w_floating && HAS_KEY_X(config, win) && parent && parent_tp != win_tp) { - if (!can_move(win, win_tp != parent_tp, err)) { - return; - } - if (win == curwin) { - curwin_moving_tp = true; - win_goto(win_float_find_altwin(win, NULL)); - - // Autocommands may have been a real nuisance and messed things up... - if (curwin == win) { - api_set_error(err, kErrorTypeException, "Failed to switch away from window %d", - win->handle); - return; - } - win_tp = win_find_tabpage(win); - parent_tp = win_find_tabpage(parent); - if (!win_tp || !parent_tp) { - api_set_error(err, kErrorTypeException, "Target windows were closed"); - goto restore_curwin; - } - if (!win->w_floating) { - api_set_error(err, kErrorTypeException, "Window %d was made non-floating", - win->handle); - goto restore_curwin; - } - if (!can_move(win, win_tp != parent_tp, err)) { - goto restore_curwin; - } - } - - if (win_tp != parent_tp) { - win_remove(win, win_tp == curtab ? NULL : win_tp); - win_T *target_after; - if (parent_tp == curtab) { - target_after = lastwin_nofloating(); - } else { - target_after = parent_tp->tp_lastwin; - while (target_after->w_floating) { - target_after = target_after->w_prev; - } - } - - win_append(target_after, win, parent_tp == curtab ? NULL : parent_tp); - // If `win` was the curwin of its old tabpage, select a new curwin for it. - if (win_tp != curtab && win_tp->tp_curwin == win) { - win_tp->tp_curwin = win_float_find_altwin(win, win_tp); - } - - if (parent_tp != curtab) { - if (ui_has(kUIMultigrid)) { - ui_call_win_hide(win->w_grid_alloc.handle); - } - ui_comp_remove_grid(&win->w_grid_alloc); - } else { - win->w_hl_needs_update = true; - } - } + assert(!was_split); + if (!win_config_float_tp(win, config, &fconfig, err)) { + return; } - win_config_float(win, fconfig); } if (fconfig.style == kWinStyleMinimal && old_style != fconfig.style) { @@ -764,15 +777,6 @@ void nvim_win_set_config(Window window, Dict(win_config) *config, Error *err) } else if (win == cmdline_win && fconfig._cmdline_offset == INT_MAX) { cmdline_win = NULL; } - return; - -restore_curwin: - // If `win` was the original curwin, and autocommands didn't move it outside of curtab, be a - // good citizen and try to return to it. - if (curwin_moving_tp && win_valid(win)) { - win_goto(win); - } -#undef HAS_KEY_X } #define PUT_KEY_X(d, key, value) PUT_KEY(d, win_config, key, value) @@ -1241,7 +1245,6 @@ bool parse_winborder(WinConfig *fconfig, char *border_opt, Error *err) static bool parse_win_config(win_T *wp, Dict(win_config) *config, WinConfig *fconfig, bool reconf, Error *err) { -#define HAS_KEY_X(d, key) HAS_KEY(d, win_config, key) bool has_relative = false, relative_is_win = false, is_split = false; if (config->relative.size > 0) { if (!parse_float_relative(config->relative, &fconfig->relative)) { @@ -1519,5 +1522,4 @@ static bool parse_win_config(win_T *wp, Dict(win_config) *config, WinConfig *fco fail: merge_win_config(fconfig, wp != NULL ? wp->w_config : WIN_CONFIG_INIT); return false; -#undef HAS_KEY_X } diff --git a/test/functional/api/window_spec.lua b/test/functional/api/window_spec.lua index 9a8b4db7e1..1c48c51921 100644 --- a/test/functional/api/window_spec.lua +++ b/test/functional/api/window_spec.lua @@ -2561,7 +2561,7 @@ describe('API/win', function() eq(win, layout[2][2][2]) end) - it('moves splits or floats to other tabpages', function() + it('moves windows to other tabpages', function() local first_tab = api.nvim_get_current_tabpage() local first_win = api.nvim_get_current_win() local win = api.nvim_open_win(0, false, { split = 'left' }) @@ -2585,14 +2585,19 @@ describe('API/win', function() }, }, layout) - -- convert new win to float window in new tabpage + -- 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 another tabpage + -- move to other tabpage api.nvim_win_set_config(win, { relative = 'win', win = first_win, row = 2, col = 2 }) 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 }) + ) end) it('correctly moves curwin when moving curwin to a different tabpage', function() @@ -2628,7 +2633,7 @@ describe('API/win', function() eq(tab1_win, api.nvim_tabpage_get_win(tab1)) api.nvim_set_current_tabpage(tab2) - -- convert new win to float window + -- convert new win to float api.nvim_win_set_config(win, { relative = 'editor', row = 2, col = 2, height = 2, width = 2 }) api.nvim_set_current_win(win) api.nvim_win_set_config(win, { relative = 'win', win = tab1_win, row = 3, col = 3 }) @@ -3568,20 +3573,29 @@ describe('API/win', function() end) it('cannot move autocmd window between tabpages', function() - local win_type, split_ok, err = exec_lua(function() + local win_type, split_ok, split_err, float_ok, float_err = exec_lua(function() local other_tp_win = vim.api.nvim_get_current_win() vim.cmd.tabnew() - local win_type, split_ok, err + local win_type, split_ok, split_err, float_ok, float_err vim.api.nvim_buf_call(vim.api.nvim_create_buf(true, true), function() win_type = vim.fn.win_gettype() - split_ok, err = + + 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 } + ) end) - return win_type, split_ok, err + return win_type, split_ok, split_err, float_ok, float_err end) + eq('autocmd', win_type) - eq({ false, 'Cannot move autocmd window to another tabpage' }, { split_ok, err }) + eq({ false, 'Cannot move autocmd window to another tabpage' }, { split_ok, split_err }) + eq({ false, 'Cannot move autocmd window to another tabpage' }, { float_ok, float_err }) end) it('cannot move cmdwin between tabpages', function() @@ -3710,9 +3724,19 @@ describe('API/win', function() command('tabnew') local tab3_win = api.nvim_get_current_win() - command('tabprev') + 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 } + ) + ) + eq(float_win, api.nvim_get_current_win()) + command( - ('autocmd WinLeave * ++once call nvim_win_set_config(%d, {"split": "left", "win": %d})'):format( + ('autocmd WinLeave * ++once call nvim_win_set_config(%d, #{split: "left", win: %d})'):format( float_win, tab1_win ) @@ -3725,6 +3749,40 @@ describe('API/win', function() { relative = 'win', win = tab3_win, row = 0, col = 0 } ) ) + 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, + { relative = 'win', win = tab3_win, row = 0, col = 0 } + ) + ) + eq(float_win, api.nvim_get_current_win()) end) end) end) From c924c2a7b316afb982cd7786923446517c6fec6b Mon Sep 17 00:00:00 2001 From: Sean Dewar <6256228+seandewar@users.noreply.github.com> Date: Tue, 10 Mar 2026 10:06:20 +0000 Subject: [PATCH 03/10] fix(api): win_config_float_tp grid removal, redraw Problem: when nvim_win_set_config moves a floatwin between tabpages, its grid may remain if temporarily inside another tabpage. Also, things like tablines aren't redrawn. Solution: always remove its grid. Set must_redraw so things are redrawn even if w_redr_type was already set for the old tabpage. I don't think it's necessary to do anything extra here when removing the grid: - win_ui_flush calls ui_call_win_hide anyway, and calling it manually ends up sending two win_hide events. - ui_comp_remove_grid safely does nothing if the grid doesn't exist. - w_pos_changed is set by win_config_float later, if that's needed. I think the pending_comp_index_update set by ui_comp_remove_grid is enough anyway, at least for making sure win_ui_flush sends win_hide. Added test fails with the prior approach of checking `parent_tp != curtab`, but also `win_tp == curtab`. (which is a better, but still flawed alternative) The added redrawing here also supersedes setting w_hl_needs_update, and also redraws stuff like the tabline to pass the new test. --- src/nvim/api/win_config.c | 17 +-- test/functional/ui/float_spec.lua | 214 ++++++++++++++++++++++++++++++ 2 files changed, 223 insertions(+), 8 deletions(-) diff --git a/src/nvim/api/win_config.c b/src/nvim/api/win_config.c index 34c3e56e8b..1f9550a187 100644 --- a/src/nvim/api/win_config.c +++ b/src/nvim/api/win_config.c @@ -696,14 +696,15 @@ static bool win_config_float_tp(win_T *win, const Dict(win_config) *config, win_tp->tp_curwin = win_float_find_altwin(win, win_tp); } - if (parent_tp != curtab) { - if (ui_has(kUIMultigrid)) { - ui_call_win_hide(win->w_grid_alloc.handle); - } - ui_comp_remove_grid(&win->w_grid_alloc); - } else { - win->w_hl_needs_update = true; - } + // Remove grid if present. More reliable than checking curtab, as tabpage_check_windows may + // not run when temporarily switching tabpages, meaning grids may be stale from another + // tabpage! (e.g: switch_win_noblock with no_display=true) + ui_comp_remove_grid(&win->w_grid_alloc); + + // Redraw tabline, update window's hl attribs, etc. Set must_redraw here, as redraw_later + // might not if w_redr_type >= UPD_NOT_VALID was set in the old tabpage. + redraw_later(win, UPD_NOT_VALID); + set_must_redraw(UPD_NOT_VALID); } } diff --git a/test/functional/ui/float_spec.lua b/test/functional/ui/float_spec.lua index 15b7004f7e..0369e55a30 100644 --- a/test/functional/ui/float_spec.lua +++ b/test/functional/ui/float_spec.lua @@ -11869,6 +11869,220 @@ describe('float window', function() ]]) end end) + + it('redrawn after moving tabpages via nvim_win_set_config()', function() + local tab1_win = api.nvim_get_current_win() + fn.setline(1, 'hello') + command('tab split') + local tab2_win = api.nvim_get_current_win() + -- 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 }) + vim.api.nvim__redraw({ valid = false, flush = false }) + vim.api.nvim_win_set_config(float, { relative = 'win', win = tab1_win, row = 1, col = 1 }) + return float + end) + + if multigrid then + screen:expect({ + grid = [[ + ## grid 1 + {9: }{10:2}{9:+ [No Name] }{3: + [No Name] }{5: }{9:X}| + [4:----------------------------------------]|*5 + [3:----------------------------------------]| + ## grid 2 (hidden) + hello | + {0:~ }|*5 + ## grid 3 + | + ## grid 4 + ^hello | + {0:~ }|*4 + ]], + }) + else + screen:expect([[ + {9: }{10:2}{9:+ [No Name] }{3: + [No Name] }{5: }{9:X}| + ^hello | + {0:~ }|*4 + | + ]]) + 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 }) + if multigrid then + screen:expect({ + grid = [[ + ## grid 1 + {9: + [No Name] }{3: }{11:2}{3:+ [No Name] }{5: }{9:X}| + [4:----------------------------------------]|*5 + [3:----------------------------------------]| + ## grid 2 (hidden) + hello | + {0:~ }|*5 + ## grid 3 + | + ## grid 4 + ^hello | + {0:~ }|*4 + ## grid 5 + {1:hello }| + {2:~ }|*4 + ]], + float_pos = { + [5] = { 1002, 'NW', 4, 1, 1, true, 50, 1, 1, 1 }, + }, + }) + else + screen:expect([[ + {9: + [No Name] }{3: }{11:2}{3:+ [No Name] }{5: }{9:X}| + ^h{1:hello } | + {0:~}{2:~ }{0: }|*4 + | + ]]) + end + + -- Autocommand runs within the first tabpage, but won't refresh grids when switching to it due + -- to switch_win_noblock having no_display set. + command( + ('autocmd OptionSet rightleft ++once call nvim_win_set_config(%d, #{relative: "win", win: %d, row: 0, col: 0})'):format( + float, + tab1_win + ) + ) + api.nvim_set_option_value('rightleft', true, { win = tab1_win }) + -- Stale grids should not have caused issues with removing the float's grid. + if multigrid then + screen:expect({ + grid = [[ + ## grid 1 + {9: }{10:2}{9:+ [No Name] }{3: + [No Name] }{5: }{9:X}| + [4:----------------------------------------]|*5 + [3:----------------------------------------]| + ## grid 2 (hidden) + hello | + {0:~ }|*5 + ## grid 3 + | + ## grid 4 + ^hello | + {0:~ }|*4 + ## grid 5 (hidden) + {1:hello }| + {2:~ }|*4 + ]], + }) + else + screen:expect([[ + {9: }{10:2}{9:+ [No Name] }{3: + [No Name] }{5: }{9:X}| + ^hello | + {0:~ }|*4 + | + ]]) + end + + command('tabfirst') + if multigrid then + screen:expect({ + grid = [[ + ## grid 1 + {3: }{11:2}{3:+ [No Name] }{9: + [No Name] }{5: }{9:X}| + [2:----------------------------------------]|*5 + [3:----------------------------------------]| + ## grid 2 + olle^h| + {0: ~}|*4 + ## grid 3 + | + ## grid 4 (hidden) + hello | + {0:~ }|*4 + ## grid 5 + {1:hello }| + {2:~ }|*4 + ]], + float_pos = { + [5] = { 1002, 'NW', 2, 0, 0, true, 50, 1, 1, 0 }, + }, + }) + else + screen:expect([[ + {3: }{11:2}{3:+ [No Name] }{9: + [No Name] }{5: }{9:X}| + {1:hello } olle^h| + {2:~ }{0: ~}|*4 + | + ]]) + end + + -- Check tablines are redrawn even when moving floats between two non-current tabpages. + command('tabnew') + if multigrid then + screen:expect({ + grid = [[ + ## grid 1 + {9: }{10:2}{9:+ No Name] }{3: [No Name] }{9: + [No Name] }{5: }{9:X}| + [6:----------------------------------------]|*5 + [3:----------------------------------------]| + ## grid 2 (hidden) + olleh| + {0: ~}|*4 + ## grid 3 + | + ## grid 4 (hidden) + hello | + {0:~ }|*4 + ## grid 5 (hidden) + {1:hello }| + {2:~ }|*4 + ## grid 6 + ^ | + {0: ~}|*4 + ]], + }) + else + screen:expect([[ + {9: }{10:2}{9:+ No Name] }{3: [No Name] }{9: + [No Name] }{5: }{9:X}| + ^ | + {0: ~}|*4 + | + ]]) + end + + api.nvim_win_set_config(float, { relative = 'win', win = tab2_win, row = 1, col = 1 }) + if multigrid then + screen:expect({ + grid = [[ + ## grid 1 + {9: + [No Name] }{3: [No Name] }{9: }{10:2}{9:+ No Name] }{5: }{9:X}| + [6:----------------------------------------]|*5 + [3:----------------------------------------]| + ## grid 2 (hidden) + olleh| + {0: ~}|*4 + ## grid 3 + | + ## grid 4 (hidden) + hello | + {0:~ }|*4 + ## grid 5 (hidden) + {1:hello }| + {2:~ }|*4 + ## grid 6 + ^ | + {0: ~}|*4 + ]], + }) + else + screen:expect([[ + {9: + [No Name] }{3: [No Name] }{9: }{10:2}{9:+ No Name] }{5: }{9:X}| + ^ | + {0: ~}|*4 + | + ]]) + end + end) end describe('with ext_multigrid and actual mouse grid', function() From ef084b5c225c16b23e3e456e8d4d12138f415ea0 Mon Sep 17 00:00:00 2001 From: Sean Dewar <6256228+seandewar@users.noreply.github.com> Date: Wed, 11 Mar 2026 09:46:28 +0000 Subject: [PATCH 04/10] fix(api): don't config split as floatwin relative to itself Problem: possible to configure a split as a floatwin with relative=win that is relative to itself. Solution: fix the check. Not caused by this PR; just something I noticed when about to fix the validation logic. --- src/nvim/api/win_config.c | 2 +- test/functional/ui/float_spec.lua | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/src/nvim/api/win_config.c b/src/nvim/api/win_config.c index 1f9550a187..9c396bfce1 100644 --- a/src/nvim/api/win_config.c +++ b/src/nvim/api/win_config.c @@ -1360,7 +1360,7 @@ static bool parse_win_config(win_T *wp, Dict(win_config) *config, WinConfig *fco } if (relative_is_win || is_split) { - if (reconf && relative_is_win) { + if (wp && relative_is_win) { win_T *target_win = find_window_by_handle(config->win, err); if (!target_win) { goto fail; diff --git a/test/functional/ui/float_spec.lua b/test/functional/ui/float_spec.lua index 0369e55a30..1c7f309627 100644 --- a/test/functional/ui/float_spec.lua +++ b/test/functional/ui/float_spec.lua @@ -11191,6 +11191,9 @@ 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. + command('split') + eq('floating window cannot be relative to itself', pcall_err(api.nvim_win_set_config, 0, config)) end) it('bufpos out of range', function() From dc00f628a2f2fee31143edfd457d13e85f30846c Mon Sep 17 00:00:00 2001 From: Sean Dewar <6256228+seandewar@users.noreply.github.com> Date: Tue, 10 Mar 2026 10:20:58 +0000 Subject: [PATCH 05/10] refactor(window): lastwin_nofloating takes tp --- src/nvim/api/win_config.c | 12 ++---------- src/nvim/arglist.c | 2 +- src/nvim/buffer.c | 2 +- src/nvim/ex_getln.c | 2 +- src/nvim/statusline.c | 2 +- src/nvim/window.c | 16 +++++++++------- src/nvim/winfloat.c | 11 ++++------- 7 files changed, 19 insertions(+), 28 deletions(-) diff --git a/src/nvim/api/win_config.c b/src/nvim/api/win_config.c index 9c396bfce1..5c7f3a7d32 100644 --- a/src/nvim/api/win_config.c +++ b/src/nvim/api/win_config.c @@ -680,17 +680,9 @@ static bool win_config_float_tp(win_T *win, const Dict(win_config) *config, // Check again, in case autocommands above moved windows to the same tabpage. if (win_tp != parent_tp) { win_remove(win, win_tp == curtab ? NULL : win_tp); - win_T *after; - if (parent_tp == curtab) { - after = lastwin_nofloating(); - } else { - after = parent_tp->tp_lastwin; - while (after->w_floating) { - after = after->w_prev; - } - } + tabpage_T *append_tp = parent_tp == curtab ? NULL : parent_tp; + win_append(lastwin_nofloating(append_tp), win, append_tp); - win_append(after, win, parent_tp == curtab ? NULL : parent_tp); // If `win` was the curwin of its old tabpage, select a new curwin for it. if (win_tp != curtab && win_tp->tp_curwin == win) { win_tp->tp_curwin = win_float_find_altwin(win, win_tp); diff --git a/src/nvim/arglist.c b/src/nvim/arglist.c index 1d5903aefc..7d12c92555 100644 --- a/src/nvim/arglist.c +++ b/src/nvim/arglist.c @@ -1116,7 +1116,7 @@ static void do_arg_all(int count, int forceit, int keep_tabs) last_curwin = curwin; last_curtab = curtab; // lastwin may be aucmd_win - win_enter(lastwin_nofloating(), false); + win_enter(lastwin_nofloating(NULL), false); // Open up to "count" windows. arg_all_open_windows(&aall, count); diff --git a/src/nvim/buffer.c b/src/nvim/buffer.c index 373cc6263b..94b670b6dc 100644 --- a/src/nvim/buffer.c +++ b/src/nvim/buffer.c @@ -3692,7 +3692,7 @@ void ex_buffer_all(exarg_T *eap) // Don't execute Win/Buf Enter/Leave autocommands here. autocmd_no_enter++; // lastwin may be aucmd_win - win_enter(lastwin_nofloating(), false); + win_enter(lastwin_nofloating(NULL), false); autocmd_no_leave++; for (buf_T *buf = firstbuf; buf != NULL && open_wins < count; buf = buf->b_next) { // Check if this buffer needs a window diff --git a/src/nvim/ex_getln.c b/src/nvim/ex_getln.c index 08d01700d6..9433957762 100644 --- a/src/nvim/ex_getln.c +++ b/src/nvim/ex_getln.c @@ -4062,7 +4062,7 @@ void compute_cmdrow(void) if (exmode_active || msg_scrolled != 0) { cmdline_row = Rows - 1; } else { - win_T *wp = lastwin_nofloating(); + win_T *wp = lastwin_nofloating(NULL); cmdline_row = wp->w_winrow + wp->w_height + wp->w_hsep_height + wp->w_status_height + global_stl_height(); } diff --git a/src/nvim/statusline.c b/src/nvim/statusline.c index 005a52b675..82febeb6c5 100644 --- a/src/nvim/statusline.c +++ b/src/nvim/statusline.c @@ -440,7 +440,7 @@ void win_redr_winbar(win_T *wp) void redraw_ruler(void) { static int did_ruler_col = -1; - win_T *wp = curwin->w_status_height == 0 ? curwin : lastwin_nofloating(); + win_T *wp = curwin->w_status_height == 0 ? curwin : lastwin_nofloating(NULL); bool is_stl_global = global_stl_height() > 0; // Check if ruler should be drawn, clear if it was drawn before. diff --git a/src/nvim/window.c b/src/nvim/window.c index bdc3234532..ca1689803f 100644 --- a/src/nvim/window.c +++ b/src/nvim/window.c @@ -505,7 +505,7 @@ newwindow: // cursor to bottom-right window case 'b': case Ctrl_B: - win_goto(lastwin_nofloating()); + win_goto(lastwin_nofloating(NULL)); break; // cursor to last accessed (previous) window @@ -1158,7 +1158,7 @@ win_T *win_split_ins(int size, int flags, win_T *new_wp, int dir, frame_T *to_fl oldwin = firstwin; } else if (flags & WSP_BOT || curwin->w_floating) { // can't split float, use last nonfloating window instead - oldwin = lastwin_nofloating(); + oldwin = lastwin_nofloating(NULL); } else { oldwin = curwin; } @@ -4800,7 +4800,7 @@ static void tabpage_check_windows(tabpage_T *old_curtab) if (wp->w_floating) { if (wp->w_config.external) { win_remove(wp, old_curtab); - win_append(lastwin_nofloating(), wp, NULL); + win_append(lastwin_nofloating(NULL), wp, NULL); } else { ui_comp_remove_grid(&wp->w_grid_alloc); } @@ -6237,7 +6237,7 @@ static void frame_setheight(frame_T *curfrp, int height) if (curfrp->fr_width != Columns) { room_cmdline = 0; } else { - win_T *wp = lastwin_nofloating(); + win_T *wp = lastwin_nofloating(NULL); room_cmdline = Rows - (int)p_ch - global_stl_height() - (wp->w_winrow + wp->w_height + wp->w_hsep_height + wp->w_status_height); room_cmdline = MAX(room_cmdline, 0); @@ -7051,7 +7051,7 @@ void command_height(void) int old_p_ch = (int)curtab->tp_ch_used; // Find bottom frame with width of screen. - frame_T *frp = lastwin_nofloating()->w_frame; + frame_T *frp = lastwin_nofloating(NULL)->w_frame; while (frp->fr_width != Columns && frp->fr_parent != NULL) { frp = frp->fr_parent; } @@ -7804,9 +7804,11 @@ void win_ui_flush(bool validate) msg_ui_flush(); } -win_T *lastwin_nofloating(void) +/// @return last non-floating window in `tp`, or NULL for current tabpage. +win_T *lastwin_nofloating(tabpage_T *tp) { - win_T *res = lastwin; + assert(tp != curtab || !tp); + win_T *res = tp ? tp->tp_lastwin : lastwin; while (res->w_floating) { res = res->w_prev; } diff --git a/src/nvim/winfloat.c b/src/nvim/winfloat.c index 27d4b859d9..0d8b0ee84a 100644 --- a/src/nvim/winfloat.c +++ b/src/nvim/winfloat.c @@ -46,7 +46,7 @@ win_T *win_new_float(win_T *wp, bool last, WinConfig fconfig, Error *err) { if (wp == NULL) { tabpage_T *tp = NULL; - win_T *tp_last = last ? lastwin : lastwin_nofloating(); + win_T *tp_last = last ? lastwin : lastwin_nofloating(NULL); if (fconfig.window != 0) { assert(!last); win_T *parent_wp = find_window_by_handle(fconfig.window, err); @@ -57,10 +57,7 @@ win_T *win_new_float(win_T *wp, bool last, WinConfig fconfig, Error *err) if (!tp) { return NULL; } - tp_last = tp == curtab ? lastwin : tp->tp_lastwin; - while (tp_last->w_floating && tp_last->w_prev) { - tp_last = tp_last->w_prev; - } + tp_last = lastwin_nofloating(tp == curtab ? NULL : tp); } wp = win_alloc(tp_last, false); win_init(wp, curwin, 0); @@ -77,7 +74,7 @@ win_T *win_new_float(win_T *wp, bool last, WinConfig fconfig, Error *err) } else { assert(!last); assert(!wp->w_floating); - if (firstwin == wp && lastwin_nofloating() == wp) { + if (firstwin == wp && lastwin_nofloating(NULL) == wp) { // last non-float api_set_error(err, kErrorTypeException, "Cannot change last window into float"); @@ -105,7 +102,7 @@ win_T *win_new_float(win_T *wp, bool last, WinConfig fconfig, Error *err) XFREE_CLEAR(wp->w_frame); win_comp_pos(); // recompute window positions win_remove(wp, NULL); - win_append(lastwin_nofloating(), wp, NULL); + win_append(lastwin_nofloating(NULL), wp, NULL); } wp->w_floating = true; wp->w_status_height = wp->w_p_stl && *wp->w_p_stl != NUL 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 06/10] 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 = [[ From 3325536150eda0de4dc69f25beca416c0a0b3e3a Mon Sep 17 00:00:00 2001 From: Sean Dewar <6256228+seandewar@users.noreply.github.com> Date: Thu, 12 Mar 2026 09:19:06 +0000 Subject: [PATCH 07/10] fix(winfloat): last_status when changing split to floatwin Problem: converting a split to a floatwin may not remove the last statusline when needed. (e.g: 'ls' is 1) Solution: call last_status/win_comp_pos in win_new_float, after win_remove. Also fix float_pos formatting for screen snapshots so it doesn't give a nil error for external windows. Not an issue from this PR. --- src/nvim/winfloat.c | 3 ++- test/functional/api/window_spec.lua | 40 ++++++++++++++++++++++++++++ test/functional/ui/float_spec.lua | 5 ++-- test/functional/ui/messages_spec.lua | 3 +-- test/functional/ui/screen.lua | 11 +++++--- 5 files changed, 53 insertions(+), 9 deletions(-) diff --git a/src/nvim/winfloat.c b/src/nvim/winfloat.c index 0d8b0ee84a..de9fe13417 100644 --- a/src/nvim/winfloat.c +++ b/src/nvim/winfloat.c @@ -100,8 +100,9 @@ win_T *win_new_float(win_T *wp, bool last, WinConfig fconfig, Error *err) int dir; winframe_remove(wp, &dir, NULL, NULL); XFREE_CLEAR(wp->w_frame); - win_comp_pos(); // recompute window positions win_remove(wp, NULL); + last_status(false); // may need to remove last status line + win_comp_pos(); // recompute window positions win_append(lastwin_nofloating(NULL), wp, NULL); } wp->w_floating = true; diff --git a/test/functional/api/window_spec.lua b/test/functional/api/window_spec.lua index 579eb88663..d75dc78142 100644 --- a/test/functional/api/window_spec.lua +++ b/test/functional/api/window_spec.lua @@ -3241,6 +3241,46 @@ describe('API/win', function() pcall_err(api.nvim_win_set_config, win, cfg) ) end) + + it('removes last statusline if needed', function() + local screen = Screen.new(30, 9) + command('set laststatus=1 | botright split') + screen:expect([[ + | + {1:~ }|*2 + {2:[No Name] }| + ^ | + {1:~ }|*2 + {3:[No Name] }| + | + ]]) + api.nvim_win_set_config(0, { relative = 'editor', row = 0, col = 0, width = 4, height = 4 }) + screen:expect([[ + {4:^ } | + {11:~ }{1: }|*3 + {1:~ }|*4 + | + ]]) + command('quit | set laststatus=2 | botright split') + screen:expect([[ + | + {1:~ }|*2 + {2:[No Name] }| + ^ | + {1:~ }|*2 + {3:[No Name] }| + | + ]]) + api.nvim_win_set_config(0, { relative = 'editor', row = 1, col = 5, width = 4, height = 4 }) + screen:expect([[ + | + {1:~ }{4:^ }{1: }| + {1:~ }{11:~ }{1: }|*3 + {1:~ }|*2 + {2:[No Name] }| + | + ]]) + end) end) describe('get_config', function() diff --git a/test/functional/ui/float_spec.lua b/test/functional/ui/float_spec.lua index a3d4289520..9d3e83352c 100644 --- a/test/functional/ui/float_spec.lua +++ b/test/functional/ui/float_spec.lua @@ -7758,12 +7758,11 @@ describe('float window', function() screen:expect { grid = [[ ## grid 1 - [2:----------------------------------------]|*5 - {5:[No Name] [+] }| + [2:----------------------------------------]|*6 [3:----------------------------------------]| ## grid 2 x | - {0:~ }|*4 + {0:~ }|*5 ## grid 3 | ## grid 4 diff --git a/test/functional/ui/messages_spec.lua b/test/functional/ui/messages_spec.lua index 4159563fcf..223088b3e8 100644 --- a/test/functional/ui/messages_spec.lua +++ b/test/functional/ui/messages_spec.lua @@ -2346,8 +2346,7 @@ describe('ui/ext_messages', function() screen:expect([[ | {1:~ }{4:^ }{1: }| - {1:~ }|*21 - {2:[No Name] }| + {1:~ }|*22 ]]) end) diff --git a/test/functional/ui/screen.lua b/test/functional/ui/screen.lua index d9501b8400..d60992974a 100644 --- a/test/functional/ui/screen.lua +++ b/test/functional/ui/screen.lua @@ -1795,9 +1795,14 @@ local function fmt_ext_state(name, state) elseif name == 'float_pos' then local str = '{\n' for k, v in pairs(state) do - str = str .. ' [' .. k .. '] = {' .. v[1] - for i = 2, #v do - str = str .. ', ' .. inspect(v[i]) + str = str .. ' [' .. k .. '] = {' + if v.external then + str = str .. ' external = true ' + else + str = str .. v[1] + for i = 2, #v do + str = str .. ', ' .. inspect(v[i]) + end end str = str .. '};\n' end From 094b297a3b33f6336a825818bfd6a53b25273ac8 Mon Sep 17 00:00:00 2001 From: Sean Dewar <6256228+seandewar@users.noreply.github.com> Date: Wed, 11 Mar 2026 19:06:47 +0000 Subject: [PATCH 08/10] feat(api): nvim_win_set_config can move split to other tp as floatwin Problem: not possible for nvim_win_set_config to convert a split to a floatwin, then move it to another tabpage in one call. Solution: allow it. --- runtime/doc/news.txt | 2 +- src/nvim/api/win_config.c | 138 +++++++------- src/nvim/winfloat.c | 34 ++-- test/functional/api/window_spec.lua | 96 ++++++---- test/functional/ui/float_spec.lua | 270 +++++++++++++++++++++++++++- 5 files changed, 416 insertions(+), 124 deletions(-) diff --git a/runtime/doc/news.txt b/runtime/doc/news.txt index 9f2a348025..7e5f7565b1 100644 --- a/runtime/doc/news.txt +++ b/runtime/doc/news.txt @@ -167,7 +167,7 @@ API they were so specified in `nvim_create_user_command()`. • |nvim_open_win()| floating windows can show a 'statusline'. Plugins can use `style='minimal'` or `:setlocal statusline=` to hide the statusline. -• |nvim_win_set_config()| can move floating windows to other tab pages. +• |nvim_win_set_config()| can move windows to other tab pages as floats. • Added experimental |nvim__exec_lua_fast()| to allow remote API clients to execute code while nvim is blocking for input. • |vim.secure.trust()| accepts `path` for the `allow` action. diff --git a/src/nvim/api/win_config.c b/src/nvim/api/win_config.c index cf10883d81..9328c8d064 100644 --- a/src/nvim/api/win_config.c +++ b/src/nvim/api/win_config.c @@ -384,9 +384,13 @@ static int win_split_flags(WinSplit split, bool toplevel) } /// Checks if window `wp` can be moved to tabpage `tp`. -static bool win_can_move_tp(win_T *wp, Error *err) +static bool win_can_move_tp(win_T *wp, tabpage_T *tp, Error *err) FUNC_ATTR_NONNULL_ALL { + if (one_window(wp, tp == curtab ? NULL : tp)) { + api_set_error(err, kErrorTypeException, "Cannot move last non-floating window"); + return false; + } if (is_aucmd_win(wp)) { api_set_error(err, kErrorTypeException, "Cannot move autocmd window to another tabpage"); return false; @@ -403,6 +407,17 @@ static bool win_can_move_tp(win_T *wp, Error *err) return true; } +static win_T *win_find_altwin(win_T *win, tabpage_T *tp) + FUNC_ATTR_NONNULL_ALL +{ + if (win->w_floating) { + return win_float_find_altwin(win, tp == curtab ? NULL : tp); + } else { + int dir; + return winframe_find_altwin(win, &dir, tp == curtab ? NULL : tp, NULL); + } +} + /// Configures `win` into a split, also moving it to another tabpage if requested. static bool win_config_split(win_T *win, const Dict(win_config) *config, WinConfig *fconfig, Error *err) @@ -446,7 +461,7 @@ static bool win_config_split(win_T *win, const Dict(win_config) *config, WinConf api_set_error(err, kErrorTypeException, "Cannot split a floating window"); return false; } - if (win_tp != parent_tp && !win_can_move_tp(win, err)) { + if (win_tp != parent_tp && !win_can_move_tp(win, win_tp, err)) { return false; // error already set } } @@ -460,19 +475,9 @@ static bool win_config_split(win_T *win, const Dict(win_config) *config, WinConf // window list or remove its frame (if non-floating), so it's valid for autocommands. const bool curwin_moving_tp = win == curwin && parent && win_tp != parent_tp; if (curwin_moving_tp) { - if (was_split) { - int dir; - win_T *altwin = winframe_find_altwin(win, &dir, NULL, NULL); - // Autocommands may still make this the last non-float after this check. - // That case will be caught later when trying to move the window. - if (!altwin) { - api_set_error(err, kErrorTypeException, "Cannot move last non-floating window"); - return false; - } - win_goto(altwin); - } else { - win_goto(win_float_find_altwin(win, NULL)); - } + win_T *altwin = win_find_altwin(win, win_tp); + assert(altwin); // win_can_move_tp ensures `win` is not the only window + win_goto(altwin); // Autocommands may have been a real nuisance and messed things up... if (curwin == win) { @@ -642,12 +647,21 @@ static bool win_config_float_tp(win_T *win, const Dict(win_config) *config, parent_tp = win_find_tabpage(parent); } + bool curwin_moving_tp = false; + win_T *altwin = NULL; + if (win_tp != parent_tp) { - if (!win_can_move_tp(win, err)) { + if (!win_can_move_tp(win, win_tp, err)) { return false; // error already set } + altwin = win_find_altwin(win, win_tp); + assert(altwin); // win_can_move_tp ensures `win` is not the only window + + // If we are moving curwin to another tabpage, switch windows *before* we remove it from the + // window list or remove its frame (if non-floating), so it's valid for autocommands. if (curwin == win) { - win_goto(win_float_find_altwin(win, NULL)); + curwin_moving_tp = true; + win_goto(altwin); // Autocommands may have been a real nuisance and messed things up... if (curwin == win) { @@ -658,47 +672,51 @@ static bool win_config_float_tp(win_T *win, const Dict(win_config) *config, win_tp = win_find_tabpage(win); parent_tp = win_find_tabpage(parent); - bool restore_curwin = false; if (!win_tp || !parent_tp) { api_set_error(err, kErrorTypeException, "Target windows were closed"); - restore_curwin = true; - } else if (!win->w_floating) { - api_set_error(err, kErrorTypeException, "Window %d was made non-floating", win->handle); - restore_curwin = true; - } else if (win_tp != parent_tp && !win_can_move_tp(win, err)) { - restore_curwin = true; // error already set + goto restore_curwin; } - if (restore_curwin) { - // As `win` was the original curwin, and autocommands didn't move it outside of curtab, be - // a good citizen and try to return to it. - if (win_valid(win)) { - win_goto(win); - } - return false; + if (win_tp != parent_tp && !win_can_move_tp(win, win_tp, err)) { + goto restore_curwin; // error already set } + altwin = win_find_altwin(win, win_tp); + assert(altwin); // win_can_move_tp ensures `win` is not the only window + } + } + + // Convert the window to a float if needed. + if (!win->w_floating) { + if (!win_new_float(win, false, *fconfig, err)) { +restore_curwin: + // If `win` was the original curwin, and autocommands didn't move it outside of curtab, be a + // good citizen and try to return to it. + if (curwin_moving_tp && win_valid(win)) { + win_goto(win); + } + return false; + } + redraw_later(win, UPD_NOT_VALID); + } + + if (win_tp != parent_tp) { + win_remove(win, win_tp == curtab ? NULL : win_tp); + tabpage_T *append_tp = parent_tp == curtab ? NULL : parent_tp; + win_append(lastwin_nofloating(append_tp), win, append_tp); + + // If `win` was the curwin of its old tabpage, select a new curwin for it. + if (win_tp != curtab && win_tp->tp_curwin == win) { + win_tp->tp_curwin = altwin; } - // Check again, in case autocommands above moved windows to the same tabpage. - if (win_tp != parent_tp) { - win_remove(win, win_tp == curtab ? NULL : win_tp); - tabpage_T *append_tp = parent_tp == curtab ? NULL : parent_tp; - win_append(lastwin_nofloating(append_tp), win, append_tp); + // Remove grid if present. More reliable than checking curtab, as tabpage_check_windows may not + // run when temporarily switching tabpages, meaning grids may be stale from another tabpage! + // (e.g: switch_win_noblock with no_display=true) + ui_comp_remove_grid(&win->w_grid_alloc); - // If `win` was the curwin of its old tabpage, select a new curwin for it. - if (win_tp != curtab && win_tp->tp_curwin == win) { - win_tp->tp_curwin = win_float_find_altwin(win, win_tp); - } - - // Remove grid if present. More reliable than checking curtab, as tabpage_check_windows may - // not run when temporarily switching tabpages, meaning grids may be stale from another - // tabpage! (e.g: switch_win_noblock with no_display=true) - ui_comp_remove_grid(&win->w_grid_alloc); - - // Redraw tabline, update window's hl attribs, etc. Set must_redraw here, as redraw_later - // might not if w_redr_type >= UPD_NOT_VALID was set in the old tabpage. - redraw_later(win, UPD_NOT_VALID); - set_must_redraw(UPD_NOT_VALID); - } + // Redraw tabline, update window's hl attribs, etc. Set must_redraw here, as redraw_later might + // not if w_redr_type >= UPD_NOT_VALID was set in the old tabpage. + redraw_later(win, UPD_NOT_VALID); + set_must_redraw(UPD_NOT_VALID); } win_config_float(win, *fconfig); @@ -745,27 +763,11 @@ void nvim_win_set_config(Window window, Dict(win_config) *config, Error *err) return; } - 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; - } - redraw_later(win, UPD_NOT_VALID); - } else if (to_split) { + if (to_split) { if (!win_config_split(win, config, &fconfig, err)) { return; } } else { - assert(!was_split); if (!win_config_float_tp(win, config, &fconfig, err)) { return; } diff --git a/src/nvim/winfloat.c b/src/nvim/winfloat.c index de9fe13417..f222f6b17e 100644 --- a/src/nvim/winfloat.c +++ b/src/nvim/winfloat.c @@ -35,10 +35,9 @@ #include "winfloat.c.generated.h" -/// Create a new float. +/// Creates a new float, or transforms an existing window to a float. /// /// @param wp if NULL, allocate a new window, otherwise turn existing window into a float. -/// It must then already belong to the current tabpage! /// @param last make the window the last one in the window list. /// Only used when allocating the autocommand window. /// @param config must already have been validated! @@ -74,19 +73,19 @@ win_T *win_new_float(win_T *wp, bool last, WinConfig fconfig, Error *err) } else { assert(!last); assert(!wp->w_floating); - if (firstwin == wp && lastwin_nofloating(NULL) == wp) { - // last non-float - api_set_error(err, kErrorTypeException, - "Cannot change last window into float"); - return NULL; - } else if (!win_valid(wp)) { - api_set_error(err, kErrorTypeException, - "Cannot change window from different tabpage into float"); + tabpage_T *win_tp = win_find_tabpage(wp); + assert(win_tp); + if ((win_tp == curtab && firstwin == wp && lastwin_nofloating(NULL) == wp) + || (win_tp != curtab && win_tp->tp_firstwin == wp && lastwin_nofloating(win_tp) == wp)) { + api_set_error(err, kErrorTypeException, "Cannot change last window into float"); return NULL; } else if (cmdwin_win != NULL && !cmdwin_win->w_floating) { // cmdwin can't become the only non-float. Check for others. bool other_nonfloat = false; - for (win_T *wp2 = firstwin; wp2 != NULL && !wp2->w_floating; wp2 = wp2->w_next) { + FOR_ALL_WINDOWS_IN_TAB(wp2, win_tp) { + if (wp2->w_floating) { + break; + } if (wp2 != wp && wp2 != cmdwin_win) { other_nonfloat = true; break; @@ -97,13 +96,16 @@ win_T *win_new_float(win_T *wp, bool last, WinConfig fconfig, Error *err) return NULL; } } + tabpage_T *tp = win_tp == curtab ? NULL : win_tp; int dir; - winframe_remove(wp, &dir, NULL, NULL); + winframe_remove(wp, &dir, tp, NULL); XFREE_CLEAR(wp->w_frame); - win_remove(wp, NULL); - last_status(false); // may need to remove last status line - win_comp_pos(); // recompute window positions - win_append(lastwin_nofloating(NULL), wp, NULL); + win_remove(wp, tp); + if (win_tp == curtab) { + last_status(false); // may need to remove last status line + win_comp_pos(); // recompute window positions + } + win_append(lastwin_nofloating(tp), wp, tp); } wp->w_floating = true; wp->w_status_height = wp->w_p_stl && *wp->w_p_stl != NUL diff --git a/test/functional/api/window_spec.lua b/test/functional/api/window_spec.lua index d75dc78142..9937c5270b 100644 --- a/test/functional/api/window_spec.lua +++ b/test/functional/api/window_spec.lua @@ -2585,15 +2585,17 @@ 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 } - ) + -- directly convert split into a float for a different tabpage + local win2 = api.nvim_open_win(0, true, { split = 'below' }) + eq('', api.nvim_win_get_config(win2).relative) + api.nvim_win_set_config( + win2, + { relative = 'editor', row = 0, col = 0, width = 1, height = 1, win = first_win } ) + eq(first_tab, api.nvim_win_get_tabpage(win2)) + eq('editor', api.nvim_win_get_config(win2).relative) + eq({ first_win, win2 }, api.nvim_tabpage_list_wins(first_tab)) + eq({ tab2_win, win }, api.nvim_tabpage_list_wins(new_tab)) -- convert new win to float in new tabpage api.nvim_win_set_config(win, { relative = 'editor', row = 2, col = 2, height = 2, width = 2 }) @@ -2601,10 +2603,23 @@ describe('API/win', function() -- move to other tabpage 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({ first_win, win, win2 }, 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, { win = -1 })) + + -- can't convert only window in other tabpage to float + command('tabnew') + local only_win = api.nvim_get_current_win() + command('tabprevious') + eq( + 'Cannot change last window into float', + pcall_err( + api.nvim_win_set_config, + only_win, + { relative = 'editor', width = 5, height = 5, row = 0, col = 0 } + ) + ) end) it('correctly moves curwin when moving curwin to a different tabpage', function() @@ -2773,7 +2788,6 @@ describe('API/win', function() it('messing with "win" or "parent" when moving "win" to other tabpage', function() command('split | tabnew') - local t2 = api.nvim_get_current_tabpage() local t2_win1 = api.nvim_get_current_win() command('split') local t2_win2 = api.nvim_get_current_win() @@ -2818,23 +2832,19 @@ describe('API/win', function() eq('', api.nvim_win_get_config(0).relative) eq(cur_win, api.nvim_get_current_win()) - -- Try to make "parent" floating. This should give the same error as before, but because - -- changing a split from another tabpage into a float isn't supported yet, check for that - -- error instead for now. - -- Use ":silent!" to avoid the one second delay from printing the error message. + -- Try to make "parent" floating. This should give the same error as before. exec(([[ - autocmd WinLeave * ++once silent! + autocmd WinLeave * ++once \ call nvim_win_set_config(%d, #{relative:'editor', row:0, col:0, width:5, height:5}) ]]):format(t2_win3)) cur_win = api.nvim_get_current_win() - api.nvim_win_set_config(0, { win = t2_win3, split = 'left' }) - matches( - 'Cannot change window from different tabpage into float$', - api.nvim_get_vvar('errmsg') + eq( + 'Floating state of windows to split changed', + pcall_err(api.nvim_win_set_config, 0, { win = t2_win3, split = 'left' }) ) - -- The error doesn't abort moving the window (or maybe it should, if that's wanted?) - neq(cur_win, api.nvim_get_current_win()) - eq(t2, api.nvim_win_get_tabpage(cur_win)) + eq('editor', api.nvim_win_get_config(t2_win3).relative) + eq('', api.nvim_win_get_config(0).relative) + eq(cur_win, api.nvim_get_current_win()) end) it('expected autocmds when moving window to other tabpage', function() @@ -3228,6 +3238,35 @@ describe('API/win', function() api.nvim_win_set_config(t2_cur_win, { split = 'left', win = 0 }) eq(t2_alt_win, api.nvim_tabpage_get_win(t2)) eq(t1, api.nvim_win_get_tabpage(t2_cur_win)) + + -- Very fun: move curwin between tabpages, converting from split to float, but with an autocmd + -- that deletes altwin after we're bumped to it, re-enters curwin, then switches to a 3rd + -- tabpage. tp_curwin of the window's old tabpage shouldn't be set to the freed altwin! + command('tablast | tab split | tabprevious | split') + command('autocmd WinEnter * ++once quit | let expect_alt = win_getid() | wincmd p | tabnext') + api.nvim_win_set_config(0, { + relative = 'editor', + win = api.nvim_tabpage_get_win(t1), + row = 0, + col = 0, + width = 5, + height = 5, + }) + eq(eval('g:expect_alt'), api.nvim_tabpage_get_win(t2)) + + -- Same, but for float -> float. + command('tabprevious | split') + api.nvim_open_win(0, true, { relative = 'editor', row = 0, col = 0, width = 1, height = 1 }) + command('autocmd WinEnter * ++once quit | let expect_alt = win_getid() | wincmd p | tabnext') + api.nvim_win_set_config(0, { + relative = 'editor', + win = api.nvim_tabpage_get_win(t1), + row = 0, + col = 0, + width = 5, + height = 5, + }) + eq(eval('g:expect_alt'), api.nvim_tabpage_get_win(t2)) end) it('set_config cannot change "noautocmd" #36409', function() @@ -3742,7 +3781,6 @@ describe('API/win', function() it('preserve current floating window when moving fails', function() local buf = api.nvim_create_buf(false, true) - local tab1_win = api.nvim_get_current_win() local float_win = api.nvim_open_win(buf, true, { relative = 'editor', row = 1, @@ -3770,18 +3808,6 @@ describe('API/win', function() ) eq(float_win, api.nvim_get_current_win()) - command( - ('autocmd WinLeave * ++once call nvim_win_set_config(%d, #{split: "left", win: %d})'):format( - float_win, - tab1_win - ) - ) - eq( - ('Window %d was made non-floating'):format(float_win), - 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 }) diff --git a/test/functional/ui/float_spec.lua b/test/functional/ui/float_spec.lua index 9d3e83352c..e9f608099c 100644 --- a/test/functional/ui/float_spec.lua +++ b/test/functional/ui/float_spec.lua @@ -3782,10 +3782,7 @@ describe('float window', function() it('API has proper error messages', 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( - "Must specify 'relative' or 'external' when creating a float", - pcall_err(api.nvim_open_win, buf, false, { win = 0 }) - ) + eq("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'", pcall_err(api.nvim_open_win, buf, false, { width = 20, height = 2, relative = 'editor', row = 0, col = 0, vertical = true }) @@ -12030,6 +12027,7 @@ describe('float window', function() -- Check tablines are redrawn even when moving floats between two non-current tabpages. command('tabnew') + local tab3_win = api.nvim_get_current_win() if multigrid then screen:expect({ grid = [[ @@ -12094,6 +12092,270 @@ describe('float window', function() | ]]) end + + -- Try converting a split to a float, then moving it to another tabpage in one call. + command('set norightleft | new') + fn.setline(1, 'floaty mcfloatface') + api.nvim_win_set_config(0, { relative = 'editor', win = tab1_win, row = 3, col = 3, width = 15, height = 5 }) + if multigrid then + screen:expect({ + grid = [[ + ## grid 1 + {9: }{10:2}{9:+ No Name] }{3: [No Name] }{9: }{10:2}{9:+ No Name] }{5: }{9:X}| + [6:----------------------------------------]|*5 + [3:----------------------------------------]| + ## grid 2 (hidden) + olleh| + {0: ~}|*4 + ## grid 3 + | + ## grid 4 (hidden) + hello | + {0:~ }|*4 + ## grid 5 (hidden) + {1:hello }| + {2:~ }|*4 + ## grid 6 + ^ | + {0:~ }|*4 + ## grid 7 (hidden) + floaty mcfloatface | + {0:~ }| + ]], + }) + else + screen:expect([[ + {9: }{10:2}{9:+ No Name] }{3: [No Name] }{9: }{10:2}{9:+ No Name] }{5: }{9:X}| + ^ | + {0:~ }|*4 + | + ]]) + end + + command('tabfirst') + if multigrid then + screen:expect({ + grid = [[ + ## grid 1 + {3: }{11:2}{3:+ No Name] }{9: [No Name] }{10:2}{9:+ No Name] }{5: }{9:X}| + [2:----------------------------------------]|*5 + [3:----------------------------------------]| + ## grid 2 + olle^h| + {0: ~}|*4 + ## grid 3 + | + ## grid 4 (hidden) + hello | + {0:~ }|*4 + ## grid 5 (hidden) + {1:hello }| + {2:~ }|*4 + ## grid 6 (hidden) + | + {0:~ }|*4 + ## grid 7 + {1:floaty mcfloatf}| + {1:ace }| + {2:~ }|*3 + ]], + float_pos = { + [7] = { 1004, 'NW', 1, 3, 3, true, 50, 1, 1, 3 }, + }, + }) + else + screen:expect([[ + {3: }{11:2}{3:+ No Name] }{9: [No Name] }{10:2}{9:+ No Name] }{5: }{9:X}| + {1:floaty mcfloatf} olle^h| + {0: }{1:ace }{0: ~}| + {0: }{2:~ }{0: ~}|*3 + | + ]]) + end + + -- Works when doing the same between two non-current tabpages. + local float2 = api.nvim_open_win(0, false, { split = 'below', win = tab3_win }) + api.nvim_win_set_config(float2, { relative = 'win', win = tab2_win, row = 2, col = 7, width = 4, height = 4, border = 'single' }) + if multigrid then + screen:expect({ + grid = [[ + ## grid 1 + {3: }{11:2}{3:+ No Name] }{9: [No Name] }{10:3}{9:+ No Name] }{5: }{9:X}| + [2:----------------------------------------]|*5 + [3:----------------------------------------]| + ## grid 2 + olle^h| + {0: ~}|*4 + ## grid 3 + | + ## grid 4 (hidden) + hello | + {0:~ }|*4 + ## grid 5 (hidden) + {1:hello }| + {2:~ }|*4 + ## grid 6 (hidden) + | + {0:~ }|*4 + ## grid 7 + {1:floaty mcfloatf}| + {1:ace }| + {2:~ }|*3 + ]], + float_pos = { + [7] = { 1004, 'NW', 1, 3, 3, true, 50, 1, 1, 3 }, + }, + }) + else + screen:expect([[ + {3: }{11:2}{3:+ No Name] }{9: [No Name] }{10:3}{9:+ No Name] }{5: }{9:X}| + {1:floaty mcfloatf} olle^h| + {0: }{1:ace }{0: ~}| + {0: }{2:~ }{0: ~}|*3 + | + ]]) + end + + command('tabnext') + if multigrid then + screen:expect({ + grid = [[ + ## grid 1 + {9: }{10:2}{9:+ No Name] }{3: [No Name] }{9: }{10:3}{9:+ No Name] }{5: }{9:X}| + [6:----------------------------------------]|*5 + [3:----------------------------------------]| + ## grid 2 (hidden) + olleh| + {0: ~}|*4 + ## grid 3 + | + ## grid 4 (hidden) + hello | + {0:~ }|*4 + ## grid 5 (hidden) + {1:hello }| + {2:~ }|*4 + ## grid 6 + ^ | + {0:~ }|*4 + ## grid 7 (hidden) + {1:floaty mcfloatf}| + {1:ace }| + {2:~ }|*3 + ]], + }) + else + screen:expect([[ + {9: }{10:2}{9:+ No Name] }{3: [No Name] }{9: }{10:3}{9:+ No Name] }{5: }{9:X}| + ^ | + {0:~ }|*4 + | + ]]) + end + + command('tabnext') + if multigrid then + screen:expect({ + grid = [[ + ## grid 1 + {9: }{10:2}{9:+ No Name] [No Name] }{3: }{11:3}{3:+ No Name] }{5: }{9:X}| + [4:----------------------------------------]|*5 + [3:----------------------------------------]| + ## grid 2 (hidden) + olleh| + {0: ~}|*4 + ## grid 3 + | + ## grid 4 + ^hello | + {0:~ }|*4 + ## grid 5 + {1:hello }| + {2:~ }|*4 + ## grid 6 (hidden) + | + {0:~ }|*4 + ## grid 7 (hidden) + {1:floaty mcfloatf}| + {1:ace }| + {2:~ }|*3 + ## grid 8 + {5:┌────┐}| + {5:│}{1:lleh}{5:│}| + {5:│}{1: o}{5:│}| + {5:│}{2: ~}{5:│}|*2 + {5:└────┘}| + ]], + float_pos = { + [5] = { 1002, 'NW', 4, 0, 0, true, 50, 1, 1, 0 }, + [8] = { 1005, 'NW', 4, 2, 7, true, 50, 2, 0, 7 }, + }, + }) + else + screen:expect([[ + {9: }{10:2}{9:+ No }{5:┌────┐}{9: [No Name] }{3: }{11:3}{3:+ No Name] }{5: }{9:X}| + {1:^hello }{5:│}{1:lleh}{5:│} | + {2:~ }{5:│}{1: o}{5:│}{0: }| + {2:~ }{5:│}{2: ~}{5:│}{0: }|*2 + {2:~ }{5:└────┘}{0: }| + | + ]]) + end + + -- Used relative=win on two floats relative to this window. + -- Split it to the right to ensure both follow. + command('topleft vsplit') + if multigrid then + screen:expect({ + grid = [[ + ## grid 1 + {9: }{10:2}{9:+ No Name] [No Name] }{3: }{11:4}{3:+ No Name] }{5: }{9:X}| + [9:--------------------]{5:│}[4:-------------------]|*4 + {4:[No Name] [+] }{5:[No Name] [+] }| + [3:----------------------------------------]| + ## grid 2 (hidden) + olleh| + {0: ~}|*4 + ## grid 3 + | + ## grid 4 + hello | + {0:~ }|*3 + ## grid 5 + {1:hello }| + {2:~ }|*4 + ## grid 6 (hidden) + | + {0:~ }|*4 + ## grid 7 (hidden) + {1:floaty mcfloatf}| + {1:ace }| + {2:~ }|*3 + ## grid 8 + {5:┌────┐}| + {5:│}{1:lleh}{5:│}| + {5:│}{1: o}{5:│}| + {5:│}{2: ~}{5:│}|*2 + {5:└────┘}| + ## grid 9 + ^hello | + {0:~ }|*3 + ]], + float_pos = { + [5] = { 1002, 'NW', 4, 0, 0, true, 50, 1, 1, 21 }, + [8] = { 1005, 'NW', 4, 2, 7, true, 50, 2, 0, 28 }, + }, + }) + else + screen:expect([[ + {9: }{10:2}{9:+ No Name] [No Name] }{3: }{11:4}{3:+ }{5:┌────┐}{3:e] }{5: }{9:X}| + ^hello {5:│}{1:hello }{5:│}{1:lleh}{5:│} | + {0:~ }{5:│}{2:~ }{5:│}{1: o}{5:│}{0: }| + {0:~ }{5:│}{2:~ }{5:│}{2: ~}{5:│}{0: }|*2 + {4:[No Name] [+] }{2:~ }{5:└────┘ }| + | + ]]) + end end) end From 853eea859fd31e7ffcbe6d09e1281f77f279f7a0 Mon Sep 17 00:00:00 2001 From: Sean Dewar <6256228+seandewar@users.noreply.github.com> Date: Thu, 12 Mar 2026 17:31:37 +0000 Subject: [PATCH 09/10] fix(api): disallow moving window between tabpages in more cases Problem: more cases where it may not be safe to move a window between tabpages. Solution: check them. Rather speculative... I haven't spend much time looking, but I didn't find existing code that sets these locks to skip checking win_valid. (what I did find called it anyway, like in win_close) Still, I think it's a good precaution for what future code might do. If the fact that nvim_win_set_config *actually* moves windows between tabpages causes unforeseen issues, "faking" it like ":wincmd T" may be an alternative: split a new window, close the old one, but instead also block autocmds, copy the old window's config, and give it its handle? --- src/nvim/api/win_config.c | 16 +++++++++++++ src/nvim/window.c | 19 ++++++++++++--- test/functional/api/window_spec.lua | 36 ++++++++++++++++++++++++++++- 3 files changed, 67 insertions(+), 4 deletions(-) diff --git a/src/nvim/api/win_config.c b/src/nvim/api/win_config.c index 9328c8d064..297b63e553 100644 --- a/src/nvim/api/win_config.c +++ b/src/nvim/api/win_config.c @@ -18,6 +18,8 @@ #include "nvim/drawscreen.h" #include "nvim/errors.h" #include "nvim/eval/window.h" +#include "nvim/ex_cmds_defs.h" +#include "nvim/ex_docmd.h" #include "nvim/globals.h" #include "nvim/highlight_group.h" #include "nvim/macros_defs.h" @@ -391,6 +393,20 @@ static bool win_can_move_tp(win_T *wp, tabpage_T *tp, Error *err) api_set_error(err, kErrorTypeException, "Cannot move last non-floating window"); return false; } + // Like closing, moving windows between tabpages makes win_valid return false. Helpful when e.g: + // walking the window list, as w_next/w_prev can unexpectedly refer to windows in another tabpage! + // Check related locks, in case they were set to avoid checking win_valid. + if (win_locked(wp)) { + api_set_error(err, kErrorTypeException, "Cannot move window to another tabpage whilst in use"); + return false; + } + if (window_layout_locked_err(CMD_SIZE, err)) { + return false; // error already set + } + if (textlock || expr_map_locked()) { + api_set_error(err, kErrorTypeException, "%s", e_textlock); + return false; + } if (is_aucmd_win(wp)) { api_set_error(err, kErrorTypeException, "Cannot move autocmd window to another tabpage"); return false; diff --git a/src/nvim/window.c b/src/nvim/window.c index ca1689803f..da4f74fc76 100644 --- a/src/nvim/window.c +++ b/src/nvim/window.c @@ -28,7 +28,6 @@ #include "nvim/eval/window.h" #include "nvim/ex_cmds.h" #include "nvim/ex_cmds2.h" -#include "nvim/ex_cmds_defs.h" #include "nvim/ex_docmd.h" #include "nvim/ex_eval.h" #include "nvim/ex_getln.h" @@ -143,12 +142,26 @@ bool frames_locked(void) /// error message. When closing window(s) and the command isn't easy to know, /// passing CMD_SIZE will also work. bool window_layout_locked(cmdidx_T cmd) +{ + Error err = ERROR_INIT; + const bool locked = window_layout_locked_err(cmd, &err); + if (ERROR_SET(&err)) { + emsg(_(err.msg)); + api_clear_error(&err); + } + return locked; +} + +/// Like `window_layout_locked`, but set `err` to the (untranslated) error message when locked. +/// @see window_layout_locked +bool window_layout_locked_err(cmdidx_T cmd, Error *err) { if (split_disallowed > 0 || close_disallowed > 0) { if (close_disallowed == 0 && cmd == CMD_tabnew) { - emsg(_(e_cannot_split_window_when_closing_buffer)); + api_set_error(err, kErrorTypeException, "%s", e_cannot_split_window_when_closing_buffer); } else { - emsg(_(e_not_allowed_to_change_window_layout_in_this_autocmd)); + api_set_error(err, kErrorTypeException, "%s", + e_not_allowed_to_change_window_layout_in_this_autocmd); } return true; } diff --git a/test/functional/api/window_spec.lua b/test/functional/api/window_spec.lua index 9937c5270b..f7a50bfa02 100644 --- a/test/functional/api/window_spec.lua +++ b/test/functional/api/window_spec.lua @@ -3168,7 +3168,6 @@ describe('API/win', function() ) command('quit!') - -- Can't switch away from window before moving it to a different tabpage during textlock. exec(([[ new call setline(1, 'foo') @@ -3180,6 +3179,41 @@ describe('API/win', function() pcall_err(command, 'normal! ==') ) eq(cur_win, api.nvim_get_current_win()) + exec(([[ + wincmd p + call setline(1, 'bar') + setlocal indentexpr=nvim_win_set_config(win_getid(winnr('#')),#{split:'left',win:%d}) + ]]):format(t2_win)) + neq(cur_win, api.nvim_get_current_win()) + matches( + 'E565: Not allowed to change text or change window$', + pcall_err(command, 'normal! ==') + ) + -- expr_map_lock + exec(([[ + nnoremap @ nvim_win_set_config(win_getid(winnr('#')),#{split:'left',win:%d}) + ]]):format(t2_win)) + neq(cur_win, api.nvim_get_current_win()) + matches( + 'E565: Not allowed to change text or change window$', + pcall_err(fn.feedkeys, '@', 'x') + ) + + exec(([[ + wincmd p + autocmd WinNewPre * ++once call nvim_win_set_config(0, #{relative:'editor', win:%d, row:0, col:0, width:1, height:1}) + ]]):format(t2_win)) + matches( + 'E1312: Not allowed to change the window layout in this autocmd$', + pcall_err(command, 'split') + ) + eq(cur_win, api.nvim_get_current_win()) -- :split didn't enter new window due to error + + exec(([[ + autocmd WinLeave * ++once call nvim_win_set_config(0, #{relative:'editor', win:%d, row:0, col:0, width:1, height:1}) + ]]):format(t2_win)) + matches('Cannot move window to another tabpage whilst in use$', pcall_err(command, 'quit')) + eq(cur_win, api.nvim_get_current_win()) -- :quit didn't close window due to error end) it('updates statusline when moving bottom split', function() From 3115e3d0d11e0e02ba9040ac1a7bb032afa762bb Mon Sep 17 00:00:00 2001 From: Sean Dewar <6256228+seandewar@users.noreply.github.com> Date: Fri, 13 Mar 2026 11:52:01 +0000 Subject: [PATCH 10/10] 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... --- src/nvim/api/win_config.c | 37 ++++++++++++----------- test/functional/api/window_spec.lua | 47 +++++++++++------------------ test/functional/ui/float_spec.lua | 12 ++++++++ 3 files changed, 49 insertions(+), 47 deletions(-) diff --git a/src/nvim/api/win_config.c b/src/nvim/api/win_config.c index 297b63e553..921a622c6e 100644 --- a/src/nvim/api/win_config.c +++ b/src/nvim/api/win_config.c @@ -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; diff --git a/test/functional/api/window_spec.lua b/test/functional/api/window_spec.lua index f7a50bfa02..bd7189e084 100644 --- a/test/functional/api/window_spec.lua +++ b/test/functional/api/window_spec.lua @@ -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) diff --git a/test/functional/ui/float_spec.lua b/test/functional/ui/float_spec.lua index e9f608099c..c9453e2b32 100644 --- a/test/functional/ui/float_spec.lua +++ b/test/functional/ui/float_spec.lua @@ -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()