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

@@ -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
+                  • win: |window-ID| target window. Can be in a different tab
-                    creating a float (relative="win"). When splitting,
+                    page. Determines the window to split (negative values act
-                    negative value works like |:topleft|, |:botright|.
+                    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

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").
+--- - win: `window-ID` target window. Can be in a different tab page. Determines the window to
----     When splitting, negative value works like `:topleft`, `:botright`.
+---     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:

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").
+///   - win: |window-ID| target window. Can be in a different tab page. Determines the window to
-///       When splitting, negative value works like |:topleft|, |:botright|.
+///       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 (relative_is_win || (HAS_KEY_X(config, win) && !is_split && wp && wp->w_floating
-    if (wp && relative_is_win) {
+                          && fconfig->relative == kFloatRelativeWindow)) {
-      win_T *target_win = find_window_by_handle(config->win, err);
+    // When relative=win is given, missing win field means win=0.
-      if (!target_win) {
+    win_T *target_win = find_window_by_handle(config->win, err);
-        goto fail;
+    if (!target_win) {
-      }
+      goto fail;
-
-      if (target_win == wp) {
-        api_set_error(err, kErrorTypeException, "floating window cannot be relative to itself");
-        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) {
+      if (!is_split && !has_relative && (!wp || !wp->w_floating)) {
-        fconfig->window = config->win;
+        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)) {
+    // Resolve, but skip validating. E.g: win_config_split accepts negative "win".
-    if (has_relative) {
+    if (fconfig->window == 0) {
-      api_set_error(err, kErrorTypeValidation,
+      fconfig->window = curwin->handle;
-                    "'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;
     }
   }
 

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(
+      eq('Invalid window id: -1', pcall_err(api.nvim_win_set_config, win, { win = -1 }))
-        '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()
@@ -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(
+          float_ok, float_err = pcall(vim.api.nvim_win_set_config, 0, { win = other_tp_win })
-            vim.api.nvim_win_set_config,
-            0,
-            { relative = 'win', win = other_tp_win, row = 0, col = 0 }
-          )
         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(
+        pcall_err(api.nvim_win_set_config, float_win, { win = tab2_win })
-          api.nvim_win_set_config,
-          float_win,
-          { relative = 'win', win = tab2_win, row = 0, col = 0 }
-        )
       )
       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(
+        pcall_err(api.nvim_win_set_config, float_win, { win = tab3_win })
-          api.nvim_win_set_config,
-          float_win,
-          { relative = 'win', win = tab3_win, row = 0, col = 0 }
-        )
       )
       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(
+        pcall_err(api.nvim_win_set_config, float_win, { win = tab3_win })
-          api.nvim_win_set_config,
-          float_win,
-          { relative = 'win', win = tab3_win, row = 0, col = 0 }
-        )
       )
       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(
+        pcall_err(api.nvim_win_set_config, float_win, { win = tab3_win })
-          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)

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=''",
+        "Must specify 'relative' or 'external' when creating a float",
-        pcall_err(api.nvim_open_win, buf, false, { width = 20, height = 2, relative = 'editor', row = 0, col = 0, win = 0 })
+        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 = [[