From b52d15853e89149472c1ecd9cce3a84e4af0785a Mon Sep 17 00:00:00 2001
From: Sean Dewar <6256228+seandewar@users.noreply.github.com>
Date: Sat, 9 Mar 2024 16:56:32 +0000
Subject: [PATCH] fix(api): win_set_config set tp_curwin of win moved from
 other tabpage

Problem: nvim_win_set_config does not update the tp_curwin of win's original
tabpage when moving it to another.

Solution: update it if win was the tp_curwin. Add a test.
---
 src/nvim/api/win_config.c           | 23 ++++++++++++----
 src/nvim/window.c                   | 15 +----------
 src/nvim/winfloat.c                 | 18 +++++++++++++
 test/functional/api/window_spec.lua | 41 +++++++++++++++++++++++++++++
 4 files changed, 78 insertions(+), 19 deletions(-)

diff --git a/src/nvim/api/win_config.c b/src/nvim/api/win_config.c
index 32b2156313..8b8eca62ca 100644
--- a/src/nvim/api/win_config.c
+++ b/src/nvim/api/win_config.c
@@ -468,7 +468,7 @@ void nvim_win_set_config(Window window, Dict(win_config) *config, Error *err)
         int dir;
         win_goto(winframe_find_altwin(win, &dir, NULL, NULL));
       } else {
-        win_goto(win_valid(prevwin) && prevwin != win ? prevwin : firstwin);
+        win_goto(win_float_find_altwin(win, NULL));
       }
 
       // Autocommands may have been a real nuisance and messed things up...
@@ -490,6 +490,8 @@ void nvim_win_set_config(Window window, Dict(win_config) *config, Error *err)
 
     int dir = 0;
     frame_T *unflat_altfr = NULL;
+    win_T *altwin = NULL;
+
     if (was_split) {
       // If the window is the last in the tabpage or `fconfig.win` is
       // a handle to itself, we can't split it.
@@ -534,10 +536,11 @@ void nvim_win_set_config(Window window, Dict(win_config) *config, Error *err)
           }
           // If the frame doesn't have a parent, the old frame
           // was the root frame and we need to create a top-level split.
-          winframe_remove(win, &dir, win_tp == curtab ? NULL : win_tp, &unflat_altfr);
+          altwin = winframe_remove(win, &dir, win_tp == curtab ? NULL : win_tp, &unflat_altfr);
         } else if (n_frames == 2) {
           // There are two windows in the frame, we can just rotate it.
-          neighbor = winframe_remove(win, &dir, win_tp == curtab ? NULL : win_tp, &unflat_altfr);
+          altwin = winframe_remove(win, &dir, win_tp == curtab ? NULL : win_tp, &unflat_altfr);
+          neighbor = altwin;
         } else {
           // There is only one window in the frame, we can't split it.
           api_set_error(err, kErrorTypeException, "Cannot split window into itself");
@@ -546,9 +549,12 @@ void nvim_win_set_config(Window window, Dict(win_config) *config, Error *err)
         // Set the parent to whatever the correct neighbor window was determined to be.
         parent = neighbor;
       } else {
-        winframe_remove(win, &dir, win_tp == curtab ? NULL : win_tp, &unflat_altfr);
+        altwin = winframe_remove(win, &dir, win_tp == curtab ? NULL : win_tp, &unflat_altfr);
       }
+    } else {
+      altwin = win_float_find_altwin(win, win_tp == curtab ? NULL : win_tp);
     }
+
     win_remove(win, win_tp == curtab ? NULL : win_tp);
     if (win_tp == curtab) {
       last_status(false);  // may need to remove last status line
@@ -556,12 +562,14 @@ void nvim_win_set_config(Window window, Dict(win_config) *config, Error *err)
     }
 
     int flags = win_split_flags(fconfig.split, parent == NULL) | WSP_NOENTER;
+    tabpage_T *const parent_tp = parent ? win_find_tabpage(parent) : curtab;
+
     TRY_WRAP(err, {
       const bool need_switch = parent != NULL && parent != curwin;
       switchwin_T switchwin;
       if (need_switch) {
         // `parent` is valid in its tabpage, so switch_win should not fail.
-        const int result = switch_win(&switchwin, parent, win_find_tabpage(parent), true);
+        const int result = switch_win(&switchwin, parent, parent_tp, true);
         (void)result;
         assert(result == OK);
       }
@@ -593,6 +601,11 @@ restore_curwin:
       return;
     }
 
+    // If `win` moved tabpages and was the curwin of its old one, select a new curwin for it.
+    if (win_tp != parent_tp && win_tp->tp_curwin == win) {
+      win_tp->tp_curwin = altwin;
+    }
+
     if (HAS_KEY_X(config, width)) {
       win_setwidth_win(fconfig.width, win);
     }
diff --git a/src/nvim/window.c b/src/nvim/window.c
index 4dc6ed370e..ecd2e83500 100644
--- a/src/nvim/window.c
+++ b/src/nvim/window.c
@@ -3044,20 +3044,7 @@ static win_T *win_free_mem(win_T *win, int *dirp, tabpage_T *tp)
     xfree(frp);
   } else {
     *dirp = 'h';  // Dummy value.
-    if (tp == NULL) {
-      if (win_valid(prevwin) && prevwin != win) {
-        wp = prevwin;
-      } else {
-        wp = firstwin;
-      }
-    } else {
-      assert(tp != curtab);
-      if (tabpage_win_valid(tp, tp->tp_prevwin) && tp->tp_prevwin != win) {
-        wp = tp->tp_prevwin;
-      } else {
-        wp = tp->tp_firstwin;
-      }
-    }
+    wp = win_float_find_altwin(win, tp);
   }
   win_free(win, tp);
 
diff --git a/src/nvim/winfloat.c b/src/nvim/winfloat.c
index 3ddff8aa5a..65d2c1306b 100644
--- a/src/nvim/winfloat.c
+++ b/src/nvim/winfloat.c
@@ -319,3 +319,21 @@ win_T *win_float_find_preview(void)
   }
   return NULL;
 }
+
+/// Select an alternative window to `win` (assumed floating) in tabpage `tp`.
+///
+/// Useful for finding a window to switch to if `win` is the current window, but is then closed or
+/// moved to a different tabpage.
+///
+/// @param  tp  `win`'s original tabpage, or NULL for current.
+win_T *win_float_find_altwin(const win_T *win, const tabpage_T *tp)
+  FUNC_ATTR_NONNULL_ARG(1)
+{
+  if (tp == NULL) {
+    return (win_valid(prevwin) && prevwin != win) ? prevwin : firstwin;
+  }
+
+  assert(tp != curtab);
+  return (tabpage_win_valid(tp, tp->tp_prevwin) && tp->tp_prevwin != win) ? tp->tp_prevwin
+                                                                          : tp->tp_firstwin;
+}
diff --git a/test/functional/api/window_spec.lua b/test/functional/api/window_spec.lua
index 6b7c550ca8..a10c8f48ef 100644
--- a/test/functional/api/window_spec.lua
+++ b/test/functional/api/window_spec.lua
@@ -2401,6 +2401,47 @@ describe('API/win', function()
                     |
       ]])
     end)
+
+    it("updates tp_curwin of moved window's original tabpage", function()
+      local t1 = api.nvim_get_current_tabpage()
+      command('tab split | split')
+      local t2 = api.nvim_get_current_tabpage()
+      local t2_alt_win = api.nvim_get_current_win()
+      command('vsplit')
+      local t2_cur_win = api.nvim_get_current_win()
+      command('tabprevious')
+      eq(t2_cur_win, api.nvim_tabpage_get_win(t2))
+
+      -- tp_curwin is unchanged when moved within the same tabpage.
+      api.nvim_win_set_config(t2_cur_win, { split = 'left', win = t2_alt_win })
+      eq(t2_cur_win, api.nvim_tabpage_get_win(t2))
+
+      -- Also unchanged if the move failed.
+      command('let &winwidth = &columns | let &winminwidth = &columns')
+      matches(
+        'E36: Not enough room$',
+        pcall_err(api.nvim_win_set_config, t2_cur_win, { split = 'left', win = 0 })
+      )
+      eq(t2_cur_win, api.nvim_tabpage_get_win(t2))
+      command('set winminwidth& winwidth&')
+
+      -- But is changed if successfully moved to a different tabpage.
+      api.nvim_win_set_config(t2_cur_win, { split = 'left', win = 0 })
+      eq(t2_alt_win, api.nvim_tabpage_get_win(t2))
+      eq(t1, api.nvim_win_get_tabpage(t2_cur_win))
+
+      -- Now do it for a float, which has different altwin logic.
+      command('tabnext')
+      t2_cur_win =
+        api.nvim_open_win(0, true, { relative = 'editor', row = 5, col = 5, width = 5, height = 5 })
+      eq(t2_alt_win, fn.win_getid(fn.winnr('#')))
+      command('tabprevious')
+      eq(t2_cur_win, api.nvim_tabpage_get_win(t2))
+
+      api.nvim_win_set_config(t2_cur_win, { split = 'left', win = 0 })
+      eq(t2_alt_win, api.nvim_tabpage_get_win(t2))
+      eq(t1, api.nvim_win_get_tabpage(t2_cur_win))
+    end)
   end)
 
   describe('get_config', function()