From e2a0748cb28fb44f6ee42f7470ef50a9dda6e320 Mon Sep 17 00:00:00 2001 From: glepnir Date: Fri, 5 Sep 2025 13:35:27 +0800 Subject: 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 +++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 104 insertions(+), 7 deletions(-) (limited to 'src/nvim') 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 } -- cgit 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: 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. --- src/nvim/api/win_config.c | 206 +++++++++++++++++++++++----------------------- 1 file changed, 104 insertions(+), 102 deletions(-) (limited to 'src/nvim') 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 } -- cgit 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: 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 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) (limited to 'src/nvim') 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); } } -- cgit 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: 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 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/nvim') 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; -- cgit 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: 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(-) (limited to 'src/nvim') 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 -- cgit 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: 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. --- src/nvim/api/win_config.c | 59 +++++++++++++++++++++++++++-------------------- 1 file changed, 34 insertions(+), 25 deletions(-) (limited to 'src/nvim') 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; } } -- cgit 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: 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 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'src/nvim') 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; -- cgit 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: 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. --- src/nvim/api/win_config.c | 132 +++++++++++++++++++++++----------------------- src/nvim/winfloat.c | 34 ++++++------ 2 files changed, 85 insertions(+), 81 deletions(-) (limited to 'src/nvim') 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 } + } - // 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); - - // 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); + // 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); + } - // 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_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); - // 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); + // 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; } + + // 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); } 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 -- cgit 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: 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 ++++++++++++++++--- 2 files changed, 32 insertions(+), 3 deletions(-) (limited to 'src/nvim') 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; } -- cgit 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: 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 +++++++++++++++++++------------------ 1 file changed, 19 insertions(+), 18 deletions(-) (limited to 'src/nvim') 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; -- cgit