refactor: rework the duplicated keymap logic

Author: AntoineGS

lua/copilot/config/init.lua

@@ -80,6 +80,7 @@ function M.validate(config)
   require("copilot.config.root_dir").validate(config.root_dir)
   require("copilot.config.should_attach").validate(config.should_attach)
   require("copilot.config.nes").validate(config.nes)
+  require("copilot.keymaps").validate(config)
 end
 
 return M

lua/copilot/keymaps/init.lua

@@ -1,10 +1,17 @@
 local logger = require("copilot.logger")
-local config = require("copilot.config")
 
 local M = {}
 local previous_keymaps = {}
 
+---@param bufnr integer
+---@param mode string
+---@param key string
 local function get_keymap_key(bufnr, mode, key)
+  if not bufnr or not mode or not key then
+    logger.error("Invalid parameters to get_keymap_key" .. vim.inspect({ bufnr, mode, key }))
+    return "invalid"
+  end
+
   return bufnr .. ":" .. mode .. ":" .. key
 end
 ---@param mode string
@@ -22,6 +29,12 @@ function M.register_keymap(mode, key, action, desc, bufnr)
     return
   end
 
+  local keymap_key = get_keymap_key(bufnr, mode, key)
+  if previous_keymaps[keymap_key] then
+    logger.trace("Keymap already registered for " .. keymap_key)
+    return
+  end
+
   vim.keymap.set(mode, key, function()
     action()
   end, {
@@ -30,7 +43,7 @@ function M.register_keymap(mode, key, action, desc, bufnr)
     buffer = bufnr,
   })
 
-  previous_keymaps[get_keymap_key(bufnr, mode, key)] = { type = "none", value = nil }
+  previous_keymaps[keymap_key] = { type = "none", value = nil }
 end
 
 ---@param mode string
@@ -109,33 +122,48 @@ end
 ---@param key string|false
 ---@param bufnr integer
 function M.unset_keymap_if_exists(mode, key, bufnr)
-  if not key then
+  if not key or not bufnr then
     return
   end
 
   local ok, err = pcall(vim.api.nvim_buf_del_keymap, bufnr, mode, key)
   previous_keymaps[get_keymap_key(bufnr, mode, key)] = nil
 
   if not ok then
-    local suggestion_keymaps = config.suggestion.keymap or {}
-    local nes_keymaps = config.nes.keymap or {}
-    local panel_keymaps = config.panel.keymap or {}
-    local found = false
-
-    for _, tbl in ipairs({ suggestion_keymaps, nes_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
+    logger.error("Could not unset keymap for " .. (mode or "nil") .. " " .. key .. ", bufnr " .. bufnr .. ": " .. err)
+  end
+end
+
+---@param config CopilotConfig
+function M.validate(config)
+  local suggestion_keymaps = config.suggestion.keymap or {}
+  local nes_keymaps = config.nes.keymap or {}
+  local panel_keymaps = config.panel.keymap or {}
+  local seen = {}
+  local duplicates = {}
+
+  for _, cfg in ipairs({ suggestion_keymaps, nes_keymaps, panel_keymaps }) do
+    for action, km in pairs(cfg) do
+      if not km then
+        goto continue
+      end
+
+      -- TODO: find a better way to determine mode, this is prone to maintenance bugs
+      -- TODO: Not sure how to validate keymaps, since some COULD be duplicates and valid
+      local mode = (action == config.panel.keymap.open or vim.tbl_contains(config.nes.keymap, action)) and "n" or "i"
+      local keymap_key = get_keymap_key(0, mode, km)
+      if seen[keymap_key] then
+        duplicates[keymap_key] = (duplicates[keymap_key] or 1) + 1
+      else
+        seen[keymap_key] = true
       end
+
+      ::continue::
     end
+  end
 
-    logger.error("Could not unset keymap for " .. mode .. " " .. key .. ", bufnr " .. bufnr .. ": " .. err)
+  for key, count in pairs(duplicates) do
+    logger.error("Duplicate keymap detected: " .. key .. " (" .. count .. " times), please review your configuration.")
   end
 end
 

lua/copilot/panel/init.lua

@@ -254,38 +254,10 @@ function panel:close()
 end
 
 function M.set_keymap(bufnr)
-  if panel.keymap.accept then
-    vim.keymap.set("n", panel.keymap.accept, M.accept, {
-      buffer = bufnr,
-      desc = "[copilot] (panel) accept",
-      silent = true,
-    })
-  end
-
-  if panel.keymap.jump_prev then
-    vim.keymap.set("n", panel.keymap.jump_prev, M.jump_prev, {
-      buffer = bufnr,
-      desc = "[copilot] (panel) jump prev",
-      silent = true,
-    })
-  end
-
-  if panel.keymap.jump_next then
-    vim.keymap.set("n", panel.keymap.jump_next, M.jump_next, {
-      buffer = bufnr,
-      desc = "[copilot] (panel) jump next",
-      silent = true,
-    })
-  end
-
-  if panel.keymap.refresh then
-    vim.keymap.set("n", panel.keymap.refresh, M.refresh, {
-      buffer = bufnr,
-      desc = "[copilot] (panel) refresh",
-      silent = true,
-    })
-  end
-
+  keymaps.register_keymap("n", panel.keymap.accept, M.accept, "[copilot] (panel) accept", bufnr)
+  keymaps.register_keymap("n", panel.keymap.jump_prev, M.jump_prev, "[copilot] (panel) jump prev", bufnr)
+  keymaps.register_keymap("n", panel.keymap.jump_next, M.jump_next, "[copilot] (panel) jump next", bufnr)
+  keymaps.register_keymap("n", panel.keymap.refresh, M.refresh, "[copilot] (panel) refresh", bufnr)
   M.keymaps_set = true
 end
 
@@ -580,10 +552,10 @@ function M.setup()
   end
 
   panel.auto_refresh = opts.auto_refresh or false
-
   panel.keymap = opts.keymap or {}
   panel.layout = vim.tbl_deep_extend("force", panel.layout, opts.layout or {})
 
+  -- not buffer specific
   if panel.keymap.open then
     vim.keymap.set("i", panel.keymap.open, M.open, {
       desc = "[copilot] (panel) open",