Merge pull request #23228 from seandewar/cmdwin-jail

fix(api): use `text_locked()` to check for textlock

runtime/doc/api.txt

@@ -1200,6 +1200,9 @@ nvim_open_term({buffer}, {opts})                            *nvim_open_term()*
     |nvim_chan_send()| can be called immediately to process sequences in a
     virtual terminal having the intended size.
 
+    Attributes: ~
+        not allowed when |textlock| is active
+
     Parameters: ~
       • {buffer}  the buffer to use (expected to be empty)
       • {opts}    Optional parameters.
@@ -1365,7 +1368,7 @@ nvim_set_current_buf({buffer})                        *nvim_set_current_buf()*
     Sets the current buffer.
 
     Attributes: ~
-        not allowed when |textlock| is active
+        not allowed when |textlock| is active or in the |cmdwin|
 
     Parameters: ~
       • {buffer}  Buffer handle
@@ -1389,7 +1392,7 @@ nvim_set_current_tabpage({tabpage})               *nvim_set_current_tabpage()*
     Sets the current tabpage.
 
     Attributes: ~
-        not allowed when |textlock| is active
+        not allowed when |textlock| is active or in the |cmdwin|
 
     Parameters: ~
       • {tabpage}  Tabpage handle
@@ -1398,7 +1401,7 @@ nvim_set_current_win({window})                        *nvim_set_current_win()*
     Sets the current window.
 
     Attributes: ~
-        not allowed when |textlock| is active
+        not allowed when |textlock| is active or in the |cmdwin|
 
     Parameters: ~
       • {window}  Window handle
@@ -2163,7 +2166,7 @@ nvim_buf_delete({buffer}, {opts})                          *nvim_buf_delete()*
     Deletes the buffer. See |:bwipeout|
 
     Attributes: ~
-        not allowed when |textlock| is active
+        not allowed when |textlock| is active or in the |cmdwin|
 
     Parameters: ~
       • {buffer}  Buffer handle, or 0 for current buffer
@@ -2426,6 +2429,9 @@ nvim_buf_set_text({buffer}, {start_row}, {start_col}, {end_row}, {end_col},
     Prefer |nvim_buf_set_lines()| if you are only adding or deleting entire
     lines.
 
+    Attributes: ~
+        not allowed when |textlock| is active
+
     Parameters: ~
       • {buffer}       Buffer handle, or 0 for current buffer
       • {start_row}    First line index
@@ -2894,7 +2900,7 @@ nvim_win_hide({window})                                      *nvim_win_hide()*
     or |nvim_win_close()|, which will close the buffer.
 
     Attributes: ~
-        not allowed when |textlock| is active
+        not allowed when |textlock| is active or in the |cmdwin|
 
     Parameters: ~
       • {window}  Window handle, or 0 for current window
@@ -2912,7 +2918,7 @@ nvim_win_set_buf({window}, {buffer})                      *nvim_win_set_buf()*
     Sets the current buffer in a window, without side effects
 
     Attributes: ~
-        not allowed when |textlock| is active
+        not allowed when |textlock| is active or in the |cmdwin|
 
     Parameters: ~
       • {window}  Window handle, or 0 for current window
@@ -2999,7 +3005,7 @@ nvim_open_win({buffer}, {enter}, {*config})                  *nvim_open_win()*
 <
 
     Attributes: ~
-        not allowed when |textlock| is active
+        not allowed when |textlock| is active or in the |cmdwin|
 
     Parameters: ~
       • {buffer}  Buffer to display, or 0 for current buffer

scripts/gen_vimdoc.py

@@ -310,7 +310,8 @@ param_exclude = (
 # Annotations are displayed as line items after API function descriptions.
 annotation_map = {
     'FUNC_API_FAST': '|api-fast|',
-    'FUNC_API_CHECK_TEXTLOCK': 'not allowed when |textlock| is active',
+    'FUNC_API_TEXTLOCK': 'not allowed when |textlock| is active or in the |cmdwin|',
+    'FUNC_API_TEXTLOCK_ALLOW_CMDWIN': 'not allowed when |textlock| is active',
     'FUNC_API_REMOTE_ONLY': '|RPC| only',
     'FUNC_API_LUA_ONLY': 'Lua |vim.api| only',
 }

src/nvim/api/buffer.c

@@ -347,7 +347,7 @@ end:
 void nvim_buf_set_lines(uint64_t channel_id, Buffer buffer, Integer start, Integer end,
                         Boolean strict_indexing, ArrayOf(String) replacement, Error *err)
   FUNC_API_SINCE(1)
-  FUNC_API_CHECK_TEXTLOCK
+  FUNC_API_TEXTLOCK_ALLOW_CMDWIN
 {
   buf_T *buf = find_buffer_by_handle(buffer, err);
 
@@ -515,6 +515,7 @@ end:
 void nvim_buf_set_text(uint64_t channel_id, Buffer buffer, Integer start_row, Integer start_col,
                        Integer end_row, Integer end_col, ArrayOf(String) replacement, Error *err)
   FUNC_API_SINCE(7)
+  FUNC_API_TEXTLOCK_ALLOW_CMDWIN
 {
   MAXSIZE_TEMP_ARRAY(scratch, 1);
   if (replacement.size == 0) {
@@ -1061,7 +1062,7 @@ Boolean nvim_buf_is_loaded(Buffer buffer)
 ///          - unload: Unloaded only, do not delete. See |:bunload|
 void nvim_buf_delete(Buffer buffer, Dictionary opts, Error *err)
   FUNC_API_SINCE(7)
-  FUNC_API_CHECK_TEXTLOCK
+  FUNC_API_TEXTLOCK
 {
   buf_T *buf = find_buffer_by_handle(buffer, err);
 

src/nvim/api/vim.c

@@ -612,7 +612,7 @@ String nvim_get_current_line(Error *err)
 /// @param[out] err Error details, if any
 void nvim_set_current_line(String line, Error *err)
   FUNC_API_SINCE(1)
-  FUNC_API_CHECK_TEXTLOCK
+  FUNC_API_TEXTLOCK_ALLOW_CMDWIN
 {
   buffer_set_line(curbuf->handle, curwin->w_cursor.lnum - 1, line, err);
 }
@@ -622,7 +622,7 @@ void nvim_set_current_line(String line, Error *err)
 /// @param[out] err Error details, if any
 void nvim_del_current_line(Error *err)
   FUNC_API_SINCE(1)
-  FUNC_API_CHECK_TEXTLOCK
+  FUNC_API_TEXTLOCK_ALLOW_CMDWIN
 {
   buffer_del_line(curbuf->handle, curwin->w_cursor.lnum - 1, err);
 }
@@ -803,7 +803,7 @@ Buffer nvim_get_current_buf(void)
 /// @param[out] err Error details, if any
 void nvim_set_current_buf(Buffer buffer, Error *err)
   FUNC_API_SINCE(1)
-  FUNC_API_CHECK_TEXTLOCK
+  FUNC_API_TEXTLOCK
 {
   buf_T *buf = find_buffer_by_handle(buffer, err);
 
@@ -858,7 +858,7 @@ Window nvim_get_current_win(void)
 /// @param[out] err Error details, if any
 void nvim_set_current_win(Window window, Error *err)
   FUNC_API_SINCE(1)
-  FUNC_API_CHECK_TEXTLOCK
+  FUNC_API_TEXTLOCK
 {
   win_T *win = find_window_by_handle(window, err);
 
@@ -951,12 +951,18 @@ fail:
 /// @return Channel id, or 0 on error
 Integer nvim_open_term(Buffer buffer, DictionaryOf(LuaRef) opts, Error *err)
   FUNC_API_SINCE(7)
+  FUNC_API_TEXTLOCK_ALLOW_CMDWIN
 {
   buf_T *buf = find_buffer_by_handle(buffer, err);
   if (!buf) {
     return 0;
   }
 
+  if (cmdwin_type != 0 && buf == curbuf) {
+    api_set_error(err, kErrorTypeException, "%s", _(e_cmdwin));
+    return 0;
+  }
+
   LuaRef cb = LUA_NOREF;
   for (size_t i = 0; i < opts.size; i++) {
     String k = opts.items[i].key;
@@ -1084,7 +1090,7 @@ Tabpage nvim_get_current_tabpage(void)
 /// @param[out] err Error details, if any
 void nvim_set_current_tabpage(Tabpage tabpage, Error *err)
   FUNC_API_SINCE(1)
-  FUNC_API_CHECK_TEXTLOCK
+  FUNC_API_TEXTLOCK
 {
   tabpage_T *tp = find_tab_by_handle(tabpage, err);
 
@@ -1126,7 +1132,7 @@ void nvim_set_current_tabpage(Tabpage tabpage, Error *err)
 ///     - false: Client must cancel the paste.
 Boolean nvim_paste(String data, Boolean crlf, Integer phase, Error *err)
   FUNC_API_SINCE(6)
-  FUNC_API_CHECK_TEXTLOCK
+  FUNC_API_TEXTLOCK_ALLOW_CMDWIN
 {
   static bool draining = false;
   bool cancel = false;
@@ -1197,7 +1203,7 @@ theend:
 /// @param[out] err Error details, if any
 void nvim_put(ArrayOf(String) lines, String type, Boolean after, Boolean follow, Error *err)
   FUNC_API_SINCE(6)
-  FUNC_API_CHECK_TEXTLOCK
+  FUNC_API_TEXTLOCK_ALLOW_CMDWIN
 {
   yankreg_T *reg = xcalloc(1, sizeof(yankreg_T));
   VALIDATE_S((prepare_yankreg_from_object(reg, type, lines.size)), "type", type.data, {

src/nvim/api/win_config.c

@@ -158,7 +158,7 @@
 /// @return Window handle, or 0 on error
 Window nvim_open_win(Buffer buffer, Boolean enter, Dict(float_config) *config, Error *err)
   FUNC_API_SINCE(6)
-  FUNC_API_CHECK_TEXTLOCK
+  FUNC_API_TEXTLOCK
 {
   FloatConfig fconfig = FLOAT_CONFIG_INIT;
   if (!parse_float_config(config, &fconfig, false, true, err)) {

src/nvim/api/window.c

@@ -48,7 +48,7 @@ Buffer nvim_win_get_buf(Window window, Error *err)
 /// @param[out] err Error details, if any
 void nvim_win_set_buf(Window window, Buffer buffer, Error *err)
   FUNC_API_SINCE(5)
-  FUNC_API_CHECK_TEXTLOCK
+  FUNC_API_TEXTLOCK
 {
   win_set_buf(window, buffer, false, err);
 }
@@ -351,7 +351,7 @@ Boolean nvim_win_is_valid(Window window)
 /// @param[out] err Error details, if any
 void nvim_win_hide(Window window, Error *err)
   FUNC_API_SINCE(7)
-  FUNC_API_CHECK_TEXTLOCK
+  FUNC_API_TEXTLOCK
 {
   win_T *win = find_window_by_handle(window, err);
   if (!win) {
@@ -383,7 +383,7 @@ void nvim_win_hide(Window window, Error *err)
 /// @param[out] err Error details, if any
 void nvim_win_close(Window window, Boolean force, Error *err)
   FUNC_API_SINCE(6)
-  FUNC_API_CHECK_TEXTLOCK
+  FUNC_API_TEXTLOCK_ALLOW_CMDWIN
 {
   win_T *win = find_window_by_handle(window, err);
   if (!win) {

src/nvim/eval/funcs.c

@@ -8385,7 +8385,10 @@ static void f_termopen(typval_T *argvars, typval_T *rettv, EvalFuncData fptr)
   if (check_secure()) {
     return;
   }
-
+  if (text_locked()) {
+    text_locked_msg();
+    return;
+  }
   if (curbuf->b_changed) {
     emsg(_("Can only call this function in an unmodified buffer"));
     return;

src/nvim/func_attr.h

@@ -226,8 +226,10 @@
 # define FUNC_API_REMOTE_ONLY
 /// API function not exposed in Vimscript/remote.
 # define FUNC_API_LUA_ONLY
-/// API function checked textlock.
-# define FUNC_API_CHECK_TEXTLOCK
+/// API function fails during textlock.
+# define FUNC_API_TEXTLOCK
+/// API function fails during textlock, but allows cmdwin.
+# define FUNC_API_TEXTLOCK_ALLOW_CMDWIN
 /// API function introduced at the given API level.
 # define FUNC_API_SINCE(X)
 /// API function deprecated since the given API level.

src/nvim/generators/c_grammar.lua

@@ -47,7 +47,8 @@ local c_proto = Ct(
   (fill * Cg((P('FUNC_API_NOEXPORT') * Cc(true)), 'noexport') ^ -1) *
   (fill * Cg((P('FUNC_API_REMOTE_ONLY') * Cc(true)), 'remote_only') ^ -1) *
   (fill * Cg((P('FUNC_API_LUA_ONLY') * Cc(true)), 'lua_only') ^ -1) *
-  (fill * Cg((P('FUNC_API_CHECK_TEXTLOCK') * Cc(true)), 'check_textlock') ^ -1) *
+  (fill * (Cg(P('FUNC_API_TEXTLOCK_ALLOW_CMDWIN') * Cc(true), 'textlock_allow_cmdwin') +
+           Cg(P('FUNC_API_TEXTLOCK') * Cc(true), 'textlock')) ^ -1) *
   (fill * Cg((P('FUNC_API_REMOTE_IMPL') * Cc(true)), 'remote_impl') ^ -1) *
   (fill * Cg((P('FUNC_API_COMPOSITOR_IMPL') * Cc(true)), 'compositor_impl') ^ -1) *
   (fill * Cg((P('FUNC_API_CLIENT_IMPL') * Cc(true)), 'client_impl') ^ -1) *

src/nvim/generators/gen_api_dispatch.lua

@@ -210,10 +210,11 @@ local keysets_defs = io.open(keysets_outputf, 'wb')
 --  so that the dispatcher can find the C functions that you are creating!
 -- ===========================================================================
 output:write([[
+#include "nvim/ex_docmd.h"
+#include "nvim/ex_getln.h"
 #include "nvim/log.h"
 #include "nvim/map.h"
 #include "nvim/msgpack_rpc/helpers.h"
-#include "nvim/vim.h"
 
 #include "nvim/api/autocmd.h"
 #include "nvim/api/buffer.h"
@@ -389,8 +390,13 @@ for i = 1, #functions do
       args[#args + 1] = converted
     end
 
-    if fn.check_textlock then
-      output:write('\n  if (textlock != 0) {')
+    if fn.textlock then
+      output:write('\n  if (text_locked()) {')
+      output:write('\n    api_set_error(error, kErrorTypeException, "%s", get_text_locked_msg());')
+      output:write('\n    goto cleanup;')
+      output:write('\n  }\n')
+    elseif fn.textlock_allow_cmdwin then
+      output:write('\n  if (textlock != 0 || expr_map_locked()) {')
       output:write('\n    api_set_error(error, kErrorTypeException, "%s", e_textlock);')
       output:write('\n    goto cleanup;')
       output:write('\n  }\n')
@@ -525,6 +531,8 @@ output:write([[
 #include <lualib.h>
 #include <lauxlib.h>
 
+#include "nvim/ex_docmd.h"
+#include "nvim/ex_getln.h"
 #include "nvim/func_attr.h"
 #include "nvim/api/private/defs.h"
 #include "nvim/api/private/helpers.h"
@@ -565,9 +573,16 @@ local function process_function(fn)
     ]], fn.name))
   end
 
-  if fn.check_textlock then
+  if fn.textlock then
     write_shifted_output(output, [[
-    if (textlock != 0) {
+    if (text_locked()) {
+      api_set_error(&err, kErrorTypeException, "%s", get_text_locked_msg());
+      goto exit_0;
+    }
+    ]])
+  elseif fn.textlock_allow_cmdwin then
+    write_shifted_output(output, [[
+    if (textlock != 0 || expr_map_locked()) {
       api_set_error(&err, kErrorTypeException, "%s", e_textlock);
       goto exit_0;
     }

src/uncrustify.cfg

@@ -3636,7 +3636,8 @@ set_numbering_for_html_output   = false    # true/false
 #       `macro-close END_MESSAGE_MAP`
 #
 #
-set CLASS_COLON                  FUNC_API_CHECK_TEXTLOCK
+set CLASS_COLON                  FUNC_API_TEXTLOCK
+set CLASS_COLON                  FUNC_API_TEXTLOCK_ALLOW_CMDWIN
 set CLASS_COLON                  FUNC_API_DEPRECATED_SINCE
 set CLASS_COLON                  FUNC_API_FAST
 set CLASS_COLON                  FUNC_API_LUA_ONLY

test/functional/terminal/buffer_spec.lua

@@ -498,3 +498,17 @@ if is_os('win') then
     end)
   end)
 end
+
+describe('termopen()', function()
+  before_each(clear)
+
+  it('disallowed when textlocked and in cmdwin buffer', function()
+    command("autocmd TextYankPost <buffer> ++once call termopen('foo')")
+    matches("Vim%(call%):E565: Not allowed to change text or change window$",
+      pcall_err(command, "normal! yy"))
+
+    feed("q:")
+    eq("Vim:E11: Invalid in command-line window; <CR> executes, CTRL-C quits",
+      pcall_err(funcs.termopen, "bar"))
+  end)
+end)

test/functional/vimscript/api_functions_spec.lua

@@ -7,6 +7,7 @@ local exc_exec, expect, eval = helpers.exc_exec, helpers.expect, helpers.eval
 local insert, pcall_err = helpers.insert, helpers.pcall_err
 local matches = helpers.matches
 local meths = helpers.meths
+local feed = helpers.feed
 
 describe('eval-API', function()
   before_each(clear)
@@ -48,10 +49,46 @@ describe('eval-API', function()
     eq('Vim(call):E5555: API call: Invalid buffer id: 17', err)
   end)
 
-  it('cannot change texts if textlocked', function()
+  it('cannot change text or window if textlocked', function()
     command("autocmd TextYankPost <buffer> ++once call nvim_buf_set_lines(0, 0, -1, v:false, [])")
     matches('Vim%(call%):E5555: API call: E565: Not allowed to change text or change window$',
        pcall_err(command, "normal! yy"))
+
+    command("autocmd TextYankPost <buffer> ++once call nvim_open_term(0, {})")
+    matches('Vim%(call%):E5555: API call: E565: Not allowed to change text or change window$',
+       pcall_err(command, "normal! yy"))
+
+    -- Functions checking textlock should also not be usable from <expr> mappings.
+    command("inoremap <expr> <f2> nvim_win_close(0, 1)")
+    eq('Vim(normal):E5555: API call: E565: Not allowed to change text or change window',
+       pcall_err(command, [[execute "normal i\<f2>"]]))
+
+    -- Text-changing functions gave a "Failed to save undo information" error when called from an
+    -- <expr> mapping outside do_cmdline() (msg_list == NULL), so use feed() to test this.
+    command("inoremap <expr> <f2> nvim_buf_set_text(0, 0, 0, 0, 0, ['hi'])")
+    meths.set_vvar('errmsg', '')
+    feed("i<f2><esc>")
+    eq('E5555: API call: E565: Not allowed to change text or change window',
+       meths.get_vvar('errmsg'))
+
+    -- Some functions checking textlock (usually those that may change the current window or buffer)
+    -- also ought to not be usable in the cmdwin.
+    feed("q:")
+    eq('E11: Invalid in command-line window; <CR> executes, CTRL-C quits',
+       pcall_err(meths.win_hide, 0))
+
+    -- But others, like nvim_buf_set_lines(), which just changes text, is OK.
+    curbufmeths.set_lines(0, -1, 1, {"wow!"})
+    eq({'wow!'}, curbufmeths.get_lines(0, -1, 1))
+
+    -- Turning the cmdwin buffer into a terminal buffer would be pretty weird.
+    eq('E11: Invalid in command-line window; <CR> executes, CTRL-C quits',
+       pcall_err(meths.open_term, 0, {}))
+
+    -- But turning a different buffer into a terminal from the cmdwin is OK.
+    local term_buf = meths.create_buf(false, true)
+    meths.open_term(term_buf, {})
+    eq('terminal', meths.get_option_value("buftype", {buf = term_buf}))
   end)
 
   it("use buffer numbers and windows ids as handles", function()