fix(float): handle buffer deletion with floating windows

zeertzjq · zeertzjq · commit 3fdb7b528d9d · 2022-03-23T09:07:21.000+08:00
diff --git a/src/nvim/buffer.c b/src/nvim/buffer.c
@@ -1042,6 +1042,10 @@ static int empty_curbuf(int close_others, int forceit, int action)
   set_bufref(&bufref, buf);
 
   if (close_others) {
+    if (curwin->w_floating) {
+      // Last window must be non-floating.
+      curwin = firstwin;
+    }
     // Close any other windows on this buffer, then make it empty.
     close_windows(buf, true);
   }
@@ -1224,11 +1228,12 @@ int do_buffer(int action, int start, int dir, int count, int forceit)
     }
 
     // If the deleted buffer is the current one, close the current window
-    // (unless it's the only window).  Repeat this so long as we end up in
-    // a window with this buffer.
+    // (unless it's the only non-floating window).
+    // When the autocommand window is involved win_close() may need to print an error message.
+    // Repeat this so long as we end up in a window with this buffer.
     while (buf == curbuf
            && !(curwin->w_closing || curwin->w_buffer->b_locked > 0)
-           && (!ONE_WINDOW || first_tabpage->tp_next != NULL)) {
+           && (lastwin == aucmd_win || !last_window(curwin))) {
       if (win_close(curwin, false, false) == FAIL) {
         break;
       }
diff --git a/src/nvim/window.c b/src/nvim/window.c
@@ -2350,17 +2350,21 @@ void entering_window(win_T *const win)
   }
 }
 
-/// Closes all windows for buffer `buf`.
+/// Closes all windows for buffer `buf` until there is only one non-floating window.
 ///
-/// @param keep_curwin don't close `curwin`
-void close_windows(buf_T *buf, int keep_curwin)
+/// @param keep_curwin  don't close `curwin`, but caller must ensure `curwin` is non-floating.
+void close_windows(buf_T *buf, bool keep_curwin)
 {
   tabpage_T *tp, *nexttp;
   int h = tabline_height();
 
   ++RedrawingDisabled;
 
-  for (win_T *wp = firstwin; wp != NULL && !ONE_WINDOW;) {
+  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));) {
     if (wp->w_buffer == buf && (!keep_curwin || wp != curwin)
         && !(wp->w_closing || wp->w_buffer->b_locked > 0)) {
       if (win_close(wp, false, false) == FAIL) {
@@ -2369,9 +2373,9 @@ void close_windows(buf_T *buf, int keep_curwin)
       }
 
       // Start all over, autocommands may change the window layout.
-      wp = firstwin;
+      wp = lastwin;
     } else {
-      wp = wp->w_next;
+      wp = wp->w_prev;
     }
   }
 
@@ -2406,7 +2410,7 @@ void close_windows(buf_T *buf, int keep_curwin)
 ///
 /// @return  true if the specified window is the only window that exists,
 ///          false if there is another, possibly in another tab page.
-static bool last_window(win_T *win) FUNC_ATTR_PURE FUNC_ATTR_WARN_UNUSED_RESULT
+bool last_window(win_T *win) FUNC_ATTR_PURE FUNC_ATTR_WARN_UNUSED_RESULT
 {
   return one_window(win) && first_tabpage->tp_next == NULL;
 }
@@ -2442,12 +2446,14 @@ bool last_nonfloat(win_T *wp) FUNC_ATTR_PURE FUNC_ATTR_WARN_UNUSED_RESULT
   return wp != NULL && firstwin == wp && !(wp->w_next && !wp->w_floating);
 }
 
-/// Check if floating windows can be closed.
+/// Check if floating windows in the current tab can be closed.
+/// Do not call this when the autocommand window is in use!
 ///
 /// @return true if all floating windows can be closed
-static bool can_close_floating_windows(tabpage_T *tab)
+static bool can_close_floating_windows(void)
 {
-  FOR_ALL_WINDOWS_IN_TAB(wp, tab) {
+  assert(lastwin != aucmd_win);
+  for (win_T *wp = lastwin; wp->w_floating; wp = wp->w_prev) {
     buf_T *buf = wp->w_buffer;
     int need_hide = (bufIsChanged(buf) && buf->b_nwindows <= 1);
 
@@ -2546,18 +2552,18 @@ int win_close(win_T *win, bool free_buf, bool force)
     emsg(_(e_autocmd_close));
     return FAIL;
   }
-  if (lastwin == aucmd_win && one_window(win)) {
-    emsg(_("E814: Cannot close window, only autocmd window would remain"));
-    return FAIL;
-  }
-  if ((firstwin == win && lastwin_nofloating() == win)
-      && lastwin->w_floating) {
-    if (force || can_close_floating_windows(curtab)) {
-      win_T *nextwp;
-      for (win_T *wpp = firstwin; wpp != NULL; wpp = nextwp) {
-        nextwp = wpp->w_next;
-        if (wpp->w_floating) {
-          win_close(wpp, free_buf, force);
+  if (lastwin->w_floating && one_window(win)) {
+    if (lastwin == aucmd_win) {
+      emsg(_("E814: Cannot close window, only autocmd window would remain"));
+      return FAIL;
+    }
+    if (force || can_close_floating_windows()) {
+      // close the last window until the there are no floating windows
+      while (lastwin->w_floating) {
+        // `force` flag isn't actually used when closing a floating window.
+        if (win_close(lastwin, free_buf, true) == FAIL) {
+          // If closing the window fails give up, to avoid looping forever.
+          return FAIL;
         }
       }
     } else {
diff --git a/test/functional/ui/float_spec.lua b/test/functional/ui/float_spec.lua
@@ -8,6 +8,7 @@ local command, feed_command = helpers.command, helpers.feed_command
 local eval = helpers.eval
 local eq = helpers.eq
 local neq = helpers.neq
+local expect = helpers.expect
 local exec_lua = helpers.exec_lua
 local insert = helpers.insert
 local meths = helpers.meths
@@ -418,40 +419,154 @@ describe('float window', function()
   end)
 
   describe('with only one tabpage', function()
+    local old_buf, old_win
+    local float_opts = {relative = 'editor', row = 1, col = 1, width = 1, height = 1}
+    before_each(function()
+      old_buf = meths.get_current_buf()
+      old_win = meths.get_current_win()
+    end)
     describe('closing the last non-floating window gives E444', function()
-      local old_win
       before_each(function()
-        old_win = meths.get_current_win()
-        meths.open_win(0, true, {relative = 'editor', row = 1, col = 1, width = 1, height = 1})
+        meths.open_win(old_buf, true, float_opts)
       end)
       it('if called from non-floating window', function()
         meths.set_current_win(old_win)
-        eq('Vim:E444: Cannot close last window', pcall_err(meths.win_close, old_win, false))
+        eq('Vim:E444: Cannot close last window',
+           pcall_err(meths.win_close, old_win, false))
       end)
       it('if called from floating window', function()
-        eq('Vim:E444: Cannot close last window', pcall_err(meths.win_close, old_win, false))
+        eq('Vim:E444: Cannot close last window',
+           pcall_err(meths.win_close, old_win, false))
+      end)
+    end)
+    describe("deleting the last non-floating window's buffer", function()
+      before_each(function()
+        insert('foo')
+      end)
+      describe('when there is only one buffer leaves that window with an empty 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})
+          eq(old_win, meths.get_current_win())
+          expect('')
+          eq(1, #meths.list_wins())
+        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('when there are other buffers closes other windows with that buffer', function()
+        local other_buf, same_buf_win, other_buf_win
+        before_each(function()
+          same_buf_win = 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)
+          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())
+          eq(other_buf, meths.get_current_buf())
+          expect('bar')
+          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.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.buf_delete(old_buf, {force = true})
+        end)
       end)
     end)
   end)
 
   describe('with multiple tabpages', function()
+    local old_tabpage, old_buf, old_win
+    local float_opts = {relative = 'editor', row = 1, col = 1, width = 1, height = 1}
+    before_each(function()
+      old_tabpage = meths.get_current_tabpage()
+      insert('oldtab')
+      command('tabnew')
+      old_buf = meths.get_current_buf()
+      old_win = meths.get_current_win()
+    end)
     describe('closing the last non-floating window', function()
-      local old_tabpage, old_win
-      before_each(function()
-        old_tabpage = meths.get_current_tabpage()
-        command('tabnew')
-        old_win = meths.get_current_win()
-        meths.open_win(0, true, {relative = 'editor', row = 1, col = 1, width = 1, height = 1})
-      end)
       describe('when all floating windows are closeable closes the tabpage', 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)
           eq(old_tabpage, meths.get_current_tabpage())
+          expect('oldtab')
+          eq(1, #meths.list_tabpages())
         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('when there are non-closeable floating windows gives E5601', function()
+        before_each(function()
+          command('set nohidden')
+          local other_buf = meths.create_buf(true, false)
+          meths.open_win(other_buf, true, float_opts)
+          insert('foo')
+        end)
+        it('if called from non-floating window', function()
+          meths.set_current_win(old_win)
+          eq('Vim:E5601: Cannot close window, only floating window would remain',
+             pcall_err(meths.win_close, old_win, false))
+        end)
+        it('if called from floating window', function()
+          eq('Vim:E5601: Cannot close window, only floating window would remain',
+             pcall_err(meths.win_close, old_win, false))
+        end)
+      end)
+    end)
+    describe("deleting the last non-floating window's buffer", function()
+      describe('when all floating windows are closeable closes the tabpage', function()
+        local other_buf, same_buf_win, other_buf_win
+        before_each(function()
+          same_buf_win = 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)
+          meths.set_current_win(old_win)
+        end)
+        it('if called from non-floating window', function()
+          meths.buf_delete(old_buf, {force = false})
+          eq(old_tabpage, meths.get_current_tabpage())
+          expect('oldtab')
+          eq(1, #meths.list_tabpages())
+        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()
+          meths.set_current_win(other_buf_win)
+          meths.buf_delete(old_buf, {force = false})
         end)
       end)
     end)
