fix(api, lua): handle setting v: variables properly (#25325)

This commit is contained in:
zeertzjq 2023-09-24 10:57:09 +08:00 committed by GitHub
parent 046c9a83f7
commit 4d3a38ac07
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 219 additions and 71 deletions

View File

@ -21,8 +21,10 @@
#include "nvim/buffer_defs.h"
#include "nvim/eval/typval.h"
#include "nvim/eval/typval_defs.h"
#include "nvim/eval/vars.h"
#include "nvim/ex_eval.h"
#include "nvim/garray.h"
#include "nvim/globals.h"
#include "nvim/highlight_group.h"
#include "nvim/lua/executor.h"
#include "nvim/map.h"
@ -235,8 +237,7 @@ Object dict_set_var(dict_T *dict, String key, Object value, bool del, bool retva
// Delete the key
if (di == NULL) {
// Doesn't exist, fail
api_set_error(err, kErrorTypeValidation, "Key not found: %s",
key.data);
api_set_error(err, kErrorTypeValidation, "Key not found: %s", key.data);
} else {
// Notify watchers
if (watched) {
@ -265,13 +266,23 @@ Object dict_set_var(dict_T *dict, String key, Object value, bool del, bool retva
di = tv_dict_item_alloc_len(key.data, key.size);
tv_dict_add(dict, di);
} else {
if (watched) {
tv_copy(&di->di_tv, &oldtv);
}
// Return the old value
if (retval) {
rv = vim_to_object(&di->di_tv);
}
bool type_error = false;
if (dict == &vimvardict
&& !before_set_vvar(key.data, di, &tv, true, watched, &type_error)) {
tv_clear(&tv);
if (type_error) {
api_set_error(err, kErrorTypeValidation,
"Setting v:%s to value with wrong type", key.data);
}
return rv;
}
if (watched) {
tv_copy(&di->di_tv, &oldtv);
}
tv_clear(&di->di_tv);
}

View File

@ -52,8 +52,8 @@
static const char *e_letunexp = N_("E18: Unexpected characters in :let");
static const char *e_lock_unlock = N_("E940: Cannot lock or unlock variable %s");
static const char e_setting_str_to_value_with_wrong_type[]
= N_("E963: Setting %s to value with wrong type");
static const char e_setting_v_str_to_value_with_wrong_type[]
= N_("E963: Setting v:%s to value with wrong type");
static const char e_cannot_use_heredoc_here[]
= N_("E991: Cannot use =<< here");
@ -1389,6 +1389,62 @@ static void list_one_var_a(const char *prefix, const char *name, const ptrdiff_t
}
}
/// Additional handling for setting a v: variable.
///
/// @return true if the variable should be set normally,
/// false if nothing else needs to be done.
bool before_set_vvar(const char *const varname, dictitem_T *const di, typval_T *const tv,
const bool copy, const bool watched, bool *const type_error)
{
if (di->di_tv.v_type == VAR_STRING) {
typval_T oldtv = TV_INITIAL_VALUE;
if (watched) {
tv_copy(&di->di_tv, &oldtv);
}
XFREE_CLEAR(di->di_tv.vval.v_string);
if (copy || tv->v_type != VAR_STRING) {
const char *const val = tv_get_string(tv);
// Careful: when assigning to v:errmsg and tv_get_string()
// causes an error message the variable will already be set.
if (di->di_tv.vval.v_string == NULL) {
di->di_tv.vval.v_string = xstrdup(val);
}
} else {
// Take over the string to avoid an extra alloc/free.
di->di_tv.vval.v_string = tv->vval.v_string;
tv->vval.v_string = NULL;
}
// Notify watchers
if (watched) {
tv_dict_watcher_notify(&vimvardict, varname, &di->di_tv, &oldtv);
tv_clear(&oldtv);
}
return false;
} else if (di->di_tv.v_type == VAR_NUMBER) {
typval_T oldtv = TV_INITIAL_VALUE;
if (watched) {
tv_copy(&di->di_tv, &oldtv);
}
di->di_tv.vval.v_number = tv_get_number(tv);
if (strcmp(varname, "searchforward") == 0) {
set_search_direction(di->di_tv.vval.v_number ? '/' : '?');
} else if (strcmp(varname, "hlsearch") == 0) {
no_hlsearch = !di->di_tv.vval.v_number;
redraw_all_later(UPD_SOME_VALID);
}
// Notify watchers
if (watched) {
tv_dict_watcher_notify(&vimvardict, varname, &di->di_tv, &oldtv);
tv_clear(&oldtv);
}
return false;
} else if (di->di_tv.v_type != tv->v_type) {
*type_error = true;
return false;
}
return true;
}
/// Set variable to the given value
///
/// If the variable already exists, the value is updated. Otherwise the variable
@ -1418,31 +1474,29 @@ void set_var_const(const char *name, const size_t name_len, typval_T *const tv,
const bool is_const)
FUNC_ATTR_NONNULL_ALL
{
dictitem_T *v;
hashtab_T *ht;
dict_T *dict;
const char *varname;
ht = find_var_ht_dict(name, name_len, &varname, &dict);
dict_T *dict;
hashtab_T *ht = find_var_ht_dict(name, name_len, &varname, &dict);
const bool watched = tv_dict_is_watched(dict);
if (ht == NULL || *varname == NUL) {
semsg(_(e_illvar), name);
return;
}
v = find_var_in_ht(ht, 0, varname, name_len - (size_t)(varname - name), true);
const size_t varname_len = name_len - (size_t)(varname - name);
dictitem_T *di = find_var_in_ht(ht, 0, varname, varname_len, true);
// Search in parent scope which is possible to reference from lambda
if (v == NULL) {
v = find_var_in_scoped_ht(name, name_len, true);
if (di == NULL) {
di = find_var_in_scoped_ht(name, name_len, true);
}
if (tv_is_func(*tv) && var_wrong_func_name(name, v == NULL)) {
if (tv_is_func(*tv) && var_wrong_func_name(name, di == NULL)) {
return;
}
typval_T oldtv = TV_INITIAL_VALUE;
if (v != NULL) {
if (di != NULL) {
if (is_const) {
emsg(_(e_cannot_mod));
return;
@ -1452,9 +1506,9 @@ void set_var_const(const char *name, const size_t name_len, typval_T *const tv,
// - Whether the variable is read-only
// - Whether the variable value is locked
// - Whether the variable is locked
if (var_check_ro(v->di_flags, name, name_len)
|| value_check_lock(v->di_tv.v_lock, name, name_len)
|| var_check_lock(v->di_flags, name, name_len)) {
if (var_check_ro(di->di_flags, name, name_len)
|| value_check_lock(di->di_tv.v_lock, name, name_len)
|| var_check_lock(di->di_flags, name, name_len)) {
return;
}
@ -1462,42 +1516,19 @@ void set_var_const(const char *name, const size_t name_len, typval_T *const tv,
// Handle setting internal v: variables separately where needed to
// prevent changing the type.
if (is_vimvarht(ht)) {
if (v->di_tv.v_type == VAR_STRING) {
XFREE_CLEAR(v->di_tv.vval.v_string);
if (copy || tv->v_type != VAR_STRING) {
const char *const val = tv_get_string(tv);
// Careful: when assigning to v:errmsg and tv_get_string()
// causes an error message the variable will already be set.
if (v->di_tv.vval.v_string == NULL) {
v->di_tv.vval.v_string = xstrdup(val);
}
} else {
// Take over the string to avoid an extra alloc/free.
v->di_tv.vval.v_string = tv->vval.v_string;
tv->vval.v_string = NULL;
}
return;
} else if (v->di_tv.v_type == VAR_NUMBER) {
v->di_tv.vval.v_number = tv_get_number(tv);
if (strcmp(varname, "searchforward") == 0) {
set_search_direction(v->di_tv.vval.v_number ? '/' : '?');
} else if (strcmp(varname, "hlsearch") == 0) {
no_hlsearch = !v->di_tv.vval.v_number;
redraw_all_later(UPD_SOME_VALID);
}
return;
} else if (v->di_tv.v_type != tv->v_type) {
semsg(_(e_setting_str_to_value_with_wrong_type), name);
return;
bool type_error = false;
if (is_vimvarht(ht)
&& !before_set_vvar(varname, di, tv, copy, watched, &type_error)) {
if (type_error) {
semsg(_(e_setting_v_str_to_value_with_wrong_type), varname);
}
return;
}
if (watched) {
tv_copy(&v->di_tv, &oldtv);
tv_copy(&di->di_tv, &oldtv);
}
tv_clear(&v->di_tv);
tv_clear(&di->di_tv);
} else { // Add a new variable.
// Can't add "v:" or "a:" variable.
if (is_vimvarht(ht) || ht == get_funccal_args_ht()) {
@ -1513,28 +1544,28 @@ void set_var_const(const char *name, const size_t name_len, typval_T *const tv,
// Make sure dict is valid
assert(dict != NULL);
v = xmalloc(offsetof(dictitem_T, di_key) + strlen(varname) + 1);
STRCPY(v->di_key, varname);
if (hash_add(ht, v->di_key) == FAIL) {
xfree(v);
di = xmalloc(offsetof(dictitem_T, di_key) + varname_len + 1);
memcpy(di->di_key, varname, varname_len + 1);
if (hash_add(ht, di->di_key) == FAIL) {
xfree(di);
return;
}
v->di_flags = DI_FLAGS_ALLOC;
di->di_flags = DI_FLAGS_ALLOC;
if (is_const) {
v->di_flags |= DI_FLAGS_LOCK;
di->di_flags |= DI_FLAGS_LOCK;
}
}
if (copy || tv->v_type == VAR_NUMBER || tv->v_type == VAR_FLOAT) {
tv_copy(tv, &v->di_tv);
tv_copy(tv, &di->di_tv);
} else {
v->di_tv = *tv;
v->di_tv.v_lock = VAR_UNLOCKED;
di->di_tv = *tv;
di->di_tv.v_lock = VAR_UNLOCKED;
tv_init(tv);
}
if (watched) {
tv_dict_watcher_notify(dict, v->di_key, &v->di_tv, &oldtv);
tv_dict_watcher_notify(dict, di->di_key, &di->di_tv, &oldtv);
tv_clear(&oldtv);
}
@ -1542,7 +1573,7 @@ void set_var_const(const char *name, const size_t name_len, typval_T *const tv,
// Like :lockvar! name: lock the value and what it contains, but only
// if the reference count is up to one. That locks only literal
// values.
tv_item_lock(&v->di_tv, DICT_MAXNEST, true, true);
tv_item_lock(&di->di_tv, DICT_MAXNEST, true, true);
}
}

View File

@ -24,6 +24,7 @@
#include "nvim/buffer_defs.h"
#include "nvim/eval/typval.h"
#include "nvim/eval/typval_defs.h"
#include "nvim/eval/vars.h"
#include "nvim/ex_eval.h"
#include "nvim/fold.h"
#include "nvim/globals.h"
@ -397,6 +398,15 @@ int nlua_setvar(lua_State *lstate)
di = tv_dict_item_alloc_len(key.data, key.size);
tv_dict_add(dict, di);
} else {
bool type_error = false;
if (dict == &vimvardict
&& !before_set_vvar(key.data, di, &tv, true, watched, &type_error)) {
tv_clear(&tv);
if (type_error) {
return luaL_error(lstate, "Setting v:%s to value with wrong type", key.data);
}
return 0;
}
if (watched) {
tv_copy(&di->di_tv, &oldtv);
}

View File

@ -1350,15 +1350,59 @@ describe('API', function()
end)
it('nvim_get_vvar, nvim_set_vvar', function()
-- Set readonly v: var.
eq('Key is read-only: count',
pcall_err(request, 'nvim_set_vvar', 'count', 42))
-- Set non-existent v: var.
eq('Dictionary is locked',
pcall_err(request, 'nvim_set_vvar', 'nosuchvar', 42))
-- Set writable v: var.
eq('Key is read-only: count', pcall_err(request, 'nvim_set_vvar', 'count', 42))
eq('Dictionary is locked', pcall_err(request, 'nvim_set_vvar', 'nosuchvar', 42))
meths.set_vvar('errmsg', 'set by API')
eq('set by API', meths.get_vvar('errmsg'))
meths.set_vvar('errmsg', 42)
eq('42', eval('v:errmsg'))
meths.set_vvar('oldfiles', { 'one', 'two' })
eq({ 'one', 'two' }, eval('v:oldfiles'))
meths.set_vvar('oldfiles', {})
eq({}, eval('v:oldfiles'))
eq('Setting v:oldfiles to value with wrong type', pcall_err(meths.set_vvar, 'oldfiles', 'a'))
eq({}, eval('v:oldfiles'))
feed('i foo foo foo<Esc>0/foo<CR>')
eq({1, 1}, meths.win_get_cursor(0))
eq(1, eval('v:searchforward'))
feed('n')
eq({1, 5}, meths.win_get_cursor(0))
meths.set_vvar('searchforward', 0)
eq(0, eval('v:searchforward'))
feed('n')
eq({1, 1}, meths.win_get_cursor(0))
meths.set_vvar('searchforward', 1)
eq(1, eval('v:searchforward'))
feed('n')
eq({1, 5}, meths.win_get_cursor(0))
local screen = Screen.new(60, 3)
screen:set_default_attr_ids({
[0] = {bold = true, foreground = Screen.colors.Blue},
[1] = {background = Screen.colors.Yellow},
})
screen:attach()
eq(1, eval('v:hlsearch'))
screen:expect{grid=[[
{1:foo} {1:^foo} {1:foo} |
{0:~ }|
|
]]}
meths.set_vvar('hlsearch', 0)
eq(0, eval('v:hlsearch'))
screen:expect{grid=[[
foo ^foo foo |
{0:~ }|
|
]]}
meths.set_vvar('hlsearch', 1)
eq(1, eval('v:hlsearch'))
screen:expect{grid=[[
{1:foo} {1:^foo} {1:foo} |
{0:~ }|
|
]]}
end)
it('vim_set_var returns the old value', function()

View File

@ -1491,8 +1491,60 @@ describe('lua stdlib', function()
eq(NIL, funcs.luaeval "vim.v.null")
matches([[attempt to index .* nil value]],
pcall_err(exec_lua, 'return vim.v[0].progpath'))
eq('Key is read-only: count', pcall_err(exec_lua, 'vim.v.count = 42'))
eq('Dictionary is locked', pcall_err(exec_lua, 'vim.v.nosuchvar = 42'))
eq('Key is read-only: count', pcall_err(exec_lua, [[vim.v.count = 42]]))
eq('Dictionary is locked', pcall_err(exec_lua, [[vim.v.nosuchvar = 42]]))
eq('Key is fixed: errmsg', pcall_err(exec_lua, [[vim.v.errmsg = nil]]))
exec_lua([[vim.v.errmsg = 'set by Lua']])
eq('set by Lua', eval('v:errmsg'))
exec_lua([[vim.v.errmsg = 42]])
eq('42', eval('v:errmsg'))
exec_lua([[vim.v.oldfiles = { 'one', 'two' }]])
eq({ 'one', 'two' }, eval('v:oldfiles'))
exec_lua([[vim.v.oldfiles = {}]])
eq({}, eval('v:oldfiles'))
eq('Setting v:oldfiles to value with wrong type', pcall_err(exec_lua, [[vim.v.oldfiles = 'a']]))
eq({}, eval('v:oldfiles'))
feed('i foo foo foo<Esc>0/foo<CR>')
eq({1, 1}, meths.win_get_cursor(0))
eq(1, eval('v:searchforward'))
feed('n')
eq({1, 5}, meths.win_get_cursor(0))
exec_lua([[vim.v.searchforward = 0]])
eq(0, eval('v:searchforward'))
feed('n')
eq({1, 1}, meths.win_get_cursor(0))
exec_lua([[vim.v.searchforward = 1]])
eq(1, eval('v:searchforward'))
feed('n')
eq({1, 5}, meths.win_get_cursor(0))
local screen = Screen.new(60, 3)
screen:set_default_attr_ids({
[0] = {bold = true, foreground = Screen.colors.Blue},
[1] = {background = Screen.colors.Yellow},
})
screen:attach()
eq(1, eval('v:hlsearch'))
screen:expect{grid=[[
{1:foo} {1:^foo} {1:foo} |
{0:~ }|
|
]]}
exec_lua([[vim.v.hlsearch = 0]])
eq(0, eval('v:hlsearch'))
screen:expect{grid=[[
foo ^foo foo |
{0:~ }|
|
]]}
exec_lua([[vim.v.hlsearch = 1]])
eq(1, eval('v:hlsearch'))
screen:expect{grid=[[
{1:foo} {1:^foo} {1:foo} |
{0:~ }|
|
]]}
end)
it('vim.bo', function()