Fixing buffer menu having stale items with terminal / cmdline window

Currently, when using special buffers like terminals / command-line
window / quickfix / location list, these buffers will get added to the
"Buffers" menu, but they don't get removed when the buffers are gone,
leaving stale menu items. Fix buffer menu to be more robust.

1. Currently the buffer menu works by using the buffer name to
   add/remove the entries, but it's error-prone because the buffer could
   have been renamed under-the-hood. While it uses BufFilePre/Post
   autocommands to handle normal buffer renames, it doesn't work for the
   command-line window (accessible via `q:`) which gets renamed without
   sending the autocommand. Instead, change the menus to cached a
   dictionary a bufnum -> menu name, so it will always know how to
   remove a buffer from itself.
2. Add BufFilePre/Post autocommands to command-line windows when it
   changes the buffer name to "[Command Line]".
3. Add BufFilePre/Post autocommands to terminal windows when it
   changes the buffer name to the command, e.g. "!/bin/zsh".

Either (1) or (2)+(3) will fix the issue, but just doing all of them as
this seems like the right thing to do (2 + 3) also means the menu items
show the correct names instead of just saying "[No Name]".

This doesn't fix the following which needs to be fixed later:
1. Quickfix and Location List don't send BufDeleted autocmds. This leads
   to them also leaving stale buffer menu items as there's no way for
   the script to know that those buffers are gone.

Also add unit tests for cmdline-win / terminal buffer menus

Note: This fix misc test_cmdline failures in MacVim due to the menu item
not being able to be removed.

This is a duplicate of vim/vim#5787

Author: ychin

runtime/menu.vim

@@ -711,6 +711,11 @@ if !exists("no_buffers_menu")
 " startup faster.
 let s:bmenu_wait = 1
 
+" dictionary of buffer ID to name. This helps prevent bugs where a buffer is
+" somehow being renamed and we can't remove it from the menu because we are
+" using the wrong menu name.
+let s:bmenu_items = {}
+
 if !exists("bmenu_priority")
   let bmenu_priority = 60
 endif
@@ -733,14 +738,13 @@ func! s:BMRemove()
     if isdirectory(name)
       return
     endif
-    let munge = <SID>BMMunge(name, expand("<abuf>"))
-
-    if s:bmenu_short == 0
-      exe 'silent! aun &Buffers.' . munge
-    else
-      exe 'silent! aun &Buffers.' . <SID>BMHash2(munge) . munge
+    let bufnum = expand("<abuf>")
+    if s:bmenu_items->has_key(bufnum)
+      let menu_name = s:bmenu_items[bufnum]
+      exe 'silent! aun &Buffers.' . menu_name
+      let s:bmenu_count = s:bmenu_count - 1
+      unlet s:bmenu_items[bufnum]
     endif
-    let s:bmenu_count = s:bmenu_count - 1
   endif
 endfunc
 
@@ -749,6 +753,7 @@ func! s:BMShow(...)
   let s:bmenu_wait = 1
   let s:bmenu_short = 1
   let s:bmenu_count = 0
+  let s:bmenu_items = {}
   "
   " get new priority, if exists
   if a:0 == 1
@@ -844,9 +849,12 @@ func! s:BMFilename(name, num)
   let munge = <SID>BMMunge(a:name, a:num)
   let hash = <SID>BMHash(munge)
   if s:bmenu_short == 0
+    let s:bmenu_items[a:num] = munge
     let name = 'an ' . g:bmenu_priority . '.' . hash . ' &Buffers.' . munge
   else
-    let name = 'an ' . g:bmenu_priority . '.' . hash . '.' . hash . ' &Buffers.' . <SID>BMHash2(munge) . munge
+    let menu_name = <SID>BMHash2(munge) . munge
+    let s:bmenu_items[a:num] = l:menu_name
+    let name = 'an ' . g:bmenu_priority . '.' . hash . '.' . hash . ' &Buffers.' . menu_name
   endif
   " set 'cpo' to include the <CR>
   let cpo_save = &cpo

src/ex_getln.c

@@ -4206,7 +4206,9 @@ open_cmdwin(void)
 
     // Create the command-line buffer empty.
     (void)do_ecmd(0, NULL, NULL, NULL, ECMD_ONE, ECMD_HIDE, NULL);
+    apply_autocmds(EVENT_BUFFILEPRE, NULL, NULL, FALSE, curbuf);
     (void)setfname(curbuf, (char_u *)"[Command Line]", NULL, TRUE);
+    apply_autocmds(EVENT_BUFFILEPOST, NULL, NULL, FALSE, curbuf);
     set_option_value((char_u *)"bt", 0L, (char_u *)"nofile", OPT_LOCAL);
     curbuf->b_p_ma = TRUE;
 #ifdef FEAT_FOLDING

src/terminal.c

@@ -523,6 +523,8 @@ term_start(
     term->tl_next = first_term;
     first_term = term;
 
+    apply_autocmds(EVENT_BUFFILEPRE, NULL, NULL, FALSE, curbuf);
+
     if (opt->jo_term_name != NULL)
 	curbuf->b_ffname = vim_strsave(opt->jo_term_name);
     else if (argv != NULL)
@@ -571,6 +573,8 @@ term_start(
     curbuf->b_sfname = vim_strsave(curbuf->b_ffname);
     curbuf->b_fname = curbuf->b_ffname;
 
+    apply_autocmds(EVENT_BUFFILEPOST, NULL, NULL, FALSE, curbuf);
+
     if (opt->jo_term_opencmd != NULL)
 	term->tl_opencmd = vim_strsave(opt->jo_term_opencmd);
 

src/testdir/test_cmdline.vim

@@ -1030,13 +1030,6 @@ func Test_cmdline_expand_special()
 endfunc
 
 func Test_cmdwin_jump_to_win()
-  if has('gui_macvim') && has('gui_running')
-    " Due to a mix of MacVim-specific menu behaviors (calling BMShow at start
-    " instead of VimEnter), and buffer menu stale item bugs in Vim, this test
-    " doesn't work in GUI for now. Will be re-enabled after buffer menu bugs
-    " are fixed.
-    return
-  endif
   call assert_fails('call feedkeys("q:\<C-W>\<C-W>\<CR>", "xt")', 'E11:')
   new
   set modified

src/testdir/test_menu.vim

@@ -23,6 +23,43 @@ func Test_load_menu()
   call assert_equal('', v:errmsg)
 endfunc
 
+func Test_buffer_menu_special_buffers()
+  " Load in runtime menus
+  try
+    source $VIMRUNTIME/menu.vim
+  catch
+    call assert_report('error while loading menus: ' . v:exception)
+  endtry
+
+  let v:errmsg = ''
+  if !has("gui_macvim")
+    " MacVim initializes buffer menus differently (by calling BMShow
+    " immediately) so this is unnecessary and would break the test.
+    doautocmd LoadBufferMenu VimEnter
+  endif
+  call assert_equal('', v:errmsg)
+
+  let orig_buffer_menus = execute("nmenu Buffers")
+
+  " Make a new command-line window, test that it creates a new buffer menu,
+  " and test that when we exits the command-line window, the menu item got removed.
+  call feedkeys("q::let cmdline_buffer_menus=execute('nmenu Buffers')\<CR>:q\<CR>", 'ntx')
+  call assert_equal(len(split(orig_buffer_menus, "\n")) + 2, len(split(cmdline_buffer_menus, "\n")))
+  call assert_equal(orig_buffer_menus, execute("nmenu Buffers"))
+
+  " Make a terminal window, and also test that it creates and removes the
+  " buffer menu item.
+  terminal
+  let term_buffer_menus = execute('nmenu Buffers')
+  call assert_equal(len(split(orig_buffer_menus, "\n")) + 2, len(split(term_buffer_menus, "\n")))
+  bd!
+  call assert_equal(orig_buffer_menus, execute("nmenu Buffers"))
+
+  " Remove menus to clean up
+  source $VIMRUNTIME/delmenu.vim
+  call assert_equal('', v:errmsg)
+endfunc
+
 func Test_translate_menu()
   if !has('multi_lang')
     return