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

This reverts commit 0371d0f7af.

> 'bufhidden' option exists. I don't think we should assume autoclosing
windows are fine just because 'hidden' is set.
This commit is contained in:
zeertzjq
2023-01-24 18:31:07 +08:00
committed by GitHub
parent 39630265c4
commit c6ab8dfc15
11 changed files with 84 additions and 64 deletions

View File

@@ -374,7 +374,7 @@ void nvim_win_hide(Window window, Error *err)
if (is_aucmd_win(win)) { if (is_aucmd_win(win)) {
emsg(_(e_autocmd_close)); emsg(_(e_autocmd_close));
} else if (tabpage == curtab) { } else if (tabpage == curtab) {
win_close(win, false); win_close(win, false, false);
} else { } else {
win_close_othertab(win, false, tabpage); win_close_othertab(win, false, tabpage);
} }

View File

@@ -927,7 +927,7 @@ static void arg_all_close_unused_windows(arg_all_state_T *aall)
&& (first_tabpage->tp_next == NULL || !aall->had_tab)) { && (first_tabpage->tp_next == NULL || !aall->had_tab)) {
aall->use_firstwin = true; aall->use_firstwin = true;
} else { } else {
win_close(wp, !buf_hide(buf) && !bufIsChanged(buf)); win_close(wp, !buf_hide(buf) && !bufIsChanged(buf), false);
// check if autocommands removed the next window // check if autocommands removed the next window
if (!win_valid(wpnext)) { if (!win_valid(wpnext)) {
// start all over... // start all over...

View File

@@ -933,7 +933,7 @@ void goto_buffer(exarg_T *eap, int start, int dir, int count)
enter_cleanup(&cs); enter_cleanup(&cs);
// Quitting means closing the split window, nothing else. // Quitting means closing the split window, nothing else.
win_close(curwin, true); win_close(curwin, true, false);
swap_exists_action = SEA_NONE; swap_exists_action = SEA_NONE;
swap_exists_did_quit = true; 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 while (buf == curbuf
&& !(curwin->w_closing || curwin->w_buffer->b_locked > 0) && !(curwin->w_closing || curwin->w_buffer->b_locked > 0)
&& (is_aucmd_win(lastwin) || !last_window(curwin))) { && (is_aucmd_win(lastwin) || !last_window(curwin))) {
if (win_close(curwin, false) == FAIL) { if (win_close(curwin, false, false) == FAIL) {
break; break;
} }
} }
@@ -3620,7 +3620,7 @@ void ex_buffer_all(exarg_T *eap)
&& !ONE_WINDOW && !ONE_WINDOW
&& !(wp->w_closing && !(wp->w_closing
|| wp->w_buffer->b_locked > 0)) { || wp->w_buffer->b_locked > 0)) {
win_close(wp, false); win_close(wp, false, false);
wpnext = firstwin; // just in case an autocommand does wpnext = firstwin; // just in case an autocommand does
// something strange with windows // something strange with windows
tpnext = first_tabpage; // start all over... tpnext = first_tabpage; // start all over...
@@ -3700,7 +3700,7 @@ void ex_buffer_all(exarg_T *eap)
enter_cleanup(&cs); enter_cleanup(&cs);
// User selected Quit at ATTENTION prompt; close this window. // User selected Quit at ATTENTION prompt; close this window.
win_close(curwin, true); win_close(curwin, true, false);
open_wins--; open_wins--;
swap_exists_action = SEA_NONE; swap_exists_action = SEA_NONE;
swap_exists_did_quit = true; swap_exists_did_quit = true;
@@ -3740,7 +3740,7 @@ void ex_buffer_all(exarg_T *eap)
// BufWrite Autocommands made the window invalid, start over // BufWrite Autocommands made the window invalid, start over
wp = lastwin; wp = lastwin;
} else if (r) { } else if (r) {
win_close(wp, !buf_hide(wp->w_buffer)); win_close(wp, !buf_hide(wp->w_buffer), false);
open_wins--; open_wins--;
wp = lastwin; wp = lastwin;
} else { } else {

View File

@@ -4553,7 +4553,7 @@ static void ex_quit(exarg_T *eap)
} }
not_exiting(); not_exiting();
// close window; may free buffer // close window; may free buffer
win_close(wp, !buf_hide(wp->w_buffer) || eap->forceit); win_close(wp, !buf_hide(wp->w_buffer) || eap->forceit, 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 // free buffer when not hiding it or when it's a scratch buffer
if (tp == NULL) { if (tp == NULL) {
win_close(win, !need_hide && !buf_hide(buf)); win_close(win, !need_hide && !buf_hide(buf), forceit);
} else { } else {
win_close_othertab(win, !need_hide && !buf_hide(buf), tp); 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) { if (eap->addr_count == 0) {
win_close(curwin, false); // don't free buffer win_close(curwin, false, eap->forceit); // don't free buffer
} else { } else {
int winnr = 0; int winnr = 0;
win_T *win = NULL; win_T *win = NULL;
@@ -4826,7 +4826,7 @@ static void ex_hide(exarg_T *eap)
if (win == NULL) { if (win == NULL) {
win = lastwin; win = lastwin;
} }
win_close(win, false); win_close(win, false, eap->forceit);
} }
} }
@@ -4883,7 +4883,7 @@ static void ex_exit(exarg_T *eap)
} }
not_exiting(); not_exiting();
// Quit current window, may free the buffer. // Quit current window, may free the buffer.
win_close(curwin, !buf_hide(curwin->w_buffer)); win_close(curwin, !buf_hide(curwin->w_buffer), eap->forceit);
} }
} }
@@ -5282,7 +5282,7 @@ void do_exedit(exarg_T *eap, win_T *old_curwin)
// Reset the error/interrupt/exception state here so that // Reset the error/interrupt/exception state here so that
// aborting() returns false when closing a window. // aborting() returns false when closing a window.
enter_cleanup(&cs); enter_cleanup(&cs);
win_close(curwin, !need_hide && !buf_hide(curbuf)); win_close(curwin, !need_hide && !buf_hide(curbuf), false);
// Restore the error/interrupt/exception state if not // Restore the error/interrupt/exception state if not
// discarded by a new aborting error, interrupt, or // discarded by a new aborting error, interrupt, or

View File

@@ -4355,7 +4355,7 @@ static int open_cmdwin(void)
// Create empty command-line buffer. // Create empty command-line buffer.
if (buf_open_scratch(0, _("[Command Line]")) == FAIL) { if (buf_open_scratch(0, _("[Command Line]")) == FAIL) {
// Some autocommand messed it up? // Some autocommand messed it up?
win_close(curwin, true); win_close(curwin, true, false);
ga_clear(&winsizes); ga_clear(&winsizes);
cmdwin_type = 0; cmdwin_type = 0;
return Ctrl_C; return Ctrl_C;
@@ -4520,7 +4520,7 @@ static int open_cmdwin(void)
// win_goto() may trigger an autocommand that already closes the // win_goto() may trigger an autocommand that already closes the
// cmdline window. // cmdline window.
if (win_valid(wp) && wp != curwin) { if (win_valid(wp) && wp != curwin) {
win_close(wp, true); win_close(wp, true, false);
} }
// win_close() may have already wiped the buffer when 'bh' is // win_close() may have already wiped the buffer when 'bh' is

View File

@@ -220,7 +220,7 @@ void ex_helpclose(exarg_T *eap)
{ {
FOR_ALL_WINDOWS_IN_TAB(win, curtab) { FOR_ALL_WINDOWS_IN_TAB(win, curtab) {
if (bt_help(win->w_buffer)) { if (bt_help(win->w_buffer)) {
win_close(win, false); win_close(win, false, eap->forceit);
return; return;
} }
} }

View File

@@ -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()). // When w_arg_idx is -1 remove the window (see create_windows()).
if (curwin->w_arg_idx == -1) { if (curwin->w_arg_idx == -1) {
win_close(curwin, true); win_close(curwin, true, false);
advance = false; 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()). // When w_arg_idx is -1 remove the window (see create_windows()).
if (curwin->w_arg_idx == -1) { if (curwin->w_arg_idx == -1) {
arg_idx++; arg_idx++;
win_close(curwin, true); win_close(curwin, true, false);
advance = false; advance = false;
continue; continue;
} }
@@ -1782,7 +1782,7 @@ static void edit_buffers(mparm_T *parmp, char *cwd)
did_emsg = false; // avoid hit-enter prompt did_emsg = false; // avoid hit-enter prompt
getout(1); getout(1);
} }
win_close(curwin, true); win_close(curwin, true, false);
advance = false; advance = false;
} }
if (arg_idx == GARGCOUNT - 1) { if (arg_idx == GARGCOUNT - 1) {

View File

@@ -3006,7 +3006,7 @@ static void qf_jump_newwin(qf_info_T *qi, int dir, int errornr, int forceit, boo
if (retval != OK) { if (retval != OK) {
if (opened_window) { if (opened_window) {
win_close(curwin, true); // Close opened window win_close(curwin, true, false); // Close opened window
} }
if (qf_ptr != NULL && qf_ptr->qf_fnum != 0) { if (qf_ptr != NULL && qf_ptr->qf_fnum != 0) {
// Couldn't open file, so put index back where it was. This could // 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. // Find existing quickfix window and close it.
win_T *win = qf_find_win(qi); win_T *win = qf_find_win(qi);
if (win != NULL) { if (win != NULL) {
win_close(win, false); win_close(win, false, false);
} }
} }
@@ -5709,7 +5709,7 @@ static void wipe_dummy_buffer(buf_T *buf, char *dirname_start)
if (firstwin->w_next != NULL) { if (firstwin->w_next != NULL) {
for (win_T *wp = firstwin; wp != NULL; wp = wp->w_next) { for (win_T *wp = firstwin; wp != NULL; wp = wp->w_next) {
if (wp->w_buffer == buf) { if (wp->w_buffer == buf) {
if (win_close(wp, false) == OK) { if (win_close(wp, false, false) == OK) {
did_one = true; did_one = true;
} }
break; break;

View File

@@ -3093,7 +3093,7 @@ static int jumpto_tag(const char *lbuf_arg, int forceit, int keep_help)
} else { } else {
RedrawingDisabled--; RedrawingDisabled--;
if (postponed_split) { // close the window if (postponed_split) { // close the window
win_close(curwin, false); win_close(curwin, false, false);
postponed_split = 0; postponed_split = 0;
} }
} }

View File

@@ -368,7 +368,7 @@ newwindow:
newtab = curtab; newtab = curtab;
goto_tabpage_tp(oldtab, true, true); goto_tabpage_tp(oldtab, true, true);
if (curwin == wp) { if (curwin == wp) {
win_close(curwin, false); win_close(curwin, false, false);
} }
if (valid_tabpage(newtab)) { if (valid_tabpage(newtab)) {
goto_tabpage_tp(newtab, true, true); goto_tabpage_tp(newtab, true, true);
@@ -517,7 +517,7 @@ wingotofile:
RESET_BINDING(curwin); RESET_BINDING(curwin);
if (do_ecmd(0, ptr, NULL, NULL, ECMD_LASTL, ECMD_HIDE, NULL) == FAIL) { if (do_ecmd(0, ptr, NULL, NULL, ECMD_LASTL, ECMD_HIDE, NULL) == FAIL) {
// Failed to open the file, close the window opened for it. // Failed to open the file, close the window opened for it.
win_close(curwin, false); win_close(curwin, false, false);
goto_tabpage_win(oldtab, oldwin); goto_tabpage_win(oldtab, oldwin);
} else if (nchar == 'F' && lnum >= 0) { } else if (nchar == 'F' && lnum >= 0) {
curwin->w_cursor.lnum = lnum; 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));) { for (win_T *wp = lastwin; wp != NULL && (is_aucmd_win(lastwin) || !one_window(wp));) {
if (wp->w_buffer == buf && (!keep_curwin || wp != curwin) if (wp->w_buffer == buf && (!keep_curwin || wp != curwin)
&& !(wp->w_closing || wp->w_buffer->b_locked > 0)) { && !(wp->w_closing || wp->w_buffer->b_locked > 0)) {
if (win_close(wp, false) == FAIL) { if (win_close(wp, false, false) == FAIL) {
// If closing the window fails give up, to avoid looping forever. // If closing the window fails give up, to avoid looping forever.
break; break;
} }
@@ -2567,6 +2567,24 @@ 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); 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. /// Close the possibly last window in a tab page.
/// ///
/// @param win window to close /// @param win window to close
@@ -2658,7 +2676,7 @@ static void win_close_buffer(win_T *win, bool free_buf, bool abort_if_last)
// //
// Called by :quit, :close, :xit, :wq and findtag(). // Called by :quit, :close, :xit, :wq and findtag().
// Returns FAIL when the window was not closed. // Returns FAIL when the window was not closed.
int win_close(win_T *win, bool free_buf) int win_close(win_T *win, bool free_buf, bool force)
{ {
tabpage_T *prev_curtab = curtab; tabpage_T *prev_curtab = curtab;
frame_T *win_frame = win->w_floating ? NULL : win->w_frame->fr_parent; frame_T *win_frame = win->w_floating ? NULL : win->w_frame->fr_parent;
@@ -2682,15 +2700,19 @@ int win_close(win_T *win, bool free_buf)
emsg(_("E814: Cannot close window, only autocmd window would remain")); emsg(_("E814: Cannot close window, only autocmd window would remain"));
return FAIL; return FAIL;
} }
if (force || can_close_floating_windows()) {
// close the last window until the there are no floating windows // close the last window until the there are no floating windows
while (lastwin->w_floating) { while (lastwin->w_floating) {
buf_T *buf = lastwin->w_buffer; // `force` flag isn't actually used when closing a floating window.
bool need_hide = (bufIsChanged(buf) && buf->b_nwindows <= 1); if (win_close(lastwin, free_buf, true) == FAIL) {
if (win_close(lastwin, !need_hide && !buf_hide(buf)) == FAIL) {
// If closing the window fails give up, to avoid looping forever. // If closing the window fails give up, to avoid looping forever.
return FAIL; return FAIL;
} }
} }
} else {
emsg(e_floatonly);
return FAIL;
}
} }
// When closing the last window in a tab page first go to another tab page // When closing the last window in a tab page first go to another tab page
@@ -3879,7 +3901,7 @@ void close_others(int message, int forceit)
continue; continue;
} }
} }
win_close(wp, !buf_hide(wp->w_buffer) && !bufIsChanged(wp->w_buffer)); win_close(wp, !buf_hide(wp->w_buffer) && !bufIsChanged(wp->w_buffer), false);
} }
if (message && !ONE_WINDOW) { if (message && !ONE_WINDOW) {

View File

@@ -462,9 +462,6 @@ describe('float window', function()
end) end)
end) end)
describe("deleting the last non-floating window's buffer", function() 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() describe('leaves one window with an empty buffer when there is only one buffer', function()
local same_buf_float local same_buf_float
before_each(function() before_each(function()
@@ -555,9 +552,6 @@ describe('float window', function()
end) end)
end) end)
describe('with splits, deleting the last listed buffer creates an empty buffer', function() 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() describe('when a non-floating window has an unlisted buffer', function()
local same_buf_float local same_buf_float
before_each(function() before_each(function()
@@ -603,7 +597,6 @@ describe('float window', function()
same_buf_float = meths.open_win(old_buf, false, float_opts).id same_buf_float = meths.open_win(old_buf, false, float_opts).id
end) end)
after_each(function() after_each(function()
eq(false, meths.buf_is_valid(old_buf))
expect('') expect('')
eq(2, #meths.list_wins()) eq(2, #meths.list_wins())
eq(2, #meths.list_tabpages()) eq(2, #meths.list_tabpages())
@@ -636,7 +629,6 @@ describe('float window', function()
same_buf_float = meths.open_win(old_buf, false, float_opts).id same_buf_float = meths.open_win(old_buf, false, float_opts).id
end) end)
after_each(function() after_each(function()
eq(false, meths.buf_is_valid(old_buf))
expect('') expect('')
eq(3, #meths.list_wins()) eq(3, #meths.list_wins())
eq(2, #meths.list_tabpages()) eq(2, #meths.list_tabpages())
@@ -664,18 +656,12 @@ describe('float window', function()
old_win = curwin().id old_win = curwin().id
end) end)
describe('closing the last non-floating window', function() describe('closing the last non-floating window', function()
describe('closes the tabpage force-closing floating windows', function() describe('closes the tabpage when all floating windows are closeable', function()
local same_buf_float, other_buf, other_buf_float local same_buf_float
before_each(function() before_each(function()
command('set nohidden')
same_buf_float = meths.open_win(old_buf, false, float_opts).id 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) end)
after_each(function() after_each(function()
eq(true, meths.buf_is_valid(other_buf))
eq(old_tabpage, curtab().id) eq(old_tabpage, curtab().id)
expect('oldtab') expect('oldtab')
eq(1, #meths.list_tabpages()) eq(1, #meths.list_tabpages())
@@ -683,30 +669,41 @@ describe('float window', function()
it('if called from non-floating window', function() it('if called from non-floating window', function()
meths.win_close(old_win, false) meths.win_close(old_win, false)
end) end)
it('if called from floating window with the same buffer', function() it('if called from floating window', function()
meths.set_current_win(same_buf_float) meths.set_current_win(same_buf_float)
meths.win_close(old_win, false) meths.win_close(old_win, false)
end) end)
it('if called from floating window with another buffer', function()
meths.set_current_win(other_buf_float)
meths.win_close(old_win, false)
end) end)
end) describe('gives E5601 when there are non-closeable floating windows', function()
end) local other_buf_float
describe("deleting the last non-floating window's buffer", function()
describe('closes the tabpage force-closing floating windows', function()
local same_buf_float, other_buf, other_buf_float
before_each(function() before_each(function()
command('set nohidden') command('set nohidden')
same_buf_float = meths.open_win(old_buf, false, float_opts).id local other_buf = meths.create_buf(true, false).id
other_buf = meths.create_buf(true, false).id
other_buf_float = meths.open_win(other_buf, true, float_opts).id other_buf_float = meths.open_win(other_buf, true, float_opts).id
insert('foo') insert('foo')
meths.set_current_win(old_win) meths.set_current_win(old_win)
end) 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()
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))
end)
end)
end)
describe("deleting the last non-floating window's buffer", function()
describe('closes the tabpage when all floating windows are closeable', function()
local same_buf_float, other_buf, other_buf_float
before_each(function()
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
meths.set_current_win(old_win)
end)
after_each(function() after_each(function()
eq(false, meths.buf_is_valid(old_buf))
eq(true, meths.buf_is_valid(other_buf))
eq(old_tabpage, curtab().id) eq(old_tabpage, curtab().id)
expect('oldtab') expect('oldtab')
eq(1, #meths.list_tabpages()) eq(1, #meths.list_tabpages())
@@ -724,6 +721,7 @@ describe('float window', function()
meths.buf_delete(old_buf, {force = false}) meths.buf_delete(old_buf, {force = false})
end) end)
end) end)
-- TODO: what to do when there are non-closeable floating windows?
end) end)
end) end)