IntenseFinance/neovim
3
0
Fork 0
mirror of https://github.com/neovim/neovim.git synced 2025-02-25 18:55:25 -06:00
Code Issues Packages Projects Releases Wiki Activity

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

Browse Source 
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.
This commit is contained in:
zeertzjq 2023-01-23 18:55:11 +08:00 committed by GitHub
parent d58bf4ff30
commit 0371d0f7af
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
11 changed files with 59 additions and 79 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
src/nvim/api/window.c
View File

@ -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);
}

2
src/nvim/arglist.c
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)) {
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...

10
src/nvim/buffer.c
View File

@ -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 {

12
src/nvim/ex_docmd.c
View File

@ -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

4
src/nvim/ex_getln.c
View File

@ -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

2
src/nvim/help.c
View File

@ -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;
}
}

6
src/nvim/main.c
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()).
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) {

6
src/nvim/quickfix.c
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 (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;

2
src/nvim/tag.c
View File

@ -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;
}
}

46
src/nvim/window.c
View File

@ -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) {

46
test/functional/ui/float_spec.lua
View File

@ -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)