vim-patch:8.2.0614: get ml_get error when deleting a line in 'completefunc' (#19244)

Problem:    Get ml_get error when deleting a line in 'completefunc'. (Yegappan
            Lakshmanan)
Solution:   Lock the text while evaluating 'completefunc'.
ff06f283e3

Fix a mistake in the porting of patch 8.1.0098.
Cherry-pick Test_run_excmd_with_text_locked() from patch 8.2.0270.
Cherry-pick test_gf.vim changes from patch 8.2.0369.
Cherry-pick message change from later patches.

runtime/doc/insert.txt

@@ -654,8 +654,10 @@ Note: The keys that are valid in CTRL-X mode are not mapped.  This allows for
 ends CTRL-X mode (any key that is not a valid CTRL-X mode command) is mapped.
 Also, when doing completion with 'complete' mappings apply as usual.
 
-Note: While completion is active Insert mode can't be used recursively.
+								*E565*
-Mappings that somehow invoke ":normal i.." will generate an E523 error.
+Note: While completion is active Insert mode can't be used recursively and
+buffer text cannot be changed.  Mappings that somehow invoke ":normal i.."
+will generate an E565 error.
 
 The following mappings are suggested to make typing the completion commands
 a bit easier (although they will hide other commands): >

src/nvim/edit.c

@@ -1409,14 +1409,9 @@ bool edit(int cmdchar, bool startln, long count)
 
   // Don't allow changes in the buffer while editing the cmdline.  The
   // caller of getcmdline() may get confused.
-  if (textlock != 0) {
-    emsg(_(e_secure));
-    return false;
-  }
-
   // Don't allow recursive insert mode when busy with completion.
-  if (compl_started || compl_busy || pum_visible()) {
+  if (textlock != 0 || compl_started || compl_busy || pum_visible()) {
-    emsg(_(e_secure));
+    emsg(_(e_textlock));
     return false;
   }
 
@@ -3966,6 +3961,8 @@ static void expand_by_function(int type, char_u *base)
   pos = curwin->w_cursor;
   curwin_save = curwin;
   curbuf_save = curbuf;
+  // Lock the text to avoid weird things from happening.
+  textlock++;
 
   // Call a function, which returns a list or dict.
   if (call_vim_function((char *)funcname, 2, args, &rettv) == OK) {
@@ -3984,6 +3981,7 @@ static void expand_by_function(int type, char_u *base)
       break;
     }
   }
+  textlock--;
 
   if (curwin_save != curwin || curbuf_save != curbuf) {
     emsg(_(e_complwin));

src/nvim/eval/funcs.c

@@ -1062,6 +1062,11 @@ static void f_complete(typval_T *argvars, typval_T *rettv, FunPtr fptr)
     return;
   }
 
+  const int save_textlock = textlock;
+  // "textlock" is set when evaluating 'completefunc' but we can change text
+  // here.
+  textlock = 0;
+
   // Check for undo allowed here, because if something was already inserted
   // the line was already saved for undo and this check isn't done.
   if (!undo_allowed(curbuf)) {
@@ -1070,15 +1075,13 @@ static void f_complete(typval_T *argvars, typval_T *rettv, FunPtr fptr)
 
   if (argvars[1].v_type != VAR_LIST) {
     emsg(_(e_invarg));
-    return;
+  } else {
+    const colnr_T startcol = tv_get_number_chk(&argvars[0], NULL);
+    if (startcol > 0) {
+      set_completion(startcol - 1, argvars[1].vval.v_list);
+    }
   }
-
+  textlock = save_textlock;
-  const colnr_T startcol = tv_get_number_chk(&argvars[0], NULL);
-  if (startcol <= 0) {
-    return;
-  }
-
-  set_completion(startcol - 1, argvars[1].vval.v_list);
 }
 
 /// "complete_add()" function

src/nvim/ex_getln.c

@@ -2722,7 +2722,7 @@ char *get_text_locked_msg(void)
   if (cmdwin_type != 0) {
     return e_cmdwin;
   } else {
-    return e_secure;
+    return e_textlock;
   }
 }
 

src/nvim/generators/gen_api_dispatch.lua

@@ -292,7 +292,7 @@ for i = 1, #functions do
 
     if fn.check_textlock then
       output:write('\n  if (textlock != 0) {')
-      output:write('\n    api_set_error(error, kErrorTypeException, "%s", e_secure);')
+      output:write('\n    api_set_error(error, kErrorTypeException, "%s", e_textlock);')
       output:write('\n    goto cleanup;')
       output:write('\n  }\n')
     end
@@ -435,7 +435,7 @@ local function process_function(fn)
   if fn.check_textlock then
     write_shifted_output(output, [[
     if (textlock != 0) {
-      api_set_error(&err, kErrorTypeException, "%s", e_secure);
+      api_set_error(&err, kErrorTypeException, "%s", e_textlock);
       goto exit_0;
     }
     ]])

src/nvim/globals.h

@@ -509,7 +509,7 @@ EXTERN int full_screen INIT(= false);
 /// .vimrc in current directory.
 EXTERN int secure INIT(= 0);
 
-/// Non-zero when changing text and jumping to another window/buffer is not
+/// Non-zero when changing text and jumping to another window or editing another buffer is not
 /// allowed.
 EXTERN int textlock INIT(= 0);
 
@@ -955,6 +955,7 @@ EXTERN char e_listdictblobarg[] INIT(= N_("E896: Argument of %s must be a List,
 EXTERN char e_readerrf[] INIT(= N_("E47: Error while reading errorfile"));
 EXTERN char e_sandbox[] INIT(= N_("E48: Not allowed in sandbox"));
 EXTERN char e_secure[] INIT(= N_("E523: Not allowed here"));
+EXTERN char e_textlock[] INIT(= N_("E565: Not allowed to change text or change window"));
 EXTERN char e_screenmode[] INIT(= N_("E359: Screen mode setting not supported"));
 EXTERN char e_scroll[] INIT(= N_("E49: Invalid scroll size"));
 EXTERN char e_shellempty[] INIT(= N_("E91: 'shell' option is empty"));

src/nvim/match.c

@@ -494,10 +494,10 @@ static void next_search_hl(win_T *win, match_T *search_hl, match_T *shl, linenr_
       shl->lnum += shl->rm.startpos[0].lnum;
       break;                            // useful match found
     }
-
-    // Restore called_emsg for assert_fails().
-    called_emsg = save_called_emsg;
   }
+
+  // Restore called_emsg for assert_fails().
+  called_emsg = save_called_emsg;
 }
 
 /// Advance to the match in window "wp" line "lnum" or past it.

src/nvim/testdir/test_edit.vim

@@ -985,6 +985,22 @@ func Test_edit_CTRL_U()
   bw!
 endfunc
 
+func Test_edit_completefunc_delete()
+  func CompleteFunc(findstart, base)
+    if a:findstart == 1
+      return col('.') - 1
+    endif
+    normal dd
+    return ['a', 'b']
+  endfunc
+  new
+  set completefunc=CompleteFunc
+  call setline(1, ['', 'abcd', ''])
+  2d
+  call assert_fails("normal 2G$a\<C-X>\<C-U>", 'E565:')
+  bwipe!
+endfunc
+
 func Test_edit_CTRL_Z()
   " Ctrl-Z when insertmode is not set inserts it literally
   new
@@ -1331,7 +1347,7 @@ func Test_edit_forbidden()
   try
     call feedkeys("ix\<esc>", 'tnix')
     call assert_fails(1, 'textlock')
-  catch /^Vim\%((\a\+)\)\=:E523/ " catch E523: not allowed here
+  catch /^Vim\%((\a\+)\)\=:E565/ " catch E565: not allowed here
   endtry
   " TODO: Might be a bug: should x really be inserted here
   call assert_equal(['xa'], getline(1, '$'))
@@ -1356,7 +1372,7 @@ func Test_edit_forbidden()
   try
     call feedkeys("i\<c-x>\<c-u>\<esc>", 'tnix')
     call assert_fails(1, 'change in complete function')
-  catch /^Vim\%((\a\+)\)\=:E523/ " catch E523
+  catch /^Vim\%((\a\+)\)\=:E565/ " catch E565
   endtry
   delfu Complete
   set completefunc=

src/nvim/testdir/test_ex_mode.vim

@@ -67,13 +67,13 @@ endfunc
 func Test_ex_mode_errors()
   " Not allowed to enter ex mode when text is locked
   au InsertCharPre <buffer> normal! gQ<CR>
-  let caught_e523 = 0
+  let caught_e565 = 0
   try
     call feedkeys("ix\<esc>", 'xt')
-  catch /^Vim\%((\a\+)\)\=:E523/ " catch E523
+  catch /^Vim\%((\a\+)\)\=:E565/ " catch E565
-    let caught_e523 = 1
+    let caught_e565 = 1
   endtry
-  call assert_equal(1, caught_e523)
+  call assert_equal(1, caught_e565)
   au! InsertCharPre
 
   new

src/nvim/testdir/test_excmd.vim

@@ -401,6 +401,30 @@ func Test_winsize_cmd()
   " Actually changing the window size would be flaky.
 endfunc
 
+" Test for running Ex commands when text is locked.
+" <C-\>e in the command line is used to lock the text
+func Test_run_excmd_with_text_locked()
+  " :quit
+  let cmd = ":\<C-\>eexecute('quit')\<CR>\<C-C>"
+  call assert_fails("call feedkeys(cmd, 'xt')", 'E565:')
+
+  " :qall
+  let cmd = ":\<C-\>eexecute('qall')\<CR>\<C-C>"
+  call assert_fails("call feedkeys(cmd, 'xt')", 'E565:')
+
+  " :exit
+  let cmd = ":\<C-\>eexecute('exit')\<CR>\<C-C>"
+  call assert_fails("call feedkeys(cmd, 'xt')", 'E565:')
+
+  " :close - should be ignored
+  new
+  let cmd = ":\<C-\>eexecute('close')\<CR>\<C-C>"
+  call assert_equal(2, winnr('$'))
+  close
+
+  call assert_fails("call feedkeys(\":\<C-R>=execute('bnext')\<CR>\", 'xt')", 'E565:')
+endfunc
+
 func Test_not_break_expression_register()
   call setreg('=', '1+1')
   if 0

src/nvim/testdir/test_gf.vim

@@ -147,5 +147,20 @@ func Test_gf_error()
   call setline(1, '/doesnotexist')
   call assert_fails('normal gf', 'E447:')
   call assert_fails('normal gF', 'E447:')
+  call assert_fails('normal [f', 'E447:')
+
+  " gf is not allowed when text is locked
+  au InsertCharPre <buffer> normal! gF<CR>
+  let caught_e565 = 0
+  try
+    call feedkeys("ix\<esc>", 'xt')
+  catch /^Vim\%((\a\+)\)\=:E565/ " catch E565
+    let caught_e565 = 1
+  endtry
+  call assert_equal(1, caught_e565)
+  au! InsertCharPre
+
   bwipe!
 endfunc
+
+" vim: shiftwidth=2 sts=2 expandtab

src/nvim/testdir/test_popup.vim

@@ -349,19 +349,17 @@ func DummyCompleteOne(findstart, base)
   endif
 endfunc
 
-" Test that nothing happens if the 'completefunc' opens
+" Test that nothing happens if the 'completefunc' tries to open
-" a new window (no completion, no crash)
+" a new window (fails to open window, continues)
 func Test_completefunc_opens_new_window_one()
   new
   let winid = win_getid()
   setlocal completefunc=DummyCompleteOne
   call setline(1, 'one')
   /^one
-  call assert_fails('call feedkeys("A\<C-X>\<C-U>\<C-N>\<Esc>", "x")', 'E839:')
+  call assert_fails('call feedkeys("A\<C-X>\<C-U>\<C-N>\<Esc>", "x")', 'E565:')
-  call assert_notequal(winid, win_getid())
-  q!
   call assert_equal(winid, win_getid())
-  call assert_equal('', getline(1))
+  call assert_equal('oneDEF', getline(1))
   q!
 endfunc
 

src/nvim/undo.c

@@ -297,7 +297,7 @@ bool undo_allowed(buf_T *buf)
   // Don't allow changes in the buffer while editing the cmdline.  The
   // caller of getcmdline() may get confused.
   if (textlock != 0) {
-    emsg(_(e_secure));
+    emsg(_(e_textlock));
     return false;
   }
 

test/functional/ui/inccommand_spec.lua

@@ -1778,11 +1778,11 @@ describe("'inccommand' and :cnoremap", function()
       feed_command("cnoremap <expr> x execute('bwipeout!')[-1].'x'")
 
       feed(":%s/tw/tox<enter>")
-      screen:expect{any=[[{14:^E523:]]}
+      screen:expect{any=[[{14:^E565:]]}
       feed('<c-c>')
 
       -- error thrown b/c of the mapping
-      neq(nil, eval('v:errmsg'):find('^E523:'))
+      neq(nil, eval('v:errmsg'):find('^E565:'))
       expect([[
       Inc substitution on
       toxo lines

test/functional/vimscript/api_functions_spec.lua

@@ -49,7 +49,8 @@ describe('eval-API', function()
 
   it('cannot change texts if textlocked', function()
     command("autocmd TextYankPost <buffer> ++once call nvim_buf_set_lines(0, 0, -1, v:false, [])")
-    eq('Vim(call):E5555: API call: E523: Not allowed here', pcall_err(command, "normal! yy"))
+    eq('Vim(call):E5555: API call: E565: Not allowed to change text or change window',
+       pcall_err(command, "normal! yy"))
   end)
 
   it("use buffer numbers and windows ids as handles", function()