refactor(win_close): remove "force", don't pass on "free_buf" (#21921)

Problem:
The "force" flag of win_close() complicates the code and adds edge cases
where it is not clear what the correct behavior should be.
The "free_buf" flag of win_close() is passed on to float windows when
closing the last window of a tabpage, which doesn't make much sense.

Solution:
Remove the "force" flag and always close float windows as if :close! is
used when closing the last window of a tabpage, and set the "free_buf"
flag for a float window based on whether its buffer can be freed.
As 'hidden' is on by default, this change shouldn't affect many people.

11 files changed, 59 insertions, 79 deletions

diff --git a/src/nvim/api/window.c b/src/nvim/api/window.c
index df8ad165ba..76ea4b6d7f 100644
--- a/src/nvim/api/window.c
+++ b/src/nvim/api/window.c
@@ -370,7 +370,7 @@ void nvim_win_hide(Window window, Error *err)
   TryState tstate;
   try_enter(&tstate);
   if (tabpage == curtab) {
-    win_close(win, false, false);
+    win_close(win, false);
   } else {
     win_close_othertab(win, false, tabpage);
   }
diff --git a/src/nvim/arglist.c b/src/nvim/arglist.c
index c6a4be7e13..6c7886dcf3 100644
--- a/src/nvim/arglist.c
+++ b/src/nvim/arglist.c
@@ -927,7 +927,7 @@ static void arg_all_close_unused_windows(arg_all_state_T *aall)
               && (first_tabpage->tp_next == NULL || !aall->had_tab)) {
             aall->use_firstwin = true;
           } else {
-            win_close(wp, !buf_hide(buf) && !bufIsChanged(buf), false);
+            win_close(wp, !buf_hide(buf) && !bufIsChanged(buf));
             // check if autocommands removed the next window
             if (!win_valid(wpnext)) {
               // start all over...
diff --git a/src/nvim/buffer.c b/src/nvim/buffer.c
index 5dcb10751f..332c6cadb8 100644
--- a/src/nvim/buffer.c
+++ b/src/nvim/buffer.c
@@ -933,7 +933,7 @@ void goto_buffer(exarg_T *eap, int start, int dir, int count)
     enter_cleanup(&cs);
 
     // Quitting means closing the split window, nothing else.
-    win_close(curwin, true, false);
+    win_close(curwin, true);
     swap_exists_action = SEA_NONE;
     swap_exists_did_quit = true;
 
@@ -1335,7 +1335,7 @@ int do_buffer(int action, int start, int dir, int count, int forceit)
     while (buf == curbuf
            && !(curwin->w_closing || curwin->w_buffer->b_locked > 0)
            && (is_aucmd_win(lastwin) || !last_window(curwin))) {
-      if (win_close(curwin, false, false) == FAIL) {
+      if (win_close(curwin, false) == FAIL) {
         break;
       }
     }
@@ -3620,7 +3620,7 @@ void ex_buffer_all(exarg_T *eap)
           && !ONE_WINDOW
           && !(wp->w_closing
                || wp->w_buffer->b_locked > 0)) {
-        win_close(wp, false, false);
+        win_close(wp, false);
         wpnext = firstwin;              // just in case an autocommand does
                                         // something strange with windows
         tpnext = first_tabpage;         // start all over...
@@ -3700,7 +3700,7 @@ void ex_buffer_all(exarg_T *eap)
         enter_cleanup(&cs);
 
         // User selected Quit at ATTENTION prompt; close this window.
-        win_close(curwin, true, false);
+        win_close(curwin, true);
         open_wins--;
         swap_exists_action = SEA_NONE;
         swap_exists_did_quit = true;
@@ -3740,7 +3740,7 @@ void ex_buffer_all(exarg_T *eap)
       // BufWrite Autocommands made the window invalid, start over
       wp = lastwin;
     } else if (r) {
-      win_close(wp, !buf_hide(wp->w_buffer), false);
+      win_close(wp, !buf_hide(wp->w_buffer));
       open_wins--;
       wp = lastwin;
     } else {
diff --git a/src/nvim/ex_docmd.c b/src/nvim/ex_docmd.c
index a24e8458a6..f7f38e225c 100644
--- a/src/nvim/ex_docmd.c
+++ b/src/nvim/ex_docmd.c
@@ -4553,7 +4553,7 @@ static void ex_quit(exarg_T *eap)
     }
     not_exiting();
     // close window; may free buffer
-    win_close(wp, !buf_hide(wp->w_buffer) || eap->forceit, eap->forceit);
+    win_close(wp, !buf_hide(wp->w_buffer) || eap->forceit);
   }
 }
 
@@ -4663,7 +4663,7 @@ void ex_win_close(int forceit, win_T *win, tabpage_T *tp)
 
   // free buffer when not hiding it or when it's a scratch buffer
   if (tp == NULL) {
-    win_close(win, !need_hide && !buf_hide(buf), forceit);
+    win_close(win, !need_hide && !buf_hide(buf));
   } else {
     win_close_othertab(win, !need_hide && !buf_hide(buf), tp);
   }
@@ -4811,7 +4811,7 @@ static void ex_hide(exarg_T *eap)
   }
 
   if (eap->addr_count == 0) {
-    win_close(curwin, false, eap->forceit);  // don't free buffer
+    win_close(curwin, false);  // don't free buffer
   } else {
     int winnr = 0;
     win_T *win = NULL;
@@ -4826,7 +4826,7 @@ static void ex_hide(exarg_T *eap)
     if (win == NULL) {
       win = lastwin;
     }
-    win_close(win, false, eap->forceit);
+    win_close(win, false);
   }
 }
 
@@ -4883,7 +4883,7 @@ static void ex_exit(exarg_T *eap)
     }
     not_exiting();
     // Quit current window, may free the buffer.
-    win_close(curwin, !buf_hide(curwin->w_buffer), eap->forceit);
+    win_close(curwin, !buf_hide(curwin->w_buffer));
   }
 }
 
@@ -5282,7 +5282,7 @@ void do_exedit(exarg_T *eap, win_T *old_curwin)
           // Reset the error/interrupt/exception state here so that
           // aborting() returns false when closing a window.
           enter_cleanup(&cs);
-          win_close(curwin, !need_hide && !buf_hide(curbuf), false);
+          win_close(curwin, !need_hide && !buf_hide(curbuf));
 
           // Restore the error/interrupt/exception state if not
           // discarded by a new aborting error, interrupt, or
diff --git a/src/nvim/ex_getln.c b/src/nvim/ex_getln.c
index c62142310d..de671872dd 100644
--- a/src/nvim/ex_getln.c
+++ b/src/nvim/ex_getln.c
@@ -4355,7 +4355,7 @@ static int open_cmdwin(void)
   // Create empty command-line buffer.
   if (buf_open_scratch(0, _("[Command Line]")) == FAIL) {
     // Some autocommand messed it up?
-    win_close(curwin, true, false);
+    win_close(curwin, true);
     ga_clear(&winsizes);
     cmdwin_type = 0;
     return Ctrl_C;
@@ -4520,7 +4520,7 @@ static int open_cmdwin(void)
     // win_goto() may trigger an autocommand that already closes the
     // cmdline window.
     if (win_valid(wp) && wp != curwin) {
-      win_close(wp, true, false);
+      win_close(wp, true);
     }
 
     // win_close() may have already wiped the buffer when 'bh' is
diff --git a/src/nvim/help.c b/src/nvim/help.c
index bbc552fa4c..5d92d4d2ac 100644
--- a/src/nvim/help.c
+++ b/src/nvim/help.c
@@ -220,7 +220,7 @@ void ex_helpclose(exarg_T *eap)
 {
   FOR_ALL_WINDOWS_IN_TAB(win, curtab) {
     if (bt_help(win->w_buffer)) {
-      win_close(win, false, eap->forceit);
+      win_close(win, false);
       return;
     }
   }
diff --git a/src/nvim/main.c b/src/nvim/main.c
index bbe877356d..fa930b2b1e 100644
--- a/src/nvim/main.c
+++ b/src/nvim/main.c
@@ -1723,7 +1723,7 @@ static void edit_buffers(mparm_T *parmp, char *cwd)
 
   // When w_arg_idx is -1 remove the window (see create_windows()).
   if (curwin->w_arg_idx == -1) {
-    win_close(curwin, true, false);
+    win_close(curwin, true);
     advance = false;
   }
 
@@ -1735,7 +1735,7 @@ static void edit_buffers(mparm_T *parmp, char *cwd)
     // When w_arg_idx is -1 remove the window (see create_windows()).
     if (curwin->w_arg_idx == -1) {
       arg_idx++;
-      win_close(curwin, true, false);
+      win_close(curwin, true);
       advance = false;
       continue;
     }
@@ -1782,7 +1782,7 @@ static void edit_buffers(mparm_T *parmp, char *cwd)
           did_emsg = false;             // avoid hit-enter prompt
           getout(1);
         }
-        win_close(curwin, true, false);
+        win_close(curwin, true);
         advance = false;
       }
       if (arg_idx == GARGCOUNT - 1) {
diff --git a/src/nvim/quickfix.c b/src/nvim/quickfix.c
index 44daefddc7..2e0218741a 100644
--- a/src/nvim/quickfix.c
+++ b/src/nvim/quickfix.c
@@ -3006,7 +3006,7 @@ static void qf_jump_newwin(qf_info_T *qi, int dir, int errornr, int forceit, boo
 
   if (retval != OK) {
     if (opened_window) {
-      win_close(curwin, true, false);          // Close opened window
+      win_close(curwin, true);          // Close opened window
     }
     if (qf_ptr != NULL && qf_ptr->qf_fnum != 0) {
       // Couldn't open file, so put index back where it was.  This could
@@ -3548,7 +3548,7 @@ void ex_cclose(exarg_T *eap)
   // Find existing quickfix window and close it.
   win_T *win = qf_find_win(qi);
   if (win != NULL) {
-    win_close(win, false, false);
+    win_close(win, false);
   }
 }
 
@@ -5709,7 +5709,7 @@ static void wipe_dummy_buffer(buf_T *buf, char *dirname_start)
     if (firstwin->w_next != NULL) {
       for (win_T *wp = firstwin; wp != NULL; wp = wp->w_next) {
         if (wp->w_buffer == buf) {
-          if (win_close(wp, false, false) == OK) {
+          if (win_close(wp, false) == OK) {
             did_one = true;
           }
           break;
diff --git a/src/nvim/tag.c b/src/nvim/tag.c
index 18c7684584..4e3a45fed7 100644
--- a/src/nvim/tag.c
+++ b/src/nvim/tag.c
@@ -3093,7 +3093,7 @@ static int jumpto_tag(const char *lbuf_arg, int forceit, int keep_help)
   } else {
     RedrawingDisabled--;
     if (postponed_split) {              // close the window
-      win_close(curwin, false, false);
+      win_close(curwin, false);
       postponed_split = 0;
     }
   }
diff --git a/src/nvim/window.c b/src/nvim/window.c
index 92d11cef0a..62dc706beb 100644
--- a/src/nvim/window.c
+++ b/src/nvim/window.c
@@ -368,7 +368,7 @@ newwindow:
         newtab = curtab;
         goto_tabpage_tp(oldtab, true, true);
         if (curwin == wp) {
-          win_close(curwin, false, false);
+          win_close(curwin, false);
         }
         if (valid_tabpage(newtab)) {
           goto_tabpage_tp(newtab, true, true);
@@ -517,7 +517,7 @@ wingotofile:
         RESET_BINDING(curwin);
         if (do_ecmd(0, ptr, NULL, NULL, ECMD_LASTL, ECMD_HIDE, NULL) == FAIL) {
           // Failed to open the file, close the window opened for it.
-          win_close(curwin, false, false);
+          win_close(curwin, false);
           goto_tabpage_win(oldtab, oldwin);
         } else if (nchar == 'F' && lnum >= 0) {
           curwin->w_cursor.lnum = lnum;
@@ -2491,7 +2491,7 @@ void close_windows(buf_T *buf, bool keep_curwin)
   for (win_T *wp = lastwin; wp != NULL && (is_aucmd_win(lastwin) || !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) {
+      if (win_close(wp, false) == FAIL) {
         // If closing the window fails give up, to avoid looping forever.
         break;
       }
@@ -2568,24 +2568,6 @@ 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 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(void)
-{
-  assert(!is_aucmd_win(lastwin));
-  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);
-
-    if (need_hide && !buf_hide(buf)) {
-      return false;
-    }
-  }
-  return true;
-}
-
 /// Close the possibly last window in a tab page.
 ///
 /// @param  win          window to close
@@ -2677,7 +2659,7 @@ static void win_close_buffer(win_T *win, bool free_buf, bool abort_if_last)
 //
 // Called by :quit, :close, :xit, :wq and findtag().
 // Returns FAIL when the window was not closed.
-int win_close(win_T *win, bool free_buf, bool force)
+int win_close(win_T *win, bool free_buf)
 {
   tabpage_T *prev_curtab = curtab;
   frame_T *win_frame = win->w_floating ? NULL : win->w_frame->fr_parent;
@@ -2701,18 +2683,14 @@ int win_close(win_T *win, bool free_buf, bool force)
       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;
-        }
+    // close the last window until the there are no floating windows
+    while (lastwin->w_floating) {
+      buf_T *buf = lastwin->w_buffer;
+      bool need_hide = (bufIsChanged(buf) && buf->b_nwindows <= 1);
+      if (win_close(lastwin, !need_hide && !buf_hide(buf)) == FAIL) {
+        // If closing the window fails give up, to avoid looping forever.
+        return FAIL;
       }
-    } else {
-      emsg(e_floatonly);
-      return FAIL;
     }
   }
 
@@ -3902,7 +3880,7 @@ void close_others(int message, int forceit)
         continue;
       }
     }
-    win_close(wp, !buf_hide(wp->w_buffer) && !bufIsChanged(wp->w_buffer), false);
+    win_close(wp, !buf_hide(wp->w_buffer) && !bufIsChanged(wp->w_buffer));
   }
 
   if (message && !ONE_WINDOW) {
diff --git a/test/functional/ui/float_spec.lua b/test/functional/ui/float_spec.lua
index 6759510ad1..60952543e7 100644
--- a/test/functional/ui/float_spec.lua
+++ b/test/functional/ui/float_spec.lua
@@ -462,6 +462,9 @@ describe('float window', function()
       end)
     end)
     describe("deleting the last non-floating window's buffer", function()
+      after_each(function()
+        eq(false, meths.buf_is_valid(old_buf))
+      end)
       describe('leaves one window with an empty buffer when there is only one buffer', function()
         local same_buf_float
         before_each(function()
@@ -552,6 +555,9 @@ describe('float window', function()
       end)
     end)
     describe('with splits, deleting the last listed buffer creates an empty buffer', function()
+      after_each(function()
+        eq(false, meths.buf_is_valid(old_buf))
+      end)
       describe('when a non-floating window has an unlisted buffer', function()
         local same_buf_float
         before_each(function()
@@ -597,6 +603,7 @@ describe('float window', function()
         same_buf_float = meths.open_win(old_buf, false, float_opts).id
       end)
       after_each(function()
+        eq(false, meths.buf_is_valid(old_buf))
         expect('')
         eq(2, #meths.list_wins())
         eq(2, #meths.list_tabpages())
@@ -629,6 +636,7 @@ describe('float window', function()
         same_buf_float = meths.open_win(old_buf, false, float_opts).id
       end)
       after_each(function()
+        eq(false, meths.buf_is_valid(old_buf))
         expect('')
         eq(3, #meths.list_wins())
         eq(2, #meths.list_tabpages())
@@ -656,12 +664,18 @@ describe('float window', function()
       old_win = curwin().id
     end)
     describe('closing the last non-floating window', function()
-      describe('closes the tabpage when all floating windows are closeable', function()
-        local same_buf_float
+      describe('closes the tabpage force-closing floating windows', function()
+        local same_buf_float, other_buf, other_buf_float
         before_each(function()
+          command('set nohidden')
           same_buf_float = meths.open_win(old_buf, false, float_opts).id
+          other_buf = meths.create_buf(true, false).id
+          other_buf_float = meths.open_win(other_buf, true, float_opts).id
+          insert('foo')
+          meths.set_current_win(old_win)
         end)
         after_each(function()
+          eq(true, meths.buf_is_valid(other_buf))
           eq(old_tabpage, curtab().id)
           expect('oldtab')
           eq(1, #meths.list_tabpages())
@@ -669,41 +683,30 @@ describe('float window', function()
         it('if called from non-floating window', function()
           meths.win_close(old_win, false)
         end)
-        it('if called from floating window', function()
+        it('if called from floating window with the same buffer', function()
           meths.set_current_win(same_buf_float)
           meths.win_close(old_win, false)
         end)
-      end)
-      describe('gives E5601 when there are non-closeable floating windows', function()
-        local other_buf_float
-        before_each(function()
-          command('set nohidden')
-          local other_buf = meths.create_buf(true, false).id
-          other_buf_float = meths.open_win(other_buf, true, float_opts).id
-          insert('foo')
-          meths.set_current_win(old_win)
-        end)
-        it('if called from non-floating window', function()
-          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()
+        it('if called from floating window with another buffer', function()
           meths.set_current_win(other_buf_float)
-          eq('Vim:E5601: Cannot close window, only floating window would remain',
-             pcall_err(meths.win_close, old_win, false))
+          meths.win_close(old_win, false)
         end)
       end)
     end)
     describe("deleting the last non-floating window's buffer", function()
-      describe('closes the tabpage when all floating windows are closeable', function()
+      describe('closes the tabpage force-closing floating windows', function()
         local same_buf_float, other_buf, other_buf_float
         before_each(function()
+          command('set nohidden')
           same_buf_float = meths.open_win(old_buf, false, float_opts).id
           other_buf = meths.create_buf(true, false).id
           other_buf_float = meths.open_win(other_buf, true, float_opts).id
+          insert('foo')
           meths.set_current_win(old_win)
         end)
         after_each(function()
+          eq(false, meths.buf_is_valid(old_buf))
+          eq(true, meths.buf_is_valid(other_buf))
           eq(old_tabpage, curtab().id)
           expect('oldtab')
           eq(1, #meths.list_tabpages())
@@ -721,7 +724,6 @@ describe('float window', function()
           meths.buf_delete(old_buf, {force = false})
         end)
       end)
-      -- TODO: what to do when there are non-closeable floating windows?
     end)
   end)