Merge #5500

Closes #5246

9 files changed, 201 insertions, 77 deletions

diff --git a/runtime/doc/tabpage.txt b/runtime/doc/tabpage.txt
index 70e6953211..4ab5b3c759 100644
--- a/runtime/doc/tabpage.txt
+++ b/runtime/doc/tabpage.txt
@@ -83,14 +83,21 @@ In the GUI tab pages line you can use the right mouse button to open menu.
 		Execute {cmd} and when it opens a new window open a new tab
 		page instead.  Doesn't work for |:diffsplit|, |:diffpatch|,
 		|:execute| and |:normal|.
-		When [count] is omitted the tab page appears after the current
-		one.
-		When [count] is specified the new tab page comes after tab
-		page [count].  Use ":0tab cmd" to get the new tab page as the
-		first one.
+		If [count] is given the new tab page appears after the tab
+		page [count] otherwise the new tab page will appear after the
+		current one.
 		Examples: >
-			:tab split	" opens current buffer in new tab page
-			:tab help gt	" opens tab page with help for "gt"
+		    :tab split	    " opens current buffer in new tab page
+		    :tab help gt    " opens tab page with help for "gt"
+		    :.tab help gt   " as above
+		    :+tab help	    " opens tab page with help after the next
+				    " tab page
+		    :-tab help	    " opens tab page with help before the
+				    " current one
+		    :0tab help	    " opens tab page with help before the
+				    " first one
+		    :$tab help	    " opens tab page with help after the last
+				    " one
 
 CTRL-W gf	Open a new tab page and edit the file name under the cursor.
 		See |CTRL-W_gf|.
@@ -140,7 +147,7 @@ something else.
 
 :{count}tabo[nly][!]
 		Close all tab pages except the {count}th one. >
-		    :.tabonly " one
+		    :.tabonly " as above
 		    :-tabonly " close all tab pages except the previous one
 		    :+tabonly " close all tab pages except the next one
 		    :1tabonly " close all tab pages except the first one
diff --git a/src/nvim/buffer.c b/src/nvim/buffer.c
index a573c20648..a66fdc1304 100644
--- a/src/nvim/buffer.c
+++ b/src/nvim/buffer.c
@@ -323,13 +323,14 @@ void close_buffer(win_T *win, buf_T *buf, int action, int abort_if_last)
     wipe_buf = true;
   }
 
-  if (win_valid(win)) {
-    /* Set b_last_cursor when closing the last window for the buffer.
-     * Remember the last cursor position and window options of the buffer.
-     * This used to be only for the current window, but then options like
-     * 'foldmethod' may be lost with a ":only" command. */
-    if (buf->b_nwindows == 1)
+  if (win_valid_any_tab(win)) {
+    // Set b_last_cursor when closing the last window for the buffer.
+    // Remember the last cursor position and window options of the buffer.
+    // This used to be only for the current window, but then options like
+    // 'foldmethod' may be lost with a ":only" command.
+    if (buf->b_nwindows == 1) {
       set_last_cursor(win);
+    }
     buflist_setfpos(buf, win,
         win->w_cursor.lnum == 1 ? 0 : win->w_cursor.lnum,
         win->w_cursor.col, TRUE);
@@ -402,7 +403,7 @@ void close_buffer(win_T *win, buf_T *buf, int action, int abort_if_last)
   buf->b_nwindows = nwindows;
 
   buf_freeall(buf, (del_buf ? BFA_DEL : 0) + (wipe_buf ? BFA_WIPE : 0));
-  if (win_valid(win) && win->w_buffer == buf) {
+  if (win_valid_any_tab(win) && win->w_buffer == buf) {
     win->w_buffer = NULL;  // make sure we don't use the buffer now
   }
 
@@ -478,17 +479,20 @@ void buf_clear_file(buf_T *buf)
   buf->b_ml.ml_flags = ML_EMPTY;                /* empty buffer */
 }
 
-/*
- * buf_freeall() - free all things allocated for a buffer that are related to
- * the file.  flags:
- * BFA_DEL	  buffer is going to be deleted
- * BFA_WIPE	  buffer is going to be wiped out
- * BFA_KEEP_UNDO  do not free undo information
- */
+/// buf_freeall() - free all things allocated for a buffer that are related to
+/// the file.  Careful: get here with "curwin" NULL when exiting.
+///
+/// @param flags BFA_DEL buffer is going to be deleted
+///              BFA_WIPE buffer is going to be wiped out
+///              BFA_KEEP_UNDO  do not free undo information
 void buf_freeall(buf_T *buf, int flags)
 {
   bool is_curbuf = (buf == curbuf);
+  int is_curwin = (curwin != NULL && curwin->w_buffer == buf);
+  win_T *the_curwin = curwin;
+  tabpage_T *the_curtab = curtab;
 
+  // Make sure the buffer isn't closed by autocommands.
   buf->b_closing = true;
   apply_autocmds(EVENT_BUFUNLOAD, buf->b_fname, buf->b_fname, FALSE, buf);
   if (!buf_valid(buf))              /* autocommands may delete the buffer */
@@ -505,8 +509,18 @@ void buf_freeall(buf_T *buf, int flags)
       return;
   }
   buf->b_closing = false;
-  if (aborting())           /* autocmds may abort script processing */
+
+  // If the buffer was in curwin and the window has changed, go back to that
+  // window, if it still exists.  This avoids that ":edit x" triggering a
+  // "tabnext" BufUnload autocmd leaves a window behind without a buffer.
+  if (is_curwin && curwin != the_curwin &&  win_valid_any_tab(the_curwin)) {
+    block_autocmds();
+    goto_tabpage_win(the_curtab, the_curwin);
+    unblock_autocmds();
+  }
+  if (aborting()) {  // autocmds may abort script processing
     return;
+  }
 
   /*
    * It's possible that autocommands change curbuf to the one being deleted.
diff --git a/src/nvim/ex_cmds.c b/src/nvim/ex_cmds.c
index 96eeff8973..17780c58e4 100644
--- a/src/nvim/ex_cmds.c
+++ b/src/nvim/ex_cmds.c
@@ -2250,28 +2250,28 @@ do_ecmd (
         xfree(new_name);
         goto theend;
       }
-      if (buf == curbuf)                /* already in new buffer */
-        auto_buf = TRUE;
-      else {
-        if (curbuf == old_curbuf)
+      if (buf == curbuf) {  // already in new buffer
+        auto_buf = true;
+      } else {
+        win_T *the_curwin = curwin;
+
+        // Set the w_closing flag to avoid that autocommands close the window.
+        the_curwin->w_closing = true;
+        if (curbuf == old_curbuf) {
           buf_copy_options(buf, BCO_ENTER);
+        }
 
-        /* close the link to the current buffer */
-        u_sync(FALSE);
+        // Close the link to the current buffer. This will set
+        // curwin->w_buffer to NULL.
+        u_sync(false);
         close_buffer(oldwin, curbuf,
-            (flags & ECMD_HIDE) || curbuf->terminal ? 0 : DOBUF_UNLOAD, FALSE);
-
-        /* Autocommands may open a new window and leave oldwin open
-         * which leads to crashes since the above call sets
-         * oldwin->w_buffer to NULL. */
-        if (curwin != oldwin && oldwin != aucmd_win && win_valid(oldwin)) {
-          assert(oldwin);
-          if (oldwin->w_buffer == NULL) {
-            win_close(oldwin, FALSE);
-          }
-        }
+                     (flags & ECMD_HIDE) || curbuf->terminal ? 0 : DOBUF_UNLOAD,
+                     false);
+
+        the_curwin->w_closing = false;
 
-        if (aborting()) {           /* autocmds may abort script processing */
+        // autocmds may abort script processing
+        if (aborting() && curwin->w_buffer != NULL) {
           xfree(new_name);
           goto theend;
         }
diff --git a/src/nvim/ex_docmd.c b/src/nvim/ex_docmd.c
index 9f83688e1e..5e418bf099 100644
--- a/src/nvim/ex_docmd.c
+++ b/src/nvim/ex_docmd.c
@@ -1302,8 +1302,7 @@ static char_u * do_one_cmd(char_u **cmdlinep,
      * 2. Handle command modifiers.
      */
     p = ea.cmd;
-    if (ascii_isdigit(*ea.cmd))
-      p = skipwhite(skipdigits(ea.cmd));
+    p = skip_range(ea.cmd, NULL);
     switch (*p) {
     /* When adding an entry, also modify cmd_exists(). */
     case 'a':   if (!checkforcmd(&ea.cmd, "aboveleft", 3))
@@ -1406,12 +1405,18 @@ static char_u * do_one_cmd(char_u **cmdlinep,
       continue;
 
     case 't':   if (checkforcmd(&p, "tab", 3)) {
-        if (ascii_isdigit(*ea.cmd))
-          cmdmod.tab = atoi((char *)ea.cmd) + 1;
-        else
-          cmdmod.tab = tabpage_index(curtab) + 1;
-        ea.cmd = p;
-        continue;
+      long tabnr = get_address(&ea, &ea.cmd, ADDR_TABS, ea.skip, false);
+      if (tabnr == MAXLNUM) {
+        cmdmod.tab = tabpage_index(curtab) + 1;
+      } else {
+        if (tabnr < 0 || tabnr > LAST_TAB_NR) {
+          errormsg = (char_u *)_(e_invrange);
+          goto doend;
+        }
+        cmdmod.tab = tabnr + 1;
+      }
+      ea.cmd = p;
+      continue;
     }
       if (!checkforcmd(&ea.cmd, "topleft", 2))
         break;
@@ -1766,11 +1771,8 @@ static char_u * do_one_cmd(char_u **cmdlinep,
 
     if (text_locked() && !(ea.argt & CMDWIN)
         && !IS_USER_CMDIDX(ea.cmdidx)) {
-      /* Command not allowed when editing the command line. */
-      if (cmdwin_type != 0)
-        errormsg = (char_u *)_(e_cmdwin);
-      else
-        errormsg = (char_u *)_(e_secure);
+      // Command not allowed when editing the command line.
+      errormsg = get_text_locked_msg();
       goto doend;
     }
     /* Disallow editing another buffer when "curbuf_lock" is set.
diff --git a/src/nvim/ex_getln.c b/src/nvim/ex_getln.c
index 7444eb8a38..e525c949bd 100644
--- a/src/nvim/ex_getln.c
+++ b/src/nvim/ex_getln.c
@@ -1688,10 +1688,15 @@ int text_locked(void) {
  */
 void text_locked_msg(void)
 {
-  if (cmdwin_type != 0)
-    EMSG(_(e_cmdwin));
-  else
-    EMSG(_(e_secure));
+  EMSG(_(get_text_locked_msg()));
+}
+
+char_u * get_text_locked_msg(void) {
+  if (cmdwin_type != 0) {
+    return e_cmdwin;
+  } else {
+    return e_secure;
+  }
 }
 
 /*
diff --git a/src/nvim/testdir/test_tabpage.vim b/src/nvim/testdir/test_tabpage.vim
index e6b85d6e14..0bf7d056de 100644
--- a/src/nvim/testdir/test_tabpage.vim
+++ b/src/nvim/testdir/test_tabpage.vim
@@ -186,4 +186,62 @@ function Test_tabpage_with_autocmd()
   bw!
 endfunction
 
+function Test_tabpage_with_tab_modifier()
+  for n in range(4)
+    tabedit
+  endfor
+
+  function s:check_tab(pre_nr, cmd, post_nr)
+    exec 'tabnext ' . a:pre_nr
+    exec a:cmd
+    call assert_equal(a:post_nr, tabpagenr())
+    call assert_equal('help', &filetype)
+    helpclose
+  endfunc
+
+  call s:check_tab(1, 'tab help', 2)
+  call s:check_tab(1, '3tab help', 4)
+  call s:check_tab(1, '.tab help', 2)
+  call s:check_tab(1, '.+1tab help', 3)
+  call s:check_tab(1, '0tab help', 1)
+  call s:check_tab(2, '+tab help', 4)
+  call s:check_tab(2, '+2tab help', 5)
+  call s:check_tab(4, '-tab help', 4)
+  call s:check_tab(4, '-2tab help', 3)
+  call s:check_tab(3, '$tab help', 6)
+  call assert_fails('99tab help', 'E16:')
+  call assert_fails('+99tab help', 'E16:')
+  call assert_fails('-99tab help', 'E16:')
+
+  delfunction s:check_tab
+  tabonly!
+  bw!
+endfunction
+
+func Test_tabnext_on_buf_unload1()
+  " This once caused a crash
+  new
+  tabedit
+  tabfirst
+  au BufUnload <buffer> tabnext
+  q
+
+  while tabpagenr('$') > 1
+    bwipe!
+  endwhile
+endfunc
+
+func Test_tabnext_on_buf_unload2()
+  " This once caused a crash
+  tabedit
+  autocmd BufUnload <buffer> tabnext
+  file x
+  edit y
+
+  while tabpagenr('$') > 1
+    bwipe!
+  endwhile
+endfunc
+
+
 " vim: shiftwidth=2 sts=2 expandtab
diff --git a/src/nvim/version.c b/src/nvim/version.c
index 29c0532a7f..498afa9656 100644
--- a/src/nvim/version.c
+++ b/src/nvim/version.c
@@ -129,10 +129,10 @@ static int included_patches[] = {
   // 2315,
   // 2314,
   // 2313,
-  // 2312,
+  2312,
   // 2311,
   // 2310 NA
-  // 2309,
+  2309,
   // 2308 NA
   // 2307,
   // 2306,
@@ -204,7 +204,7 @@ static int included_patches[] = {
   // 2240,
   // 2239,
   // 2238 NA
-  // 2237,
+  2237,
   // 2236,
   // 2235,
   // 2234 NA
@@ -229,7 +229,7 @@ static int included_patches[] = {
   // 2215,
   // 2214 NA
   2213,
-  // 2212,
+  2212,
   // 2211 NA
   // 2210 NA
   // 2209,
diff --git a/src/nvim/window.c b/src/nvim/window.c
index 03a2e9a842..9c6a2e26a6 100644
--- a/src/nvim/window.c
+++ b/src/nvim/window.c
@@ -1065,6 +1065,23 @@ bool win_valid(win_T *win) FUNC_ATTR_PURE FUNC_ATTR_WARN_UNUSED_RESULT
   return false;
 }
 
+/// Check if "win" is a pointer to an existing window in any tabpage.
+///
+/// @param  win  window to check
+bool win_valid_any_tab(win_T *win) FUNC_ATTR_PURE FUNC_ATTR_WARN_UNUSED_RESULT
+{
+  if (win == NULL) {
+    return false;
+  }
+
+  FOR_ALL_TAB_WINDOWS(tp, wp) {
+    if (wp == win) {
+      return true;
+    }
+  }
+  return false;
+}
+
 /*
  * Return the number of windows.
  */
@@ -1918,7 +1935,7 @@ int win_close(win_T *win, int free_buf)
   if (win->w_buffer != NULL) {
     win->w_closing = true;
     close_buffer(win, win->w_buffer, free_buf ? DOBUF_UNLOAD : 0, true);
-    if (win_valid(win)) {
+    if (win_valid_any_tab(win)) {
       win->w_closing = false;
     }
 
@@ -1938,11 +1955,19 @@ int win_close(win_T *win, int free_buf)
       curwin->w_buffer = curbuf;
     getout(0);
   }
-  /* Autocommands may have closed the window already, or closed the only
-   * other window or moved to another tab page. */
-  else if (!win_valid(win) || last_window() || curtab != prev_curtab
-           || close_last_window_tabpage(win, free_buf, prev_curtab))
+  // Autocommands may have moved to another tab page.
+  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);
     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) || last_window()
+      || close_last_window_tabpage(win, free_buf, prev_curtab)) {
+    return FAIL;
+  }
 
   // let terminal buffers know that this window dimensions may be ignored
   win->w_closing = true;
@@ -2019,12 +2044,16 @@ void win_close_othertab(win_T *win, int free_buf, tabpage_T *tp)
   tabpage_T   *ptp = NULL;
   int free_tp = FALSE;
 
-  assert(win->w_buffer);  // to avoid np dereference warning in next line
-  if (win->w_closing || win->w_buffer->b_closing)
-    return;     /* window is already being closed */
+  // Get here with win->w_buffer == NULL when win_close() detects the tab page
+  // changed.
+  if (win->w_closing || (win->w_buffer != NULL && win->w_buffer->b_closing)) {
+    return;  // window is already being closed
+  }
 
-  /* Close the link to the buffer. */
-  close_buffer(win, win->w_buffer, free_buf ? DOBUF_UNLOAD : 0, FALSE);
+  if (win->w_buffer != NULL) {
+    // Close the link to the buffer.
+    close_buffer(win, win->w_buffer, free_buf ? DOBUF_UNLOAD : 0, false);
+  }
 
   /* Careful: Autocommands may have closed the tab page or made it the
    * current tab page.  */
@@ -3213,11 +3242,8 @@ void goto_tabpage(int n)
   int i;
 
   if (text_locked()) {
-    /* Not allowed when editing the command line. */
-    if (cmdwin_type != 0)
-      EMSG(_(e_cmdwin));
-    else
-      EMSG(_(e_secure));
+    // Not allowed when editing the command line.
+    text_locked_msg();
     return;
   }
 
diff --git a/test/functional/shada/marks_spec.lua b/test/functional/shada/marks_spec.lua
index 646b0b692e..b743c3a4b7 100644
--- a/test/functional/shada/marks_spec.lua
+++ b/test/functional/shada/marks_spec.lua
@@ -102,6 +102,18 @@ describe('ShaDa support code', function()
     eq(2, nvim_current_line())
   end)
 
+  it('is able to dump and read back mark " from a closed tab', function()
+    nvim_command('edit ' .. testfilename)
+    nvim_command('edit ' .. testfilename_2)
+    nvim_command('2')
+    nvim_command('q!')
+    nvim_command('qall')
+    reset()
+    nvim_command('edit ' .. testfilename_2)
+    nvim_command('normal! `"')
+    eq(2, nvim_current_line())
+  end)
+
   it('is able to populate v:oldfiles', function()
     nvim_command('edit ' .. testfilename)
     local tf_full = curbufmeths.get_name()