From 7470e6326068cf2d5ce140a1238da42aa2f63c05 Mon Sep 17 00:00:00 2001 From: zeertzjq Date: Tue, 20 Jan 2026 06:06:29 +0800 Subject: [PATCH] vim-patch:9.1.2097: TabClosedPre may be triggered twice for the same tab page Problem: TabClosedPre may be triggered twice for the same tab page when closing another tab page in BufWinLeave (after 9.1.1211). Solution: Store whether TabClosedPre was triggered in tabpage_T (zeertzjq). Also fix the inconsistency that :tabclose! triggers TabClosedPre after a failed :tabclose, but :close! doesn't even if there is only one window in the tab page. closes: vim/vim#19211 https://github.com/vim/vim/commit/9168a04e0c63c95eec643dab14a8e0a8933d90e7 --- src/nvim/buffer_defs.h | 5 +-- src/nvim/ex_docmd.c | 28 ++++++++++++--- src/nvim/window.c | 22 +++--------- test/old/testdir/test_autocmd.vim | 58 +++++++++++++++++++++++++++++++ 4 files changed, 89 insertions(+), 24 deletions(-) diff --git a/src/nvim/buffer_defs.h b/src/nvim/buffer_defs.h index 80efe8a640..06a52ccc88 100644 --- a/src/nvim/buffer_defs.h +++ b/src/nvim/buffer_defs.h @@ -841,9 +841,10 @@ struct tabpage_S { win_T *tp_firstwin; ///< first window in this Tab page win_T *tp_lastwin; ///< last window in this Tab page int64_t tp_old_Rows_avail; ///< ROWS_AVAIL when Tab page was left - int64_t tp_old_Columns; ///< Columns when Tab page was left, -1 when - ///< calling win_new_screen_cols() postponed + int64_t tp_old_Columns; ///< Columns when Tab page was left, -1 when + ///< calling win_new_screen_cols() postponed OptInt tp_ch_used; ///< value of 'cmdheight' when frame size was set + bool tp_did_tabclosedpre; ///< whether TabClosedPre was triggered diff_T *tp_first_diff; buf_T *(tp_diffbuf[DB_COUNT]); diff --git a/src/nvim/ex_docmd.c b/src/nvim/ex_docmd.c index 0428ab8ee7..8aa3f02626 100644 --- a/src/nvim/ex_docmd.c +++ b/src/nvim/ex_docmd.c @@ -5150,7 +5150,9 @@ void tabpage_close(int forceit) return; } - trigger_tabclosedpre(curtab, true); + trigger_tabclosedpre(curtab); + curtab->tp_did_tabclosedpre = true; + tabpage_T *const save_curtab = curtab; // First close all the windows but the current one. If that worked then // close the last window in this tab, that will close it. @@ -5163,6 +5165,11 @@ void tabpage_close(int forceit) if (ONE_WINDOW) { ex_win_close(forceit, curwin, NULL); } + if (curtab == save_curtab) { + // When closing the tab page failed, reset tp_did_tabclosedpre so that + // TabClosedPre behaves consistently on next :close vs :tabclose. + curtab->tp_did_tabclosedpre = false; + } } /// Close tab page "tp", which is not the current tab page. @@ -5178,7 +5185,8 @@ void tabpage_close_other(tabpage_T *tp, int forceit) return; } - trigger_tabclosedpre(tp, true); + trigger_tabclosedpre(tp); + tp->tp_did_tabclosedpre = true; // Limit to 1000 windows, autocommands may add a window while we close // one. OK, so I'm paranoid... @@ -5187,11 +5195,21 @@ void tabpage_close_other(tabpage_T *tp, int forceit) win_T *wp = tp->tp_lastwin; ex_win_close(forceit, wp, tp); - // Autocommands may delete the tab page under our fingers and we may - // fail to close a window with a modified buffer. - if (!valid_tabpage(tp) || tp->tp_lastwin == wp) { + // Autocommands may delete the tab page under our fingers. + if (!valid_tabpage(tp)) { break; } + // We may fail to close a window with a modified buffer. + if (tp->tp_lastwin == wp) { + done = 1000; + break; + } + } + if (done >= 1000) { + // When closing the tab page failed, reset tp_did_tabclosedpre so that + // TabClosedPre behaves consistently on next :close vs :tabclose. + tp->tp_did_tabclosedpre = false; + return; } } diff --git a/src/nvim/window.c b/src/nvim/window.c index 07402aaaec..7d9bf04609 100644 --- a/src/nvim/window.c +++ b/src/nvim/window.c @@ -3110,12 +3110,9 @@ static void do_autocmd_winclosed(win_T *win) recursive = false; } -/// directly is true if the window is closed by ':tabclose' or ':tabonly'. -/// This allows saving the session before closing multi-window tab. -void trigger_tabclosedpre(tabpage_T *tp, bool directly) +void trigger_tabclosedpre(tabpage_T *tp) { static bool recursive = false; - static bool skip = false; tabpage_T *ptp = curtab; // Quickly return when no TabClosedPre autocommands to be executed or @@ -3124,17 +3121,8 @@ void trigger_tabclosedpre(tabpage_T *tp, bool directly) return; } - // Skip if the event have been triggered by ':tabclose' recently - if (skip) { - skip = false; - return; - } - if (valid_tabpage(tp)) { goto_tabpage_tp(tp, false, false); - if (directly) { - skip = true; - } } recursive = true; window_layout_lock(); @@ -3143,10 +3131,10 @@ void trigger_tabclosedpre(tabpage_T *tp, bool directly) recursive = false; // tabpage may have been modified or deleted by autocmds if (valid_tabpage(ptp)) { - // try to recover the tappage first + // try to recover the tabpage first goto_tabpage_tp(ptp, false, false); } else { - // fall back to the first tappage + // fall back to the first tabpage goto_tabpage_tp(first_tabpage, false, false); } } @@ -3211,8 +3199,8 @@ bool win_close_othertab(win_T *win, int free_buf, tabpage_T *tp, bool force) } } - if (tp->tp_firstwin == tp->tp_lastwin) { - trigger_tabclosedpre(tp, false); + if (tp->tp_firstwin == tp->tp_lastwin && !tp->tp_did_tabclosedpre) { + trigger_tabclosedpre(tp); // autocmd may have freed the window already. if (!win_valid_any_tab(win)) { return false; diff --git a/test/old/testdir/test_autocmd.vim b/test/old/testdir/test_autocmd.vim index 5ad3ef6570..b3c9a44d15 100644 --- a/test/old/testdir/test_autocmd.vim +++ b/test/old/testdir/test_autocmd.vim @@ -4704,6 +4704,64 @@ func Test_autocmd_TabClosedPre() call assert_equal([1, 2], g:tabpagenr_pre) call assert_equal([2, 3], g:tabpagenr_post) + " Test failing to close tab page + let g:tabpagenr_pre = [] + let g:tabpagenr_post = [] + let t:testvar = 1 + call setline(1, 'foo') + setlocal bufhidden=wipe + tabnew + let t:testvar = 2 + tabnew + let t:testvar = 3 + call setline(1, 'bar') + setlocal bufhidden=wipe + tabnew + let t:testvar = 4 + call setline(1, 'baz') + setlocal bufhidden=wipe + new + call assert_fails('tabclose', 'E445:') + call assert_equal([4], g:tabpagenr_pre) + call assert_equal([], g:tabpagenr_post) + " :tabclose! after failed :tabclose should trigger TabClosedPre again. + tabclose! + call assert_equal([4, 4], g:tabpagenr_pre) + call assert_equal([3], g:tabpagenr_post) + call assert_fails('tabclose', 'E37:') + call assert_equal([4, 4, 3], g:tabpagenr_pre) + call assert_equal([3], g:tabpagenr_post) + " The same for :close! if the tab page only has one window. + close! + call assert_equal([4, 4, 3, 3], g:tabpagenr_pre) + call assert_equal([3, 2], g:tabpagenr_post) + " Also test with :close! after failed :tabonly. + call assert_fails('tabonly', 'E37:') + call assert_equal([4, 4, 3, 3, 1], g:tabpagenr_pre) + call assert_equal([3, 2], g:tabpagenr_post) + tabprevious | close! + call assert_equal([4, 4, 3, 3, 1, 1], g:tabpagenr_pre) + call assert_equal([3, 2, 2], g:tabpagenr_post) + %bwipe! + + " Test closing another tab page in BufWinLeave + let g:tabpagenr_pre = [] + let g:tabpagenr_post = [] + split + let t:testvar = 1 + tabnew + let t:testvar = 2 + tabnew Xsomebuf + let t:testvar = 3 + new + autocmd BufWinLeave Xsomebuf ++once ++nested tabclose 1 + tabclose + " TabClosedPre should not be triggered for tab page 3 twice. + call assert_equal([3, 1], g:tabpagenr_pre) + " When tab page 1 was closed, tab page 3 was still the current tab page. + call assert_equal([3, 2], g:tabpagenr_post) + %bwipe! + func ClearAutocmdAndCreateTabs() au! TabClosedPre bw!