fix(api): improve autocmd error handling

lewis6991 · lewis6991 · commit 929293815bc6 · 2022-03-31T14:23:53.000+01:00
- nvim_del_augroup_* now works with pcall
- nvim_del_autocmd now errors for invalid ids
diff --git a/src/nvim/api/autocmd.c b/src/nvim/api/autocmd.c
@@ -609,10 +609,16 @@ Integer nvim_create_autocmd(uint64_t channel_id, Object event, Dict(create_autoc
 /// NOTE: Only autocommands created via the API have an id.
 /// @param id Integer The id returned by nvim_create_autocmd
 /// @see |nvim_create_autocmd()|
-void nvim_del_autocmd(Integer id)
+void nvim_del_autocmd(Integer id, Error *err)
   FUNC_API_SINCE(9)
 {
-  autocmd_delete_id(id);
+  if (id <= 0) {
+    api_set_error(err, kErrorTypeException, "Invalid autocmd id");
+    return;
+  }
+  if (!autocmd_delete_id(id)) {
+    api_set_error(err, kErrorTypeException, "Failed to delete autocmd");
+  }
 }
 
 /// Create or get an autocommand group |autocmd-groups|.
@@ -664,11 +670,15 @@ Integer nvim_create_augroup(uint64_t channel_id, String name, Dict(create_augrou
 /// @param id Integer The id of the group.
 /// @see |nvim_del_augroup_by_name()|
 /// @see |nvim_create_augroup()|
-void nvim_del_augroup_by_id(Integer id)
+void nvim_del_augroup_by_id(Integer id, Error *err)
   FUNC_API_SINCE(9)
 {
-  char *name = augroup_name((int)id);
-  augroup_del(name, false);
+  TRY_WRAP({
+    try_start();
+    char *name = augroup_name((int)id);
+    augroup_del(name, false);
+    try_end(err);
+  });
 }
 
 /// Delete an autocommand group by name.
@@ -677,10 +687,14 @@ void nvim_del_augroup_by_id(Integer id)
 /// this group will also be deleted and cleared. This group will no longer exist.
 /// @param name String The name of the group.
 /// @see |autocommand-groups|
-void nvim_del_augroup_by_name(String name)
+void nvim_del_augroup_by_name(String name, Error *err)
   FUNC_API_SINCE(9)
 {
-  augroup_del(name.data, false);
+  TRY_WRAP({
+    try_start();
+    augroup_del(name.data, false);
+    try_end(err);
+  });
 }
 
 /// Execute an autocommand |autocmd-execute|.
diff --git a/src/nvim/autocmd.c b/src/nvim/autocmd.c
@@ -2350,17 +2350,20 @@ int autocmd_delete_event(int group, event_T event, char_u *pat)
 /// Deletes an autocmd by ID.
 /// Only autocmds created via the API have IDs associated with them. There
 /// is no way to delete a specific autocmd created via :autocmd
-void autocmd_delete_id(int64_t id)
+bool autocmd_delete_id(int64_t id)
 {
+  assert(id > 0);
   FOR_ALL_AUEVENTS(event) {
     FOR_ALL_AUPATS_IN_EVENT(event, ap) {
       for (AutoCmd *ac = ap->cmds; ac != NULL; ac = ac->next) {
         if (ac->id == id) {
           aucmd_del(ac);
+          return true;
         }
       }
     }
   }
+  return false;
 }
 
 // ===========================================================================
diff --git a/test/functional/api/autocmd_spec.lua b/test/functional/api/autocmd_spec.lua
@@ -809,6 +809,14 @@ describe('autocmd api', function()
       eq(2, get_executed_count(), "No additional counts")
     end)
 
+    it('can delete non-existent groups with pcall', function()
+      eq(false, exec_lua[[return pcall(vim.api.nvim_del_augroup_by_name, 'noexist')]])
+      eq('Vim:E367: No such group: "noexist"', pcall_err(meths.del_augroup_by_name, 'noexist'))
+
+      eq(false, exec_lua[[return pcall(vim.api.nvim_del_augroup_by_id, -12342)]])
+      eq('Vim:E367: No such group: "--Deleted--"', pcall_err(meths.del_augroup_by_id, -12312))
+    end)
+
     it('groups work with once', function()
       local augroup = "TestGroup"
 

Original file line number	Diff line number	Diff line change
`@@ -2350,17 +2350,20 @@ int autocmd_delete_event(int group, event_T event, char_u *pat)`
`2350`	`2350`	`/// Deletes an autocmd by ID.`
`2351`	`2351`	`/// Only autocmds created via the API have IDs associated with them. There`
`2352`	`2352`	`/// is no way to delete a specific autocmd created via :autocmd`
`2353`		`-void autocmd_delete_id(int64_t id)`
	`2353`	`+bool autocmd_delete_id(int64_t id)`
`2354`	`2354`	`{`
	`2355`	`+ assert(id > 0);`
`2355`	`2356`	`FOR_ALL_AUEVENTS(event) {`
`2356`	`2357`	`FOR_ALL_AUPATS_IN_EVENT(event, ap) {`
`2357`	`2358`	`for (AutoCmd *ac = ap->cmds; ac != NULL; ac = ac->next) {`
`2358`	`2359`	`if (ac->id == id) {`
`2359`	`2360`	`aucmd_del(ac);`
	`2361`	`+ return true;`
`2360`	`2362`	`}`
`2361`	`2363`	`}`
`2362`	`2364`	`}`
`2363`	`2365`	`}`
	`2366`	`+ return false;`
`2364`	`2367`	`}`
`2365`	`2368`
`2366`	`2369`	`// ===========================================================================`