fix(float): fix some other crashes with :unhide or :all (#25328)

3 files changed, 178 insertions, 9 deletions

diff --git a/src/nvim/arglist.c b/src/nvim/arglist.c
index 74348052b0..9c2b3ba6d8 100644
--- a/src/nvim/arglist.c
+++ b/src/nvim/arglist.c
@@ -857,12 +857,17 @@ static void arg_all_close_unused_windows(arg_all_state_T *aall)
   while (true) {
     win_T *wpnext = NULL;
     tabpage_T *tpnext = curtab->tp_next;
-    for (win_T *wp = firstwin; wp != NULL; wp = wpnext) {
+    // Try to close floating windows first
+    for (win_T *wp = lastwin->w_floating ? lastwin : firstwin; wp != NULL; wp = wpnext) {
       int i;
-      wpnext = wp->w_next;
+      wpnext = wp->w_floating
+        ? wp->w_prev->w_floating ? wp->w_prev : firstwin
+        : (wp->w_next == NULL || wp->w_next->w_floating) ? NULL : wp->w_next;
       buf_T *buf = wp->w_buffer;
       if (buf->b_ffname == NULL
-          || (!aall->keep_tabs && (buf->b_nwindows > 1 || wp->w_width != Columns))) {
+          || (!aall->keep_tabs
+              && (buf->b_nwindows > 1 || wp->w_width != Columns
+                  || (wp->w_floating && !is_aucmd_win(wp))))) {
         i = aall->opened_len;
       } else {
         // check if the buffer in this window is in the arglist
@@ -917,7 +922,7 @@ static void arg_all_close_unused_windows(arg_all_state_T *aall)
             (void)autowrite(buf, false);
             // Check if autocommands removed the window.
             if (!win_valid(wp) || !bufref_valid(&bufref)) {
-              wpnext = firstwin;  // Start all over...
+              wpnext = lastwin->w_floating ? lastwin : firstwin;  // Start all over...
               continue;
             }
           }
@@ -930,7 +935,7 @@ static void arg_all_close_unused_windows(arg_all_state_T *aall)
             // check if autocommands removed the next window
             if (!win_valid(wpnext)) {
               // start all over...
-              wpnext = firstwin;
+              wpnext = lastwin->w_floating ? lastwin : firstwin;
             }
           }
         }
@@ -977,6 +982,8 @@ static void arg_all_open_windows(arg_all_state_T *aall, int count)
             if (aall->keep_tabs) {
               aall->new_curwin = wp;
               aall->new_curtab = curtab;
+            } else if (wp->w_floating) {
+              break;
             } else if (wp->w_frame->fr_parent != curwin->w_frame->fr_parent) {
               emsg(_(e_window_layout_changed_unexpectedly));
               i = count;
@@ -1098,7 +1105,8 @@ static void do_arg_all(int count, int forceit, int keep_tabs)
   autocmd_no_leave++;
   last_curwin = curwin;
   last_curtab = curtab;
-  win_enter(lastwin, false);
+  // lastwin may be aucmd_win
+  win_enter(lastwin_nofloating(), 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 f2174e055b..68cef67c8a 100644
--- a/src/nvim/buffer.c
+++ b/src/nvim/buffer.c
@@ -3622,6 +3622,7 @@ void ex_buffer_all(exarg_T *eap)
         ? wp->w_prev->w_floating ? wp->w_prev : firstwin
         : (wp->w_next == NULL || wp->w_next->w_floating) ? NULL : wp->w_next;
       if ((wp->w_buffer->b_nwindows > 1
+           || wp->w_floating
            || ((cmdmod.cmod_split & WSP_VERT)
                ? wp->w_height + wp->w_hsep_height + wp->w_status_height < Rows - p_ch
                - tabline_height() - global_stl_height()
@@ -3656,6 +3657,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);
   autocmd_no_leave++;
   for (buf = firstbuf; buf != NULL && open_wins < count; buf = buf->b_next) {
@@ -3674,7 +3676,7 @@ void ex_buffer_all(exarg_T *eap)
     } else {
       // Check if this buffer already has a window
       for (wp = firstwin; wp != NULL; wp = wp->w_next) {
-        if (wp->w_buffer == buf) {
+        if (!wp->w_floating && wp->w_buffer == buf) {
           break;
         }
       }
diff --git a/test/functional/ui/float_spec.lua b/test/functional/ui/float_spec.lua
index e21e4e4aad..6db9e7af3e 100644
--- a/test/functional/ui/float_spec.lua
+++ b/test/functional/ui/float_spec.lua
@@ -522,8 +522,8 @@ describe('float window', function()
     eq(5, meths.get_option_value('scroll', {win=float_win.id}))
   end)
 
-  it(':unhide works when there are floating windows #17797', function()
-    local float_opts = {relative = 'editor', row = 1, col = 1, width = 10, height = 10}
+  it(':unhide works when there are floating windows', function()
+    local float_opts = {relative = 'editor', row = 1, col = 1, width = 5, height = 5}
     local w0 = curwin()
     meths.open_win(0, false, float_opts)
     meths.open_win(0, false, float_opts)
@@ -532,6 +532,17 @@ describe('float window', function()
     eq({ w0 }, meths.list_wins())
   end)
 
+  it(':all works when there are floating windows', function()
+    command('args Xa.txt')
+    local float_opts = {relative = 'editor', row = 1, col = 1, width = 5, height = 5}
+    local w0 = curwin()
+    meths.open_win(0, false, float_opts)
+    meths.open_win(0, false, float_opts)
+    eq(3, #meths.list_wins())
+    command('all')
+    eq({ w0 }, meths.list_wins())
+  end)
+
   describe('with only one tabpage,', function()
     local float_opts = {relative = 'editor', row = 1, col = 1, width = 1, height = 1}
     local old_buf, old_win
@@ -3222,6 +3233,154 @@ describe('float window', function()
       end
     end)
 
+    describe('no crash when rearranging windows', function()
+      local function test_rearrange_windows(cmd)
+        command('set laststatus=2')
+        screen:try_resize(40, 13)
+
+        command('args X1 X2 X3 X4 X5 X6')
+        command('sargument 2')
+        command('sargument 3')
+        local w3 = curwin()
+        command('sargument 4')
+        local w4 = curwin()
+        command('sargument 5')
+        command('sargument 6')
+
+        local float_opts = { relative = 'editor', row = 6, col = 0, width = 40, height = 1 }
+        meths.win_set_config(w3, float_opts)
+        meths.win_set_config(w4, float_opts)
+        command('wincmd =')
+        if multigrid then
+          screen:expect{grid=[[
+          ## grid 1
+            [8:----------------------------------------]|
+            [8:----------------------------------------]|
+            {4:X6                                      }|
+            [7:----------------------------------------]|
+            [7:----------------------------------------]|
+            {5:X5                                      }|
+            [4:----------------------------------------]|
+            [4:----------------------------------------]|
+            {5:X2                                      }|
+            [2:----------------------------------------]|
+            [2:----------------------------------------]|
+            {5:X1                                      }|
+            [3:----------------------------------------]|
+          ## grid 2
+                                                    |
+            {0:~                                       }|
+          ## grid 3
+                                                    |
+          ## grid 4
+                                                    |
+            {0:~                                       }|
+          ## grid 5
+            {1:                                        }|
+          ## grid 6
+            {1:                                        }|
+          ## grid 7
+                                                    |
+            {0:~                                       }|
+          ## grid 8
+            ^                                        |
+            {0:~                                       }|
+          ]], float_pos={
+            [5] = {{id = 1002}, "NW", 1, 6, 0, true, 50};
+            [6] = {{id = 1003}, "NW", 1, 6, 0, true, 50};
+          }, win_viewport={
+            [2] = {win = {id = 1000}, topline = 0, botline = 2, curline = 0, curcol = 0, linecount = 1, sum_scroll_delta = 0};
+            [4] = {win = {id = 1001}, topline = 0, botline = 2, curline = 0, curcol = 0, linecount = 1, sum_scroll_delta = 0};
+            [5] = {win = {id = 1002}, topline = 0, botline = 1, curline = 0, curcol = 0, linecount = 1, sum_scroll_delta = 0};
+            [6] = {win = {id = 1003}, topline = 0, botline = 1, curline = 0, curcol = 0, linecount = 1, sum_scroll_delta = 0};
+            [7] = {win = {id = 1004}, topline = 0, botline = 2, curline = 0, curcol = 0, linecount = 1, sum_scroll_delta = 0};
+            [8] = {win = {id = 1005}, topline = 0, botline = 2, curline = 0, curcol = 0, linecount = 1, sum_scroll_delta = 0};
+          }}
+        else
+          screen:expect{grid=[[
+            ^                                        |
+            {0:~                                       }|
+            {4:X6                                      }|
+                                                    |
+            {0:~                                       }|
+            {5:X5                                      }|
+            {1:                                        }|
+            {0:~                                       }|
+            {5:X2                                      }|
+                                                    |
+            {0:~                                       }|
+            {5:X1                                      }|
+                                                    |
+          ]]}
+        end
+
+        command(cmd)
+        if multigrid then
+          screen:expect{grid=[[
+          ## grid 1
+            [2:----------------------------------------]|
+            {4:X1                                      }|
+            [4:----------------------------------------]|
+            {5:X2                                      }|
+            [9:----------------------------------------]|
+            {5:X3                                      }|
+            [10:----------------------------------------]|
+            {5:X4                                      }|
+            [7:----------------------------------------]|
+            {5:X5                                      }|
+            [8:----------------------------------------]|
+            {5:X6                                      }|
+            [3:----------------------------------------]|
+          ## grid 2
+            ^                                        |
+          ## grid 3
+                                                    |
+          ## grid 4
+                                                    |
+          ## grid 7
+                                                    |
+          ## grid 8
+                                                    |
+          ## grid 9
+                                                    |
+          ## grid 10
+                                                    |
+          ]], win_viewport={
+            [2] = {win = {id = 1000}, topline = 0, botline = 1, curline = 0, curcol = 0, linecount = 1, sum_scroll_delta = 0};
+            [4] = {win = {id = 1001}, topline = 0, botline = 1, curline = 0, curcol = 0, linecount = 1, sum_scroll_delta = 0};
+            [7] = {win = {id = 1004}, topline = 0, botline = 1, curline = 0, curcol = 0, linecount = 1, sum_scroll_delta = 0};
+            [8] = {win = {id = 1005}, topline = 0, botline = 1, curline = 0, curcol = 0, linecount = 1, sum_scroll_delta = 0};
+            [9] = {win = {id = 1006}, topline = 0, botline = 1, curline = 0, curcol = 0, linecount = 1, sum_scroll_delta = 0};
+            [10] = {win = {id = 1007}, topline = 0, botline = 1, curline = 0, curcol = 0, linecount = 1, sum_scroll_delta = 0};
+          }}
+        else
+          screen:expect{grid=[[
+            ^                                        |
+            {4:X1                                      }|
+                                                    |
+            {5:X2                                      }|
+                                                    |
+            {5:X3                                      }|
+                                                    |
+            {5:X4                                      }|
+                                                    |
+            {5:X5                                      }|
+                                                    |
+            {5:X6                                      }|
+                                                    |
+          ]]}
+        end
+      end
+
+      it('using :unhide', function()
+        test_rearrange_windows('unhide')
+      end)
+
+      it('using :all', function()
+        test_rearrange_windows('all')
+      end)
+    end)
+
     it('API has proper error messages', function()
       local buf = meths.create_buf(false,false)
       eq("Invalid key: 'bork'",