Merge pull request #1025 from ychin/mvim-fix-cmdline-term-buffer-menu-stale-items

Fixing buffer menu having stale items with terminal / cmdline window

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