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.
2026-03-28 03:12:00 +00:00 · 2026-03-10 09:05:27 +00:00
parent e2a0748cb2
commit e5240b35c3
3 changed files with 174 additions and 113 deletions
--- 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.
--- 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
 }
--- 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)