fix: warn the user if unregistering a keymap fails

Fixes zbirenbaum#533

Author: AntoineGS

lua/copilot/panel/init.lua

@@ -243,7 +243,7 @@ function panel:close()
   end
 end
 
-local function set_keymap(bufnr)
+function M.set_keymap(bufnr)
   if panel.keymap.accept then
     vim.keymap.set("n", panel.keymap.accept, M.accept, {
       buffer = bufnr,
@@ -275,6 +275,8 @@ local function set_keymap(bufnr)
       silent = true,
     })
   end
+
+  M.keymaps_set = true
 end
 
 function panel:ensure_bufnr()
@@ -293,7 +295,7 @@ function panel:ensure_bufnr()
       vim.api.nvim_set_option_value(name, value, { buf = self.bufnr })
     end
 
-    set_keymap(self.bufnr)
+    M.set_keymap(self.bufnr)
   end
 
   vim.api.nvim_buf_set_name(self.bufnr, self.panel_uri)
@@ -581,12 +583,16 @@ function M.teardown()
     return
   end
 
-  if panel.keymap.open then
-    vim.keymap.del("i", panel.keymap.open)
+  util.unset_keymap_if_exists("i", panel.keymap.open)
+
+  if M.keymaps_set then
+    M.unset_keymap_if_exists("n", panel.keymap.accept)
+    M.unset_keymap_if_exists("n", panel.keymap.jump_prev)
+    M.unset_keymap_if_exists("n", panel.keymap.jump_next)
+    M.unset_keymap_if_exists("n", panel.keymap.refresh)
   end
 
   panel:close()
-
   panel.setup_done = false
 end
 

lua/copilot/suggestion/init.lua

@@ -179,29 +179,12 @@ local function set_keymap(keymap)
 end
 
 local function unset_keymap(keymap)
-  if keymap.accept then
-    vim.keymap.del("i", keymap.accept)
-  end
-
-  if keymap.accept_word then
-    vim.keymap.del("i", keymap.accept_word)
-  end
-
-  if keymap.accept_line then
-    vim.keymap.del("i", keymap.accept_line)
-  end
-
-  if keymap.next then
-    vim.keymap.del("i", keymap.next)
-  end
-
-  if keymap.prev then
-    vim.keymap.del("i", keymap.prev)
-  end
-
-  if keymap.dismiss then
-    vim.keymap.del("i", keymap.dismiss)
-  end
+  util.unset_keymap_if_exists("i", keymap.accept)
+  util.unset_keymap_if_exists("i", keymap.accept_word)
+  util.unset_keymap_if_exists("i", keymap.accept_line)
+  util.unset_keymap_if_exists("i", keymap.next)
+  util.unset_keymap_if_exists("i", keymap.prev)
+  util.unset_keymap_if_exists("i", keymap.dismiss)
 end
 
 local function stop_timer()

lua/copilot/util.lua

@@ -113,4 +113,35 @@ function M.strutf16len(str)
   end
 end
 
+---@param mode string
+---@param key string|false
+function M.unset_keymap_if_exists(mode, key)
+  if not key then
+    return
+  end
+
+  local ok, err = pcall(vim.api.nvim_del_keymap, mode, key)
+
+  if not ok then
+    local suggestion_keymaps = config.suggestion.keymap or {}
+    local panel_keymaps = config.panel.keymap or {}
+    local found = false
+
+    for _, tbl in ipairs({ suggestion_keymaps, panel_keymaps }) do
+      for _, v in pairs(tbl) do
+        if v == key then
+          if found then
+            logger.error("Keymap " .. key .. " is used for two different actions, please review your configuration.")
+            return
+          else
+            found = true
+          end
+        end
+      end
+    end
+
+    logger.error("Could not unset keymap for " .. mode .. " " .. key .. ": " .. err)
+  end
+end
+
 return M

tests/screenshots/tests-test_suggestion.lua---suggestion()---duplicated-keymap-yields-correct-error-message

@@ -0,0 +1,51 @@
+--|---------|---------|---------|---------|---------|---------|---------|---------|
+01|1, 2, 3,                                                                        
+02|4, 5, 6,                                                                        
+03|7, 8, 9,                                                                        
+04|~                                                                               
+05|~                                                                               
+06|~                                                                               
+07|~                                                                               
+08|~                                                                               
+09|~                                                                               
+10|~                                                                               
+11|~                                                                               
+12|~                                                                               
+13|~                                                                               
+14|~                                                                               
+15|~                                                                               
+16|~                                                                               
+17|~                                                                               
+18|~                                                                               
+19|~                                                                               
+20|~                                                                               
+21|~                                                                               
+22|~                                                                               
+23|[No Name] [+]                                                 3,10-17        All
+24|-- INSERT --                                                                    
+
+--|---------|---------|---------|---------|---------|---------|---------|---------|
+01|00000000000000000000000000000000000000000000000000000000000000000000000000000000
+02|00000000000000000000000000000000000000000000000000000000000000000000000000000000
+03|00000000000000000000000000000000000000000000000000000000000000000000000000000000
+04|11111111111111111111111111111111111111111111111111111111111111111111111111111111
+05|11111111111111111111111111111111111111111111111111111111111111111111111111111111
+06|11111111111111111111111111111111111111111111111111111111111111111111111111111111
+07|11111111111111111111111111111111111111111111111111111111111111111111111111111111
+08|11111111111111111111111111111111111111111111111111111111111111111111111111111111
+09|11111111111111111111111111111111111111111111111111111111111111111111111111111111
+10|11111111111111111111111111111111111111111111111111111111111111111111111111111111
+11|11111111111111111111111111111111111111111111111111111111111111111111111111111111
+12|11111111111111111111111111111111111111111111111111111111111111111111111111111111
+13|11111111111111111111111111111111111111111111111111111111111111111111111111111111
+14|11111111111111111111111111111111111111111111111111111111111111111111111111111111
+15|11111111111111111111111111111111111111111111111111111111111111111111111111111111
+16|11111111111111111111111111111111111111111111111111111111111111111111111111111111
+17|11111111111111111111111111111111111111111111111111111111111111111111111111111111
+18|11111111111111111111111111111111111111111111111111111111111111111111111111111111
+19|11111111111111111111111111111111111111111111111111111111111111111111111111111111
+20|11111111111111111111111111111111111111111111111111111111111111111111111111111111
+21|11111111111111111111111111111111111111111111111111111111111111111111111111111111
+22|11111111111111111111111111111111111111111111111111111111111111111111111111111111
+23|22222222222222222222222222222222222222222222222222222222222222222222222222222222
+24|33333333333344444444444444444444444444444444444444444444444444444444444444444444

tests/test_suggestion.lua

@@ -167,4 +167,18 @@ T["suggestion()"]["accept_line, 1 line, then accept"] = function()
   reference_screenshot(child.get_screenshot(), nil, { ignore_text = { 49, 50 }, ignore_attr = { 49, 50 } })
 end
 
+T["suggestion()"]["duplicated keymap yields correct error message"] = function()
+  child.config.suggestion = child.config.suggestion .. "auto_trigger = true," .. "keymap = { accept = '<M-CR>' },"
+  child.configure_copilot()
+  child.type_keys("i1, 2, 3,", "<Esc>", "o4, 5, 6,", "<Esc>", "o7, ")
+  child.wait_for_suggestion()
+  child.type_keys("<M-CR>", "<Tab>")
+  child.cmd_capture("Copilot disable")
+  local mess = child.cmd_capture("messages")
+  assert(mess:match("E31") == nil, "Error E31 should have been handled")
+  assert(mess:match("please review your configuration") ~= nil, "Should have logged a message about keymap conflict")
+
+  reference_screenshot(child.get_screenshot(), nil, { ignore_text = { 49, 50 }, ignore_attr = { 49, 50 } })
+end
+
 return T