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] 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)