diff --git a/src/nvim/api/win_config.c b/src/nvim/api/win_config.c index d64b9085e3..2af6e56a3d 100644 --- a/src/nvim/api/win_config.c +++ b/src/nvim/api/win_config.c @@ -290,6 +290,13 @@ Window nvim_open_win(Buffer buffer, Boolean enter, Dict(win_config) *config, Err } } } else { + // Unlike check_split_disallowed_err, ignore `split_disallowed`, as opening a float shouldn't + // mess with the frame structure. Still check `b_locked_split` to avoid opening more windows + // into a closing buffer, though. + if (curwin->w_buffer->b_locked_split) { // Can't instead check `buf` in case win_set_buf fails! + api_set_error(err, kErrorTypeException, "E1159: Cannot open a float when closing the buffer"); + goto cleanup; + } wp = win_new_float(NULL, false, fconfig, err); } if (!wp) { @@ -336,6 +343,7 @@ Window nvim_open_win(Buffer buffer, Boolean enter, Dict(win_config) *config, Err } } if (!tp) { + api_clear_error(err); // may have been set by win_set_buf api_set_error(err, kErrorTypeException, "Window was closed immediately"); goto cleanup; } @@ -489,7 +497,14 @@ void nvim_win_set_config(Window window, Dict(win_config) *config, Error *err) if (curwin_moving_tp) { if (was_split) { int dir; - win_goto(winframe_find_altwin(win, &dir, NULL, NULL)); + win_T *altwin = winframe_find_altwin(win, &dir, NULL, NULL); + // Autocommands may still make this the last non-float after this check. + // That case will be caught later when trying to move the window. + if (!altwin) { + api_set_error(err, kErrorTypeException, "Cannot move last non-floating window"); + return; + } + win_goto(altwin); } else { win_goto(win_float_find_altwin(win, NULL)); } @@ -522,7 +537,7 @@ void nvim_win_set_config(Window window, Dict(win_config) *config, Error *err) // FIXME(willothy): if the window is the last in the tabpage but there is another tabpage // and the target window is in that other tabpage, should we move the window to that // tabpage and close the previous one, or just error? - api_set_error(err, kErrorTypeException, "Cannot move last window"); + api_set_error(err, kErrorTypeException, "Cannot move last non-floating window"); goto restore_curwin; } else if (parent != NULL && parent->handle == win->handle) { int n_frames = 0; diff --git a/src/nvim/api/window.c b/src/nvim/api/window.c index cde629dab3..2bdb11d285 100644 --- a/src/nvim/api/window.c +++ b/src/nvim/api/window.c @@ -374,7 +374,7 @@ void nvim_win_hide(Window window, Error *err) } else if (tabpage == curtab) { win_close(win, false, false); } else { - win_close_othertab(win, false, tabpage); + win_close_othertab(win, false, tabpage, false); } }); } diff --git a/src/nvim/buffer.c b/src/nvim/buffer.c index 8c0fb9a230..4d46a3ec7c 100644 --- a/src/nvim/buffer.c +++ b/src/nvim/buffer.c @@ -500,7 +500,7 @@ static bool can_unload_buffer(buf_T *buf) /// close all other windows. /// @param ignore_abort /// If true, don't abort even when aborting() returns true. -/// @return true when we got to the end and b_nwindows was decremented. +/// @return true if b_nwindows was decremented directly by this call (e.g: not via autocmds). bool close_buffer(win_T *win, buf_T *buf, int action, bool abort_if_last, bool ignore_abort) { bool unload_buf = (action != 0); @@ -571,7 +571,7 @@ bool close_buffer(win_T *win, buf_T *buf, int action, bool abort_if_last, bool i } buf->b_locked--; buf->b_locked_split--; - if (abort_if_last && one_window(win)) { + if (abort_if_last && win != NULL && one_window(win, NULL)) { // Autocommands made this the only window. emsg(_(e_auabort)); return false; @@ -590,7 +590,7 @@ bool close_buffer(win_T *win, buf_T *buf, int action, bool abort_if_last, bool i } buf->b_locked--; buf->b_locked_split--; - if (abort_if_last && one_window(win)) { + if (abort_if_last && win != NULL && one_window(win, NULL)) { // Autocommands made this the only window. emsg(_(e_auabort)); return false; @@ -625,7 +625,7 @@ bool close_buffer(win_T *win, buf_T *buf, int action, bool abort_if_last, bool i // Return when a window is displaying the buffer or when it's not // unloaded. if (buf->b_nwindows > 0 || !unload_buf) { - return false; + return true; } if (buf->terminal) { @@ -702,7 +702,7 @@ bool close_buffer(win_T *win, buf_T *buf, int action, bool abort_if_last, bool i } // Do not wipe out the buffer if it is used in a window. if (buf->b_nwindows > 0) { - return false; + return true; } FOR_ALL_TAB_WINDOWS(tp, wp) { mark_forget_file(wp, buf->b_fnum); diff --git a/src/nvim/ex_docmd.c b/src/nvim/ex_docmd.c index 0a52b7e909..a3abe75150 100644 --- a/src/nvim/ex_docmd.c +++ b/src/nvim/ex_docmd.c @@ -4915,7 +4915,7 @@ void ex_win_close(int forceit, win_T *win, tabpage_T *tp) if (tp == NULL) { win_close(win, !need_hide && !buf_hide(buf), forceit); } else { - win_close_othertab(win, !need_hide && !buf_hide(buf), tp); + win_close_othertab(win, !need_hide && !buf_hide(buf), tp, forceit); } } diff --git a/src/nvim/version.c b/src/nvim/version.c index 755288f137..400a9719bf 100644 --- a/src/nvim/version.c +++ b/src/nvim/version.c @@ -2720,7 +2720,7 @@ bool may_show_intro(void) && (curbuf->b_fname == NULL) && (curbuf->handle == 1) && (curwin->handle == LOWEST_WIN_ID) - && one_window(curwin) + && one_window(curwin, NULL) && (vim_strchr(p_shm, SHM_INTRO) == NULL)); } diff --git a/src/nvim/window.c b/src/nvim/window.c index 7cfe436337..9e36d93953 100644 --- a/src/nvim/window.c +++ b/src/nvim/window.c @@ -427,7 +427,7 @@ newwindow: // move window to new tab page case 'T': CHECK_CMDWIN; - if (one_window(curwin)) { + if (one_window(curwin, NULL)) { msg(_(m_onlyone), 0); } else { tabpage_T *oldtab = curtab; @@ -500,7 +500,7 @@ newwindow: case 'H': case 'L': CHECK_CMDWIN; - if (one_window(curwin)) { + if (one_window(curwin, NULL)) { beep_flush(); } else { const int dir = ((nchar == 'H' || nchar == 'L') ? WSP_VERT : 0) @@ -754,15 +754,17 @@ static void cmd_with_count(char *cmd, char *bufp, size_t bufsize, int64_t Prenum void win_set_buf(win_T *win, buf_T *buf, Error *err) FUNC_ATTR_NONNULL_ALL { + const handle_T win_handle = win->handle; tabpage_T *tab = win_find_tabpage(win); // no redrawing and don't set the window title RedrawingDisabled++; switchwin_T switchwin; + int win_result; TRY_WRAP(err, { - int win_result = switch_win_noblock(&switchwin, win, tab, true); + win_result = switch_win_noblock(&switchwin, win, tab, true); if (win_result != FAIL) { const int save_acd = p_acd; if (!switchwin.sw_same_win) { @@ -777,6 +779,9 @@ void win_set_buf(win_T *win, buf_T *buf, Error *err) } } }); + if (win_result == FAIL && !ERROR_SET(err)) { + api_set_error(err, kErrorTypeException, "Failed to switch to window %d", win_handle); + } // If window is not current, state logic will not validate its cursor. So do it now. // Still needed if do_buffer returns FAIL (e.g: autocmds abort script after buffer was set). @@ -1081,7 +1086,7 @@ win_T *win_split_ins(int size, int flags, win_T *new_wp, int dir, frame_T *to_fl bool toplevel = flags & (WSP_TOP | WSP_BOT); // add a status line when p_ls == 1 and splitting the first window - if (one_window(firstwin) && p_ls == 1 && oldwin->w_status_height == 0) { + if (one_window(firstwin, NULL) && p_ls == 1 && oldwin->w_status_height == 0) { if (oldwin->w_height <= p_wmh) { emsg(_(e_noroom)); return NULL; @@ -1791,7 +1796,7 @@ static void win_exchange(int Prenum) return; } - if (one_window(curwin)) { + if (one_window(curwin, NULL)) { // just one window beep_flush(); return; @@ -1883,7 +1888,7 @@ static void win_rotate(bool upwards, int count) return; } - if (count <= 0 || one_window(curwin)) { + if (count <= 0 || one_window(curwin, NULL)) { // nothing to do beep_flush(); return; @@ -1966,7 +1971,7 @@ int win_splitmove(win_T *wp, int size, int flags) int dir = 0; int height = wp->w_height; - if (one_window(wp)) { + if (one_window(wp, NULL)) { return OK; // nothing to do } if (is_aucmd_win(wp) || check_split_disallowed(wp) == FAIL) { @@ -2495,7 +2500,7 @@ void close_windows(buf_T *buf, bool keep_curwin) // 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 && (is_aucmd_win(lastwin) || !one_window(wp));) { + for (win_T *wp = lastwin; wp != NULL && (is_aucmd_win(lastwin) || !one_window(wp, NULL));) { if (wp->w_buffer == buf && (!keep_curwin || wp != curwin) && !(win_locked(wp) || wp->w_buffer->b_locked > 0)) { if (win_close(wp, false, false) == FAIL) { @@ -2520,7 +2525,10 @@ void close_windows(buf_T *buf, bool keep_curwin) for (win_T *wp = tp->tp_lastwin; wp != NULL; wp = wp->w_prev) { if (wp->w_buffer == buf && !(win_locked(wp) || wp->w_buffer->b_locked > 0)) { - win_close_othertab(wp, false, tp); + if (!win_close_othertab(wp, false, tp, false)) { + // If closing the window fails give up, to avoid looping forever. + break; + } // Start all over, the tab page may be closed and // autocommands may change the window layout. @@ -2537,27 +2545,30 @@ void close_windows(buf_T *buf, bool keep_curwin) /// Check if "win" is the last non-floating window that exists. bool last_window(win_T *win) FUNC_ATTR_PURE FUNC_ATTR_WARN_UNUSED_RESULT { - return one_window(win) && first_tabpage->tp_next == NULL; + return one_window(win, NULL) && first_tabpage->tp_next == NULL; } -/// Check if "win" is the only non-floating window in the current tabpage. +/// Check if "win" is the only non-floating window in tabpage "tp", or NULL for current tabpage. /// /// This should be used in place of ONE_WINDOW when necessary, /// with "firstwin" or the affected window as argument depending on the situation. -bool one_window(win_T *win) FUNC_ATTR_PURE FUNC_ATTR_WARN_UNUSED_RESULT +bool one_window(win_T *win, tabpage_T *tp) + FUNC_ATTR_PURE FUNC_ATTR_WARN_UNUSED_RESULT FUNC_ATTR_NONNULL_ARG(1) { - assert(!firstwin->w_floating); - return firstwin == win && (win->w_next == NULL || win->w_next->w_floating); + win_T *first = tp ? tp->tp_firstwin : firstwin; + assert((!tp || tp != curtab) && !first->w_floating); + return first == win && (win->w_next == NULL || win->w_next->w_floating); } -/// Check if floating windows in the current tab can be closed. +/// Check if floating windows in tabpage `tp` can be closed. /// Do not call this when the autocommand window is in use! /// +/// @param tp tabpage to check. Must be NULL for the current tabpage. /// @return true if all floating windows can be closed -static bool can_close_floating_windows(void) +static bool can_close_floating_windows(tabpage_T *tp) { - assert(!is_aucmd_win(lastwin)); - for (win_T *wp = lastwin; wp->w_floating; wp = wp->w_prev) { + assert(tp != curtab && (tp || !is_aucmd_win(lastwin))); + for (win_T *wp = tp ? tp->tp_lastwin : lastwin; wp->w_floating; wp = wp->w_prev) { buf_T *buf = wp->w_buffer; int need_hide = (bufIsChanged(buf) && buf->b_nwindows <= 1); @@ -2618,10 +2629,10 @@ static bool close_last_window_tabpage(win_T *win, bool free_buf, tabpage_T *prev // case they have already been triggered. goto_tabpage_tp(alt_tabpage(), false, win->w_buffer != NULL); - // Safety check: Autocommands may have closed the window when jumping - // to the other tab page. - if (valid_tabpage(prev_curtab) && prev_curtab->tp_firstwin == win) { - win_close_othertab(win, free_buf, prev_curtab); + // Safety check: Autocommands may have switched back to the old tab page + // or closed the window when jumping to the other tab page. + if (curtab != prev_curtab && valid_tabpage(prev_curtab) && prev_curtab->tp_firstwin == win) { + win_close_othertab(win, free_buf, prev_curtab, false); } entering_window(curwin); @@ -2639,7 +2650,9 @@ static bool close_last_window_tabpage(win_T *win, bool free_buf, tabpage_T *prev /// "action" can also be zero (do nothing). /// "abort_if_last" is passed to close_buffer(): abort closing if all other /// windows are closed. -static void win_close_buffer(win_T *win, int action, bool abort_if_last) +/// +/// @return whether close_buffer() decremented b_nwindows +static bool win_close_buffer(win_T *win, int action, bool abort_if_last) FUNC_ATTR_NONNULL_ALL { // Free independent synblock before the buffer is freed. @@ -2654,12 +2667,13 @@ static void win_close_buffer(win_T *win, int action, bool abort_if_last) win->w_buffer->b_p_bl = false; } + bool retval = false; // Close the link to the buffer. if (win->w_buffer != NULL) { bufref_T bufref; set_bufref(&bufref, curbuf); win->w_locked = true; - close_buffer(win, win->w_buffer, action, abort_if_last, true); + retval = close_buffer(win, win->w_buffer, action, abort_if_last, true); if (win_valid_any_tab(win)) { win->w_locked = false; } @@ -2670,6 +2684,27 @@ static void win_close_buffer(win_T *win, int action, bool abort_if_last) curbuf = firstbuf; } } + + return retval; +} + +/// When failing to close a window after already calling close_buffer() on it, +/// call this to make the window have a buffer again. +/// +/// @param bufref reference to win->w_buffer before calling close_buffer() +/// @param did_decrement whether close_buffer() decremented b_nwindows +static void win_unclose_buffer(win_T *win, bufref_T *bufref, bool did_decrement) +{ + if (win->w_buffer == NULL) { + // If the buffer was removed from the window we have to give it any buffer. + win->w_buffer = firstbuf; + firstbuf->b_nwindows++; + win_init_empty(win); + } else if (did_decrement && win->w_buffer == bufref->br_buf && bufref_valid(bufref)) { + // close_buffer() decremented the window count, but we're keeping the window. + // As the window is still viewing the buffer, increment the count. + win->w_buffer->b_nwindows++; + } } // Close window "win". Only works for the current tab page. @@ -2697,12 +2732,12 @@ int win_close(win_T *win, bool free_buf, bool force) emsg(_(e_autocmd_close)); return FAIL; } - if (lastwin->w_floating && one_window(win)) { + if (lastwin->w_floating && one_window(win, NULL)) { if (is_aucmd_win(lastwin)) { emsg(_("E814: Cannot close window, only autocmd window would remain")); return FAIL; } - if (force || can_close_floating_windows()) { + if (force || can_close_floating_windows(NULL)) { // 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. @@ -2793,7 +2828,10 @@ int win_close(win_T *win, bool free_buf, bool force) return OK; } - win_close_buffer(win, free_buf ? DOBUF_UNLOAD : 0, true); + bufref_T bufref; + set_bufref(&bufref, win->w_buffer); + + bool did_decrement = win_close_buffer(win, free_buf ? DOBUF_UNLOAD : 0, true); if (win_valid(win) && win->w_buffer == NULL && !win->w_floating && last_window(win)) { @@ -2808,14 +2846,23 @@ int win_close(win_T *win, bool free_buf, bool force) if (curtab != prev_curtab && win_valid_any_tab(win) && win->w_buffer == NULL) { // Need to close the window anyway, since the buffer is NULL. - win_close_othertab(win, false, prev_curtab); + win_close_othertab(win, false, prev_curtab, force); return FAIL; } // Autocommands may have closed the window already, or closed the only // other window or moved to another tab page. - if (!win_valid(win) || (!win->w_floating && last_window(win)) - || close_last_window_tabpage(win, free_buf, prev_curtab)) { + if (!win_valid(win)) { + return FAIL; + } + if (one_window(win, NULL) && (first_tabpage->tp_next == NULL || lastwin->w_floating)) { + if (first_tabpage->tp_next != NULL) { + emsg(e_floatonly); + } + win_unclose_buffer(win, &bufref, did_decrement); + return FAIL; + } + if (close_last_window_tabpage(win, free_buf, prev_curtab)) { return FAIL; } @@ -2974,14 +3021,43 @@ static void do_autocmd_winclosed(win_T *win) // thus "tp" may become invalid! // Caller must check if buffer is hidden and whether the tabline needs to be // updated. -void win_close_othertab(win_T *win, int free_buf, tabpage_T *tp) +// @return false when the window was not closed as a direct result of this call +// (e.g: not via autocmds). +bool win_close_othertab(win_T *win, int free_buf, tabpage_T *tp, bool force) FUNC_ATTR_NONNULL_ALL { + assert(tp != curtab); + bool did_decrement = false; + // Get here with win->w_buffer == NULL when win_close() detects the tab page // changed. if (win_locked(win) || (win->w_buffer != NULL && win->w_buffer->b_locked > 0)) { - return; // window is already being closed + return false; // window is already being closed + } + if (is_aucmd_win(win)) { + emsg(_(e_autocmd_close)); + return false; + } + + // Check if closing this window would leave only floating windows. + if (tp->tp_lastwin->w_floating && one_window(win, tp)) { + if (force || can_close_floating_windows(tp)) { + // close the last window until the there are no floating windows + while (tp->tp_lastwin->w_floating) { + // `force` flag isn't actually used when closing a floating window. + if (!win_close_othertab(tp->tp_lastwin, free_buf, tp, true)) { + // If closing the window fails give up, to avoid looping forever. + goto leave_open; + } + } + if (!win_valid_any_tab(win)) { + return false; // window already closed by autocommands + } + } else { + emsg(e_floatonly); + goto leave_open; + } } // Fire WinClosed just before starting to free window-related resources. @@ -2991,87 +3067,83 @@ void win_close_othertab(win_T *win, int free_buf, tabpage_T *tp) do_autocmd_winclosed(win); // autocmd may have freed the window already. if (!win_valid_any_tab(win)) { - return; + return false; } } + bufref_T bufref; + set_bufref(&bufref, win->w_buffer); + if (win->w_buffer != NULL) { // Close the link to the buffer. - close_buffer(win, win->w_buffer, free_buf ? DOBUF_UNLOAD : 0, false, true); + did_decrement = close_buffer(win, win->w_buffer, free_buf ? DOBUF_UNLOAD : 0, false, true); } - tabpage_T *ptp = NULL; - // Careful: Autocommands may have closed the tab page or made it the // current tab page. - for (ptp = first_tabpage; ptp != NULL && ptp != tp; ptp = ptp->tp_next) {} - if (ptp == NULL || tp == curtab) { - // If the buffer was removed from the window we have to give it any - // buffer. - if (win_valid_any_tab(win) && win->w_buffer == NULL) { - win->w_buffer = firstbuf; - firstbuf->b_nwindows++; - win_init_empty(win); - } - return; + if (!valid_tabpage(tp) || tp == curtab) { + goto leave_open; + } + // Autocommands may have closed the window already, or nvim_win_set_config + // moved it to a different tab page. + if (!tabpage_win_valid(tp, win)) { + goto leave_open; + } + // Autocommands may again cause closing this window to leave only floats. + // Check again; we'll not bother closing floating windows this time. + if (tp->tp_lastwin->w_floating && one_window(win, tp)) { + emsg(e_floatonly); + goto leave_open; } - // Autocommands may have closed the window already. - { - bool found_window = false; - FOR_ALL_WINDOWS_IN_TAB(wp, tp) { - if (wp == win) { - found_window = true; - break; - } - } - if (!found_window) { - return; - } - } - - bool free_tp = false; + int free_tp_idx = 0; // When closing the last window in a tab page remove the tab page. if (tp->tp_firstwin == tp->tp_lastwin) { - char prev_idx[NUMBUFLEN]; - if (has_event(EVENT_TABCLOSED)) { - vim_snprintf(prev_idx, NUMBUFLEN, "%i", tabpage_index(tp)); - } - + free_tp_idx = tabpage_index(tp); int h = tabline_height(); if (tp == first_tabpage) { first_tabpage = tp->tp_next; } else { + tabpage_T *ptp; for (ptp = first_tabpage; ptp != NULL && ptp->tp_next != tp; ptp = ptp->tp_next) { // loop } if (ptp == NULL) { internal_error("win_close_othertab()"); - return; + return false; } ptp->tp_next = tp->tp_next; } - free_tp = true; redraw_tabline = true; if (h != tabline_height()) { win_new_screen_rows(); } - - if (has_event(EVENT_TABCLOSED)) { - apply_autocmds(EVENT_TABCLOSED, prev_idx, prev_idx, false, win->w_buffer); - } } // Free the memory used for the window. + buf_T *buf = win->w_buffer; int dir; win_free_mem(win, &dir, tp); - if (free_tp) { + if (free_tp_idx > 0) { free_tabpage(tp); + + if (has_event(EVENT_TABCLOSED)) { + char prev_idx[NUMBUFLEN]; + vim_snprintf(prev_idx, NUMBUFLEN, "%i", free_tp_idx); + apply_autocmds(EVENT_TABCLOSED, prev_idx, prev_idx, false, buf); + } } + return true; + +leave_open: + if (win_valid_any_tab(win)) { + win_unclose_buffer(win, &bufref, did_decrement); + } + return false; } /// Free the memory used for a window. @@ -3219,14 +3291,15 @@ win_T *winframe_remove(win_T *win, int *dirp, tabpage_T *tp, frame_T **unflat_al /// @param tp tab page "win" is in, NULL for current /// @param altfr if not NULL, set to pointer of frame that will get the space /// -/// @return a pointer to the window that will get the freed up space. +/// @return a pointer to the window that will get the freed up space, or NULL +/// if there is no other non-float to receive the space. win_T *winframe_find_altwin(win_T *win, int *dirp, tabpage_T *tp, frame_T **altfr) FUNC_ATTR_NONNULL_ARG(1, 2) { assert(tp == NULL || tp != curtab); - // If there is only one window there is nothing to remove. - if (tp == NULL ? ONE_WINDOW : tp->tp_firstwin == tp->tp_lastwin) { + // If there is only one non-floating window there is nothing to remove. + if (one_window(win, tp)) { return NULL; } @@ -3423,7 +3496,7 @@ static frame_T *win_altframe(win_T *win, tabpage_T *tp) { assert(tp == NULL || tp != curtab); - if (tp == NULL ? ONE_WINDOW : tp->tp_firstwin == tp->tp_lastwin) { + if (one_window(win, tp)) { return alt_tabpage()->tp_curwin->w_frame; } @@ -3992,7 +4065,7 @@ void close_others(int message, int forceit) return; } - if (one_window(firstwin) && !lastwin->w_floating) { + if (one_window(firstwin, NULL) && !lastwin->w_floating) { if (message && !autocmd_busy) { msg(_(m_onlyone), 0); @@ -7049,7 +7122,7 @@ int global_stl_height(void) /// @param morewin pretend there are two or more windows if true. int last_stl_height(bool morewin) { - return (p_ls > 1 || (p_ls == 1 && (morewin || !one_window(firstwin)))) ? STATUS_HEIGHT : 0; + return (p_ls > 1 || (p_ls == 1 && (morewin || !one_window(firstwin, NULL)))) ? STATUS_HEIGHT : 0; } /// Return the minimal number of rows that is needed on the screen to display diff --git a/test/functional/api/window_spec.lua b/test/functional/api/window_spec.lua index f1579f5345..400c3611c7 100644 --- a/test/functional/api/window_spec.lua +++ b/test/functional/api/window_spec.lua @@ -1777,6 +1777,13 @@ describe('API/win', function() it('checks if splitting disallowed', function() command('split | autocmd WinEnter * ++once call nvim_open_win(0, 0, #{split: "right"})') matches("E242: Can't split a window while closing another$", pcall_err(command, 'quit')) + -- E242 is not needed for floats. + exec([[ + split + autocmd WinEnter * ++once let g:win = nvim_open_win(0, 0, #{relative: "editor", row: 0, col: 0, width: 5, height: 5}) + quit + ]]) + eq('editor', eval('nvim_win_get_config(g:win).relative')) command('only | autocmd BufHidden * ++once call nvim_open_win(0, 0, #{split: "left"})') matches( @@ -1803,6 +1810,38 @@ describe('API/win', function() .. '})' ) command('new | quit') + + -- Apply to opening floats too, as that can similarly create new views into a closing buffer. + -- For example, the following would open a float into an unloaded buffer: + exec([[ + only + new + let g:buf = bufnr() + autocmd BufUnload * ++once call nvim_open_win(g:buf, 0, #{relative: "editor", width: 5, height: 5, row: 1, col: 1}) + setlocal bufhidden=unload + ]]) + matches('E1159: Cannot open a float when closing the buffer$', pcall_err(command, 'quit')) + eq(false, eval('nvim_buf_is_loaded(g:buf)')) + eq(0, eval('win_findbuf(g:buf)->len()')) + + -- Only checking b_locked_split for the target buffer is insufficient, as naughty autocommands + -- can cause win_set_buf to remain in a closing curbuf: + exec([[ + only + new + let g:buf = bufnr() + autocmd BufWipeout * ++once ++nested let g:buf2 = nvim_create_buf(1, 0) + \| execute 'autocmd BufLeave * ++once call nvim_buf_delete(g:buf2, #{force: 1})' + \| setlocal bufhidden= + \| call nvim_open_win(g:buf2, 1, #{relative: 'editor', width: 5, height: 5, col: 5, row: 5}) + setlocal bufhidden=wipe + ]]) + matches('E1159: Cannot open a float when closing the buffer$', pcall_err(command, 'quit')) + eq(false, eval('nvim_buf_is_loaded(g:buf)')) + eq(0, eval('win_findbuf(g:buf)->len()')) + -- BufLeave shouldn't run here (buf2 isn't deleted and remains hidden) + eq(true, eval('nvim_buf_is_loaded(g:buf2)')) + eq(0, eval('win_findbuf(g:buf2)->len()')) end) it('restores last known cursor position if BufWinEnter did not move it', function() @@ -1931,6 +1970,80 @@ describe('API/win', function() ) end) + it('no crash when closing the only non-float in other tabpage #31236', function() + local tp = api.nvim_get_current_tabpage() + local split_win = api.nvim_get_current_win() + local float_win = api.nvim_open_win( + 0, + false, + { relative = 'editor', width = 5, height = 5, row = 1, col = 1 } + ) + command('tabnew') + + api.nvim_win_close(split_win, false) + eq(false, api.nvim_win_is_valid(split_win)) + eq(false, api.nvim_win_is_valid(float_win)) + eq(false, api.nvim_tabpage_is_valid(tp)) + + tp = api.nvim_get_current_tabpage() + split_win = api.nvim_get_current_win() + local float_buf = api.nvim_create_buf(true, false) + float_win = api.nvim_open_win( + float_buf, + false, + { relative = 'editor', width = 5, height = 5, row = 1, col = 1 } + ) + -- Set these options to prevent the float from being automatically closed. + api.nvim_set_option_value('modified', true, { buf = float_buf }) + api.nvim_set_option_value('bufhidden', 'wipe', { buf = float_buf }) + command('tabnew') + + matches( + 'E5601: Cannot close window, only floating window would remain$', + pcall_err(api.nvim_win_close, split_win, false) + ) + eq(true, api.nvim_win_is_valid(split_win)) + eq(true, api.nvim_win_is_valid(float_win)) + eq(true, api.nvim_tabpage_is_valid(tp)) + + api.nvim_set_current_win(float_win) + api.nvim_win_close(split_win, true) -- Force it this time. + eq(false, api.nvim_win_is_valid(split_win)) + eq(false, api.nvim_win_is_valid(float_win)) + eq(false, api.nvim_tabpage_is_valid(tp)) + + -- Ensure opening a float after the initial check (like in WinClosed) doesn't crash... + exec([[ + tabnew + let g:tp = nvim_get_current_tabpage() + let g:win = win_getid() + tabprevious + autocmd! WinClosed * ++once call nvim_open_win(0, 0, #{win: g:win, relative: 'win', width: 5, height: 5, row: 5, col: 5}) + ]]) + matches( + 'E5601: Cannot close window, only floating window would remain$', + pcall_err(command, 'call nvim_win_close(g:win, 0)') + ) + eq(true, eval 'nvim_tabpage_is_valid(g:tp)') + + exec([[ + tabnew + let g:tp = nvim_get_current_tabpage() + let g:win = win_getid() + let g:buf = bufnr() + tabprevious + let s:buf2 = nvim_create_buf(0, 0) + call setbufvar(s:buf2, '&modified', 1) + call setbufvar(s:buf2, '&bufhidden', 'wipe') + autocmd! WinClosed * ++once call nvim_open_win(s:buf2, 0, #{win: g:win, relative: 'win', width: 5, height: 5, row: 5, col: 5}) + ]]) + matches( + 'E5601: Cannot close window, only floating window would remain$', + pcall_err(command, 'call nvim_buf_delete(g:buf, #{force: 1})') + ) + eq(true, eval 'nvim_tabpage_is_valid(g:tp)') + end) + it('respects requested size for large splits', function() command('vsplit') local win = api.nvim_open_win(0, false, { win = -1, split = 'right', width = 38 }) @@ -1955,6 +2068,31 @@ describe('API/win', function() api.nvim_open_win(0, true, { split = 'below' }) eq(11, api.nvim_win_get_height(0)) end) + + it('no leak when win_set_buf fails and window is closed immediately', function() + -- Following used to leak. + command('autocmd BufEnter * ++once quit! | throw 1337') + eq( + 'Window was closed immediately', + pcall_err( + api.nvim_open_win, + api.nvim_create_buf(true, true), + true, + { relative = 'editor', width = 5, height = 5, row = 1, col = 1 } + ) + ) + -- If the window wasn't closed, still set errors from win_set_buf. + command('autocmd BufEnter * ++once throw 1337') + eq( + 'BufEnter Autocommands for "*": 1337', + pcall_err( + api.nvim_open_win, + api.nvim_create_buf(true, true), + true, + { relative = 'editor', width = 5, height = 5, row = 1, col = 1 } + ) + ) + end) end) describe('set_config', function() @@ -2005,11 +2143,42 @@ describe('API/win', function() eq('editor', api.nvim_win_get_config(win).relative) end) - it('throws error when attempting to move the last window', function() + it('throws error when attempting to move the last non-floating window', function() local err = pcall_err(api.nvim_win_set_config, 0, { vertical = false, }) - eq('Cannot move last window', err) + eq('Cannot move last non-floating window', err) + + local win1 = api.nvim_get_current_win() + command('tabnew') + eq( + 'Cannot move last non-floating window', + pcall_err(api.nvim_win_set_config, 0, { win = win1, split = 'left' }) + ) + api.nvim_open_win(0, false, { relative = 'editor', width = 5, height = 5, row = 1, col = 1 }) + eq( + 'Cannot move last non-floating window', + pcall_err(api.nvim_win_set_config, 0, { win = win1, split = 'left' }) + ) + + -- If it's no longer the last non-float, still an error if autocommands make it the last + -- non-float again before it's moved. + command('vsplit') + exec_lua(function() + vim.api.nvim_create_autocmd('WinEnter', { + once = true, + callback = function() + vim.api.nvim_win_set_config( + 0, + { relative = 'editor', width = 5, height = 5, row = 1, col = 1 } + ) + end, + }) + end) + eq( + 'Cannot move last non-floating window', + pcall_err(api.nvim_win_set_config, 0, { win = win1, split = 'left' }) + ) end) it('passing retval of get_config results in no-op', function() diff --git a/test/functional/autocmd/autocmd_spec.lua b/test/functional/autocmd/autocmd_spec.lua index fdf07a2d39..b4317a3e50 100644 --- a/test/functional/autocmd/autocmd_spec.lua +++ b/test/functional/autocmd/autocmd_spec.lua @@ -17,7 +17,6 @@ local pcall_err = t.pcall_err local fn = n.fn local expect = n.expect local command = n.command -local exc_exec = n.exc_exec local exec_lua = n.exec_lua local retry = t.retry local source = n.source @@ -151,7 +150,10 @@ describe('autocmd', function() }) command('autocmd BufLeave * bwipeout yy') - eq('Vim(edit):E143: Autocommands unexpectedly deleted new buffer yy', exc_exec('edit yy')) + eq( + 'Vim(edit):E143: Autocommands unexpectedly deleted new buffer yy', + pcall_err(command, 'edit yy') + ) expect([[ start of test file xx @@ -429,6 +431,23 @@ describe('autocmd', function() ) end) + it('cannot close `aucmd_win` in non-current tabpage', function() + exec([[ + file Xa + tabnew Xb + call setline(1, 'foo') + tabfirst + autocmd BufWriteCmd Xb tablast | bwipe! Xa + ]]) + eq( + 'BufWriteCmd Autocommands for "Xb": Vim(bwipeout):E813: Cannot close autocmd window', + pcall_err(command, 'wall') + ) + -- Sanity check: :bwipe failing to close all windows into Xa should keep it loaded. + -- (So there's no risk of it being left unloaded inside a window) + eq(1, eval('bufloaded("Xa")')) + end) + describe('closing last non-floating window in tab from `aucmd_win`', function() before_each(function() command('edit Xa.txt') diff --git a/test/functional/autocmd/tabclose_spec.lua b/test/functional/autocmd/tabclose_spec.lua index fde72cf4d7..3969f33cf1 100644 --- a/test/functional/autocmd/tabclose_spec.lua +++ b/test/functional/autocmd/tabclose_spec.lua @@ -2,8 +2,10 @@ local t = require('test.testutil') local n = require('test.functional.testnvim')() local clear, eq = n.clear, t.eq -local api = n.api local command = n.command +local eval = n.eval +local exec = n.exec +local exec_capture = n.exec_capture describe('TabClosed', function() before_each(clear) @@ -16,11 +18,11 @@ describe('TabClosed', function() ) repeat command('tabnew') - until api.nvim_eval('tabpagenr()') == 6 -- current tab is now 6 - eq('tabclosed:6:6:5', api.nvim_exec('tabclose', true)) -- close last 6, current tab is now 5 - eq('tabclosed:5:5:4', api.nvim_exec('close', true)) -- close last window on tab, closes tab - eq('tabclosed:2:2:3', api.nvim_exec('2tabclose', true)) -- close tab 2, current tab is now 3 - eq('tabclosed:1:1:2\ntabclosed:1:1:1', api.nvim_exec('tabonly', true)) -- close tabs 1 and 2 + until eval('tabpagenr()') == 6 -- current tab is now 6 + eq('tabclosed:6:6:5', exec_capture('tabclose')) -- close last 6, current tab is now 5 + eq('tabclosed:5:5:4', exec_capture('close')) -- close last window on tab, closes tab + eq('tabclosed:2:2:3', exec_capture('2tabclose')) -- close tab 2, current tab is now 3 + eq('tabclosed:1:1:2\ntabclosed:1:1:1', exec_capture('tabonly')) -- close tabs 1 and 2 end) it('is triggered when closing a window via bdelete from another tab', function() @@ -30,9 +32,9 @@ describe('TabClosed', function() command('1tabedit Xtestfile') command('1tabedit Xtestfile') command('normal! 1gt') - eq({ 1, 3 }, api.nvim_eval('[tabpagenr(), tabpagenr("$")]')) - eq('tabclosed:2:2:1\ntabclosed:2:2:1', api.nvim_exec('bdelete Xtestfile', true)) - eq({ 1, 1 }, api.nvim_eval('[tabpagenr(), tabpagenr("$")]')) + eq({ 1, 3 }, eval('[tabpagenr(), tabpagenr("$")]')) + eq('tabclosed:2:2:1\ntabclosed:2:2:1', exec_capture('bdelete Xtestfile')) + eq({ 1, 1 }, eval('[tabpagenr(), tabpagenr("$")]')) end) it('is triggered when closing a window via bdelete from current tab', function() @@ -44,9 +46,39 @@ describe('TabClosed', function() command('1tabedit Xtestfile2') -- Only one tab is closed, and the alternate file is used for the other. - eq({ 2, 3 }, api.nvim_eval('[tabpagenr(), tabpagenr("$")]')) - eq('tabclosed:2:2:2', api.nvim_exec('bdelete Xtestfile2', true)) - eq('Xtestfile1', api.nvim_eval('bufname("")')) + eq({ 2, 3 }, eval('[tabpagenr(), tabpagenr("$")]')) + eq('tabclosed:2:2:2', exec_capture('bdelete Xtestfile2')) + eq('Xtestfile1', eval('bufname("")')) + end) + + it('triggers after tab page is properly freed', function() + exec([[ + let s:tp = nvim_get_current_tabpage() + let g:buf = bufnr() + + setlocal bufhidden=wipe + tabnew + au TabClosed * ++once let g:tp_valid = nvim_tabpage_is_valid(s:tp) + \| let g:abuf = expand('') + + call nvim_buf_delete(g:buf, #{force: 1}) + ]]) + eq(false, eval('g:tp_valid')) + eq(false, eval('nvim_buf_is_valid(g:buf)')) + eq('', eval('g:abuf')) + + exec([[ + tabnew + let g:buf = bufnr() + let s:win = win_getid() + + tabfirst + au TabClosed * ++once let g:abuf = expand('') + + call nvim_win_close(s:win, 1) + ]]) + eq(true, eval('nvim_buf_is_valid(g:buf)')) + eq(eval('g:buf'), tonumber(eval('g:abuf'))) end) end) @@ -58,11 +90,11 @@ describe('TabClosed', function() command('au! TabClosed 2 echom "tabclosed:match"') repeat command('tabnew') - until api.nvim_eval('tabpagenr()') == 7 -- current tab is now 7 + until eval('tabpagenr()') == 7 -- current tab is now 7 -- sanity check, we shouldn't match on tabs with numbers other than 2 - eq('tabclosed:7:7:6', api.nvim_exec('tabclose', true)) + eq('tabclosed:7:7:6', exec_capture('tabclose')) -- close tab page 2, current tab is now 5 - eq('tabclosed:2:2:5\ntabclosed:match', api.nvim_exec('2tabclose', true)) + eq('tabclosed:2:2:5\ntabclosed:match', exec_capture('2tabclose')) end) end) @@ -72,9 +104,9 @@ describe('TabClosed', function() 'au! TabClosed * echom "tabclosed:".expand("").":".expand("").":".tabpagenr()' ) command('tabedit Xtestfile') - eq({ 2, 2 }, api.nvim_eval('[tabpagenr(), tabpagenr("$")]')) - eq('tabclosed:2:2:1', api.nvim_exec('close', true)) - eq({ 1, 1 }, api.nvim_eval('[tabpagenr(), tabpagenr("$")]')) + eq({ 2, 2 }, eval('[tabpagenr(), tabpagenr("$")]')) + eq('tabclosed:2:2:1', exec_capture('close')) + eq({ 1, 1 }, eval('[tabpagenr(), tabpagenr("$")]')) end) end) end) diff --git a/test/functional/editor/tabpage_spec.lua b/test/functional/editor/tabpage_spec.lua index e829821a64..c8be39ea6e 100644 --- a/test/functional/editor/tabpage_spec.lua +++ b/test/functional/editor/tabpage_spec.lua @@ -147,4 +147,13 @@ describe('tabpage', function() command('tabs') assert_alive() end) + + it('no crash if autocmd remains in tabpage of closing last window', function() + exec([[ + tabnew + let s:win = win_getid() + autocmd TabLeave * ++once tablast | tabonly + quit + ]]) + end) end) diff --git a/test/functional/ui/float_spec.lua b/test/functional/ui/float_spec.lua index c8492b0f3e..34d7d7be7a 100644 --- a/test/functional/ui/float_spec.lua +++ b/test/functional/ui/float_spec.lua @@ -1005,7 +1005,7 @@ describe('float window', function() assert_alive() end) - pending('does not crash if BufUnload makes it the only non-float in tabpage', function() + it('does not crash if BufUnload makes it the only non-float in tabpage', function() exec([[ tabnew let g:buf = bufnr() @@ -1017,10 +1017,60 @@ describe('float window', function() \ #{relative: 'editor', row: 5, col: 5, width: 5, height: 5}) autocmd BufUnload * ++once exe g:buf .. 'bwipe!' ]]) - command('close') + eq('Vim(close):E5601: Cannot close window, only floating window would remain', pcall_err(command, 'close')) assert_alive() end) + describe('does not crash if WinClosed makes it the only non-float', function() + before_each(function() + exec([[ + let g:buf = bufnr() + new + setlocal bufhidden=wipe + autocmd WinClosed * ++once exe g:buf .. 'bwipe!' + ]]) + end) + + local opts = { relative = 'editor', row = 5, col = 5, width = 5, height = 5 } + local floatwin + + describe('and there is a float window with the same buffer', function() + before_each(function() + floatwin = api.nvim_open_win(0, false, opts) + end) + + it('with multiple tabpages', function() + command('tabnew | tabprev') + eq('Vim(close):E5601: Cannot close window, only floating window would remain', pcall_err(command, 'close')) + api.nvim_win_close(floatwin, true) + assert_alive() + end) + + it('with only one tabpage', function() + command('close') + api.nvim_win_close(floatwin, true) + assert_alive() + end) + end) + + describe('and there is a float with a different buffer', function() + before_each(function() + floatwin = api.nvim_open_win(api.nvim_create_buf(true, false), false, opts) + end) + + it('with multiple tabpages', function() + command('tabnew | tabprev') + eq('Vim(close):E855: Autocommands caused command to abort', pcall_err(command, 'close')) + assert_alive() + end) + + it('with only one tabpage', function() + eq('Vim(close):E855: Autocommands caused command to abort', pcall_err(command, 'close')) + assert_alive() + end) + end) + end) + it('does not crash if WinClosed from floating window closes it', function() exec([[ tabnew