diff --git a/src/nvim/api/private/helpers.c b/src/nvim/api/private/helpers.c index 8fcabb3605..5463578b56 100644 --- a/src/nvim/api/private/helpers.c +++ b/src/nvim/api/private/helpers.c @@ -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); } diff --git a/src/nvim/eval/vars.c b/src/nvim/eval/vars.c index e5b1b88eef..5d1da956ee 100644 --- a/src/nvim/eval/vars.c +++ b/src/nvim/eval/vars.c @@ -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); } } diff --git a/src/nvim/lua/stdlib.c b/src/nvim/lua/stdlib.c index 8f1b5c7b86..f175a7abb0 100644 --- a/src/nvim/lua/stdlib.c +++ b/src/nvim/lua/stdlib.c @@ -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); } diff --git a/test/functional/api/vim_spec.lua b/test/functional/api/vim_spec.lua index fcf3eae5ec..d82e5f66c9 100644 --- a/test/functional/api/vim_spec.lua +++ b/test/functional/api/vim_spec.lua @@ -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 foo0/foo') + 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() diff --git a/test/functional/lua/vim_spec.lua b/test/functional/lua/vim_spec.lua index cf80478b08..bd3d8f5247 100644 --- a/test/functional/lua/vim_spec.lua +++ b/test/functional/lua/vim_spec.lua @@ -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 foo0/foo') + 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()