refactor(mappings)!: mapblock_fill_dict() use API Dictionary (#20020)

This introduces the following breaking changes:
- nvim_get_keymap now always returns a LuaRef object as "callback" for a
  Lua mapping regardless of how it is called. The LuaRef object can be
  called from Lua and Vim script, but is lost over RPC.
- maparg() now returns a Funcref instead of a ref number as "callback"
  for a Lua mapping. The Funcref can be called from Lua and Vim script,
  but is lost over RPC.

This may also make nvim_get_keymap faster, but make maparg() slower.

src/nvim/api/buffer.c

@@ -945,7 +945,7 @@ Integer nvim_buf_get_changedtick(Buffer buffer, Error *err)
 /// @param[out]  err   Error details, if any
 /// @returns Array of |maparg()|-like dictionaries describing mappings.
 ///          The "buffer" key holds the associated buffer handle.
-ArrayOf(Dictionary) nvim_buf_get_keymap(uint64_t channel_id, Buffer buffer, String mode, Error *err)
+ArrayOf(Dictionary) nvim_buf_get_keymap(Buffer buffer, String mode, Error *err)
   FUNC_API_SINCE(3)
 {
   buf_T *buf = find_buffer_by_handle(buffer, err);
@@ -954,7 +954,7 @@ ArrayOf(Dictionary) nvim_buf_get_keymap(uint64_t channel_id, Buffer buffer, Stri
     return (Array)ARRAY_DICT_INIT;
   }
 
-  return keymap_array(mode, buf, channel_id == LUA_INTERNAL_CALL);
+  return keymap_array(mode, buf);
 }
 
 /// Sets a buffer-local |mapping| for the given mode.

src/nvim/api/vim.c

@@ -1421,10 +1421,10 @@ Dictionary nvim_get_mode(void)
 /// @param  mode       Mode short-name ("n", "i", "v", ...)
 /// @returns Array of |maparg()|-like dictionaries describing mappings.
 ///          The "buffer" key is always zero.
-ArrayOf(Dictionary) nvim_get_keymap(uint64_t channel_id, String mode)
+ArrayOf(Dictionary) nvim_get_keymap(String mode)
   FUNC_API_SINCE(3)
 {
-  return keymap_array(mode, NULL, channel_id == LUA_INTERNAL_CALL);
+  return keymap_array(mode, NULL);
 }
 
 /// Sets a global |mapping| for the given mode.

src/nvim/mapping.c

@@ -1989,16 +1989,18 @@ void f_hasmapto(typval_T *argvars, typval_T *rettv, EvalFuncData fptr)
   }
 }
 
-/// Fill a dictionary with all applicable maparg() like dictionaries
+/// Fill a Dictionary with all applicable maparg() like dictionaries
 ///
-/// @param dict          The dictionary to be filled
 /// @param mp            The maphash that contains the mapping information
 /// @param buffer_value  The "buffer" value
 /// @param compatible    True for compatible with old maparg() dict
-static void mapblock_fill_dict(dict_T *const dict, const mapblock_T *const mp,
+///
-                               const char *lhsrawalt, long buffer_value, bool compatible)
+/// @return  A Dictionary.
-  FUNC_ATTR_NONNULL_ARG(1, 2)
+static Dictionary mapblock_fill_dict(const mapblock_T *const mp, const char *lhsrawalt,
+                                     const long buffer_value, const bool compatible)
+  FUNC_ATTR_NONNULL_ARG(1)
 {
+  Dictionary dict = ARRAY_DICT_INIT;
   char *const lhs = str2special_save((const char *)mp->m_keys,
                                      compatible, !compatible);
   char *const mapmode = map_mode_to_chars(mp->m_mode);
@@ -2015,37 +2017,35 @@ static void mapblock_fill_dict(dict_T *const dict, const mapblock_T *const mp,
   }
 
   if (mp->m_luaref != LUA_NOREF) {
-    tv_dict_add_nr(dict, S_LEN("callback"), mp->m_luaref);
+    PUT(dict, "callback", LUAREF_OBJ(api_new_luaref(mp->m_luaref)));
   } else {
-    if (compatible) {
+    PUT(dict, "rhs", STRING_OBJ(compatible
-      tv_dict_add_str(dict, S_LEN("rhs"), (const char *)mp->m_orig_str);
+                                ? cstr_to_string(mp->m_orig_str)
-    } else {
+                                : cstr_as_string(str2special_save(mp->m_str, false, true))));
-      tv_dict_add_allocated_str(dict, S_LEN("rhs"),
-                                str2special_save((const char *)mp->m_str, false,
-                                                 true));
-    }
   }
   if (mp->m_desc != NULL) {
-    tv_dict_add_allocated_str(dict, S_LEN("desc"), xstrdup(mp->m_desc));
+    PUT(dict, "desc", STRING_OBJ(cstr_to_string(mp->m_desc)));
   }
-  tv_dict_add_allocated_str(dict, S_LEN("lhs"), lhs);
+  PUT(dict, "lhs", STRING_OBJ(cstr_as_string(lhs)));
-  tv_dict_add_str(dict, S_LEN("lhsraw"), (const char *)mp->m_keys);
+  PUT(dict, "lhsraw", STRING_OBJ(cstr_to_string((const char *)mp->m_keys)));
   if (lhsrawalt != NULL) {
     // Also add the value for the simplified entry.
-    tv_dict_add_str(dict, S_LEN("lhsrawalt"), lhsrawalt);
+    PUT(dict, "lhsrawalt", STRING_OBJ(cstr_to_string(lhsrawalt)));
   }
-  tv_dict_add_nr(dict, S_LEN("noremap"), noremap_value);
+  PUT(dict, "noremap", INTEGER_OBJ(noremap_value));
-  tv_dict_add_nr(dict, S_LEN("script"), mp->m_noremap == REMAP_SCRIPT ? 1 : 0);
+  PUT(dict, "script", INTEGER_OBJ(mp->m_noremap == REMAP_SCRIPT ? 1 : 0));
-  tv_dict_add_nr(dict, S_LEN("expr"),  mp->m_expr ? 1 : 0);
+  PUT(dict, "expr", INTEGER_OBJ(mp->m_expr ? 1 : 0));
-  tv_dict_add_nr(dict, S_LEN("silent"), mp->m_silent ? 1 : 0);
+  PUT(dict, "silent", INTEGER_OBJ(mp->m_silent ? 1 : 0));
-  tv_dict_add_nr(dict, S_LEN("sid"), (varnumber_T)mp->m_script_ctx.sc_sid);
+  PUT(dict, "sid", INTEGER_OBJ((varnumber_T)mp->m_script_ctx.sc_sid));
-  tv_dict_add_nr(dict, S_LEN("lnum"), (varnumber_T)mp->m_script_ctx.sc_lnum);
+  PUT(dict, "lnum", INTEGER_OBJ((varnumber_T)mp->m_script_ctx.sc_lnum));
-  tv_dict_add_nr(dict, S_LEN("buffer"), (varnumber_T)buffer_value);
+  PUT(dict, "buffer", INTEGER_OBJ((varnumber_T)buffer_value));
-  tv_dict_add_nr(dict, S_LEN("nowait"), mp->m_nowait ? 1 : 0);
+  PUT(dict, "nowait", INTEGER_OBJ(mp->m_nowait ? 1 : 0));
   if (mp->m_replace_keycodes) {
-    tv_dict_add_nr(dict, S_LEN("replace_keycodes"), 1);
+    PUT(dict, "replace_keycodes", INTEGER_OBJ(1));
   }
-  tv_dict_add_allocated_str(dict, S_LEN("mode"), mapmode);
+  PUT(dict, "mode", STRING_OBJ(cstr_as_string(mapmode)));
+
+  return dict;
 }
 
 static void get_maparg(typval_T *argvars, typval_T *rettv, int exact)
@@ -2114,11 +2114,14 @@ static void get_maparg(typval_T *argvars, typval_T *rettv, int exact)
       rettv->vval.v_string = nlua_funcref_str(mp->m_luaref);
     }
   } else {
+    // Return a dictionary.
     tv_dict_alloc_ret(rettv);
     if (mp != NULL && (rhs != NULL || rhs_lua != LUA_NOREF)) {
-      // Return a dictionary.
+      Dictionary dict = mapblock_fill_dict(mp,
-      mapblock_fill_dict(rettv->vval.v_dict, mp, did_simplify ? (char *)keys_simplified : NULL,
+                                           did_simplify ? (char *)keys_simplified : NULL,
-                         buffer_local, true);
+                                           buffer_local, true);
+      (void)object_to_vim(DICTIONARY_OBJ(dict), rettv, NULL);
+      api_free_dictionary(dict);
     }
   }
 
@@ -2599,12 +2602,10 @@ fail_and_free:
 ///
 /// @param  mode  The abbreviation for the mode
 /// @param  buf  The buffer to get the mapping array. NULL for global
-/// @param  from_lua  Whether it is called from internal Lua api.
 /// @returns Array of maparg()-like dictionaries describing mappings
-ArrayOf(Dictionary) keymap_array(String mode, buf_T *buf, bool from_lua)
+ArrayOf(Dictionary) keymap_array(String mode, buf_T *buf)
 {
   Array mappings = ARRAY_DICT_INIT;
-  dict_T *const dict = tv_dict_alloc();
 
   // Convert the string mode to the integer mode
   // that is stored within each mapblock
@@ -2623,25 +2624,11 @@ ArrayOf(Dictionary) keymap_array(String mode, buf_T *buf, bool from_lua)
       }
       // Check for correct mode
       if (int_mode & current_maphash->m_mode) {
-        mapblock_fill_dict(dict, current_maphash, NULL, buffer_value, false);
+        ADD(mappings,
-        Object api_dict = vim_to_object((typval_T[]) { { .v_type = VAR_DICT,
+            DICTIONARY_OBJ(mapblock_fill_dict(current_maphash, NULL, buffer_value, false)));
-                                                         .vval.v_dict = dict } });
-        if (from_lua) {
-          Dictionary d = api_dict.data.dictionary;
-          for (size_t j = 0; j < d.size; j++) {
-            if (strequal("callback", d.items[j].key.data)) {
-              d.items[j].value.type = kObjectTypeLuaRef;
-              d.items[j].value.data.luaref = api_new_luaref((LuaRef)d.items[j].value.data.integer);
-              break;
-            }
-          }
-        }
-        ADD(mappings, api_dict);
-        tv_dict_clear(dict);
       }
     }
   }
-  tv_dict_free(dict);
 
   return mappings;
 }

test/functional/api/keymap_spec.lua

@@ -6,6 +6,7 @@ local command = helpers.command
 local curbufmeths = helpers.curbufmeths
 local eq, neq = helpers.eq, helpers.neq
 local exec_lua = helpers.exec_lua
+local exec = helpers.exec
 local feed = helpers.feed
 local funcs = helpers.funcs
 local meths = helpers.meths
@@ -336,21 +337,26 @@ describe('nvim_get_keymap', function()
   end)
 
   it('can handle lua mappings', function()
-    eq(0, exec_lua [[
+    eq(0, exec_lua([[
       GlobalCount = 0
-      vim.api.nvim_set_keymap ('n', 'asdf', '', {callback = function() GlobalCount = GlobalCount + 1 end })
+      vim.api.nvim_set_keymap('n', 'asdf', '', {callback = function() GlobalCount = GlobalCount + 1 end })
       return GlobalCount
-    ]])
+    ]]))
 
     feed('asdf\n')
-    eq(1, exec_lua[[return GlobalCount]])
+    eq(1, exec_lua([[return GlobalCount]]))
 
-    eq(2, exec_lua[[
+    eq(2, exec_lua([[
       vim.api.nvim_get_keymap('n')[1].callback()
       return GlobalCount
+    ]]))
+
+    exec([[
+      call nvim_get_keymap('n')[0].callback()
     ]])
+    eq(3, exec_lua([[return GlobalCount]]))
+
     local mapargs = meths.get_keymap('n')
-    assert(type(mapargs[1].callback) == 'number', 'callback is not luaref number')
     mapargs[1].callback = nil
     eq({
       lhs='asdf',
@@ -834,17 +840,29 @@ describe('nvim_set_keymap, nvim_del_keymap', function()
   end)
 
   it ('maparg() returns lua mapping correctly', function()
-    exec_lua [[
+    eq(0, exec_lua([[
-      vim.api.nvim_set_keymap ('n', 'asdf', '', {callback = function() print('jkl;') end })
+      GlobalCount = 0
-    ]]
+      vim.api.nvim_set_keymap('n', 'asdf', '', {callback = function() GlobalCount = GlobalCount + 1 end })
-    assert.truthy(string.match(funcs.maparg('asdf', 'n'),
+      return GlobalCount
-                  "^<Lua %d+>"))
+    ]]))
+
+    assert.truthy(string.match(funcs.maparg('asdf', 'n'), "^<Lua %d+>"))
+
     local mapargs = funcs.maparg('asdf', 'n', false, true)
-    assert(type(mapargs.callback) == 'number', 'callback is not luaref number')
     mapargs.callback = nil
     mapargs.lhsraw = nil
     mapargs.lhsrawalt = nil
     eq(generate_mapargs('n', 'asdf', nil, {sid=sid_lua}), mapargs)
+
+    eq(1, exec_lua([[
+      vim.fn.maparg('asdf', 'n', false, true).callback()
+      return GlobalCount
+    ]]))
+
+    exec([[
+      call maparg('asdf', 'n', v:false, v:true).callback()
+    ]])
+    eq(2, exec_lua([[return GlobalCount]]))
   end)
 
   it('can make lua expr mappings replacing keycodes', function()