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: #19211

Signed-off-by: zeertzjq <zeertzjq@outlook.com>
Signed-off-by: Christian Brabandt <cb@256bit.org>

diff --git a/src/ex_docmd.c b/src/ex_docmd.c
index 42ca2e7bff0be5dacd3c308741865ca8a0551633..1eb1de3e366bdac34120efd734e24a4e05b3c6f3 100644 (file)
--- a/src/ex_docmd.c
+++ b/src/ex_docmd.c
@@ -6570,7 +6570,9 @@ tabpage_close(int forceit)
     if (window_layout_locked(CMD_tabclose))
        return;
 
-    trigger_tabclosedpre(curtab, TRUE);
+    trigger_tabclosedpre(curtab);
+    curtab->tp_did_tabclosedpre = TRUE;
+    tabpage_T *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.
@@ -6578,6 +6580,10 @@ tabpage_close(int forceit)
        close_others(TRUE, 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;
 #ifdef FEAT_GUI
     need_mouse_correct = TRUE;
 #endif
@@ -6598,7 +6604,8 @@ tabpage_close_other(tabpage_T *tp, int forceit)
     if (window_layout_locked(CMD_SIZE))
        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...
@@ -6607,10 +6614,22 @@ tabpage_close_other(tabpage_T *tp, int forceit)
        wp = tp->tp_firstwin;
        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_firstwin == 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_firstwin == 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;
     }
 
     apply_autocmds(EVENT_TABCLOSED, NULL, NULL, FALSE, curbuf);

diff --git a/src/proto/window.pro b/src/proto/window.pro
index 939c220762ab6a916b4719aaf981b3f46be51b93..62d8bc639aac2fe873ce79eca48a3813350f45ca 100644 (file)
--- a/src/proto/window.pro
+++ b/src/proto/window.pro
@@ -28,7 +28,7 @@ void close_windows(buf_T *buf, int keep_curwin);
 int last_window(void);
 int one_window(void);
 int win_close(win_T *win, int free_buf);
-void trigger_tabclosedpre(tabpage_T *tp, int directly);
+void trigger_tabclosedpre(tabpage_T *tp);
 void snapshot_windows_scroll_size(void);
 void may_make_initial_scroll_size_snapshot(void);
 void may_trigger_win_scrolled_resized(void);

diff --git a/src/structs.h b/src/structs.h
index 58046abf292344d87761081cbeb304a93283266d..83d35c53f3950fd448efc9d6c1bdde0a554be21d 100644 (file)
--- a/src/structs.h
+++ b/src/structs.h
@@ -3743,6 +3743,7 @@ struct tabpage_S
     int                    tp_prev_which_scrollbars[3];
                                    // previous value of which_scrollbars
 #endif
+    int                    tp_did_tabclosedpre;  // whether TabClosedPre was triggered
 
     char_u         *tp_localdir;       // absolute path of local directory or
                                        // NULL

diff --git a/src/testdir/test_autocmd.vim b/src/testdir/test_autocmd.vim
index ad10af7c75d760ef8af9c46fa4c3957b56441227..82d62e7636d954ac51925a4c4e9b35f0018e9987 100644 (file)
--- a/src/testdir/test_autocmd.vim
+++ b/src/testdir/test_autocmd.vim
@@ -5315,6 +5315,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!

diff --git a/src/version.c b/src/version.c
index cbf48413c7bb4ababab2555df30cde6d1a36a0cd..ede881cf95649c040f1af95c478031ffa84bbfae 100644 (file)
--- a/src/version.c
+++ b/src/version.c
@@ -734,6 +734,8 @@ static char *(features[]) =
 
 static int included_patches[] =
 {   /* Add new patch number below this line */
+/**/
+    2097,
 /**/
     2096,
 /**/

diff --git a/src/window.c b/src/window.c
index e34574bb08a46112f200214977fc5972beba9b8e..4573591261d6b25c39bbecfc00f0821dd9dba87b 100644 (file)
--- a/src/window.c
+++ b/src/window.c
@@ -3021,15 +3021,10 @@ trigger_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, int directly)
+trigger_tabclosedpre(tabpage_T *tp)
 {
     static int recursive = FALSE;
-    static int skip = FALSE;
     tabpage_T  *ptp = curtab;
 
     // Quickly return when no TabClosedPre autocommands to be executed or
@@ -3037,19 +3032,8 @@ trigger_tabclosedpre(tabpage_T *tp, int directly)
     if (!has_tabclosedpre() || recursive)
        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();
     apply_autocmds(EVENT_TABCLOSEDPRE, NULL, NULL, FALSE, NULL);
@@ -3057,10 +3041,10 @@ trigger_tabclosedpre(tabpage_T *tp, int 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);
 }
 
@@ -3447,9 +3431,9 @@ win_close_othertab(win_T *win, int free_buf, tabpage_T *tp)
            return;
     }
 
-    if (tp->tp_firstwin == tp->tp_lastwin)
+    if (tp->tp_firstwin == tp->tp_lastwin && !tp->tp_did_tabclosedpre)
     {
-       trigger_tabclosedpre(tp, FALSE);
+       trigger_tabclosedpre(tp);
        // autocmd may have freed the window already.
        if (!win_valid_any_tab(win))
            return;