From 086db743287ddc9f83c91a0b4a9fc8f1a226c383 Mon Sep 17 00:00:00 2001 From: Masaaki Hirotsu Date: Sat, 3 Oct 2026 09:05:41 +0900 Subject: [PATCH 1/2] fix(keymaps): restore shadowed source buffer mappings as Neovim resolves them Source popups looked up the mapping to shadow by comparing lhs strings, so , h and os never matched Neovim's normalized form (, h, the expanded leader) and those buffer-local mappings were lost after the popup closed or lost focus. - Snapshot mappings with maparg() in the source buffer and restore them with mapset(), keeping callback, expr, and multi-mode mappings. - Take every snapshot before installing any popup key and delete before restoring, so lhs spellings that resolve to the same keys stay intact. - Cover Ctrl, leader, and Lua callback mappings across close and WinLeave, and leave global mappings global. --- lua/peekstack/ui/keymaps.lua | 60 ++++++------- tests/popup_source_mode_spec.lua | 139 +++++++++++++++++++++++++++++++ 2 files changed, 165 insertions(+), 34 deletions(-) diff --git a/lua/peekstack/ui/keymaps.lua b/lua/peekstack/ui/keymaps.lua index ab794ec..25eb4df 100644 --- a/lua/peekstack/ui/keymaps.lua +++ b/lua/peekstack/ui/keymaps.lua @@ -9,7 +9,7 @@ local M = {} ---@field winid integer ---@field bufnr integer ---@field lhs string[] ----@field original table +---@field original table[] maparg() dicts of the buffer-local mappings that were shadowed --- Buffer-local keymaps temporarily installed for the currently focused --- source-mode popup window. They are restored on WinLeave/close so the @@ -166,43 +166,29 @@ local function mapping_specs() return keymap_spec.normalize(raw) end +--- Snapshot the buffer-local normal-mode mapping that `lhs` resolves to. +--- `maparg()` applies the same key resolution as Neovim (`` vs ``, +--- ``, ``), and the returned dict keeps callback, expr, `` +--- and script context so `mapset()` can recreate it exactly. ---@param bufnr integer ---@param lhs string ----@return vim.api.keyset.get_keymap? +---@return table? local function get_buffer_map(bufnr, lhs) - for _, item in ipairs(vim.api.nvim_buf_get_keymap(bufnr, "n")) do - if item.lhs == lhs then - return item - end + local item = vim.api.nvim_buf_call(bufnr, function() + return vim.fn.maparg(lhs, "n", false, true) + end) + if item.buffer ~= 1 then + return nil end - return nil + return item end ---@param bufnr integer ----@param item vim.api.keyset.get_keymap? +---@param item table local function restore_buffer_map(bufnr, item) - if not item or not item.lhs or item.lhs == "" then - return - end - - local opts = { - buffer = bufnr, - desc = item.desc ~= "" and item.desc or nil, - expr = item.expr == 1, - nowait = item.nowait == 1, - remap = item.noremap == 0, - script = item.script == 1, - silent = item.silent == 1, - } - - if item.callback ~= nil then - vim.keymap.set("n", item.lhs, item.callback, opts) - return - end - - if type(item.rhs) == "string" then - vim.keymap.set("n", item.lhs, item.rhs, opts) - end + vim.api.nvim_buf_call(bufnr, function() + vim.fn.mapset(item) + end) end local function deactivate_active_source_popup() @@ -216,9 +202,13 @@ local function deactivate_active_source_popup() return end + -- Delete every installed mapping before restoring, so a restored mapping is + -- never removed again by another lhs spelling that resolves to the same keys. for _, lhs in ipairs(active.lhs) do pcall(vim.keymap.del, "n", lhs, { buffer = active.bufnr }) - restore_buffer_map(active.bufnr, active.original[lhs]) + end + for _, item in ipairs(active.original) do + restore_buffer_map(active.bufnr, item) end end @@ -234,16 +224,18 @@ local function activate_source_popup(popup) deactivate_active_source_popup() local specs = mapping_specs() - ---@type table + ---@type table[] local original = {} ---@type string[] local lhs_list = {} + -- Snapshot every original mapping before installing any, so an lhs that + -- resolves to the same keys as an earlier spec does not capture our own map. for _, spec in ipairs(specs) do - original[spec.lhs] = get_buffer_map(popup.bufnr, spec.lhs) - keymap_spec.set(popup.bufnr, spec) + original[#original + 1] = get_buffer_map(popup.bufnr, spec.lhs) lhs_list[#lhs_list + 1] = spec.lhs end + keymap_spec.apply(popup.bufnr, specs) active_source_maps = { winid = popup.winid, diff --git a/tests/popup_source_mode_spec.lua b/tests/popup_source_mode_spec.lua index 01d029b..70103f4 100644 --- a/tests/popup_source_mode_spec.lua +++ b/tests/popup_source_mode_spec.lua @@ -215,6 +215,145 @@ describe("popup source mode", function() vim.fn.delete(temp) end) + describe("existing buffer-local mappings with other spellings", function() + local saved_leader + local temp + local script + local source_bufnr + local root_win + + --- Resolve a mapping the same way Neovim does. Lua callbacks are re-wrapped + --- by maparg(), so only their presence is compared; behaviour is checked + --- by assert_originals_work(). + ---@param lhs string + ---@param mode string + ---@return table + local function resolve_mode(lhs, mode) + local item = vim.api.nvim_buf_call(source_bufnr, function() + return vim.fn.maparg(lhs, mode, false, true) + end) + item.lnum = nil + item.callback = item.callback and true or nil + return item + end + + ---@param lhs string + ---@return table + local function resolve(lhs) + return resolve_mode(lhs, "n") + end + + before_each(function() + saved_leader = vim.g.mapleader + vim.g.mapleader = "," + temp = vim.fn.tempname() .. ".lua" + vim.fn.writefile({ "print('peekstack')" }, temp) + vim.api.nvim_cmd({ cmd = "edit", args = { temp } }, {}) + root_win = vim.api.nvim_get_current_win() + source_bufnr = vim.api.nvim_get_current_buf() + + -- Mappings written with different spellings than peekstack's specs, + -- including a script-local expr mapping and a Lua callback. + script = vim.fn.tempname() .. ".vim" + vim.fn.writefile({ + "function! s:Original() abort", + " let g:peekstack_original_sid = 1", + " return ''", + "endfunction", + "nnoremap Original()", + "noremap os :let g:peekstack_original_leader = 1", + }, script) + vim.api.nvim_cmd({ cmd = "source", args = { script } }, {}) + vim.keymap.set("n", "h", function() + vim.g.peekstack_original_callback = 1 + end, { buffer = source_bufnr, desc = "Original window left" }) + end) + + after_each(function() + vim.g.mapleader = saved_leader + vim.g.peekstack_original_sid = nil + vim.g.peekstack_original_leader = nil + vim.g.peekstack_original_callback = nil + vim.fn.delete(temp) + vim.fn.delete(script) + end) + + local function open_source_popup() + local model = popup.create({ + uri = vim.uri_from_fname(temp), + range = { start = { line = 0, character = 0 }, ["end"] = { line = 0, character = 0 } }, + provider = "test", + }, { + buffer_mode = "source", + }) + assert.is_not_nil(model) + return model + end + + ---@param keys string + local function feed(keys) + vim.api.nvim_feedkeys(vim.api.nvim_replace_termcodes(keys, true, false, true), "x", false) + end + + local function assert_originals_work() + vim.api.nvim_set_current_win(root_win) + feed("") + feed("os") + feed("h") + assert.equals(1, vim.g.peekstack_original_sid) + assert.equals(1, vim.g.peekstack_original_leader) + assert.equals(1, vim.g.peekstack_original_callback) + end + + it("restores Ctrl, leader and mappings after popup close", function() + local lhs_list = { "", "os", "h" } + local before = vim.tbl_map(resolve, lhs_list) + local visual_before = resolve_mode(",os", "x") + + local model = open_source_popup() + assert.equals("Peekstack focus next", resolve("").desc) + assert.equals("Peekstack stack view", resolve("os").desc) + assert.equals("Peekstack navigate left", resolve("h").desc) + + popup.close(model) + + for i, lhs in ipairs(lhs_list) do + assert.same(before[i], resolve(lhs)) + end + -- The `:noremap` mapping also keeps its visual/operator-pending modes. + assert.same(visual_before, resolve_mode(",os", "x")) + assert_originals_work() + end) + + it("restores Ctrl, leader and mappings when leaving the popup", function() + require("peekstack.core.events").setup() + local model = open_source_popup() + assert.equals(model.winid, vim.api.nvim_get_current_win()) + assert.equals("Peekstack focus next", resolve("").desc) + + vim.api.nvim_set_current_win(root_win) + + assert.is_nil(resolve("").desc) + assert.equals("Original window left", resolve("h").desc) + assert_originals_work() + + popup.close(model) + end) + + it("does not turn global mappings into buffer-local ones", function() + vim.keymap.del("n", "", { buffer = source_bufnr }) + vim.keymap.set("n", "", "", { desc = "Global next" }) + + local model = open_source_popup() + popup.close(model) + + local item = resolve("") + assert.equals(0, item.buffer) + assert.equals("Global next", item.desc) + vim.keymap.del("n", "") + end) + end) + it("installs hjkl navigation keymaps on copy-mode popups", function() local loc = make_location() local model = popup.create(loc, { buffer_mode = "copy" }) From 38c86387d3ab5ca70c30c40b36d954cd50130477 Mon Sep 17 00:00:00 2001 From: Masaaki Hirotsu Date: Sat, 3 Oct 2026 09:07:28 +0900 Subject: [PATCH 2/2] fix(keymaps): limit source popup cleanup to the keys it installed Restoring a whole :map snapshot reset visual and operator-pending mappings that the popup never touched, discarding changes made while it was open. Cleanup also re-resolved at close time, so changing mapleader removed an unrelated mapping and leaked the popup's own. - Restore snapshots in normal mode only. - Remember the resolved lhs of each installed mapping and delete by it. - Cover both cases, including closing while another buffer is current. --- lua/peekstack/ui/keymaps.lua | 26 ++++++++++++++++++-------- tests/popup_source_mode_spec.lua | 31 ++++++++++++++++++++++++++++++- 2 files changed, 48 insertions(+), 9 deletions(-) diff --git a/lua/peekstack/ui/keymaps.lua b/lua/peekstack/ui/keymaps.lua index 25eb4df..34953f2 100644 --- a/lua/peekstack/ui/keymaps.lua +++ b/lua/peekstack/ui/keymaps.lua @@ -8,7 +8,7 @@ local M = {} ---@class PeekstackSourcePopupMapState ---@field winid integer ---@field bufnr integer ----@field lhs string[] +---@field installed string[] lhs of the installed mappings, as resolved when they were set ---@field original table[] maparg() dicts of the buffer-local mappings that were shadowed --- Buffer-local keymaps temporarily installed for the currently focused @@ -183,11 +183,14 @@ local function get_buffer_map(bufnr, lhs) return item end +--- Recreate a snapshot in normal mode only. Installing a normal-mode map +--- leaves the other modes of a `:map` mapping untouched, so they must not be +--- reset to the snapshot. ---@param bufnr integer ---@param item table local function restore_buffer_map(bufnr, item) vim.api.nvim_buf_call(bufnr, function() - vim.fn.mapset(item) + vim.fn.mapset("n", false, item) end) end @@ -204,7 +207,7 @@ local function deactivate_active_source_popup() -- Delete every installed mapping before restoring, so a restored mapping is -- never removed again by another lhs spelling that resolves to the same keys. - for _, lhs in ipairs(active.lhs) do + for _, lhs in ipairs(active.installed) do pcall(vim.keymap.del, "n", lhs, { buffer = active.bufnr }) end for _, item in ipairs(active.original) do @@ -226,21 +229,28 @@ local function activate_source_popup(popup) local specs = mapping_specs() ---@type table[] local original = {} - ---@type string[] - local lhs_list = {} - -- Snapshot every original mapping before installing any, so an lhs that -- resolves to the same keys as an earlier spec does not capture our own map. for _, spec in ipairs(specs) do original[#original + 1] = get_buffer_map(popup.bufnr, spec.lhs) - lhs_list[#lhs_list + 1] = spec.lhs end keymap_spec.apply(popup.bufnr, specs) + -- Record the resolved keys so cleanup does not depend on `mapleader` at + -- that later time. + ---@type string[] + local installed = {} + for _, spec in ipairs(specs) do + local item = get_buffer_map(popup.bufnr, spec.lhs) + if item then + installed[#installed + 1] = item.lhs + end + end + active_source_maps = { winid = popup.winid, bufnr = popup.bufnr, - lhs = lhs_list, + installed = installed, original = original, } end diff --git a/tests/popup_source_mode_spec.lua b/tests/popup_source_mode_spec.lua index 70103f4..cc989b4 100644 --- a/tests/popup_source_mode_spec.lua +++ b/tests/popup_source_mode_spec.lua @@ -224,7 +224,8 @@ describe("popup source mode", function() --- Resolve a mapping the same way Neovim does. Lua callbacks are re-wrapped --- by maparg(), so only their presence is compared; behaviour is checked - --- by assert_originals_work(). + --- by assert_originals_work(). A `:map` entry may come back split per mode, + --- so the combined mode spelling is ignored. ---@param lhs string ---@param mode string ---@return table @@ -234,6 +235,8 @@ describe("popup source mode", function() end) item.lnum = nil item.callback = item.callback and true or nil + item.mode = nil + item.mode_bits = nil return item end @@ -309,6 +312,7 @@ describe("popup source mode", function() local lhs_list = { "", "os", "h" } local before = vim.tbl_map(resolve, lhs_list) local visual_before = resolve_mode(",os", "x") + local pending_before = resolve_mode(",os", "o") local model = open_source_popup() assert.equals("Peekstack focus next", resolve("").desc) @@ -322,6 +326,7 @@ describe("popup source mode", function() end -- The `:noremap` mapping also keeps its visual/operator-pending modes. assert.same(visual_before, resolve_mode(",os", "x")) + assert.same(pending_before, resolve_mode(",os", "o")) assert_originals_work() end) @@ -340,6 +345,30 @@ describe("popup source mode", function() popup.close(model) end) + it("keeps other modes of a :map mapping changed while the popup is open", function() + local model = open_source_popup() + vim.keymap.set("x", ",os", "", { buffer = source_bufnr, desc = "Updated visual" }) + vim.api.nvim_set_current_win(root_win) + vim.api.nvim_cmd({ cmd = "enew" }, {}) + assert.is_not.equals(source_bufnr, vim.api.nvim_get_current_buf()) + + popup.close(model) + + assert.equals("Updated visual", resolve_mode(",os", "x").desc) + assert.equals(":let g:peekstack_original_leader = 1", resolve("os").rhs) + end) + + it("cleans up leader mappings after mapleader changes", function() + vim.keymap.set("n", ";os", "", { buffer = source_bufnr, desc = "Unrelated" }) + local model = open_source_popup() + vim.g.mapleader = ";" + + popup.close(model) + + assert.equals("Unrelated", resolve(";os").desc) + assert.equals(":let g:peekstack_original_leader = 1", resolve(",os").rhs) + end) + it("does not turn global mappings into buffer-local ones", function() vim.keymap.del("n", "", { buffer = source_bufnr }) vim.keymap.set("n", "", "", { desc = "Global next" })