feat(api): autocmd group can be either name or id (#17559)

* feat(api): `group` can be either string or int

This affects the following API functions:
- `vim.api.nvim_create_autocmd`
- `vim.api.nvim_get_autocmds`
- `vim.api.nvim_do_autocmd`

closes #17552

* refactor: add two maps for fast lookups

* fix: delete augroup info from id->name map

When in "stupid_legacy_mode", the value in name->id map would be updated
to `AUGROUP_DELETED`, but the entry would still remain in id->name. This
would create a problem in `augroup_name` function which would return the
name of the augroup instead of `--DELETED--`.

The id->name map is only used for fast loopup in `augroup_name` function
so there's no point in keeping the entry of deleted augroup in it.

Co-authored-by: TJ DeVries <devries.timothyj@gmail.com>

runtime/doc/api.txt

@@ -3165,7 +3165,7 @@ nvim_create_autocmd({event}, {*opts})                  *nvim_create_autocmd()*
                               • create a |autocmd-buflocal| autocmd.
                               • NOTE: Cannot be used with {pattern}
 
-                            • group: (string) The augroup name
+                            • group: (string|int) The augroup name or id
                             • once: (boolean) - See |autocmd-once|
                             • nested: (boolean) - See |autocmd-nested|
                             • desc: (string) - Description of the autocmd
@@ -3213,7 +3213,7 @@ nvim_do_autocmd({event}, {*opts})                          *nvim_do_autocmd()*
                                "*".
                                • NOTE: Cannot be used with {buffer}
 
-                             • group (string) - autocmd group name
+                             • group (string|int) - autocmd group name or id
                              • modeline (boolean) - Default true, see
                                |<nomodeline>|
 
@@ -3224,7 +3224,8 @@ nvim_get_autocmds({*opts})                               *nvim_get_autocmds()*
                     {opts}  Optional Parameters:
                             • event : Name or list of name of events to match
                               against
-                            • group (string): Name of group to match against
+                            • group (string|int): Name or id of group to match
+                              against
                             • pattern: Pattern or list of patterns to match
                               against. Cannot be used with {buffer}
                             • buffer: Buffer number or list of buffer numbers

src/nvim/api/autocmd.c

@@ -40,7 +40,7 @@ static int64_t next_autocmd_id = 1;
 ///
 /// @param opts Optional Parameters:
 ///     - event : Name or list of name of events to match against
-///     - group (string): Name of group to match against
+///     - group (string|int): Name or id of group to match against
 ///     - pattern: Pattern or list of patterns to match against. Cannot be used with {buffer}
 ///     - buffer: Buffer number or list of buffer numbers for buffer local autocommands
 ///               |autocmd-buflocal|. Cannot be used with {pattern}
@@ -62,19 +62,27 @@ Array nvim_get_autocmds(Dict(get_autocmds) *opts, Error *err)
 
   int group = 0;
 
-  if (opts->group.type != kObjectTypeNil) {
-    Object v = opts->group;
-    if (v.type != kObjectTypeString) {
-      api_set_error(err, kErrorTypeValidation, "group must be a string.");
+  switch (opts->group.type) {
+    case kObjectTypeNil:
+      break;
+    case kObjectTypeString:
+      group = augroup_find(opts->group.data.string.data);
+      if (group < 0) {
+        api_set_error(err, kErrorTypeValidation, "invalid augroup passed.");
+        goto cleanup;
+      }
+      break;
+    case kObjectTypeInteger:
+      group = (int)opts->group.data.integer;
+      char *name = augroup_name(group);
+      if (!augroup_exists(name)) {
+        api_set_error(err, kErrorTypeValidation, "invalid augroup passed.");
+        goto cleanup;
+      }
+      break;
+    default:
+      api_set_error(err, kErrorTypeValidation, "group must be a string or an integer.");
       goto cleanup;
-    }
-
-    group = augroup_find(v.data.string.data);
-
-    if (group < 0) {
-      api_set_error(err, kErrorTypeValidation, "invalid augroup passed.");
-      goto cleanup;
-    }
   }
 
   if (opts->event.type != kObjectTypeNil) {
@@ -321,7 +329,7 @@ cleanup:
 ///         - buffer: (bufnr)
 ///             - create a |autocmd-buflocal| autocmd.
 ///             - NOTE: Cannot be used with {pattern}
-///         - group: (string) The augroup name
+///         - group: (string|int) The augroup name or id
 ///         - once: (boolean) - See |autocmd-once|
 ///         - nested: (boolean) - See |autocmd-nested|
 ///         - desc: (string) - Description of the autocmd
@@ -406,23 +414,29 @@ Integer nvim_create_autocmd(uint64_t channel_id, Object event, Dict(create_autoc
   bool is_once = api_object_to_bool(opts->once, "once", false, err);
   bool is_nested = api_object_to_bool(opts->nested, "nested", false, err);
 
-  // TODO(tjdevries): accept number for namespace instead
-  if (opts->group.type != kObjectTypeNil) {
-    Object *v = &opts->group;
-    if (v->type != kObjectTypeString) {
-      api_set_error(err, kErrorTypeValidation, "'group' must be a string");
+  switch (opts->group.type) {
+    case kObjectTypeNil:
+      break;
+    case kObjectTypeString:
+      au_group = augroup_find(opts->group.data.string.data);
+      if (au_group == AUGROUP_ERROR) {
+        api_set_error(err,
+                      kErrorTypeValidation,
+                      "invalid augroup: %s", opts->group.data.string.data);
+        goto cleanup;
+      }
+      break;
+    case kObjectTypeInteger:
+      au_group = (int)opts->group.data.integer;
+      char *name = augroup_name(au_group);
+      if (!augroup_exists(name)) {
+        api_set_error(err, kErrorTypeValidation, "invalid augroup: %d", au_group);
+        goto cleanup;
+      }
+      break;
+    default:
+      api_set_error(err, kErrorTypeValidation, "'group' must be a string or an integer.");
       goto cleanup;
-    }
-
-    au_group = augroup_find(v->data.string.data);
-
-    if (au_group == AUGROUP_ERROR) {
-      api_set_error(err,
-                    kErrorTypeException,
-                    "invalid augroup: %s", v->data.string.data);
-
-      goto cleanup;
-    }
   }
 
   if (opts->pattern.type != kObjectTypeNil && opts->buffer.type != kObjectTypeNil) {
@@ -624,7 +638,7 @@ void nvim_del_augroup_by_name(String name)
 ///             - NOTE: Cannot be used with {pattern}
 ///         - pattern (string|table) - optional, defaults to "*".
 ///             - NOTE: Cannot be used with {buffer}
-///         - group (string) - autocmd group name
+///         - group (string|int) - autocmd group name or id
 ///         - modeline (boolean) - Default true, see |<nomodeline>|
 void nvim_do_autocmd(Object event, Dict(do_autocmd) *opts, Error *err)
   FUNC_API_SINCE(9)
@@ -644,21 +658,29 @@ void nvim_do_autocmd(Object event, Dict(do_autocmd) *opts, Error *err)
     goto cleanup;
   }
 
-  if (opts->group.type != kObjectTypeNil) {
-    if (opts->group.type != kObjectTypeString) {
-      api_set_error(err, kErrorTypeValidation, "'group' must be a string");
+  switch (opts->group.type) {
+    case kObjectTypeNil:
+      break;
+    case kObjectTypeString:
+      au_group = augroup_find(opts->group.data.string.data);
+      if (au_group == AUGROUP_ERROR) {
+        api_set_error(err,
+                      kErrorTypeValidation,
+                      "invalid augroup: %s", opts->group.data.string.data);
+        goto cleanup;
+      }
+      break;
+    case kObjectTypeInteger:
+      au_group = (int)opts->group.data.integer;
+      char *name = augroup_name(au_group);
+      if (!augroup_exists(name)) {
+        api_set_error(err, kErrorTypeValidation, "invalid augroup: %d", au_group);
+        goto cleanup;
+      }
+      break;
+    default:
+      api_set_error(err, kErrorTypeValidation, "'group' must be a string or an integer.");
       goto cleanup;
-    }
-
-    au_group = augroup_find(opts->group.data.string.data);
-
-    if (au_group == AUGROUP_ERROR) {
-      api_set_error(err,
-                    kErrorTypeException,
-                    "invalid augroup: %s", opts->group.data.string.data);
-
-      goto cleanup;
-    }
   }
 
   if (opts->buffer.type != kObjectTypeNil) {

src/nvim/autocmd.c

@@ -105,15 +105,24 @@ static char_u *old_termresponse = NULL;
 #define FOR_ALL_AUPATS_IN_EVENT(event, ap) \
   for (AutoPat *ap = first_autopat[event]; ap != NULL; ap = ap->next)  // NOLINT
 
-// Map of autocmd group names.
+// Map of autocmd group names and ids.
 //  name -> ID
-static Map(String, int) augroup_map = MAP_INIT;
+//  ID -> name
+static Map(String, int) map_augroup_name_to_id = MAP_INIT;
+static Map(int, String) map_augroup_id_to_name = MAP_INIT;
 
-static void augroup_map_del(char *name)
+static void augroup_map_del(int id, char *name)
 {
-  String key = map_key(String, int)(&augroup_map, cstr_as_string(name));
-  map_del(String, int)(&augroup_map, key);
-  api_free_string(key);
+  if (name != NULL) {
+    String key = map_key(String, int)(&map_augroup_name_to_id, cstr_as_string(name));
+    map_del(String, int)(&map_augroup_name_to_id, key);
+    api_free_string(key);
+  }
+  if (id > 0) {
+    String mapped = map_get(int, String)(&map_augroup_id_to_name, id);
+    api_free_string(mapped);
+    map_del(int, String)(&map_augroup_id_to_name, id);
+  }
 }
 
 
@@ -382,12 +391,14 @@ int augroup_add(char *name)
   }
 
   if (existing_id == AUGROUP_DELETED) {
-    augroup_map_del(name);
+    augroup_map_del(existing_id, name);
   }
 
   int next_id = next_augroup_id++;
-  String name_copy = cstr_to_string(name);
-  map_put(String, int)(&augroup_map, name_copy, next_id);
+  String name_key = cstr_to_string(name);
+  String name_val = cstr_to_string(name);
+  map_put(String, int)(&map_augroup_name_to_id, name_key, next_id);
+  map_put(int, String)(&map_augroup_id_to_name, next_id, name_val);
 
   return next_id;
 }
@@ -416,7 +427,8 @@ void augroup_del(char *name, bool stupid_legacy_mode)
         FOR_ALL_AUPATS_IN_EVENT(event, ap) {
           if (ap->group == i && ap->pat != NULL) {
             give_warning((char_u *)_("W19: Deleting augroup that is still in use"), true);
-            map_put(String, int)(&augroup_map, cstr_as_string(name), AUGROUP_DELETED);
+            map_put(String, int)(&map_augroup_name_to_id, cstr_as_string(name), AUGROUP_DELETED);
+            augroup_map_del(ap->group, NULL);
             return;
           }
         }
@@ -432,7 +444,7 @@ void augroup_del(char *name, bool stupid_legacy_mode)
     }
 
     // Remove the group because it's not currently in use.
-    augroup_map_del(name);
+    augroup_map_del(i, name);
     au_cleanup();
   }
 }
@@ -445,7 +457,7 @@ void augroup_del(char *name, bool stupid_legacy_mode)
 int augroup_find(const char *name)
   FUNC_ATTR_PURE FUNC_ATTR_WARN_UNUSED_RESULT
 {
-  int existing_id = map_get(String, int)(&augroup_map, cstr_as_string((char *)name));
+  int existing_id = map_get(String, int)(&map_augroup_name_to_id, cstr_as_string((char *)name));
   if (existing_id == AUGROUP_DELETED) {
     return existing_id;
   }
@@ -487,13 +499,10 @@ char *augroup_name(int group)
     return NULL;
   }
 
-  String key;
-  int value;
-  map_foreach(&augroup_map, key, value, {
-    if (value == group) {
-      return key.data;
-    }
-  });
+  String key = map_get(int, String)(&map_augroup_id_to_name, group);
+  if (key.data != NULL) {
+    return key.data;
+  }
 
   // If it's not in the map anymore, then it must have been deleted.
   return (char *)get_deleted_augroup();
@@ -526,7 +535,7 @@ void do_augroup(char_u *arg, int del_group)
 
     String name;
     int value;
-    map_foreach(&augroup_map, name, value, {
+    map_foreach(&map_augroup_name_to_id, name, value, {
       if (value > 0) {
         msg_puts(name.data);
       } else {
@@ -556,11 +565,17 @@ void free_all_autocmds(void)
   // Delete the augroup_map, including free the data
   String name;
   int id;
-  map_foreach(&augroup_map, name, id, {
+  map_foreach(&map_augroup_name_to_id, name, id, {
     (void)id;
     api_free_string(name);
   })
-  map_destroy(String, int)(&augroup_map);
+  map_destroy(String, int)(&map_augroup_name_to_id);
+
+  map_foreach(&map_augroup_id_to_name, id, name, {
+    (void)id;
+    api_free_string(name);
+  })
+  map_destroy(int, String)(&map_augroup_id_to_name);
 }
 #endif
 

src/nvim/map.c

@@ -178,6 +178,7 @@ MAP_IMPL(String, MsgpackRpcRequestHandler, MSGPACK_HANDLER_INITIALIZER)
 MAP_IMPL(HlEntry, int, DEFAULT_INITIALIZER)
 MAP_IMPL(String, handle_T, 0)
 MAP_IMPL(String, int, DEFAULT_INITIALIZER)
+MAP_IMPL(int, String, DEFAULT_INITIALIZER)
 
 MAP_IMPL(ColorKey, ColorItem, COLOR_ITEM_INITIALIZER)
 

src/nvim/map.h

@@ -47,6 +47,7 @@ MAP_DECLS(String, MsgpackRpcRequestHandler)
 MAP_DECLS(HlEntry, int)
 MAP_DECLS(String, handle_T)
 MAP_DECLS(String, int)
+MAP_DECLS(int, String)
 
 MAP_DECLS(ColorKey, ColorItem)
 

test/functional/api/autocmd_spec.lua

@@ -281,6 +281,31 @@ describe('autocmd api', function()
 
         eq("Too many buffers. Please limit yourself to 256 or fewer", pcall_err(meths.get_autocmds, { event = "InsertEnter", buffer = bufs }))
       end)
+
+      it('should return autocmds when group is specified by id', function()
+        local auid = meths.create_augroup("nvim_test_augroup", { clear = true })
+        meths.create_autocmd("FileType", { group = auid, command = 'echo "1"' })
+        meths.create_autocmd("FileType", { group = auid, command = 'echo "2"' })
+
+        local aus = meths.get_autocmds { group = auid }
+        eq(2, #aus)
+
+        local aus2 = meths.get_autocmds { group = auid, event = "InsertEnter" }
+        eq(0, #aus2)
+      end)
+
+      it('should return autocmds when group is specified by name', function()
+        local auname = "nvim_test_augroup"
+        meths.create_augroup(auname, { clear = true })
+        meths.create_autocmd("FileType", { group = auname, command = 'echo "1"' })
+        meths.create_autocmd("FileType", { group = auname, command = 'echo "2"' })
+
+        local aus = meths.get_autocmds { group = auname }
+        eq(2, #aus)
+
+        local aus2 = meths.get_autocmds { group = auname, event = "InsertEnter" }
+        eq(0, #aus2)
+      end)
     end)
 
     describe('groups', function()
@@ -331,7 +356,7 @@ describe('autocmd api', function()
     end)
 
     describe('groups: 2', function()
-      it('raises error for undefined augroup', function()
+      it('raises error for undefined augroup name', function()
         local success, code = unpack(meths.exec_lua([[
           return {pcall(function()
             vim.api.nvim_create_autocmd("FileType", {
@@ -345,6 +370,39 @@ describe('autocmd api', function()
         eq(false, success)
         matches('invalid augroup: NotDefined', code)
       end)
+
+      it('raises error for undefined augroup id', function()
+        local success, code = unpack(meths.exec_lua([[
+          return {pcall(function()
+            -- Make sure the augroup is deleted
+            vim.api.nvim_del_augroup_by_id(1)
+
+            vim.api.nvim_create_autocmd("FileType", {
+              pattern = "*",
+              group = 1,
+              command = "echo 'hello'",
+            })
+          end)}
+        ]], {}))
+
+        eq(false, success)
+        matches('invalid augroup: 1', code)
+      end)
+
+      it('raises error for invalid group type', function()
+        local success, code = unpack(meths.exec_lua([[
+          return {pcall(function()
+            vim.api.nvim_create_autocmd("FileType", {
+              pattern = "*",
+              group = true,
+              command = "echo 'hello'",
+            })
+          end)}
+        ]], {}))
+
+        eq(false, success)
+        matches("'group' must be a string or an integer", code)
+      end)
     end)
 
     describe('patterns', function()
@@ -497,6 +555,35 @@ describe('autocmd api', function()
       meths.do_autocmd("User", { pattern = "TestCommand" })
       eq('matched', meths.get_var('matched'))
     end)
+
+    it('can pass group by id', function()
+      meths.set_var("group_executed", false)
+
+      local auid = meths.create_augroup("nvim_test_augroup", { clear = true })
+      meths.create_autocmd("FileType", {
+        group = auid,
+        command = 'let g:group_executed = v:true',
+      })
+
+      eq(false, meths.get_var("group_executed"))
+      meths.do_autocmd("FileType", { group = auid })
+      eq(true, meths.get_var("group_executed"))
+    end)
+
+    it('can pass group by name', function()
+      meths.set_var("group_executed", false)
+
+      local auname = "nvim_test_augroup"
+      meths.create_augroup(auname, { clear = true })
+      meths.create_autocmd("FileType", {
+        group = auname,
+        command = 'let g:group_executed = v:true',
+      })
+
+      eq(false, meths.get_var("group_executed"))
+      meths.do_autocmd("FileType", { group = auname })
+      eq(true, meths.get_var("group_executed"))
+    end)
   end)
 
   describe('nvim_create_augroup', function()