From b6b190291d26a5e6e4ce1e07259ec48341b81a4e Mon Sep 17 00:00:00 2001 From: bfredl Date: Mon, 7 Sep 2026 14:43:11 +0200 Subject: [PATCH] fix(api): do not pull a fast one for nvim_del_augroup_by_id This is a bit more boilerplate because of painful impedance mismatch (is augroup names the root source of truth? is ids the source of truth? who knows? what does truth even mean?), but show an error reflecting what the caller actually tried (they passed in an invalid id, not a null string) --- scripts/linterrcodes.lua | 1 + src/nvim/api/autocmd.c | 5 ++--- src/nvim/autocmd.c | 24 +++++++++++++++++++++--- test/functional/api/autocmd_spec.lua | 13 ++++++++++--- 4 files changed, 34 insertions(+), 9 deletions(-) diff --git a/scripts/linterrcodes.lua b/scripts/linterrcodes.lua index 8e1eb33c0b..e7b8f33b7c 100644 --- a/scripts/linterrcodes.lua +++ b/scripts/linterrcodes.lua @@ -32,6 +32,7 @@ local dup_allowed = { E312 = 2, E317 = 4, E319 = 2, + E367 = 3, E423 = 3, E474 = 60, E475 = 6, diff --git a/src/nvim/api/autocmd.c b/src/nvim/api/autocmd.c index f046ce72bd..f3b5aa780a 100644 --- a/src/nvim/api/autocmd.c +++ b/src/nvim/api/autocmd.c @@ -663,8 +663,7 @@ void nvim_del_augroup_by_id(Integer id, Error *err) FUNC_API_SINCE(9) { TRY_WRAP(err, { - char *name = id == 0 ? NULL : augroup_name((int)id); - augroup_del(name, false); + augroup_del_by_id((int)id); }); } @@ -678,7 +677,7 @@ void nvim_del_augroup_by_name(String name, Error *err) FUNC_API_SINCE(9) { TRY_WRAP(err, { - augroup_del(name.data, false); + augroup_del(name.data, -1, false); }); } diff --git a/src/nvim/autocmd.c b/src/nvim/autocmd.c index 65d1543f96..191dd3aa90 100644 --- a/src/nvim/autocmd.c +++ b/src/nvim/autocmd.c @@ -421,6 +421,17 @@ int augroup_add(const char *name) return next_id; } +void augroup_del_by_id(int id) +{ + char *name = id <= 0 ? NULL : augroup_name(id); + if (name == NULL) { + semsg(_("E367: No such group id: %d"), id); + } else { + // NB: this can still error, but the error might be "semantic" like "-- Deleted --" + augroup_del(name, id, false); + } +} + /// Delete the augroup that matches name. /// @param stupid_legacy_mode bool: This parameter determines whether to run the augroup /// deletion in the same fashion as `:augroup! {name}` where if there are any remaining @@ -432,11 +443,18 @@ int augroup_add(const char *name) /// I did not consider this good behavior, so now when NOT in stupid_legacy_mode, we actually /// delete these groups and their commands, like you would expect (and don't leave hanging /// `--- DELETED ---` groups around) -void augroup_del(char *name, bool stupid_legacy_mode) +/// @param id_for_error only to provide context for an error message: +/// if >= 0, we tried to look up the group by id +/// if < 0, only use `name` for error +void augroup_del(char *name, int id_for_error, bool stupid_legacy_mode) { int group = augroup_find(name); if (group == AUGROUP_ERROR) { // the group doesn't exist - semsg(_("E367: No such group: \"%s\""), name); + if (id_for_error > 0) { + semsg(_("E367: No such group id: %d \"%s\""), id_for_error, name); + } else { + semsg(_("E367: No such group: \"%s\""), name); + } return; } else if (group == current_augroup) { emsg(_("E936: Cannot delete the current group")); @@ -548,7 +566,7 @@ void do_augroup(char *arg, bool del_group) if (*arg == NUL) { emsg(_(e_argreq)); } else { - augroup_del(arg, true); + augroup_del(arg, -1, true); } } else if (STRICMP(arg, "end") == 0) { // ":aug end": back to group 0 current_augroup = AUGROUP_DEFAULT; diff --git a/test/functional/api/autocmd_spec.lua b/test/functional/api/autocmd_spec.lua index 7cc84a1023..a4588dd314 100644 --- a/test/functional/api/autocmd_spec.lua +++ b/test/functional/api/autocmd_spec.lua @@ -1671,13 +1671,13 @@ describe('autocmd api', function() eq('Vim:E367: No such group: "noexist"', pcall_err(api.nvim_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(api.nvim_del_augroup_by_id, -12312)) + eq('Vim:E367: No such group id: -12312', pcall_err(api.nvim_del_augroup_by_id, -12312)) eq(false, exec_lua [[return pcall(vim.api.nvim_del_augroup_by_id, 0)]]) - eq('Vim:E367: No such group: "[NULL]"', pcall_err(api.nvim_del_augroup_by_id, 0)) + eq('Vim:E367: No such group id: 0', pcall_err(api.nvim_del_augroup_by_id, 0)) eq(false, exec_lua [[return pcall(vim.api.nvim_del_augroup_by_id, 12342)]]) - eq('Vim:E367: No such group: "[NULL]"', pcall_err(api.nvim_del_augroup_by_id, 12312)) + eq('Vim:E367: No such group id: 12312', pcall_err(api.nvim_del_augroup_by_id, 12312)) end) it('groups work with once', function() @@ -1828,6 +1828,13 @@ describe('autocmd api', function() -- so now this works as expected eq(false, pcall(api.nvim_get_autocmds, { group = 'TEMP_ABCD' })) eq(0, #api.nvim_get_autocmds { event = 'BufReadPost' }) + + -- deleting an already deleted group gives NICE error message: + -- exact value of augroup_id here depends on defaults.lua and bullshit, splice it in! + eq( + 'Vim:E367: No such group id: ' .. augroup_id .. ' "--Deleted--"', + pcall_err(api.nvim_del_augroup_by_id, augroup_id) + ) end) it('api: should clear and not return any autocmds for delete groups by name', function()