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

Problem: some API functions that check textlock (usually those that can change
curwin or curbuf) can break the cmdwin.

Solution: make FUNC_API_CHECK_TEXTLOCK call text_locked() instead, which already
checks for textlock, cmdwin and `<expr>` status.

Add FUNC_API_TEXTLOCK_ALLOW_CMDWIN to allow such functions to be usable in the
cmdwin if they can work properly there; the opt-in nature of this attribute
should hopefully help mitigate future bugs.

Also fix a regression in #22634 that made functions checking textlock usable in
`<expr>` mappings, and rename FUNC_API_CHECK_TEXTLOCK to FUNC_API_TEXTLOCK.
This commit is contained in:
Sean Dewar 2023-04-20 21:17:25 +01:00
parent b2e8c0df20
commit 77118d0da8
No known key found for this signature in database
GPG Key ID: 08CC2C83AD41B581
11 changed files with 67 additions and 31 deletions

View File

@ -1365,7 +1365,7 @@ nvim_set_current_buf({buffer}) *nvim_set_current_buf()*
Sets the current buffer. Sets the current buffer.
Attributes: ~ Attributes: ~
not allowed when |textlock| is active not allowed when |textlock| is active or in the |cmdwin|
Parameters: ~ Parameters: ~
• {buffer} Buffer handle • {buffer} Buffer handle
@ -1389,7 +1389,7 @@ nvim_set_current_tabpage({tabpage}) *nvim_set_current_tabpage()*
Sets the current tabpage. Sets the current tabpage.
Attributes: ~ Attributes: ~
not allowed when |textlock| is active not allowed when |textlock| is active or in the |cmdwin|
Parameters: ~ Parameters: ~
• {tabpage} Tabpage handle • {tabpage} Tabpage handle
@ -1398,7 +1398,7 @@ nvim_set_current_win({window}) *nvim_set_current_win()*
Sets the current window. Sets the current window.
Attributes: ~ Attributes: ~
not allowed when |textlock| is active not allowed when |textlock| is active or in the |cmdwin|
Parameters: ~ Parameters: ~
• {window} Window handle • {window} Window handle
@ -2163,7 +2163,7 @@ nvim_buf_delete({buffer}, {opts}) *nvim_buf_delete()*
Deletes the buffer. See |:bwipeout| Deletes the buffer. See |:bwipeout|
Attributes: ~ Attributes: ~
not allowed when |textlock| is active not allowed when |textlock| is active or in the |cmdwin|
Parameters: ~ Parameters: ~
• {buffer} Buffer handle, or 0 for current buffer • {buffer} Buffer handle, or 0 for current buffer
@ -2894,7 +2894,7 @@ nvim_win_hide({window}) *nvim_win_hide()*
or |nvim_win_close()|, which will close the buffer. or |nvim_win_close()|, which will close the buffer.
Attributes: ~ Attributes: ~
not allowed when |textlock| is active not allowed when |textlock| is active or in the |cmdwin|
Parameters: ~ Parameters: ~
• {window} Window handle, or 0 for current window • {window} Window handle, or 0 for current window
@ -2912,7 +2912,7 @@ nvim_win_set_buf({window}, {buffer}) *nvim_win_set_buf()*
Sets the current buffer in a window, without side effects Sets the current buffer in a window, without side effects
Attributes: ~ Attributes: ~
not allowed when |textlock| is active not allowed when |textlock| is active or in the |cmdwin|
Parameters: ~ Parameters: ~
• {window} Window handle, or 0 for current window • {window} Window handle, or 0 for current window
@ -2999,7 +2999,7 @@ nvim_open_win({buffer}, {enter}, {*config}) *nvim_open_win()*
< <
Attributes: ~ Attributes: ~
not allowed when |textlock| is active not allowed when |textlock| is active or in the |cmdwin|
Parameters: ~ Parameters: ~
• {buffer} Buffer to display, or 0 for current buffer • {buffer} Buffer to display, or 0 for current buffer

View File

@ -310,7 +310,8 @@ param_exclude = (
# Annotations are displayed as line items after API function descriptions. # Annotations are displayed as line items after API function descriptions.
annotation_map = { annotation_map = {
'FUNC_API_FAST': '|api-fast|', '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_REMOTE_ONLY': '|RPC| only',
'FUNC_API_LUA_ONLY': 'Lua |vim.api| only', 'FUNC_API_LUA_ONLY': 'Lua |vim.api| only',
} }

View File

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

View File

@ -612,7 +612,7 @@ String nvim_get_current_line(Error *err)
/// @param[out] err Error details, if any /// @param[out] err Error details, if any
void nvim_set_current_line(String line, Error *err) void nvim_set_current_line(String line, Error *err)
FUNC_API_SINCE(1) 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); 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 /// @param[out] err Error details, if any
void nvim_del_current_line(Error *err) void nvim_del_current_line(Error *err)
FUNC_API_SINCE(1) FUNC_API_SINCE(1)
FUNC_API_CHECK_TEXTLOCK FUNC_API_TEXTLOCK_ALLOW_CMDWIN
{ {
buffer_del_line(curbuf->handle, curwin->w_cursor.lnum - 1, err); 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 /// @param[out] err Error details, if any
void nvim_set_current_buf(Buffer buffer, Error *err) void nvim_set_current_buf(Buffer buffer, Error *err)
FUNC_API_SINCE(1) FUNC_API_SINCE(1)
FUNC_API_CHECK_TEXTLOCK FUNC_API_TEXTLOCK
{ {
buf_T *buf = find_buffer_by_handle(buffer, err); 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 /// @param[out] err Error details, if any
void nvim_set_current_win(Window window, Error *err) void nvim_set_current_win(Window window, Error *err)
FUNC_API_SINCE(1) FUNC_API_SINCE(1)
FUNC_API_CHECK_TEXTLOCK FUNC_API_TEXTLOCK
{ {
win_T *win = find_window_by_handle(window, err); win_T *win = find_window_by_handle(window, err);
@ -1084,7 +1084,7 @@ Tabpage nvim_get_current_tabpage(void)
/// @param[out] err Error details, if any /// @param[out] err Error details, if any
void nvim_set_current_tabpage(Tabpage tabpage, Error *err) void nvim_set_current_tabpage(Tabpage tabpage, Error *err)
FUNC_API_SINCE(1) FUNC_API_SINCE(1)
FUNC_API_CHECK_TEXTLOCK FUNC_API_TEXTLOCK
{ {
tabpage_T *tp = find_tab_by_handle(tabpage, err); tabpage_T *tp = find_tab_by_handle(tabpage, err);
@ -1126,7 +1126,7 @@ void nvim_set_current_tabpage(Tabpage tabpage, Error *err)
/// - false: Client must cancel the paste. /// - false: Client must cancel the paste.
Boolean nvim_paste(String data, Boolean crlf, Integer phase, Error *err) Boolean nvim_paste(String data, Boolean crlf, Integer phase, Error *err)
FUNC_API_SINCE(6) FUNC_API_SINCE(6)
FUNC_API_CHECK_TEXTLOCK FUNC_API_TEXTLOCK_ALLOW_CMDWIN
{ {
static bool draining = false; static bool draining = false;
bool cancel = false; bool cancel = false;
@ -1197,7 +1197,7 @@ theend:
/// @param[out] err Error details, if any /// @param[out] err Error details, if any
void nvim_put(ArrayOf(String) lines, String type, Boolean after, Boolean follow, Error *err) void nvim_put(ArrayOf(String) lines, String type, Boolean after, Boolean follow, Error *err)
FUNC_API_SINCE(6) FUNC_API_SINCE(6)
FUNC_API_CHECK_TEXTLOCK FUNC_API_TEXTLOCK_ALLOW_CMDWIN
{ {
yankreg_T *reg = xcalloc(1, sizeof(yankreg_T)); yankreg_T *reg = xcalloc(1, sizeof(yankreg_T));
VALIDATE_S((prepare_yankreg_from_object(reg, type, lines.size)), "type", type.data, { VALIDATE_S((prepare_yankreg_from_object(reg, type, lines.size)), "type", type.data, {

View File

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

View File

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

View File

@ -226,8 +226,10 @@
# define FUNC_API_REMOTE_ONLY # define FUNC_API_REMOTE_ONLY
/// API function not exposed in Vimscript/remote. /// API function not exposed in Vimscript/remote.
# define FUNC_API_LUA_ONLY # define FUNC_API_LUA_ONLY
/// API function checked textlock. /// API function fails during textlock.
# define FUNC_API_CHECK_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. /// API function introduced at the given API level.
# define FUNC_API_SINCE(X) # define FUNC_API_SINCE(X)
/// API function deprecated since the given API level. /// API function deprecated since the given API level.

View File

@ -47,7 +47,8 @@ local c_proto = Ct(
(fill * Cg((P('FUNC_API_NOEXPORT') * Cc(true)), 'noexport') ^ -1) * (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_REMOTE_ONLY') * Cc(true)), 'remote_only') ^ -1) *
(fill * Cg((P('FUNC_API_LUA_ONLY') * Cc(true)), 'lua_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_REMOTE_IMPL') * Cc(true)), 'remote_impl') ^ -1) *
(fill * Cg((P('FUNC_API_COMPOSITOR_IMPL') * Cc(true)), 'compositor_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) * (fill * Cg((P('FUNC_API_CLIENT_IMPL') * Cc(true)), 'client_impl') ^ -1) *

View File

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

View File

@ -3636,7 +3636,8 @@ set_numbering_for_html_output = false # true/false
# `macro-close END_MESSAGE_MAP` # `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_DEPRECATED_SINCE
set CLASS_COLON FUNC_API_FAST set CLASS_COLON FUNC_API_FAST
set CLASS_COLON FUNC_API_LUA_ONLY set CLASS_COLON FUNC_API_LUA_ONLY

View File

@ -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 insert, pcall_err = helpers.insert, helpers.pcall_err
local matches = helpers.matches local matches = helpers.matches
local meths = helpers.meths local meths = helpers.meths
local feed = helpers.feed
describe('eval-API', function() describe('eval-API', function()
before_each(clear) before_each(clear)
@ -48,10 +49,25 @@ describe('eval-API', function()
eq('Vim(call):E5555: API call: Invalid buffer id: 17', err) eq('Vim(call):E5555: API call: Invalid buffer id: 17', err)
end) 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, [])") 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$', matches('Vim%(call%):E5555: API call: E565: Not allowed to change text or change window$',
pcall_err(command, "normal! yy")) 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>"]]))
-- 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))
end) end)
it("use buffer numbers and windows ids as handles", function() it("use buffer numbers and windows ids as handles", function()