fix(float): don't always switch window when deleting last listed buffer (#17836)

zeertzjq · web-flow · commit ff82b2785f16 · 2022-03-24T11:56:22.000+08:00
diff --git a/src/nvim/buffer.c b/src/nvim/buffer.c
@@ -1043,8 +1043,20 @@ static int empty_curbuf(int close_others, int forceit, int action)
 
   if (close_others) {
     if (curwin->w_floating) {
-      // Last window must be non-floating.
-      curwin = firstwin;
+      bool can_close_all_others = false;
+      for (win_T *wp = firstwin; !wp->w_floating; wp = wp->w_next) {
+        if (wp->w_buffer != curbuf) {
+          // Found another non-floating window with a different (probably unlisted) buffer.
+          // Closing all other windows with the this buffer is fine in this case.
+          can_close_all_others = true;
+          break;
+        }
+      }
+      if (!can_close_all_others) {
+        // Closing all other windows with this buffer will close all non-floating windows.
+        // Move to a non-floating window then.
+        curwin = firstwin;
+      }
     }
     // Close any other windows on this buffer, then make it empty.
     close_windows(buf, true);
diff --git a/src/nvim/window.c b/src/nvim/window.c
@@ -2350,18 +2350,16 @@ void entering_window(win_T *const win)
   }
 }
 
-/// Closes all windows for buffer `buf` until there is only one non-floating window.
+/// Closes all windows for buffer `buf` unless there is only one non-floating window.
 ///
-/// @param keep_curwin  don't close `curwin`, but caller must ensure `curwin` is non-floating.
+/// @param keep_curwin  don't close `curwin`
 void close_windows(buf_T *buf, bool keep_curwin)
 {
   tabpage_T *tp, *nexttp;
   int h = tabline_height();
 
   ++RedrawingDisabled;
 
-  assert(!keep_curwin || !curwin->w_floating);
-
   // Start from lastwin to close floating windows with the same buffer first.
   // When the autocommand window is involved win_close() may need to print an error message.
   for (win_T *wp = lastwin; wp != NULL && (lastwin == aucmd_win || !one_window(wp));) {
diff --git a/test/functional/ui/float_spec.lua b/test/functional/ui/float_spec.lua
@@ -418,10 +418,11 @@ describe('float window', function()
     eq(winids, eval('winids'))
   end)
 
-  describe('with only one tabpage', function()
-    local old_buf, old_win
+  describe('with only one tabpage,', function()
     local float_opts = {relative = 'editor', row = 1, col = 1, width = 1, height = 1}
+    local old_buf, old_win
     before_each(function()
+      insert('foo')
       old_buf = meths.get_current_buf()
       old_win = meths.get_current_win()
     end)
@@ -440,92 +441,170 @@ describe('float window', function()
       end)
     end)
     describe("deleting the last non-floating window's buffer", function()
-      before_each(function()
-        insert('foo')
-      end)
       describe('leaves one window with an empty buffer when there is only one buffer', function()
         before_each(function()
           meths.open_win(old_buf, true, float_opts)
         end)
-        it('if called from non-floating window', function()
-          meths.set_current_win(old_win)
-          meths.buf_delete(old_buf, {force = true})
+        after_each(function()
           eq(old_win, meths.get_current_win())
           expect('')
           eq(1, #meths.list_wins())
         end)
+        it('if called from non-floating window', function()
+          meths.set_current_win(old_win)
+          meths.buf_delete(old_buf, {force = true})
+        end)
         it('if called from floating window', function()
           meths.buf_delete(old_buf, {force = true})
-          eq(old_win, meths.get_current_win())
-          expect('')
-          eq(1, #meths.list_wins())
         end)
       end)
       describe('closes other windows with that buffer when there are other buffers', function()
-        local same_buf_win, other_buf, other_buf_win
+        local same_buf_float, other_buf, other_buf_float
         before_each(function()
-          same_buf_win = meths.open_win(old_buf, false, float_opts)
+          same_buf_float = meths.open_win(old_buf, false, float_opts)
           other_buf = meths.create_buf(true, false)
-          other_buf_win = meths.open_win(other_buf, true, float_opts)
+          other_buf_float = meths.open_win(other_buf, true, float_opts)
           insert('bar')
           meths.set_current_win(old_win)
         end)
-        it('if called from non-floating window', function()
-          meths.buf_delete(old_buf, {force = true})
-          eq(old_win, meths.get_current_win())
+        after_each(function()
           eq(other_buf, meths.get_current_buf())
           expect('bar')
           eq(2, #meths.list_wins())
         end)
+        it('if called from non-floating window', function()
+          meths.buf_delete(old_buf, {force = true})
+          eq(old_win, meths.get_current_win())
+        end)
         it('if called from floating window with the same buffer', function()
-          meths.set_current_win(same_buf_win)
+          meths.set_current_win(same_buf_float)
           meths.buf_delete(old_buf, {force = true})
           eq(old_win, meths.get_current_win())
-          eq(other_buf, meths.get_current_buf())
-          expect('bar')
-          eq(2, #meths.list_wins())
         end)
         -- TODO: this case is too hard to deal with
         pending('if called from floating window with another buffer', function()
-          meths.set_current_win(other_buf_win)
+          meths.set_current_win(other_buf_float)
           meths.buf_delete(old_buf, {force = true})
         end)
       end)
-      describe('creates a new buffer when there is only one listed buffer', function()
-        local same_buf_win, unlisted_buf, unlisted_buf_win
+      describe('creates an empty buffer when there is only one listed buffer', function()
+        local same_buf_float, unlisted_buf_float
         before_each(function()
-          same_buf_win = meths.open_win(old_buf, false, float_opts)
-          unlisted_buf = meths.create_buf(true, false)
-          unlisted_buf_win = meths.open_win(unlisted_buf, true, float_opts)
-          insert('bar')
+          same_buf_float = meths.open_win(old_buf, false, float_opts)
+          local unlisted_buf = meths.create_buf(true, false)
+          unlisted_buf_float = meths.open_win(unlisted_buf, true, float_opts)
+          insert('unlisted')
           command('set nobuflisted')
           meths.set_current_win(old_win)
         end)
+        after_each(function()
+          expect('')
+          eq(2, #meths.list_wins())
+        end)
         it('if called from non-floating window', function()
           meths.buf_delete(old_buf, {force = true})
           eq(old_win, meths.get_current_win())
-          expect('')
-          eq(2, #meths.list_wins())
         end)
         it('if called from floating window with the same buffer', function()
-          meths.set_current_win(same_buf_win)
+          meths.set_current_win(same_buf_float)
           meths.buf_delete(old_buf, {force = true})
           eq(old_win, meths.get_current_win())
-          expect('')
-          eq(2, #meths.list_wins())
         end)
         -- TODO: this case is too hard to deal with
         pending('if called from floating window with an unlisted buffer', function()
-          meths.set_current_win(unlisted_buf_win)
+          meths.set_current_win(unlisted_buf_float)
+          meths.buf_delete(old_buf, {force = true})
+        end)
+      end)
+    end)
+    describe('with splits, deleting the last listed buffer creates an empty buffer', function()
+      describe('when a non-floating window has an unlisted buffer', function()
+        local same_buf_float
+        before_each(function()
+          command('botright vnew')
+          insert('unlisted')
+          command('set nobuflisted')
+          meths.set_current_win(old_win)
+          same_buf_float = meths.open_win(old_buf, false, float_opts)
+        end)
+        after_each(function()
+          expect('')
+          eq(2, #meths.list_wins())
+        end)
+        it('if called from non-floating window with the deleted buffer', function()
+          meths.buf_delete(old_buf, {force = true})
+          eq(old_win, meths.get_current_win())
+        end)
+        it('if called from floating window with the deleted buffer', function()
+          meths.set_current_win(same_buf_float)
           meths.buf_delete(old_buf, {force = true})
+          eq(same_buf_float, meths.get_current_win())
         end)
       end)
     end)
   end)
 
-  describe('with multiple tabpages', function()
-    local old_tabpage, old_buf, old_win
+  describe('with mulitple tabpages but only one listed buffer,', function()
+    local float_opts = {relative = 'editor', row = 1, col = 1, width = 1, height = 1}
+    local unlisted_buf, old_buf, old_win
+    before_each(function()
+      insert('unlisted')
+      command('set nobuflisted')
+      unlisted_buf = meths.get_current_buf()
+      command('tabnew')
+      insert('foo')
+      old_buf = meths.get_current_buf()
+      old_win = meths.get_current_win()
+    end)
+    describe('without splits, deleting the last listed buffer creates an empty buffer', function()
+      local same_buf_float
+      before_each(function()
+        meths.set_current_win(old_win)
+        same_buf_float = meths.open_win(old_buf, false, float_opts)
+      end)
+      after_each(function()
+        expect('')
+        eq(2, #meths.list_wins())
+        eq(2, #meths.list_tabpages())
+      end)
+      it('if called from non-floating window', function()
+        meths.buf_delete(old_buf, {force = true})
+        eq(old_win, meths.get_current_win())
+      end)
+      it('if called from floating window with the same buffer', function()
+        meths.set_current_win(same_buf_float)
+        meths.buf_delete(old_buf, {force = true})
+        eq(old_win, meths.get_current_win())
+      end)
+    end)
+    describe('with splits, deleting the last listed buffer creates an empty buffer', function()
+      local same_buf_float
+      before_each(function()
+        command('botright vsplit')
+        meths.set_current_buf(unlisted_buf)
+        meths.set_current_win(old_win)
+        same_buf_float = meths.open_win(old_buf, false, float_opts)
+      end)
+      after_each(function()
+        expect('')
+        eq(3, #meths.list_wins())
+        eq(2, #meths.list_tabpages())
+      end)
+      it('if called from non-floating window with the deleted buffer', function()
+        meths.buf_delete(old_buf, {force = true})
+        eq(old_win, meths.get_current_win())
+      end)
+      it('if called from floating window with the deleted buffer', function()
+        meths.set_current_win(same_buf_float)
+        meths.buf_delete(old_buf, {force = true})
+        eq(same_buf_float, meths.get_current_win())
+      end)
+    end)
+  end)
+
+  describe('with multiple tabpages and multiple listed buffers,', function()
     local float_opts = {relative = 'editor', row = 1, col = 1, width = 1, height = 1}
+    local old_tabpage, old_buf, old_win
     before_each(function()
       old_tabpage = meths.get_current_tabpage()
       insert('oldtab')
@@ -538,18 +617,17 @@ describe('float window', function()
         before_each(function()
           meths.open_win(old_buf, true, float_opts)
         end)
-        it('if called from non-floating window', function()
-          meths.set_current_win(old_win)
-          meths.win_close(old_win, false)
+        after_each(function()
           eq(old_tabpage, meths.get_current_tabpage())
           expect('oldtab')
           eq(1, #meths.list_tabpages())
         end)
+        it('if called from non-floating window', function()
+          meths.set_current_win(old_win)
+          meths.win_close(old_win, false)
+        end)
         it('if called from floating window', function()
           meths.win_close(old_win, false)
-          eq(old_tabpage, meths.get_current_tabpage())
-          expect('oldtab')
-          eq(1, #meths.list_tabpages())
         end)
       end)
       describe('gives E5601 when there are non-closeable floating windows', function()
@@ -579,18 +657,17 @@ describe('float window', function()
           other_buf_win = meths.open_win(other_buf, true, float_opts)
           meths.set_current_win(old_win)
         end)
-        it('if called from non-floating window', function()
-          meths.buf_delete(old_buf, {force = false})
+        after_each(function()
           eq(old_tabpage, meths.get_current_tabpage())
           expect('oldtab')
           eq(1, #meths.list_tabpages())
         end)
+        it('if called from non-floating window', function()
+          meths.buf_delete(old_buf, {force = false})
+        end)
         it('if called from floating window with the same buffer', function()
           meths.set_current_win(same_buf_win)
           meths.buf_delete(old_buf, {force = false})
-          eq(old_tabpage, meths.get_current_tabpage())
-          expect('oldtab')
-          eq(1, #meths.list_tabpages())
         end)
         -- TODO: this case is too hard to deal with
         pending('if called from floating window with another buffer', function()

Original file line number	Diff line number	Diff line change
`@@ -2350,18 +2350,16 @@ void entering_window(win_T *const win)`
`2350`	`2350`	`}`
`2351`	`2351`	`}`
`2352`	`2352`
`2353`		-/// Closes all windows for buffer `buf` until there is only one non-floating window.
	`2353`	+/// Closes all windows for buffer `buf` unless there is only one non-floating window.
`2354`	`2354`	`///`
`2355`		-/// @param keep_curwin don't close `curwin`, but caller must ensure `curwin` is non-floating.
	`2355`	+/// @param keep_curwin don't close `curwin`
`2356`	`2356`	`void close_windows(buf_T *buf, bool keep_curwin)`
`2357`	`2357`	`{`
`2358`	`2358`	`tabpage_T tp, nexttp;`
`2359`	`2359`	`int h = tabline_height();`
`2360`	`2360`
`2361`	`2361`	`++RedrawingDisabled;`
`2362`	`2362`
`2363`		`- assert(!keep_curwin \|\| !curwin->w_floating);`
`2364`		`-`
`2365`	`2363`	`// Start from lastwin to close floating windows with the same buffer first.`
`2366`	`2364`	`// When the autocommand window is involved win_close() may need to print an error message.`
`2367`	`2365`	`for (win_T *wp = lastwin; wp != NULL && (lastwin == aucmd_win \|\| !one_window(wp));) {`