Merge pull request #5336 from bfredl/del_var

make del_var and set_var not return the old value.

scripts/gendispatch.lua

@@ -46,8 +46,8 @@ grammar = Ct((c_proto + c_comment + c_preproc + ws) ^ 1)
 assert(#arg >= 3)
 functions = {}
 
-local scriptdir = arg[1]
-package.path = scriptdir .. '/?.lua;' .. package.path
+local nvimsrcdir = arg[1]
+package.path = nvimsrcdir .. '/?.lua;' .. package.path
 
 -- names of all headers relative to the source root (for inclusion in the
 -- generated file)
@@ -57,6 +57,9 @@ outputf = arg[#arg-1]
 -- output mpack file (metadata)
 mpack_outputf = arg[#arg]
 
+-- set of function names, used to detect duplicates
+function_names = {}
+
 -- read each input file, parse and append to the api metadata
 for i = 2, #arg - 2 do
   local full_path = arg[i]
@@ -72,6 +75,7 @@ for i = 2, #arg - 2 do
     local fn = tmp[i]
     if not fn.noexport then
       functions[#functions + 1] = tmp[i]
+      function_names[fn.name] = true
       if #fn.parameters ~= 0 and fn.parameters[1][2] == 'channel_id' then
         -- this function should receive the channel id
         fn.receives_channel_id = true
@@ -104,7 +108,7 @@ end
 
 -- Export functions under older deprecated names.
 -- These will be removed eventually.
-local deprecated_aliases = require("dispatch_deprecated")
+local deprecated_aliases = require("api.dispatch_deprecated")
 for i,f in ipairs(shallowcopy(functions)) do
   local ismethod = false
   if startswith(f.name, "nvim_buf_") then
@@ -120,6 +124,13 @@ for i,f in ipairs(shallowcopy(functions)) do
   f.method = ismethod
   local newname = deprecated_aliases[f.name]
   if newname ~= nil then
+    if function_names[newname] then
+      -- duplicate
+      print("Function "..f.name.." has deprecated alias\n"
+            ..newname.." which has a separate implementation.\n"..
+            "Please remove it from src/nvim/api/dispatch_deprecated.lua")
+      os.exit(1)
+    end
     local newf = shallowcopy(f)
     newf.name = newname
     newf.impl_name = f.name

src/nvim/CMakeLists.txt

@@ -204,11 +204,12 @@ add_custom_command(OUTPUT ${GENERATED_UNICODE_TABLES}
 )
 
 add_custom_command(OUTPUT ${GENERATED_API_DISPATCH} ${API_METADATA}
-    COMMAND ${LUA_PRG} ${DISPATCH_GENERATOR} ${PROJECT_SOURCE_DIR}/scripts ${API_HEADERS} ${GENERATED_API_DISPATCH} ${API_METADATA}
+    COMMAND ${LUA_PRG} ${DISPATCH_GENERATOR} ${CMAKE_CURRENT_LIST_DIR} ${API_HEADERS} ${GENERATED_API_DISPATCH} ${API_METADATA}
   DEPENDS
     ${API_HEADERS}
     ${MSGPACK_RPC_HEADERS}
     ${DISPATCH_GENERATOR}
+    ${CMAKE_CURRENT_LIST_DIR}/api/dispatch_deprecated.lua
 )
 
 list(APPEND NEOVIM_GENERATED_SOURCES

src/nvim/api/buffer.c

@@ -432,19 +432,15 @@ Object nvim_buf_get_var(Buffer buffer, String name, Error *err)
 /// @param name The variable name
 /// @param value The variable value
 /// @param[out] err Details of an error that may have occurred
-/// @return The old value or nil if there was no previous value.
-///
-///         @warning It may return nil if there was no previous value
-///                  or if previous value was `v:null`.
-Object nvim_buf_set_var(Buffer buffer, String name, Object value, Error *err)
+void nvim_buf_set_var(Buffer buffer, String name, Object value, Error *err)
 {
   buf_T *buf = find_buffer_by_handle(buffer, err);
 
   if (!buf) {
-    return (Object) OBJECT_INIT;
+    return;
   }
 
-  return dict_set_value(buf->b_vars, name, value, false, err);
+  dict_set_value(buf->b_vars, name, value, false, false, err);
 }
 
 /// Removes a buffer-scoped (b:) variable
@@ -452,11 +448,30 @@ Object nvim_buf_set_var(Buffer buffer, String name, Object value, Error *err)
 /// @param buffer The buffer handle
 /// @param name The variable name
 /// @param[out] err Details of an error that may have occurred
+void nvim_buf_del_var(Buffer buffer, String name, Error *err)
+{
+  buf_T *buf = find_buffer_by_handle(buffer, err);
+
+  if (!buf) {
+    return;
+  }
+
+  dict_set_value(buf->b_vars, name, NIL, true, false, err);
+}
+
+/// Sets a buffer-scoped (b:) variable
+///
+/// @deprecated
+///
+/// @param buffer The buffer handle
+/// @param name The variable name
+/// @param value The variable value
+/// @param[out] err Details of an error that may have occurred
 /// @return The old value or nil if there was no previous value.
 ///
 ///         @warning It may return nil if there was no previous value
 ///                  or if previous value was `v:null`.
-Object nvim_buf_del_var(Buffer buffer, String name, Error *err)
+Object buffer_set_var(Buffer buffer, String name, Object value, Error *err)
 {
   buf_T *buf = find_buffer_by_handle(buffer, err);
 
@@ -464,9 +479,29 @@ Object nvim_buf_del_var(Buffer buffer, String name, Error *err)
     return (Object) OBJECT_INIT;
   }
 
-  return dict_set_value(buf->b_vars, name, NIL, true, err);
+  return dict_set_value(buf->b_vars, name, value, false, true, err);
 }
 
+/// Removes a buffer-scoped (b:) variable
+///
+/// @deprecated
+///
+/// @param buffer The buffer handle
+/// @param name The variable name
+/// @param[out] err Details of an error that may have occurred
+/// @return The old value
+Object buffer_del_var(Buffer buffer, String name, Error *err)
+{
+  buf_T *buf = find_buffer_by_handle(buffer, err);
+
+  if (!buf) {
+    return (Object) OBJECT_INIT;
+  }
+
+  return dict_set_value(buf->b_vars, name, NIL, true, true, err);
+}
+
+
 /// Gets a buffer option value
 ///
 /// @param buffer The buffer handle

src/nvim/api/dispatch_deprecated.lua

@@ -3,8 +3,6 @@ local deprecated_aliases = {
   nvim_buf_get_lines="buffer_get_lines",
   nvim_buf_set_lines="buffer_set_lines",
   nvim_buf_get_var="buffer_get_var",
-  nvim_buf_set_var="buffer_set_var",
-  nvim_buf_del_var="buffer_del_var",
   nvim_buf_get_option="buffer_get_option",
   nvim_buf_set_option="buffer_set_option",
   nvim_buf_get_number="buffer_get_number",
@@ -16,8 +14,6 @@ local deprecated_aliases = {
   nvim_buf_clear_highlight="buffer_clear_highlight",
   nvim_tabpage_get_windows="tabpage_get_windows",
   nvim_tabpage_get_var="tabpage_get_var",
-  nvim_tabpage_set_var="tabpage_set_var",
-  nvim_tabpage_del_var="tabpage_del_var",
   nvim_tabpage_get_window="tabpage_get_window",
   nvim_tabpage_is_valid="tabpage_is_valid",
   nvim_ui_detach="ui_detach",
@@ -33,8 +29,6 @@ local deprecated_aliases = {
   nvim_list_runtime_paths="vim_list_runtime_paths",
   nvim_change_directory="vim_change_directory",
   nvim_get_var="vim_get_var",
-  nvim_set_var="vim_set_var",
-  nvim_del_var="vim_del_var",
   nvim_get_vvar="vim_get_vvar",
   nvim_get_option="vim_get_option",
   nvim_set_option="vim_set_option",
@@ -66,8 +60,6 @@ local deprecated_aliases = {
   nvim_win_get_width="window_get_width",
   nvim_win_set_width="window_set_width",
   nvim_win_get_var="window_get_var",
-  nvim_win_set_var="window_set_var",
-  nvim_win_del_var="window_del_var",
   nvim_win_get_option="window_get_option",
   nvim_win_set_option="window_set_option",
   nvim_win_get_position="window_get_position",

src/nvim/api/private/helpers.c

@@ -104,10 +104,11 @@ Object dict_get_value(dict_T *dict, String key, Error *err)
 /// @param value The new value
 /// @param del Delete key in place of setting it. Argument `value` is ignored in
 ///            this case.
+/// @param retval If true the old value will be converted and returned.
 /// @param[out] err Details of an error that may have occurred
-/// @return the old value, if any
+/// @return The old value if `retval` is true and the key was present, else NIL
 Object dict_set_value(dict_T *dict, String key, Object value, bool del,
-                      Error *err)
+                      bool retval, Error *err)
 {
   Object rv = OBJECT_INIT;
 
@@ -135,7 +136,9 @@ Object dict_set_value(dict_T *dict, String key, Object value, bool del,
       api_set_error(err, Validation, _("Key \"%s\" doesn't exist"), key.data);
     } else {
       // Return the old value
+      if (retval) {
         rv = vim_to_object(&di->di_tv);
+      }
       // Delete the entry
       hashitem_T *hi = hash_find(&dict->dv_hashtab, di->di_key);
       hash_remove(&dict->dv_hashtab, hi);
@@ -156,7 +159,9 @@ Object dict_set_value(dict_T *dict, String key, Object value, bool del,
       dict_add(dict, di);
     } else {
       // Return the old value
+      if (retval) {
         rv = vim_to_object(&di->di_tv);
+      }
       clear_tv(&di->di_tv);
     }
 

src/nvim/api/tabpage.c

@@ -60,11 +60,7 @@ Object nvim_tabpage_get_var(Tabpage tabpage, String name, Error *err)
 /// @param name The variable name
 /// @param value The variable value
 /// @param[out] err Details of an error that may have occurred
-/// @return The old value or nil if there was no previous value.
-///
-///         @warning It may return nil if there was no previous value
-///                  or if previous value was `v:null`.
-Object nvim_tabpage_set_var(Tabpage tabpage,
+void nvim_tabpage_set_var(Tabpage tabpage,
                           String name,
                           Object value,
                           Error *err)
@@ -72,10 +68,10 @@ Object nvim_tabpage_set_var(Tabpage tabpage,
   tabpage_T *tab = find_tab_by_handle(tabpage, err);
 
   if (!tab) {
-    return (Object) OBJECT_INIT;
+    return;
   }
 
-  return dict_set_value(tab->tp_vars, name, value, false, err);
+  dict_set_value(tab->tp_vars, name, value, false, false, err);
 }
 
 /// Removes a tab-scoped (t:) variable
@@ -83,11 +79,30 @@ Object nvim_tabpage_set_var(Tabpage tabpage,
 /// @param tabpage handle
 /// @param name The variable name
 /// @param[out] err Details of an error that may have occurred
+void nvim_tabpage_del_var(Tabpage tabpage, String name, Error *err)
+{
+  tabpage_T *tab = find_tab_by_handle(tabpage, err);
+
+  if (!tab) {
+    return;
+  }
+
+  dict_set_value(tab->tp_vars, name, NIL, true, false, err);
+}
+
+/// Sets a tab-scoped (t:) variable
+///
+/// @deprecated
+///
+/// @param tabpage handle
+/// @param name The variable name
+/// @param value The variable value
+/// @param[out] err Details of an error that may have occurred
 /// @return The old value or nil if there was no previous value.
 ///
 ///         @warning It may return nil if there was no previous value
 ///                  or if previous value was `v:null`.
-Object nvim_tabpage_del_var(Tabpage tabpage, String name, Error *err)
+Object tabpage_set_var(Tabpage tabpage, String name, Object value, Error *err)
 {
   tabpage_T *tab = find_tab_by_handle(tabpage, err);
 
@@ -95,7 +110,26 @@ Object nvim_tabpage_del_var(Tabpage tabpage, String name, Error *err)
     return (Object) OBJECT_INIT;
   }
 
-  return dict_set_value(tab->tp_vars, name, NIL, true, err);
+  return dict_set_value(tab->tp_vars, name, value, false, true, err);
+}
+
+/// Removes a tab-scoped (t:) variable
+///
+/// @deprecated
+///
+/// @param tabpage handle
+/// @param name The variable name
+/// @param[out] err Details of an error that may have occurred
+/// @return The old value
+Object tabpage_del_var(Tabpage tabpage, String name, Error *err)
+{
+  tabpage_T *tab = find_tab_by_handle(tabpage, err);
+
+  if (!tab) {
+    return (Object) OBJECT_INIT;
+  }
+
+  return dict_set_value(tab->tp_vars, name, NIL, true, true, err);
 }
 
 /// Gets the current window in a tab page

src/nvim/api/vim.c

@@ -354,26 +354,46 @@ Object nvim_get_var(String name, Error *err)
 /// @param name The variable name
 /// @param value The variable value
 /// @param[out] err Details of an error that may have occurred
-/// @return The old value or nil if there was no previous value.
-///
-///         @warning It may return nil if there was no previous value
-///                  or if previous value was `v:null`.
-Object nvim_set_var(String name, Object value, Error *err)
+void nvim_set_var(String name, Object value, Error *err)
 {
-  return dict_set_value(&globvardict, name, value, false, err);
+  dict_set_value(&globvardict, name, value, false, false, err);
 }
 
 /// Removes a global variable
 ///
 /// @param name The variable name
 /// @param[out] err Details of an error that may have occurred
+void nvim_del_var(String name, Error *err)
+{
+  dict_set_value(&globvardict, name, NIL, true, false, err);
+}
+
+/// Sets a global variable
+///
+/// @deprecated
+///
+/// @param name The variable name
+/// @param value The variable value
+/// @param[out] err Details of an error that may have occurred
 /// @return The old value or nil if there was no previous value.
 ///
 ///         @warning It may return nil if there was no previous value
 ///                  or if previous value was `v:null`.
-Object nvim_del_var(String name, Error *err)
+Object vim_set_var(String name, Object value, Error *err)
 {
-  return dict_set_value(&globvardict, name, NIL, true, err);
+  return dict_set_value(&globvardict, name, value, false, true, err);
+}
+
+/// Removes a global variable
+///
+/// @deprecated
+///
+/// @param name The variable name
+/// @param[out] err Details of an error that may have occurred
+/// @return The old value
+Object vim_del_var(String name, Error *err)
+{
+  return dict_set_value(&globvardict, name, NIL, true, true, err);
 }
 
 /// Gets a vim variable

src/nvim/api/window.c

@@ -202,19 +202,15 @@ Object nvim_win_get_var(Window window, String name, Error *err)
 /// @param name The variable name
 /// @param value The variable value
 /// @param[out] err Details of an error that may have occurred
-/// @return The old value or nil if there was no previous value.
-///
-///         @warning It may return nil if there was no previous value
-///                  or if previous value was `v:null`.
-Object nvim_win_set_var(Window window, String name, Object value, Error *err)
+void nvim_win_set_var(Window window, String name, Object value, Error *err)
 {
   win_T *win = find_window_by_handle(window, err);
 
   if (!win) {
-    return (Object) OBJECT_INIT;
+    return;
   }
 
-  return dict_set_value(win->w_vars, name, value, false, err);
+  dict_set_value(win->w_vars, name, value, false, false, err);
 }
 
 /// Removes a window-scoped (w:) variable
@@ -222,11 +218,30 @@ Object nvim_win_set_var(Window window, String name, Object value, Error *err)
 /// @param window The window handle
 /// @param name The variable name
 /// @param[out] err Details of an error that may have occurred
+void nvim_win_del_var(Window window, String name, Error *err)
+{
+  win_T *win = find_window_by_handle(window, err);
+
+  if (!win) {
+    return;
+  }
+
+  dict_set_value(win->w_vars, name, NIL, true, false, err);
+}
+
+/// Sets a window-scoped (w:) variable
+///
+/// @deprecated
+///
+/// @param window The window handle
+/// @param name The variable name
+/// @param value The variable value
+/// @param[out] err Details of an error that may have occurred
 /// @return The old value or nil if there was no previous value.
 ///
 ///         @warning It may return nil if there was no previous value
 ///                  or if previous value was `v:null`.
-Object nvim_win_del_var(Window window, String name, Error *err)
+Object window_set_var(Window window, String name, Object value, Error *err)
 {
   win_T *win = find_window_by_handle(window, err);
 
@@ -234,7 +249,26 @@ Object nvim_win_del_var(Window window, String name, Error *err)
     return (Object) OBJECT_INIT;
   }
 
-  return dict_set_value(win->w_vars, name, NIL, true, err);
+  return dict_set_value(win->w_vars, name, value, false, true, err);
+}
+
+/// Removes a window-scoped (w:) variable
+///
+/// @deprecated
+///
+/// @param window The window handle
+/// @param name The variable name
+/// @param[out] err Details of an error that may have occurred
+/// @return The old value
+Object window_del_var(Window window, String name, Error *err)
+{
+  win_T *win = find_window_by_handle(window, err);
+
+  if (!win) {
+    return (Object) OBJECT_INIT;
+  }
+
+  return dict_set_value(win->w_vars, name, NIL, true, true, err);
 }
 
 /// Gets a window option value

src/nvim/eval.c

@@ -16332,9 +16332,9 @@ static void f_termopen(typval_T *argvars, typval_T *rettv, FunPtr fptr)
   // Save the job id and pid in b:terminal_job_{id,pid}
   Error err;
   dict_set_value(curbuf->b_vars, cstr_as_string("terminal_job_id"),
-                 INTEGER_OBJ(rettv->vval.v_number), false, &err);
+                 INTEGER_OBJ(rettv->vval.v_number), false, false, &err);
   dict_set_value(curbuf->b_vars, cstr_as_string("terminal_job_pid"),
-                 INTEGER_OBJ(pid), false, &err);
+                 INTEGER_OBJ(pid), false, false, &err);
 
   Terminal *term = terminal_open(topts);
   data->term = term;

src/nvim/terminal.c

@@ -623,11 +623,12 @@ static void buf_set_term_title(buf_T *buf, char *title)
     FUNC_ATTR_NONNULL_ALL
 {
   Error err;
-  api_free_object(dict_set_value(buf->b_vars,
+  dict_set_value(buf->b_vars,
                  cstr_as_string("term_title"),
                  STRING_OBJ(cstr_as_string(title)),
                  false,
-                                 &err));
+                 false,
+                 &err);
 }
 
 static int term_settermprop(VTermProp prop, VTermValue *val, void *data)

test/functional/api/buffer_spec.lua

@@ -4,6 +4,7 @@ local clear, nvim, buffer = helpers.clear, helpers.nvim, helpers.buffer
 local curbuf, curwin, eq = helpers.curbuf, helpers.curwin, helpers.eq
 local curbufmeths, ok = helpers.curbufmeths, helpers.ok
 local funcs, request = helpers.funcs, helpers.request
+local NIL = helpers.NIL
 
 describe('buffer_* functions', function()
   before_each(clear)
@@ -250,6 +251,21 @@ describe('buffer_* functions', function()
       curbufmeths.del_var('lua')
       eq(0, funcs.exists('b:lua'))
     end)
+
+    it('buffer_set_var returns the old value', function()
+      local val1 = {1, 2, {['3'] = 1}}
+      local val2 = {4, 7}
+      eq(NIL, request('buffer_set_var', 0, 'lua', val1))
+      eq(val1, request('buffer_set_var', 0, 'lua', val2))
+    end)
+
+    it('buffer_del_var returns the old value', function()
+      local val1 = {1, 2, {['3'] = 1}}
+      local val2 = {4, 7}
+      eq(NIL,  request('buffer_set_var', 0, 'lua', val1))
+      eq(val1, request('buffer_set_var', 0, 'lua', val2))
+      eq(val2, request('buffer_del_var', 0, 'lua'))
+    end)
   end)
 
   describe('{get,set}_option', function()

test/functional/api/tabpage_spec.lua

@@ -5,6 +5,8 @@ local clear, nvim, tabpage, curtab, eq, ok =
   helpers.ok
 local curtabmeths = helpers.curtabmeths
 local funcs = helpers.funcs
+local request = helpers.request
+local NIL = helpers.NIL
 
 describe('tabpage_* functions', function()
   before_each(clear)
@@ -32,6 +34,21 @@ describe('tabpage_* functions', function()
       curtabmeths.del_var('lua')
       eq(0, funcs.exists('t:lua'))
     end)
+
+    it('tabpage_set_var returns the old value', function()
+      local val1 = {1, 2, {['3'] = 1}}
+      local val2 = {4, 7}
+      eq(NIL, request('tabpage_set_var', 0, 'lua', val1))
+      eq(val1, request('tabpage_set_var', 0, 'lua', val2))
+    end)
+
+    it('tabpage_del_var returns the old value', function()
+      local val1 = {1, 2, {['3'] = 1}}
+      local val2 = {4, 7}
+      eq(NIL,  request('tabpage_set_var', 0, 'lua', val1))
+      eq(val1, request('tabpage_set_var', 0, 'lua', val2))
+      eq(val2, request('tabpage_del_var', 0, 'lua'))
+    end)
   end)
 
   describe('is_valid', function()

test/functional/api/vim_spec.lua

@@ -88,19 +88,19 @@ describe('vim_* functions', function()
       eq(0, funcs.exists('g:lua'))
     end)
 
-    it('set_var returns the old value', function()
+    it('vim_set_var returns the old value', function()
       local val1 = {1, 2, {['3'] = 1}}
       local val2 = {4, 7}
-      eq(NIL, nvim('set_var', 'lua', val1))
-      eq(val1, nvim('set_var', 'lua', val2))
+      eq(NIL, request('vim_set_var', 'lua', val1))
+      eq(val1, request('vim_set_var', 'lua', val2))
     end)
 
-    it('del_var returns the old value', function()
+    it('vim_del_var returns the old value', function()
       local val1 = {1, 2, {['3'] = 1}}
       local val2 = {4, 7}
-      eq(NIL, meths.set_var('lua', val1))
-      eq(val1, meths.set_var('lua', val2))
-      eq(val2, meths.del_var('lua'))
+      eq(NIL,  request('vim_set_var', 'lua', val1))
+      eq(val1, request('vim_set_var', 'lua', val2))
+      eq(val2, request('vim_del_var', 'lua'))
     end)
 
     it('truncates values with NULs in them', function()

test/functional/api/window_spec.lua

@@ -7,6 +7,8 @@ local clear, nvim, curbuf, curbuf_contents, window, curwin, eq, neq,
 local wait = helpers.wait
 local curwinmeths = helpers.curwinmeths
 local funcs = helpers.funcs
+local request = helpers.request
+local NIL = helpers.NIL
 
 -- check if str is visible at the beginning of some line
 local function is_visible(str)
@@ -137,6 +139,21 @@ describe('window_* functions', function()
       curwinmeths.del_var('lua')
       eq(0, funcs.exists('w:lua'))
     end)
+
+    it('window_set_var returns the old value', function()
+      local val1 = {1, 2, {['3'] = 1}}
+      local val2 = {4, 7}
+      eq(NIL, request('window_set_var', 0, 'lua', val1))
+      eq(val1, request('window_set_var', 0, 'lua', val2))
+    end)
+
+    it('window_del_var returns the old value', function()
+      local val1 = {1, 2, {['3'] = 1}}
+      local val2 = {4, 7}
+      eq(NIL,  request('window_set_var', 0, 'lua', val1))
+      eq(val1, request('window_set_var', 0, 'lua', val2))
+      eq(val2, request('window_del_var', 0, 'lua'))
+    end)
   end)
 
   describe('{get,set}_option', function()