From 6671abb842b6438a0ecd3f5bd0a43973023c4823 Mon Sep 17 00:00:00 2001 From: Yanuo Ma Date: Sat, 1 Aug 2026 01:49:18 -0400 Subject: [PATCH 1/5] refactor(keymap): own mappings through a slot registry Every codediff mapping was installed with a bare vim.keymap.set from one of 38 call sites, and cleanup deleted whatever key names happened to be in config at the time. That model could not restore what it overwrote, and it could not remove what it had actually installed. Mappings now go through a per-session registry backed by a global slot arbiter keyed by (bufnr, mode, canonical lhs). Each slot snapshots the pre-existing buffer-local mapping on first claim and hands it back after the last claim is released, so ownership is reference counted and teardown is symmetric with setup. Fixes: - a pre-existing buffer-local mapping (q, and plugin maps such as gitsigns ]c / [c) is restored on close instead of being destroyed (#289, #334, and the keymap half of #394) - ih is released from operator-pending and visual mode, not just normal - the previous file's buffer is detached when the diff switches files - conflict mode no longer deletes the user's do/dp; the view layer withdraws its claim so the original mapping is restored for the merge - two sessions sharing one buffer no longer corrupt each other's snapshot - a mapping replaced by another plugin while codediff is active is left alone: the foreign mapping wins, and is never restored over g? is now derived from the registry rather than a hand-maintained list, which fixes three drifts: gm shown when diff.compute_moves is off, do/dp shown in conflict mode, and the hunk staging keys hidden outside explorer mode. Double click, the explorer auto-open keys and the compact fold wraps are accounted for; the fold wraps opt out of the popup because they are transparent wrappers over native Vim keys. Public config, defaults and behavior are unchanged. A golden matrix fixture pins role x mode x lhs x desc for every session shape and verifies each mapping is reachable through maparg, which catches mappings that are listed but encoded twice (the trap that briefly broke <2-LeftMouse>, and ). A coverage spec asserts all 115 configured mappings resolve in every shape that provides them. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- lua/codediff/keymap/init.lua | 35 ++ lua/codediff/keymap/normalize.lua | 46 +++ lua/codediff/keymap/registry.lua | 226 +++++++++++++ lua/codediff/keymap/slots.lua | 358 ++++++++++++++++++++ lua/codediff/ui/conflict/keymaps.lua | 171 ++++------ lua/codediff/ui/explorer/keymaps.lua | 49 +-- lua/codediff/ui/explorer/render.lua | 5 +- lua/codediff/ui/history/keymaps.lua | 27 +- lua/codediff/ui/history/render.lua | 1 + lua/codediff/ui/keymap_help.lua | 113 ++++--- lua/codediff/ui/lib/tree_utils.lua | 8 +- lua/codediff/ui/lifecycle/accessors.lua | 206 +++++++++--- lua/codediff/ui/lifecycle/cleanup.lua | 4 +- lua/codediff/ui/lifecycle/init.lua | 7 + lua/codediff/ui/lifecycle/session.lua | 7 + lua/codediff/ui/view/compact.lua | 25 +- lua/codediff/ui/view/keymaps.lua | 20 +- tests/fixtures/keymap_matrix.txt | 342 +++++++++++++++++++ tests/keymap_matrix.lua | 173 ++++++++++ tests/ui/keymap/golden_matrix_spec.lua | 259 +++++++++++++++ tests/ui/keymap/keymap_coverage_spec.lua | 313 ++++++++++++++++++ tests/ui/keymap/keymap_help_spec.lua | 342 +++++++++++++++++++ tests/ui/keymap/keymap_lifecycle_spec.lua | 297 +++++++++++++++++ tests/ui/keymap/registry_spec.lua | 379 ++++++++++++++++++++++ 24 files changed, 3155 insertions(+), 258 deletions(-) create mode 100644 lua/codediff/keymap/init.lua create mode 100644 lua/codediff/keymap/normalize.lua create mode 100644 lua/codediff/keymap/registry.lua create mode 100644 lua/codediff/keymap/slots.lua create mode 100644 tests/fixtures/keymap_matrix.txt create mode 100644 tests/keymap_matrix.lua create mode 100644 tests/ui/keymap/golden_matrix_spec.lua create mode 100644 tests/ui/keymap/keymap_coverage_spec.lua create mode 100644 tests/ui/keymap/keymap_help_spec.lua create mode 100644 tests/ui/keymap/keymap_lifecycle_spec.lua create mode 100644 tests/ui/keymap/registry_spec.lua diff --git a/lua/codediff/keymap/init.lua b/lua/codediff/keymap/init.lua new file mode 100644 index 00000000..c46543b4 --- /dev/null +++ b/lua/codediff/keymap/init.lua @@ -0,0 +1,35 @@ +-- Keymap registry facade. +-- +-- Every codediff mapping is installed through a per-session registry, which +-- records exactly what it installed and what was there before, so teardown can +-- hand the key back to its previous owner instead of deleting it. +-- +-- See codediff.keymap.slots for the ownership rules. + +local M = {} + +local registry = require("codediff.keymap.registry") +local slots = require("codediff.keymap.slots") +local normalize = require("codediff.keymap.normalize") + +--- Create a registry for one session. +--- @param name string Diagnostic label +--- @return table +function M.new(name) + return registry.new(name) +end + +--- Forget every slot for a wiped buffer without touching Neovim. +--- @param bufnr number +function M.forget_buffer(bufnr) + slots.forget_buffer(bufnr) +end + +M.canonical = normalize.canonical +M.resolve = normalize.resolve + +--- Live slot count. Test/diagnostic helper. +M.slot_count = slots.count +M.inspect_slot = slots.inspect + +return M diff --git a/lua/codediff/keymap/normalize.lua b/lua/codediff/keymap/normalize.lua new file mode 100644 index 00000000..436f67d4 --- /dev/null +++ b/lua/codediff/keymap/normalize.lua @@ -0,0 +1,46 @@ +-- Key-sequence normalization for the keymap registry. +-- +-- Mapping identity must be canonical: `` and `` are the same key to +-- Neovim, and `x` depends on the value of `mapleader` at the moment +-- the mapping is created. Slots are therefore keyed by the fully expanded +-- byte sequence, computed once at claim time and stored, never recomputed. + +local M = {} + +--- Expand a key sequence to its canonical byte form. +--- @param lhs string +--- @return string|nil canonical nil when lhs is not a usable key sequence +function M.canonical(lhs) + if type(lhs) ~= "string" or lhs == "" then + return nil + end + local ok, expanded = pcall(vim.api.nvim_replace_termcodes, lhs, true, true, true) + if not ok or expanded == "" then + return nil + end + return expanded +end + +--- Resolve a configured binding value to a key sequence. +--- Preserves the existing contract: a string binds, `false` (or nil, or an +--- empty string) silently disables. No other shape is accepted. +--- @param value string|false|nil +--- @return string|nil lhs +function M.resolve(value) + if type(value) ~= "string" or value == "" then + return nil + end + return value +end + +--- Normalize a mode argument to a list of single-character modes. +--- @param modes string|string[] +--- @return string[] +function M.modes(modes) + if type(modes) == "table" then + return modes + end + return { modes } +end + +return M diff --git a/lua/codediff/keymap/registry.lua b/lua/codediff/keymap/registry.lua new file mode 100644 index 00000000..77c03d73 --- /dev/null +++ b/lua/codediff/keymap/registry.lua @@ -0,0 +1,226 @@ +-- Per-session keymap registry. +-- +-- Owns every mapping a single codediff session installs, so teardown is +-- symmetric with setup. The registry holds no mapping state itself: it records +-- which slots it claimed and delegates arbitration to codediff.keymap.slots. +-- +-- Buffer ownership matters for suspend/resume: +-- * borrowed buffers (real files shown in the diff panes) must release their +-- mappings when the user leaves the tab, or codediff's keys would appear +-- on that file everywhere it is opened; +-- * owned buffers (explorer/history panels and other codediff scratch +-- buffers) cannot leak anywhere, so their mappings stay installed until +-- the buffer goes away. Suspending them caused a past regression where +-- panel keys vanished after a tab switch. + +local M = {} + +local slots = require("codediff.keymap.slots") +local normalize = require("codediff.keymap.normalize") + +local Registry = {} +Registry.__index = Registry + +local function entry_key(bufnr, mode, lhs) + return string.format("%d\0%s\0%s", bufnr, mode, lhs) +end + +--- @param name string Diagnostic label (e.g. "session:3") +--- @return table registry +function M.new(name) + return setmetatable({ + name = name, + entries = {}, + suspended = false, + disposed = false, + }, Registry) +end + +--- Install a mapping owned by this registry. +--- @param bufnr number +--- @param modes string|string[] +--- @param lhs string|false|nil Configured binding; false/nil silently disables +--- @param rhs function|string +--- @param opts table|nil Forwarded verbatim to vim.keymap.set (minus buffer) +--- @param meta table|nil { suspendable = boolean, priority = integer, help = boolean } +--- @return boolean claimed +function Registry:claim(bufnr, modes, lhs, rhs, opts, meta) + if self.disposed then + return false + end + + local resolved = normalize.resolve(lhs) + if not resolved then + return false + end + local canonical = normalize.canonical(resolved) + if not canonical then + return false + end + if not bufnr or not vim.api.nvim_buf_is_valid(bufnr) then + return false + end + + meta = meta or {} + local suspendable = meta.suspendable ~= false + -- Transparent wrappers over native Vim keys (compact's synced folds) opt out + -- of the help popup: they do not add a codediff command to discover. + local documented = meta.help ~= false + local claimed = false + + for _, mode in ipairs(normalize.modes(modes)) do + -- `canonical` identifies the slot; `resolved` is the spelling the mapping + -- APIs accept. Passing the canonical bytes to vim.keymap.set would encode + -- keys like <2-LeftMouse> and a second time, leaving a mapping the + -- real key press can never reach. + if slots.claim(self, bufnr, mode, canonical, resolved, rhs, opts, meta.priority) then + local key = entry_key(bufnr, mode, canonical) + self.entries[key] = { + bufnr = bufnr, + mode = mode, + lhs = canonical, + suspendable = suspendable, + documented = documented, + } + -- A claim added while suspended must not be installed yet. + if self.suspended and suspendable then + slots.set_active(self, bufnr, mode, canonical, false) + end + claimed = true + end + end + + return claimed +end + +--- Keys this registry owns that should appear in the help popup. +--- @return table canonical lhs -> true +function Registry:documented_keys() + local keys = {} + for _, entry in pairs(self.entries) do + if entry.documented then + keys[entry.lhs] = true + end + end + return keys +end + +--- True when this registry currently owns `lhs`. +--- @param lhs string|false|nil Configured binding +--- @param mode string|nil Restrict to one mode; any mode when omitted +--- @param bufnr number|nil Restrict to one buffer; any buffer when omitted +--- @return boolean +function Registry:owns(lhs, mode, bufnr) + local resolved = normalize.resolve(lhs) + if not resolved then + return false + end + local canonical = normalize.canonical(resolved) + if not canonical then + return false + end + for _, entry in pairs(self.entries) do + if entry.lhs == canonical and (mode == nil or entry.mode == mode) and (bufnr == nil or entry.bufnr == bufnr) then + return true + end + end + return false +end + +--- Release a specific mapping this registry installed. +--- @param bufnr number +--- @param modes string|string[] +--- @param lhs string|false|nil +function Registry:release(bufnr, modes, lhs) + local resolved = normalize.resolve(lhs) + if not resolved or not bufnr then + return + end + local canonical = normalize.canonical(resolved) + if not canonical then + return + end + for _, mode in ipairs(normalize.modes(modes)) do + slots.release(self, bufnr, mode, canonical) + self.entries[entry_key(bufnr, mode, canonical)] = nil + end +end + +--- Release every mapping this registry installed on a buffer. +--- Used when a diff pane swaps to a different file, so the buffer that leaves +--- the session gets its original mappings back immediately. +--- @param bufnr number +function Registry:detach_buffer(bufnr) + if not bufnr then + return + end + for key, entry in pairs(self.entries) do + if entry.bufnr == bufnr then + slots.release(self, entry.bufnr, entry.mode, entry.lhs) + self.entries[key] = nil + end + end +end + +--- Release mappings on every buffer except those listed. +--- @param keep table Set of buffer numbers to retain +function Registry:detach_buffers_except(keep) + local seen = {} + for _, entry in pairs(self.entries) do + if not keep[entry.bufnr] then + seen[entry.bufnr] = true + end + end + for bufnr in pairs(seen) do + self:detach_buffer(bufnr) + end +end + +--- Uninstall suspendable mappings, keeping the claims registered. +function Registry:suspend() + if self.disposed or self.suspended then + return + end + self.suspended = true + for _, entry in pairs(self.entries) do + if entry.suspendable then + slots.set_active(self, entry.bufnr, entry.mode, entry.lhs, false) + end + end +end + +--- Reinstall previously suspended mappings. +function Registry:resume() + if self.disposed or not self.suspended then + return + end + self.suspended = false + for _, entry in pairs(self.entries) do + if entry.suspendable then + slots.set_active(self, entry.bufnr, entry.mode, entry.lhs, true) + end + end +end + +--- Release everything. Idempotent, so repeated cleanup is harmless. +function Registry:dispose() + if self.disposed then + return + end + self.disposed = true + for key, entry in pairs(self.entries) do + slots.release(self, entry.bufnr, entry.mode, entry.lhs) + self.entries[key] = nil + end +end + +--- Number of mappings currently registered. Test/diagnostic helper. +function Registry:count() + local total = 0 + for _ in pairs(self.entries) do + total = total + 1 + end + return total +end + +return M diff --git a/lua/codediff/keymap/slots.lua b/lua/codediff/keymap/slots.lua new file mode 100644 index 00000000..d7e68a94 --- /dev/null +++ b/lua/codediff/keymap/slots.lua @@ -0,0 +1,358 @@ +-- Global mapping-slot arbiter. +-- +-- A "slot" is one concrete Neovim mapping: (bufnr, mode, canonical lhs). +-- Every slot remembers the buffer-local mapping that existed before codediff +-- first touched it, and the set of claims currently competing for it. +-- +-- Invariants: +-- * The pre-existing mapping is snapshotted exactly once, on first claim. +-- * It is handed back when the last claim is released. +-- * If no buffer-local mapping existed, codediff's mapping is deleted so the +-- global/default binding becomes visible again. +-- * If another plugin replaces codediff's mapping while it is installed, the +-- foreign mapping wins: codediff neither reinstalls nor restores over it. +-- +-- Multiple sessions may claim the same slot (the same real file can be open in +-- two diff tabs). Claims are reference-counted so teardown of one session +-- cannot destroy another session's mapping or the user's original. + +local M = {} + +local normalize = require("codediff.keymap.normalize") + +-- slots[bufnr][mode][lhs] = slot +local slots = {} + +--- @class CodeDiffKeymapClaim +--- @field owner table Identity of the claiming registry +--- @field rhs function|string Callback or right-hand side +--- @field opts table Options forwarded verbatim to vim.keymap.set +--- @field priority integer Higher wins; ties resolve to the newest claim +--- @field active boolean Suspended claims stay registered but uninstalled + +local function slot_table(bufnr, mode, create) + local by_mode = slots[bufnr] + if not by_mode then + if not create then + return nil + end + by_mode = {} + slots[bufnr] = by_mode + end + local by_lhs = by_mode[mode] + if not by_lhs then + if not create then + return nil + end + by_lhs = {} + by_mode[mode] = by_lhs + end + return by_lhs +end + +--- Read the mapping currently installed for a slot, in buffer context. +local function read_current(bufnr, mode, lhs) + if not vim.api.nvim_buf_is_valid(bufnr) then + return nil + end + local ok, result = pcall(vim.api.nvim_buf_call, bufnr, function() + return vim.fn.maparg(lhs, mode, false, true) + end) + if not ok or type(result) ~= "table" or next(result) == nil then + return nil + end + return result +end + +--- True when `current` is a buffer-local mapping (not a global fallback). +local function is_buffer_local(current) + return current ~= nil and current.buffer == 1 +end + +local function drop_slot(slot) + local by_lhs = slot_table(slot.bufnr, slot.mode, false) + if by_lhs then + by_lhs[slot.key] = nil + end + local by_mode = slots[slot.bufnr] + if by_mode then + local empty = true + for _, tbl in pairs(by_mode) do + if next(tbl) ~= nil then + empty = false + break + end + end + if empty then + slots[slot.bufnr] = nil + end + end +end + +local function install(slot, claim) + local rhs = claim.rhs + if type(rhs) == "function" then + -- Wrap in a per-claim dispatcher so the installed mapping is identifiable + -- by function identity even when two claims share the same handler. + if not claim.dispatcher then + claim.dispatcher = function(...) + return claim.rhs(...) + end + end + rhs = claim.dispatcher + end + + local opts = vim.tbl_extend("force", claim.opts or {}, { buffer = slot.bufnr }) + local ok = pcall(vim.keymap.set, slot.mode, slot.lhs, rhs, opts) + if ok then + slot.applied = claim + else + slot.applied = nil + end +end + +--- Compare two mapping descriptions for identity. +local function same_map(a, b) + if not a or not b then + return false + end + if a.callback ~= nil or b.callback ~= nil then + return a.callback == b.callback + end + return a.rhs == b.rhs +end + +--- True when the mapping currently installed is the one this slot applied. +local function installed_is_ours(slot) + local claim = slot.applied + if not claim then + return false + end + local current = read_current(slot.bufnr, slot.mode, slot.lhs) + if not is_buffer_local(current) then + return false + end + local expected = claim.dispatcher or claim.rhs + if type(expected) == "function" then + return current.callback == expected + end + return current.rhs == expected +end + +--- True when codediff may take this slot: either nothing buffer-local is +--- installed, or what is installed is the snapshot we previously handed back. +local function slot_is_free_for_us(slot) + local current = read_current(slot.bufnr, slot.mode, slot.lhs) + if not is_buffer_local(current) then + return true + end + return slot.saved ~= false and same_map(current, slot.saved) +end + +--- Hand the slot back to whatever owned it before codediff. +local function restore(slot) + if not vim.api.nvim_buf_is_valid(slot.bufnr) then + slot.applied = nil + return + end + if slot.saved then + pcall(vim.api.nvim_buf_call, slot.bufnr, function() + vim.fn.mapset(slot.mode, false, slot.saved) + end) + else + pcall(vim.keymap.del, slot.mode, slot.lhs, { buffer = slot.bufnr }) + end + slot.applied = nil +end + +--- Highest-priority active claim; ties resolve to the most recent claim. +local function winner(slot) + local best, best_index + for index, claim in ipairs(slot.claims) do + if claim.active then + if not best or claim.priority > best.priority or (claim.priority == best.priority and index > best_index) then + best, best_index = claim, index + end + end + end + return best +end + +--- Bring the real Neovim mapping in line with the slot's claims. +--- The slot survives as long as any claim is registered, even when every claim +--- is suspended, because it holds the snapshot needed to restore on resume. +local function reconcile(slot) + if not vim.api.nvim_buf_is_valid(slot.bufnr) then + drop_slot(slot) + return + end + + -- Ownership check: if what is installed is no longer the mapping we put + -- there, another plugin (or the user) has taken over. Stand down for good. + if slot.applied and not installed_is_ours(slot) then + slot.displaced = true + slot.applied = nil + end + + local win = winner(slot) + + if slot.displaced then + -- Never reinstall over, or restore across, a foreign mapping. + if #slot.claims == 0 then + drop_slot(slot) + end + return + end + + if win then + if slot.applied ~= win then + if slot.applied or slot_is_free_for_us(slot) then + install(slot, win) + else + -- Something else claimed the key while we were suspended. + slot.displaced = true + slot.applied = nil + end + end + return + end + + if slot.applied then + restore(slot) + end + if #slot.claims == 0 then + drop_slot(slot) + end +end + +local function find_claim(slot, owner) + for index, claim in ipairs(slot.claims) do + if claim.owner == owner then + return index, claim + end + end + return nil +end + +--- Claim a slot for `owner`, snapshotting any pre-existing buffer-local map. +--- Re-claiming with the same owner replaces that owner's previous claim, which +--- is what happens when a session re-runs its keymap setup after a re-render. +--- +--- `key` identifies the slot; `lhs` is what Neovim is asked to map. They differ +--- for keys such as `<2-LeftMouse>` and ``, whose canonical form contains +--- K_SPECIAL bytes that the mapping APIs would encode a second time. +--- @param owner table +--- @param bufnr number +--- @param mode string +--- @param key string Canonical (already expanded) key sequence, used as identity +--- @param lhs string Key sequence in the form the mapping APIs accept +--- @param rhs function|string +--- @param opts table|nil Forwarded verbatim to vim.keymap.set +--- @param priority integer|nil +--- @return boolean claimed +function M.claim(owner, bufnr, mode, key, lhs, rhs, opts, priority) + if not bufnr or not vim.api.nvim_buf_is_valid(bufnr) or not key or not lhs then + return false + end + + local by_lhs = slot_table(bufnr, mode, true) + local slot = by_lhs[key] + + if not slot then + local current = read_current(bufnr, mode, lhs) + slot = { + bufnr = bufnr, + mode = mode, + key = key, + lhs = lhs, + -- Only buffer-local mappings are ours to restore. A global mapping must + -- never be recreated as a buffer-local one. + saved = is_buffer_local(current) and current or false, + claims = {}, + } + by_lhs[key] = slot + end + + local existing_index = find_claim(slot, owner) + if existing_index then + table.remove(slot.claims, existing_index) + end + + table.insert(slot.claims, { + owner = owner, + rhs = rhs, + opts = opts or {}, + priority = priority or 0, + active = true, + }) + + reconcile(slot) + return true +end + +--- Release `owner`'s claim on a slot. +--- @param key string Canonical key sequence used as slot identity +function M.release(owner, bufnr, mode, key) + local by_lhs = slot_table(bufnr, mode, false) + local slot = by_lhs and by_lhs[key] + if not slot then + return + end + + local index = find_claim(slot, owner) + if index then + table.remove(slot.claims, index) + end + + reconcile(slot) +end + +--- Suspend or resume `owner`'s claim without forgetting it. +--- @param key string Canonical key sequence used as slot identity +function M.set_active(owner, bufnr, mode, key, active) + local by_lhs = slot_table(bufnr, mode, false) + local slot = by_lhs and by_lhs[key] + if not slot then + return + end + + local _, claim = find_claim(slot, owner) + if not claim or claim.active == active then + return + end + claim.active = active + + reconcile(slot) +end + +--- Forget every slot for a buffer without touching Neovim. +--- For BufWipeout: the buffer and its mappings are already gone. +function M.forget_buffer(bufnr) + slots[bufnr] = nil +end + +--- Number of live slots. Test/diagnostic helper. +function M.count() + local total = 0 + for _, by_mode in pairs(slots) do + for _, by_lhs in pairs(by_mode) do + for _ in pairs(by_lhs) do + total = total + 1 + end + end + end + return total +end + +--- Inspect a slot. Test/diagnostic helper. +function M.inspect(bufnr, mode, lhs) + local by_lhs = slot_table(bufnr, mode, false) + return by_lhs and by_lhs[normalize.canonical(lhs) or lhs] or nil +end + +--- Drop all state. Test helper only. +function M.reset() + slots = {} +end + +return M diff --git a/lua/codediff/ui/conflict/keymaps.lua b/lua/codediff/ui/conflict/keymaps.lua index 9511179f..8ddc8535 100644 --- a/lua/codediff/ui/conflict/keymaps.lua +++ b/lua/codediff/ui/conflict/keymaps.lua @@ -8,7 +8,34 @@ local actions = require("codediff.ui.conflict.actions") local diffget = require("codediff.ui.conflict.diffget") local navigation = require("codediff.ui.conflict.navigation") +-- Dot-repeatable actions are expr mappings (see conflict.tracking). +local REPEATABLE_ACTIONS = { + { key = "accept_incoming", fn = actions.accept_incoming, desc = "Accept incoming change" }, + { key = "accept_current", fn = actions.accept_current, desc = "Accept current change" }, + { key = "accept_both", fn = actions.accept_both, desc = "Accept both changes" }, + { key = "discard", fn = actions.discard, desc = "Discard changes (keep base)" }, +} + +local PLAIN_ACTIONS = { + { key = "accept_all_incoming", fn = actions.accept_all_incoming, desc = "Accept ALL incoming changes" }, + { key = "accept_all_current", fn = actions.accept_all_current, desc = "Accept ALL current changes" }, + { key = "accept_all_both", fn = actions.accept_all_both, desc = "Accept ALL both changes" }, + { key = "discard_all", fn = actions.discard_all, desc = "Discard ALL, reset to base" }, + { key = "next_conflict", fn = navigation.navigate_next_conflict, desc = "Next conflict" }, + { key = "prev_conflict", fn = navigation.navigate_prev_conflict, desc = "Previous conflict" }, +} + +-- Vimdiff-style numbered diffget, only meaningful on the result buffer. +local RESULT_ONLY_ACTIONS = { + { key = "diffget_incoming", fn = diffget.diffget_incoming, desc = "Get hunk from incoming (2do)" }, + { key = "diffget_current", fn = diffget.diffget_current, desc = "Get hunk from current (3do)" }, +} + --- Setup conflict keymaps for a session +--- +--- Ordinary do/dp are not deleted here. The view layer skips claiming them +--- while a result pane exists, so any mapping the user already had on those +--- keys stays intact for the duration of the merge instead of being destroyed. --- @param tabpage number function M.setup_keymaps(tabpage) local session = lifecycle.get_session(tabpage) @@ -17,134 +44,48 @@ function M.setup_keymaps(tabpage) end local keymaps = config.options.keymaps.conflict or {} - local view_keymaps = config.options.keymaps.view or {} -- Bind to incoming (left), current (right), AND result buffers local buffers = { session.original_bufnr, session.modified_bufnr, session.result_bufnr } local base_opts = { noremap = true, silent = true, nowait = true } + local function bind(bufnr, action, opts) + if not keymaps[action.key] then + return + end + lifecycle.set_buf_keymap(tabpage, bufnr, "n", keymaps[action.key], opts.rhs, vim.tbl_extend("force", base_opts, opts.extra or {})) + end + for _, bufnr in ipairs(buffers) do if bufnr and vim.api.nvim_buf_is_valid(bufnr) then - -- Unbind normal mode do/dp from view keymaps (they don't apply in merge conflict mode) - if view_keymaps.diff_get then - pcall(vim.keymap.del, "n", view_keymaps.diff_get, { buffer = bufnr }) - end - if view_keymaps.diff_put then - pcall(vim.keymap.del, "n", view_keymaps.diff_put, { buffer = bufnr }) - end - - -- Accept incoming - if keymaps.accept_incoming then - vim.keymap.set( - "n", - keymaps.accept_incoming, - tracking.make_repeatable(function() - actions.accept_incoming(tabpage) - end), - vim.tbl_extend("force", base_opts, { buffer = bufnr, desc = "Accept incoming change", expr = true }) - ) - end - - -- Accept current - if keymaps.accept_current then - vim.keymap.set( - "n", - keymaps.accept_current, - tracking.make_repeatable(function() - actions.accept_current(tabpage) - end), - vim.tbl_extend("force", base_opts, { buffer = bufnr, desc = "Accept current change", expr = true }) - ) - end - - -- Accept both - if keymaps.accept_both then - vim.keymap.set( - "n", - keymaps.accept_both, - tracking.make_repeatable(function() - actions.accept_both(tabpage) - end), - vim.tbl_extend("force", base_opts, { buffer = bufnr, desc = "Accept both changes", expr = true }) - ) - end - - -- Discard - if keymaps.discard then - vim.keymap.set( - "n", - keymaps.discard, - tracking.make_repeatable(function() - actions.discard(tabpage) + for _, action in ipairs(REPEATABLE_ACTIONS) do + bind(bufnr, action, { + rhs = tracking.make_repeatable(function() + action.fn(tabpage) end), - vim.tbl_extend("force", base_opts, { buffer = bufnr, desc = "Discard changes (keep base)", expr = true }) - ) - end - - -- Accept ALL incoming - if keymaps.accept_all_incoming then - vim.keymap.set("n", keymaps.accept_all_incoming, function() - actions.accept_all_incoming(tabpage) - end, vim.tbl_extend("force", base_opts, { buffer = bufnr, desc = "Accept ALL incoming changes" })) - end - - -- Accept ALL current - if keymaps.accept_all_current then - vim.keymap.set("n", keymaps.accept_all_current, function() - actions.accept_all_current(tabpage) - end, vim.tbl_extend("force", base_opts, { buffer = bufnr, desc = "Accept ALL current changes" })) - end - - -- Accept ALL both - if keymaps.accept_all_both then - vim.keymap.set("n", keymaps.accept_all_both, function() - actions.accept_all_both(tabpage) - end, vim.tbl_extend("force", base_opts, { buffer = bufnr, desc = "Accept ALL both changes" })) + extra = { desc = action.desc, expr = true }, + }) end - -- Discard ALL - if keymaps.discard_all then - vim.keymap.set("n", keymaps.discard_all, function() - actions.discard_all(tabpage) - end, vim.tbl_extend("force", base_opts, { buffer = bufnr, desc = "Discard ALL, reset to base" })) + for _, action in ipairs(PLAIN_ACTIONS) do + bind(bufnr, action, { + rhs = function() + action.fn(tabpage) + end, + extra = { desc = action.desc }, + }) end - -- Navigation - if keymaps.next_conflict then - vim.keymap.set("n", keymaps.next_conflict, function() - navigation.navigate_next_conflict(tabpage) - end, vim.tbl_extend("force", base_opts, { buffer = bufnr, desc = "Next conflict" })) - end - - if keymaps.prev_conflict then - vim.keymap.set("n", keymaps.prev_conflict, function() - navigation.navigate_prev_conflict(tabpage) - end, vim.tbl_extend("force", base_opts, { buffer = bufnr, desc = "Previous conflict" })) - end - - -- Vimdiff-style diffget from incoming (2do) - only on result buffer - if keymaps.diffget_incoming and bufnr == session.result_bufnr then - vim.keymap.set( - "n", - keymaps.diffget_incoming, - tracking.make_repeatable(function() - diffget.diffget_incoming(tabpage) - end), - vim.tbl_extend("force", base_opts, { buffer = bufnr, desc = "Get hunk from incoming (2do)", expr = true }) - ) - end - - -- Vimdiff-style diffget from current (3do) - only on result buffer - if keymaps.diffget_current and bufnr == session.result_bufnr then - vim.keymap.set( - "n", - keymaps.diffget_current, - tracking.make_repeatable(function() - diffget.diffget_current(tabpage) - end), - vim.tbl_extend("force", base_opts, { buffer = bufnr, desc = "Get hunk from current (3do)", expr = true }) - ) + if bufnr == session.result_bufnr then + for _, action in ipairs(RESULT_ONLY_ACTIONS) do + bind(bufnr, action, { + rhs = tracking.make_repeatable(function() + action.fn(tabpage) + end), + extra = { desc = action.desc, expr = true }, + }) + end end end end diff --git a/lua/codediff/ui/explorer/keymaps.lua b/lua/codediff/ui/explorer/keymaps.lua index 4256c023..e253781a 100644 --- a/lua/codediff/ui/explorer/keymaps.lua +++ b/lua/codediff/ui/explorer/keymaps.lua @@ -16,9 +16,17 @@ function M.setup(explorer) local map_options = { noremap = true, silent = true, nowait = true } local explorer_keymaps = config.options.keymaps.explorer or {} + -- Panel mappings live on a codediff-owned scratch buffer, so they cannot + -- leak into user buffers and must survive tab switches (suspendable=false). + -- Required lazily to avoid a module cycle through the lifecycle package. + local lifecycle = require("codediff.ui.lifecycle") + local function panel_map(lhs, rhs, desc) + lifecycle.set_buf_keymap(explorer.tabpage, split.bufnr, "n", lhs, rhs, vim.tbl_extend("force", map_options, { desc = desc }), { suspendable = false }) + end + -- Toggle expand/collapse or select file if explorer_keymaps.select then - vim.keymap.set("n", explorer_keymaps.select, function() + panel_map(explorer_keymaps.select, function() local node = tree:get_node() if not node then return @@ -48,22 +56,22 @@ function M.setup(explorer) end end end - end, vim.tbl_extend("force", map_options, { buffer = split.bufnr, desc = "Select/toggle entry" })) + end, "Select/toggle entry") end -- Double click also works for files - vim.keymap.set("n", "<2-LeftMouse>", function() + panel_map("<2-LeftMouse>", function() local node = tree:get_node() if not node or not node.data or node.data.type == "group" or node.data.type == "directory" then return end explorer.on_file_select(node.data) - end, vim.tbl_extend("force", map_options, { buffer = split.bufnr, desc = "Select file" })) + end, "Select file") -- Hover to show full path (K key, like LSP hover) local hover_win = nil if explorer_keymaps.hover then - vim.keymap.set("n", explorer_keymaps.hover, function() + panel_map(explorer_keymaps.hover, function() -- Close existing hover window if hover_win and vim.api.nvim_win_is_valid(hover_win) then vim.api.nvim_win_close(hover_win, true) @@ -120,56 +128,56 @@ function M.setup(explorer) end end, }) - end, vim.tbl_extend("force", map_options, { buffer = split.bufnr, desc = "Show full path" })) + end, "Show full path") end -- Refresh explorer (R key) if explorer_keymaps.refresh then - vim.keymap.set("n", explorer_keymaps.refresh, function() + panel_map(explorer_keymaps.refresh, function() refresh_module.refresh(explorer) - end, vim.tbl_extend("force", map_options, { buffer = split.bufnr, desc = "Refresh explorer" })) + end, "Refresh explorer") end -- Toggle view mode (i key) - switch between 'list' and 'tree' if explorer_keymaps.toggle_view_mode then - vim.keymap.set("n", explorer_keymaps.toggle_view_mode, function() + panel_map(explorer_keymaps.toggle_view_mode, function() actions_module.toggle_view_mode(explorer) - end, vim.tbl_extend("force", map_options, { buffer = split.bufnr, desc = "Toggle list/tree view" })) + end, "Toggle list/tree view") end -- Stage all files (S key) if explorer_keymaps.stage_all then - vim.keymap.set("n", explorer_keymaps.stage_all, function() + panel_map(explorer_keymaps.stage_all, function() actions_module.stage_all(explorer) - end, vim.tbl_extend("force", map_options, { buffer = split.bufnr, desc = "Stage all files" })) + end, "Stage all files") end -- Unstage all files (U key) if explorer_keymaps.unstage_all then - vim.keymap.set("n", explorer_keymaps.unstage_all, function() + panel_map(explorer_keymaps.unstage_all, function() actions_module.unstage_all(explorer) - end, vim.tbl_extend("force", map_options, { buffer = split.bufnr, desc = "Unstage all files" })) + end, "Unstage all files") end -- Restore/discard changes (X key) if explorer_keymaps.restore then - vim.keymap.set("n", explorer_keymaps.restore, function() + panel_map(explorer_keymaps.restore, function() actions_module.restore_entry(explorer, tree) - end, vim.tbl_extend("force", map_options, { buffer = split.bufnr, desc = "Restore/discard changes" })) + end, "Restore/discard changes") end -- Toggle Changes (unstaged) group visibility if explorer_keymaps.toggle_changes then - vim.keymap.set("n", explorer_keymaps.toggle_changes, function() + panel_map(explorer_keymaps.toggle_changes, function() actions_module.toggle_group(explorer, "unstaged") - end, vim.tbl_extend("force", map_options, { buffer = split.bufnr, desc = "Toggle Changes visibility" })) + end, "Toggle Changes visibility") end -- Toggle Staged Changes group visibility if explorer_keymaps.toggle_staged then - vim.keymap.set("n", explorer_keymaps.toggle_staged, function() + panel_map(explorer_keymaps.toggle_staged, function() actions_module.toggle_group(explorer, "staged") - end, vim.tbl_extend("force", map_options, { buffer = split.bufnr, desc = "Toggle Staged Changes visibility" })) + end, "Toggle Staged Changes visibility") end -- Fold keymaps (Vim-style: zo/zO/zc/zC/za/zA/zR/zM) @@ -177,6 +185,7 @@ function M.setup(explorer) tree = tree, keymaps = explorer_keymaps, bufnr = split.bufnr, + tabpage = explorer.tabpage, }) -- Note: next_file/prev_file keymaps are set via view/keymaps.lua:setup_all_keymaps() diff --git a/lua/codediff/ui/explorer/render.lua b/lua/codediff/ui/explorer/render.lua index 24c6a6cf..8bd6cc39 100644 --- a/lua/codediff/ui/explorer/render.lua +++ b/lua/codediff/ui/explorer/render.lua @@ -594,12 +594,13 @@ function M.create(status_result, git_root, tabpage, width, base_revision, target end explorer.on_file_select(node.data) end + local lifecycle = require("codediff.ui.lifecycle") for _, key in ipairs({ "j", "k", "", "" }) do - vim.keymap.set("n", key, function() + lifecycle.set_buf_keymap(explorer.tabpage, split.bufnr, "n", key, function() local motion = key == "" and "j" or key == "" and "k" or key vim.cmd("normal! " .. motion) open_under_cursor() - end, { buffer = split.bufnr, silent = true, desc = "codediff: move and auto-open file" }) + end, { silent = true, desc = "codediff: move and auto-open file" }, { suspendable = false }) end end diff --git a/lua/codediff/ui/history/keymaps.lua b/lua/codediff/ui/history/keymaps.lua index 5622d646..faabb400 100644 --- a/lua/codediff/ui/history/keymaps.lua +++ b/lua/codediff/ui/history/keymaps.lua @@ -6,7 +6,7 @@ local M = {} -- Setup keymaps for history panel -- @param history: history object with tree, split, on_file_select, etc. --- @param opts: { is_single_file_mode, file_path, git_root, load_commit_files, navigate_next, navigate_prev } +-- @param opts: { is_single_file_mode, file_path, git_root, tabpage, load_commit_files, navigate_next, navigate_prev } function M.setup(history, opts) local tree = history.tree local split = history.split @@ -16,9 +16,17 @@ function M.setup(history, opts) local map_options = { noremap = true, silent = true, nowait = true } local history_keymaps = config.options.keymaps.history or {} + -- Panel mappings live on a codediff-owned scratch buffer, so they cannot + -- leak into user buffers and must survive tab switches (suspendable=false). + -- Required lazily to avoid a module cycle through the lifecycle package. + local lifecycle = require("codediff.ui.lifecycle") + local function panel_map(lhs, rhs, desc) + lifecycle.set_buf_keymap(opts.tabpage, split.bufnr, "n", lhs, rhs, vim.tbl_extend("force", map_options, { desc = desc }), { suspendable = false }) + end + -- Toggle expand/collapse or select file if history_keymaps.select then - vim.keymap.set("n", history_keymaps.select, function() + panel_map(history_keymaps.select, function() local node = tree:get_node() if not node then return @@ -50,11 +58,11 @@ function M.setup(history, opts) elseif node.data and node.data.type == "file" then history.on_file_select(node.data) end - end, vim.tbl_extend("force", map_options, { buffer = split.bufnr, desc = "Select/toggle entry" })) + end, "Select/toggle entry") end -- Double-click support - vim.keymap.set("n", "<2-LeftMouse>", function() + panel_map("<2-LeftMouse>", function() local node = tree:get_node() if not node then return @@ -84,14 +92,14 @@ function M.setup(history, opts) load_commit_files(node) end end - end, vim.tbl_extend("force", map_options, { buffer = split.bufnr, desc = "Select file" })) + end, "Select file") -- Note: next_file/prev_file keymaps are set via view/keymaps.lua:setup_all_keymaps() -- which uses set_tab_keymap to set them on all buffers including history panel -- Toggle view mode between list and tree if history_keymaps.toggle_view_mode then - vim.keymap.set("n", history_keymaps.toggle_view_mode, function() + panel_map(history_keymaps.toggle_view_mode, function() local history_config = config.options.history or {} local current_mode = history_config.view_mode or "list" local new_mode = (current_mode == "list") and "tree" or "list" @@ -111,15 +119,15 @@ function M.setup(history, opts) end vim.notify("History view: " .. new_mode, vim.log.levels.INFO) - end, vim.tbl_extend("force", map_options, { buffer = split.bufnr, desc = "Toggle list/tree view" })) + end, "Toggle list/tree view") end -- Refresh (R key) - re-fetch commits if history_keymaps.refresh then - vim.keymap.set("n", history_keymaps.refresh, function() + panel_map(history_keymaps.refresh, function() local refresh_module = require("codediff.ui.history.refresh") refresh_module.refresh(history) - end, vim.tbl_extend("force", map_options, { buffer = split.bufnr, desc = "Refresh history" })) + end, "Refresh history") end -- Fold keymaps (Vim-style: zo/zO/zc/zC/za/zA/zR/zM — directory nodes only) @@ -127,6 +135,7 @@ function M.setup(history, opts) tree = tree, keymaps = history_keymaps, bufnr = split.bufnr, + tabpage = opts.tabpage, }) end diff --git a/lua/codediff/ui/history/render.lua b/lua/codediff/ui/history/render.lua index 853ffe97..7355b55e 100644 --- a/lua/codediff/ui/history/render.lua +++ b/lua/codediff/ui/history/render.lua @@ -355,6 +355,7 @@ function M.create(commits, git_root, tabpage, width, opts) is_single_file_mode = is_single_file_mode, file_path = opts.file_path, git_root = git_root, + tabpage = tabpage, load_commit_files = load_commit_files, navigate_next = M.navigate_next, navigate_prev = M.navigate_prev, diff --git a/lua/codediff/ui/keymap_help.lua b/lua/codediff/ui/keymap_help.lua index 6405d597..61ee4ba4 100644 --- a/lua/codediff/ui/keymap_help.lua +++ b/lua/codediff/ui/keymap_help.lua @@ -8,6 +8,8 @@ local ns = vim.api.nvim_create_namespace("codediff-help") -- Key column width (right-aligned keys sit in this space) local KEY_COL = 14 +-- Double click, bound by the explorer and history panels. Not configurable. +local MOUSE_SELECT = "<2-LeftMouse>" -- Inter-column gap for two-column layout. Plain whitespace — no divider glyph. local COL_SEP = " " @@ -23,11 +25,14 @@ end --- Entry = { key, desc } or nil (skipped) --- Section = { title, entries[] } --- Collect a section of keymap entries, skipping nil keys -local function section(title, entries) +-- Collect a section of keymap entries, keeping only keys the session actually +-- has mapped. `is_bound` is what stops this list from drifting: an entry that +-- is disabled in config, or not applicable to the current session shape, is +-- simply not installed and therefore not advertised. +local function section(title, entries, is_bound) local items = {} for _, e in ipairs(entries) do - if e[1] then + if e[1] and is_bound(e[1]) then table.insert(items, e) end end @@ -37,49 +42,53 @@ local function section(title, entries) return { title = title, items = items } end --- Build sections based on the current session mode -local function build_sections(keymaps, is_explorer, is_history, is_conflict) +-- Build sections for the current session. +-- +-- Section inclusion follows the session shape (a standalone diff has no +-- explorer panel). Entry inclusion follows what is actually installed, so a +-- key that is disabled in config, or not applicable to the current view, is +-- never advertised. `is_bound` is what keeps this list from drifting. +local function build_sections(keymaps, is_bound, shape) local sections = {} local km = keymaps.view - -- View section - local view_items = { - { km.quit, "Close codediff tab" }, - { km.next_hunk, "Next hunk" }, - { km.prev_hunk, "Previous hunk" }, - { km.diff_get, "Get change from other buffer" }, - { km.diff_put, "Put change to other buffer" }, - { km.open_in_prev_tab, "Open buffer in previous tab" }, - } - if is_explorer or is_history then - table.insert(view_items, { km.next_file, "Next file" }) - table.insert(view_items, { km.prev_file, "Previous file" }) - end - if is_explorer then - table.insert(view_items, { km.toggle_explorer, "Toggle explorer" }) - table.insert(view_items, { km.focus_explorer, "Focus explorer" }) - table.insert(view_items, { km.toggle_stage, "Stage/unstage current file" }) - table.insert(view_items, { km.toggle_staged_view, "Toggle staged/unstaged view for current file" }) - table.insert(view_items, { km.stage_hunk, "Stage hunk under cursor" }) - table.insert(view_items, { km.unstage_hunk, "Unstage hunk under cursor" }) - table.insert(view_items, { km.discard_hunk, "Discard hunk under cursor" }) - end - table.insert(view_items, { km.toggle_layout, "Toggle inline/side-by-side layout" }) - if km.align_move then - table.insert(view_items, { km.align_move, "Align moved code block" }) - end - table.insert(view_items, { km.toggle_compact, "Toggle compact mode (fold unchanged)" }) - table.insert(view_items, { km.hunk_textobject, "Hunk textobject (visual/operator)" }) - table.insert(view_items, { km.show_help, "Toggle this help" }) - table.insert(sections, section("VIEW", view_items)) - - -- Explorer section - if is_explorer then + table.insert( + sections, + section("VIEW", { + { km.quit, "Close codediff tab" }, + { km.next_hunk, "Next hunk" }, + { km.prev_hunk, "Previous hunk" }, + { km.diff_get, "Get change from other buffer" }, + { km.diff_put, "Put change to other buffer" }, + { km.open_in_prev_tab, "Open buffer in previous tab" }, + { km.next_file, "Next file" }, + { km.prev_file, "Previous file" }, + { km.toggle_explorer, "Toggle explorer" }, + { km.focus_explorer, "Focus explorer" }, + { km.toggle_stage, "Stage/unstage current file" }, + { km.toggle_staged_view, "Toggle staged/unstaged view for current file" }, + { km.stage_hunk, "Stage hunk under cursor" }, + { km.unstage_hunk, "Unstage hunk under cursor" }, + { km.discard_hunk, "Discard hunk under cursor" }, + { km.toggle_layout, "Toggle inline/side-by-side layout" }, + { km.align_move, "Align moved code block" }, + { km.toggle_compact, "Toggle compact mode (fold unchanged)" }, + { km.hunk_textobject, "Hunk textobject (visual/operator)" }, + { km.show_help, "Toggle this help" }, + }, is_bound) + ) + + if shape.explorer then local ekm = keymaps.explorer table.insert( sections, section("EXPLORER", { { ekm.select, "Select / toggle expand" }, + { MOUSE_SELECT, "Select file (double click)" }, + { "j", "Move down / auto-open file" }, + { "k", "Move up / auto-open file" }, + { "", "Move down / auto-open file" }, + { "", "Move up / auto-open file" }, { ekm.hover, "Show full path" }, { ekm.refresh, "Refresh explorer" }, { ekm.toggle_view_mode, "Toggle list/tree view" }, @@ -96,17 +105,17 @@ local function build_sections(keymaps, is_explorer, is_history, is_conflict) { ekm.fold_toggle_recursive, "Toggle fold recursively" }, { ekm.fold_open_all, "Open all folds" }, { ekm.fold_close_all, "Close all folds" }, - }) + }, is_bound) ) end - -- History section - if is_history then + if shape.history then local hkm = keymaps.history table.insert( sections, section("HISTORY", { { hkm.select, "Select commit/file or toggle" }, + { MOUSE_SELECT, "Select commit/file (double click)" }, { hkm.toggle_view_mode, "Toggle list/tree view" }, { hkm.refresh, "Refresh history" }, { hkm.fold_open, "Open fold" }, @@ -117,12 +126,11 @@ local function build_sections(keymaps, is_explorer, is_history, is_conflict) { hkm.fold_toggle_recursive, "Toggle fold recursively" }, { hkm.fold_open_all, "Open all folds" }, { hkm.fold_close_all, "Close all folds" }, - }) + }, is_bound) ) end - -- Conflict section - if is_conflict then + if shape.conflict then local ckm = keymaps.conflict table.insert( sections, @@ -139,7 +147,7 @@ local function build_sections(keymaps, is_explorer, is_history, is_conflict) { ckm.prev_conflict, "Previous conflict" }, { ckm.diffget_incoming, "Get hunk from incoming" }, { ckm.diffget_current, "Get hunk from current" }, - }) + }, is_bound) ) end @@ -192,7 +200,6 @@ local function render_group(sections, col_width) local key_str = string.format("%" .. KEY_COL .. "s", key) local line = key_str .. " → " .. desc table.insert(lines, line) - local row = #lines - 1 table.insert(hls, { row, 0, KEY_COL, "CodeDiffHelpKey" }) table.insert(hls, { row, KEY_COL, KEY_COL + 3, "CodeDiffHelpSep" }) @@ -271,11 +278,17 @@ function M.toggle(tabpage) setup_highlights() local keymaps = config.options.keymaps - local is_explorer = session and session.mode == "explorer" - local is_history = session and session.mode == "history" - local is_conflict = session and session.result_bufnr ~= nil + local function is_bound(key) + return lifecycle.owns_keymap(tabpage, key) + end + + local shape = { + explorer = session and session.mode == "explorer" or false, + history = session and session.mode == "history" or false, + conflict = session and session.result_bufnr ~= nil or false, + } - local sections = build_sections(keymaps, is_explorer, is_history, is_conflict) + local sections = build_sections(keymaps, is_bound, shape) -- Prefer a two-column layout when there are 2+ sections and it fits on screen. -- Falls back to a single column when the terminal is too narrow. diff --git a/lua/codediff/ui/lib/tree_utils.lua b/lua/codediff/ui/lib/tree_utils.lua index 236d4dc2..0c5fc302 100644 --- a/lua/codediff/ui/lib/tree_utils.lua +++ b/lua/codediff/ui/lib/tree_utils.lua @@ -43,11 +43,12 @@ function M.get_root_node(tree) end -- Setup all fold-related keymaps on a tree buffer. --- @param opts table { tree, keymaps, bufnr } +-- @param opts table { tree, keymaps, bufnr, tabpage } function M.setup_fold_keymaps(opts) local tree = opts.tree local keymaps = opts.keymaps local bufnr = opts.bufnr + local tabpage = opts.tabpage local function update_tree_view(node) tree:render() @@ -148,10 +149,13 @@ function M.setup_fold_keymaps(opts) { key = "fold_close_all", fn = fold_close_all, desc = "Close all folds" }, } local map_options = { noremap = true, silent = true, nowait = true } + local lifecycle = require("codediff.ui.lifecycle") for _, binding in ipairs(fold_bindings) do local key = keymaps[binding.key] if key then - vim.keymap.set("n", key, binding.fn, vim.tbl_extend("force", map_options, { buffer = bufnr, desc = binding.desc })) + -- Tree panels are codediff-owned scratch buffers: their mappings cannot + -- leak elsewhere, so they stay installed across tab switches. + lifecycle.set_buf_keymap(tabpage, bufnr, "n", key, binding.fn, vim.tbl_extend("force", map_options, { desc = binding.desc }), { suspendable = false }) end end end diff --git a/lua/codediff/ui/lifecycle/accessors.lua b/lua/codediff/ui/lifecycle/accessors.lua index b29e4b61..68b5adb2 100644 --- a/lua/codediff/ui/lifecycle/accessors.lua +++ b/lua/codediff/ui/lifecycle/accessors.lua @@ -1,6 +1,8 @@ -- Accessor functions (getters and setters) for diff sessions local M = {} -local config = require("codediff.config") +-- Eagerly loaded: accessors run from scheduled callbacks that may execute +-- after the CWD changed, where a first-time require would fail. +local keymap = require("codediff.keymap") -- Lazy require to avoid circular dependency: init → session → accessors → session local function get_active_diffs() @@ -294,6 +296,25 @@ function M.update_buffers(tabpage, original_bufnr, modified_bufnr) local state = require("codediff.ui.lifecycle.state") + -- Hand mappings back to any buffer that is leaving the session. Without this + -- the previous file keeps codediff's keys until the tab is closed. + if sess.keymaps then + local keep = {} + if original_bufnr then + keep[original_bufnr] = true + end + if modified_bufnr then + keep[modified_bufnr] = true + end + if sess.explorer and sess.explorer.bufnr then + keep[sess.explorer.bufnr] = true + end + if sess.result_bufnr then + keep[sess.result_bufnr] = true + end + sess.keymaps:detach_buffers_except(keep) + end + sess.original_bufnr = original_bufnr sess.modified_bufnr = modified_bufnr @@ -453,10 +474,45 @@ function M.confirm_close_with_unsaved(tabpage) end end +--- Registry that owns every mapping this session installs. +--- Created lazily so sessions built by older call paths still work. +--- @param sess table +--- @return table|nil registry +local function registry_for(sess) + if not sess then + return nil + end + if not sess.keymaps then + sess.keymaps = keymap.new("codediff-session") + end + return sess.keymaps +end + +--- Buffers that currently belong to a session, by role. +--- @param sess table +--- @return table roles +local function session_buffers(sess) + local buffers = {} + if sess.original_bufnr and vim.api.nvim_buf_is_valid(sess.original_bufnr) then + buffers.original = sess.original_bufnr + end + if sess.modified_bufnr and vim.api.nvim_buf_is_valid(sess.modified_bufnr) then + buffers.modified = sess.modified_bufnr + end + local explorer = sess.explorer + if explorer and explorer.bufnr and vim.api.nvim_buf_is_valid(explorer.bufnr) then + buffers.panel = explorer.bufnr + end + if sess.result_bufnr and vim.api.nvim_buf_is_valid(sess.result_bufnr) then + buffers.result = sess.result_bufnr + end + return buffers +end + --- Set a keymap on all buffers in the diff tab (both diff buffers + explorer + result) --- This is the unified API for setting tab-wide keymaps --- @param tabpage number Tab page ID ---- @param mode string Keymap mode ('n', 'v', etc.) +--- @param mode string|string[] Keymap mode ('n', 'v', etc.) --- @param lhs string Left-hand side of the keymap --- @param rhs function|string Right-hand side (callback or command) --- @param opts? table Optional keymap options (will be merged with buffer-local defaults) @@ -468,63 +524,133 @@ function M.set_tab_keymap(tabpage, mode, lhs, rhs, opts) return false end - -- Track all buffers that have keymaps set (for cleanup on close) - sess.keymap_buffers = sess.keymap_buffers or {} - - opts = opts or {} + local reg = registry_for(sess) local base_opts = { noremap = true, silent = true, nowait = true } + local merged = vim.tbl_extend("force", base_opts, opts or {}) - if vim.api.nvim_buf_is_valid(sess.original_bufnr) then - vim.keymap.set(mode, lhs, rhs, vim.tbl_extend("force", base_opts, opts, { buffer = sess.original_bufnr })) - sess.keymap_buffers[sess.original_bufnr] = true + for _, bufnr in pairs(session_buffers(sess)) do + reg:claim(bufnr, mode, lhs, rhs, merged) end - if vim.api.nvim_buf_is_valid(sess.modified_bufnr) then - vim.keymap.set(mode, lhs, rhs, vim.tbl_extend("force", base_opts, opts, { buffer = sess.modified_bufnr })) - sess.keymap_buffers[sess.modified_bufnr] = true + return true +end + +--- Set a keymap on one specific buffer, owned by the session's registry. +--- Used for mappings that are scoped to a single role (hunk operations and the +--- hunk textobject on diff panes, conflict actions, panel actions). +--- @param tabpage number +--- @param bufnr number +--- @param mode string|string[] +--- @param lhs string|false|nil Configured binding; false/nil silently disables +--- @param rhs function|string +--- @param opts? table Forwarded verbatim to vim.keymap.set +--- @param meta? table { suspendable = boolean, priority = integer } +--- @return boolean success +function M.set_buf_keymap(tabpage, bufnr, mode, lhs, rhs, opts, meta) + local active_diffs = get_active_diffs() + local sess = active_diffs[tabpage] + if not sess then + -- No session to own the mapping (a panel built outside a diff tab, for + -- example). Fall back to a plain buffer-local mapping so behavior matches + -- the pre-registry implementation rather than silently binding nothing. + local resolved = keymap.resolve(lhs) + if not resolved or not bufnr or not vim.api.nvim_buf_is_valid(bufnr) then + return false + end + return pcall(vim.keymap.set, mode, resolved, rhs, vim.tbl_extend("force", opts or {}, { buffer = bufnr })) end + return registry_for(sess):claim(bufnr, mode, lhs, rhs, opts, meta) +end - local explorer = sess.explorer - if explorer and explorer.bufnr and vim.api.nvim_buf_is_valid(explorer.bufnr) then - vim.keymap.set(mode, lhs, rhs, vim.tbl_extend("force", base_opts, opts, { buffer = explorer.bufnr })) - sess.keymap_buffers[explorer.bufnr] = true +--- True when the session currently owns a mapping for `lhs`. +--- Used by the help popup so it can describe what is really bound rather than +--- a hand-maintained list that drifts. +--- @param tabpage number +--- @param lhs string|false|nil +--- @param mode string|nil Restrict to one mode; any mode when omitted +--- @param bufnr number|nil Restrict to one buffer; any session buffer when omitted +--- @return boolean +function M.owns_keymap(tabpage, lhs, mode, bufnr) + local sess = get_active_diffs()[tabpage] + if not sess or not sess.keymaps then + return false end + return sess.keymaps:owns(lhs, mode, bufnr) +end - if sess.result_bufnr and vim.api.nvim_buf_is_valid(sess.result_bufnr) then - vim.keymap.set(mode, lhs, rhs, vim.tbl_extend("force", base_opts, opts, { buffer = sess.result_bufnr })) - sess.keymap_buffers[sess.result_bufnr] = true +--- Keys the session owns that are expected to appear in the help popup. +--- @param tabpage number +--- @return table canonical lhs -> true +function M.documented_keymaps(tabpage) + local sess = get_active_diffs()[tabpage] + if not sess or not sess.keymaps then + return {} end + return sess.keymaps:documented_keys() +end - return true +--- Release a specific mapping the session installed on a buffer. +--- @param tabpage number +--- @param bufnr number +--- @param mode string|string[] +--- @param lhs string|false|nil +function M.del_buf_keymap(tabpage, bufnr, mode, lhs) + local sess = get_active_diffs()[tabpage] + if not sess or not sess.keymaps then + local resolved = keymap.resolve(lhs) + if resolved and bufnr and vim.api.nvim_buf_is_valid(bufnr) then + for _, m in ipairs(type(mode) == "table" and mode or { mode }) do + pcall(vim.keymap.del, m, resolved, { buffer = bufnr }) + end + end + return + end + sess.keymaps:release(bufnr, mode, lhs) end ---- Remove codediff keymaps from a session's buffers -function M.clear_tab_keymaps(tabpage) - local active_diffs = get_active_diffs() - local sess = active_diffs[tabpage] - if not sess then +--- Release every mapping the session installed on a buffer that is leaving it. +--- @param tabpage number +--- @param bufnr number +function M.detach_keymap_buffer(tabpage, bufnr) + local sess = get_active_diffs()[tabpage] + if not sess or not sess.keymaps then return end + sess.keymaps:detach_buffer(bufnr) +end - local function del_buf_keymaps(bufnr, keys) - if not vim.api.nvim_buf_is_valid(bufnr) then - return - end - for _, key in pairs(keys) do - if key then - pcall(vim.keymap.del, "n", key, { buffer = bufnr }) - end - end +--- Suspend the session's mappings on borrowed (real file) buffers. +--- Called on TabLeave so codediff keys do not appear on those files in other +--- tabs. Panel mappings are registered as non-suspendable and stay installed. +--- @param tabpage number +function M.clear_tab_keymaps(tabpage) + local sess = get_active_diffs()[tabpage] + if not sess or not sess.keymaps then + return end + sess.keymaps:suspend() +end - -- Delete keymaps from ALL buffers that ever had them set (not just current ones) - if sess.keymap_buffers then - for bufnr, _ in pairs(sess.keymap_buffers) do - del_buf_keymaps(bufnr, config.options.keymaps.view) - end +--- Reinstall mappings suspended by clear_tab_keymaps. +--- @param tabpage number +function M.restore_tab_keymaps(tabpage) + local sess = get_active_diffs()[tabpage] + if not sess or not sess.keymaps then + return end + sess.keymaps:resume() +end - sess.keymap_buffers = nil +--- Release every mapping the session installed, handing each key back to +--- whatever owned it before codediff. Idempotent. +--- @param tabpage number +function M.dispose_keymaps(tabpage) + local sess = get_active_diffs()[tabpage] + if not sess or not sess.keymaps then + return + end + sess.keymaps:dispose() + sess.keymaps = nil end --- Setup auto-sync on file switch: automatically update diff when user edits a different file in working buffer diff --git a/lua/codediff/ui/lifecycle/cleanup.lua b/lua/codediff/ui/lifecycle/cleanup.lua index 4a31a07d..74ff5978 100644 --- a/lua/codediff/ui/lifecycle/cleanup.lua +++ b/lua/codediff/ui/lifecycle/cleanup.lua @@ -47,8 +47,8 @@ local function cleanup_diff(tabpage) state.restore_buffer_state(diff.original_bufnr, diff.original_state) state.restore_buffer_state(diff.modified_bufnr, diff.modified_state) - -- Remove tab-scoped keymaps from all tracked buffers - accessors.clear_tab_keymaps(tabpage) + -- Hand every mapped key back to whatever owned it before codediff + accessors.dispose_keymaps(tabpage) -- Call explorer's cleanup function to stop file watchers if diff.explorer and diff.explorer._cleanup_auto_refresh then diff --git a/lua/codediff/ui/lifecycle/init.lua b/lua/codediff/ui/lifecycle/init.lua index bc5b24d2..72e613c9 100644 --- a/lua/codediff/ui/lifecycle/init.lua +++ b/lua/codediff/ui/lifecycle/init.lua @@ -65,7 +65,14 @@ M.set_conflict_blocks = accessors.set_conflict_blocks M.track_conflict_file = accessors.track_conflict_file M.confirm_close_with_unsaved = accessors.confirm_close_with_unsaved M.set_tab_keymap = accessors.set_tab_keymap +M.set_buf_keymap = accessors.set_buf_keymap +M.del_buf_keymap = accessors.del_buf_keymap +M.owns_keymap = accessors.owns_keymap +M.documented_keymaps = accessors.documented_keymaps +M.detach_keymap_buffer = accessors.detach_keymap_buffer M.clear_tab_keymaps = accessors.clear_tab_keymaps +M.restore_tab_keymaps = accessors.restore_tab_keymaps +M.dispose_keymaps = accessors.dispose_keymaps M.setup_auto_sync_on_file_switch = accessors.setup_auto_sync_on_file_switch return M diff --git a/lua/codediff/ui/lifecycle/session.lua b/lua/codediff/ui/lifecycle/session.lua index 6de79d73..3b538fed 100644 --- a/lua/codediff/ui/lifecycle/session.lua +++ b/lua/codediff/ui/lifecycle/session.lua @@ -6,6 +6,9 @@ local config = require("codediff.config") local virtual_file = require("codediff.core.virtual_file") local accessors = require("codediff.ui.lifecycle.accessors") local welcome_window = require("codediff.ui.view.welcome_window") +-- Eagerly loaded: sessions are created from scheduled callbacks that may run +-- after the CWD changed, and a first-time require would fail there. +local keymap = require("codediff.keymap") -- Track active diff sessions -- Structure: { @@ -114,6 +117,9 @@ function M.create_session( result_win = nil, conflict_files = {}, -- Tracks files opened in conflict mode for unsaved warning reapply_keymaps = reapply_keymaps, + -- Owns every mapping this session installs, so teardown can hand each key + -- back to whatever owned it before codediff. + keymaps = keymap.new("codediff-session:" .. tostring(tabpage)), } welcome_window.capture_session_profiles(active_diffs[tabpage]) @@ -198,6 +204,7 @@ function M.create_session( local current_tab = vim.api.nvim_get_current_tabpage() if current_tab == tabpage and active_diffs[tabpage] then local sess = active_diffs[tabpage] + accessors.restore_tab_keymaps(tabpage) if sess.reapply_keymaps then pcall(sess.reapply_keymaps) end diff --git a/lua/codediff/ui/view/compact.lua b/lua/codediff/ui/view/compact.lua index 443020b3..e60507a1 100644 --- a/lua/codediff/ui/view/compact.lua +++ b/lua/codediff/ui/view/compact.lua @@ -109,7 +109,8 @@ local FOLD_KEYS = { --- 3. apply the same fold action in that pane --- --- @param session table -local function setup_fold_sync(session) +--- @param tabpage number +local function setup_fold_sync(session, tabpage) if session.layout == "inline" then return -- single pane, nothing to sync end @@ -126,7 +127,7 @@ local function setup_fold_sync(session) if pane.win and vim.api.nvim_win_is_valid(pane.win) and pane.buf and vim.api.nvim_buf_is_valid(pane.buf) then for _, key in ipairs(FOLD_KEYS) do - vim.keymap.set("n", key, function() + lifecycle.set_buf_keymap(tabpage, pane.buf, "n", key, function() local count = vim.v.count > 0 and tostring(vim.v.count) or "" -- 1. Apply the fold action locally. vim.cmd("normal! " .. count .. key) @@ -168,7 +169,7 @@ local function setup_fold_sync(session) if not ok then vim.notify("[codediff] synced-fold error: " .. tostring(err), vim.log.levels.DEBUG) end - end, { buffer = pane.buf, silent = true, desc = "codediff: synced fold " .. key }) + end, { buffer = pane.buf, silent = true, desc = "codediff: synced fold " .. key }, { suspendable = false, help = false }) end end end @@ -176,14 +177,15 @@ end --- Remove the synced-fold keymap wraps from a session's panes. --- Buffer-local keymaps usually die with the buffer, but for the conflict / ---- explorer paths where buffers persist we need to delete explicitly. +--- explorer paths where buffers persist we need to release them explicitly. --- @param session table -local function teardown_fold_sync(session) +--- @param tabpage number +local function teardown_fold_sync(session, tabpage) local panes = { session.original_bufnr, session.modified_bufnr } for _, buf in ipairs(panes) do if buf and vim.api.nvim_buf_is_valid(buf) then for _, key in ipairs(FOLD_KEYS) do - pcall(vim.keymap.del, "n", key, { buffer = buf }) + lifecycle.del_buf_keymap(tabpage, buf, "n", key) end end end @@ -207,7 +209,8 @@ end --- by enable() and by every re-fold on a diff/file change. Does NOT touch --- session.compact_mode or the saved fold state — that is enable/disable's job. --- @param session table -local function apply_folds(session) +--- @param tabpage number +local function apply_folds(session, tabpage) local changes = session.stored_diff_result and session.stored_diff_result.changes if not changes or #changes == 0 then return @@ -224,7 +227,7 @@ local function apply_folds(session) vim.wo[entry.win].foldminlines = 1 end end - setup_fold_sync(session) + setup_fold_sync(session, tabpage) end --- Enable compact mode for a tabpage @@ -265,7 +268,7 @@ function M.enable(tabpage) end session.compact_mode = true - apply_folds(session) + apply_folds(session, tabpage) return true end @@ -291,7 +294,7 @@ function M.disable(tabpage) visible_lines_by_win[win] = nil end - teardown_fold_sync(session) + teardown_fold_sync(session, tabpage) session.compact_saved_fold_state = nil session.compact_mode = false @@ -357,7 +360,7 @@ function M.refresh(tabpage) return end - apply_folds(session) + apply_folds(session, tabpage) end return M diff --git a/lua/codediff/ui/view/keymaps.lua b/lua/codediff/ui/view/keymaps.lua index 51c387f4..3ef2c632 100644 --- a/lua/codediff/ui/view/keymaps.lua +++ b/lua/codediff/ui/view/keymaps.lua @@ -32,6 +32,9 @@ function M.setup_all_keymaps(tabpage, original_bufnr, modified_bufnr, is_explore local session = lifecycle.get_session(tabpage) local is_history_mode = session and session.mode == "history" local is_inline = session and session.layout == "inline" + -- Merge/conflict view: the result pane exists and do/dp are replaced by the + -- conflict mappings (see codediff.ui.conflict.keymaps). + local is_conflict = session and session.result_bufnr ~= nil -- Helper: Toggle explorer visibility (explorer mode only) local function toggle_explorer() @@ -612,12 +615,15 @@ function M.setup_all_keymaps(tabpage, original_bufnr, modified_bufnr, is_explore lifecycle.set_tab_keymap(tabpage, "n", keymaps.focus_explorer, focus_explorer, { desc = "Focus explorer panel" }) end - -- Diff get/put (do, dp) - layout-aware semantics - if keymaps.diff_get then + -- Diff get/put (do, dp) - layout-aware semantics. + -- Skipped in conflict mode: the merge view uses 2do/3do on the result pane + -- instead. Not claiming the keys here means any mapping the user already had + -- on do/dp is handed back for the duration of the merge, rather than deleted. + if keymaps.diff_get and not is_conflict then local desc = is_inline and "Revert hunk to original" or "Get change from other buffer" lifecycle.set_tab_keymap(tabpage, "n", keymaps.diff_get, diff_get, { desc = desc }) end - if keymaps.diff_put then + if keymaps.diff_put and not is_conflict then local desc = is_inline and "Accept change (no-op in inline)" or "Put change to other buffer" lifecycle.set_tab_keymap(tabpage, "n", keymaps.diff_put, diff_put, { desc = desc }) end @@ -692,13 +698,13 @@ function M.setup_all_keymaps(tabpage, original_bufnr, modified_bufnr, is_explore end for _, bufnr in ipairs(diff_bufs) do if keymaps.stage_hunk then - vim.keymap.set("n", keymaps.stage_hunk, stage_hunk, vim.tbl_extend("force", hunk_opts, { buffer = bufnr, desc = "Stage hunk under cursor" })) + lifecycle.set_buf_keymap(tabpage, bufnr, "n", keymaps.stage_hunk, stage_hunk, vim.tbl_extend("force", hunk_opts, { desc = "Stage hunk under cursor" })) end if keymaps.unstage_hunk then - vim.keymap.set("n", keymaps.unstage_hunk, unstage_hunk, vim.tbl_extend("force", hunk_opts, { buffer = bufnr, desc = "Unstage hunk under cursor" })) + lifecycle.set_buf_keymap(tabpage, bufnr, "n", keymaps.unstage_hunk, unstage_hunk, vim.tbl_extend("force", hunk_opts, { desc = "Unstage hunk under cursor" })) end if keymaps.discard_hunk then - vim.keymap.set("n", keymaps.discard_hunk, discard_hunk, vim.tbl_extend("force", hunk_opts, { buffer = bufnr, desc = "Discard hunk under cursor" })) + lifecycle.set_buf_keymap(tabpage, bufnr, "n", keymaps.discard_hunk, discard_hunk, vim.tbl_extend("force", hunk_opts, { desc = "Discard hunk under cursor" })) end -- Hunk textobject (ih) - select hunk lines in visual/operator-pending mode @@ -722,7 +728,7 @@ function M.setup_all_keymaps(tabpage, original_bufnr, modified_bufnr, is_explore vim.cmd("normal! " .. start_line .. "GV" .. (end_line - 1) .. "G") end - vim.keymap.set({ "o", "x" }, keymaps.hunk_textobject, select_hunk, vim.tbl_extend("force", hunk_opts, { buffer = bufnr, desc = "Hunk textobject" })) + lifecycle.set_buf_keymap(tabpage, bufnr, { "o", "x" }, keymaps.hunk_textobject, select_hunk, vim.tbl_extend("force", hunk_opts, { desc = "Hunk textobject" })) end end diff --git a/tests/fixtures/keymap_matrix.txt b/tests/fixtures/keymap_matrix.txt new file mode 100644 index 00000000..5d7fcb8a --- /dev/null +++ b/tests/fixtures/keymap_matrix.txt @@ -0,0 +1,342 @@ +# codediff keymap golden matrix +# role x mode x lhs x desc for each session shape. +# Regenerate with CODEDIFF_WRITE_KEYMAP_GOLDEN=1 after an intended change. + +## standalone side-by-side +original n [c Previous hunk +original n \hr Discard hunk under cursor +original n \hs Stage hunk under cursor +original n \hu Unstage hunk under cursor +original n ]c Next hunk +original n do Get change from other buffer +original n dp Put change to other buffer +original n g? Show keymap help +original n gc Toggle compact mode +original n gf Open buffer in previous tab +original n q Close codediff tab +original n t Toggle diff layout +original x ih Hunk textobject +original o ih Hunk textobject +original v ih Hunk textobject +modified n [c Previous hunk +modified n \hr Discard hunk under cursor +modified n \hs Stage hunk under cursor +modified n \hu Unstage hunk under cursor +modified n ]c Next hunk +modified n do Get change from other buffer +modified n dp Put change to other buffer +modified n g? Show keymap help +modified n gc Toggle compact mode +modified n gf Open buffer in previous tab +modified n q Close codediff tab +modified n t Toggle diff layout +modified x ih Hunk textobject +modified o ih Hunk textobject +modified v ih Hunk textobject + +## standalone inline +original n [c Previous hunk +original n \hr Discard hunk under cursor +original n \hs Stage hunk under cursor +original n \hu Unstage hunk under cursor +original n ]c Next hunk +original n do Revert hunk to original +original n dp Accept change (no-op in inline) +original n g? Show keymap help +original n gc Toggle compact mode +original n gf Open buffer in previous tab +original n q Close codediff tab +original n t Toggle diff layout +original x ih Hunk textobject +original o ih Hunk textobject +original v ih Hunk textobject +modified n [c Previous hunk +modified n \hr Discard hunk under cursor +modified n \hs Stage hunk under cursor +modified n \hu Unstage hunk under cursor +modified n ]c Next hunk +modified n do Revert hunk to original +modified n dp Accept change (no-op in inline) +modified n g? Show keymap help +modified n gc Toggle compact mode +modified n gf Open buffer in previous tab +modified n q Close codediff tab +modified n t Toggle diff layout +modified x ih Hunk textobject +modified o ih Hunk textobject +modified v ih Hunk textobject + +## explorer side-by-side +original n - Toggle stage/unstage +original n [c Previous hunk +original n [f Previous file +original n \b Toggle explorer visibility +original n \e Focus explorer panel +original n \hr Discard hunk under cursor +original n \hs Stage hunk under cursor +original n \hu Unstage hunk under cursor +original n ]c Next hunk +original n ]f Next file +original n do Get change from other buffer +original n dp Put change to other buffer +original n g? Show keymap help +original n gS Toggle staged/unstaged view for current file +original n gc Toggle compact mode +original n gf Open buffer in previous tab +original n q Close codediff tab +original n t Toggle diff layout +original x ih Hunk textobject +original o ih Hunk textobject +original v ih Hunk textobject +modified n - Toggle stage/unstage +modified n [c Previous hunk +modified n [f Previous file +modified n \b Toggle explorer visibility +modified n \e Focus explorer panel +modified n \hr Discard hunk under cursor +modified n \hs Stage hunk under cursor +modified n \hu Unstage hunk under cursor +modified n ]c Next hunk +modified n ]f Next file +modified n do Get change from other buffer +modified n dp Put change to other buffer +modified n g? Show keymap help +modified n gS Toggle staged/unstaged view for current file +modified n gc Toggle compact mode +modified n gf Open buffer in previous tab +modified n q Close codediff tab +modified n t Toggle diff layout +modified x ih Hunk textobject +modified o ih Hunk textobject +modified v ih Hunk textobject +panel n - Toggle stage/unstage +panel n <2-LeftMouse> Select file +panel n Select/toggle entry +panel n K Show full path +panel n R Refresh explorer +panel n S Stage all files +panel n U Unstage all files +panel n X Restore/discard changes +panel n [c Previous hunk +panel n [f Previous file +panel n \b Toggle explorer visibility +panel n \e Focus explorer panel +panel n ]c Next hunk +panel n ]f Next file +panel n do Get change from other buffer +panel n dp Put change to other buffer +panel n g? Show keymap help +panel n gS Toggle staged/unstaged view for current file +panel n gc Toggle compact mode +panel n gf Open buffer in previous tab +panel n gs Toggle Staged Changes visibility +panel n gu Toggle Changes visibility +panel n i Toggle list/tree view +panel n q Close codediff tab +panel n t Toggle diff layout +panel n zA Toggle fold recursively +panel n zC Close fold recursively +panel n zM Close all folds +panel n zO Open fold recursively +panel n zR Open all folds +panel n za Toggle fold +panel n zc Close fold +panel n zo Open fold + +## explorer inline +original n - Toggle stage/unstage +original n [c Previous hunk +original n [f Previous file +original n \b Toggle explorer visibility +original n \e Focus explorer panel +original n \hr Discard hunk under cursor +original n \hs Stage hunk under cursor +original n \hu Unstage hunk under cursor +original n ]c Next hunk +original n ]f Next file +original n do Revert hunk to original +original n dp Accept change (no-op in inline) +original n g? Show keymap help +original n gS Toggle staged/unstaged view for current file +original n gc Toggle compact mode +original n gf Open buffer in previous tab +original n q Close codediff tab +original n t Toggle diff layout +original x ih Hunk textobject +original o ih Hunk textobject +original v ih Hunk textobject +modified n - Toggle stage/unstage +modified n [c Previous hunk +modified n [f Previous file +modified n \b Toggle explorer visibility +modified n \e Focus explorer panel +modified n \hr Discard hunk under cursor +modified n \hs Stage hunk under cursor +modified n \hu Unstage hunk under cursor +modified n ]c Next hunk +modified n ]f Next file +modified n do Revert hunk to original +modified n dp Accept change (no-op in inline) +modified n g? Show keymap help +modified n gS Toggle staged/unstaged view for current file +modified n gc Toggle compact mode +modified n gf Open buffer in previous tab +modified n q Close codediff tab +modified n t Toggle diff layout +modified x ih Hunk textobject +modified o ih Hunk textobject +modified v ih Hunk textobject +panel n - Toggle stage/unstage +panel n <2-LeftMouse> Select file +panel n Select/toggle entry +panel n K Show full path +panel n R Refresh explorer +panel n S Stage all files +panel n U Unstage all files +panel n X Restore/discard changes +panel n [c Previous hunk +panel n [f Previous file +panel n \b Toggle explorer visibility +panel n \e Focus explorer panel +panel n ]c Next hunk +panel n ]f Next file +panel n do Revert hunk to original +panel n dp Accept change (no-op in inline) +panel n g? Show keymap help +panel n gS Toggle staged/unstaged view for current file +panel n gc Toggle compact mode +panel n gf Open buffer in previous tab +panel n gs Toggle Staged Changes visibility +panel n gu Toggle Changes visibility +panel n i Toggle list/tree view +panel n q Close codediff tab +panel n t Toggle diff layout +panel n zA Toggle fold recursively +panel n zC Close fold recursively +panel n zM Close all folds +panel n zO Open fold recursively +panel n zR Open all folds +panel n za Toggle fold +panel n zc Close fold +panel n zo Open fold + +## conflict +original n [c Previous hunk +original n [x Previous conflict +original n \cB Accept ALL both changes +original n \cO Accept ALL current changes +original n \cT Accept ALL incoming changes +original n \cX Discard ALL, reset to base +original n \cb Accept both changes +original n \co Accept current change +original n \ct Accept incoming change +original n \cx Discard changes (keep base) +original n \hr Discard hunk under cursor +original n \hs Stage hunk under cursor +original n \hu Unstage hunk under cursor +original n ]c Next hunk +original n ]x Next conflict +original n g? Show keymap help +original n gc Toggle compact mode +original n gf Open buffer in previous tab +original n q Close codediff tab +original n t Toggle diff layout +original x ih Hunk textobject +original o ih Hunk textobject +original v ih Hunk textobject +modified n [c Previous hunk +modified n [x Previous conflict +modified n \cB Accept ALL both changes +modified n \cO Accept ALL current changes +modified n \cT Accept ALL incoming changes +modified n \cX Discard ALL, reset to base +modified n \cb Accept both changes +modified n \co Accept current change +modified n \ct Accept incoming change +modified n \cx Discard changes (keep base) +modified n \hr Discard hunk under cursor +modified n \hs Stage hunk under cursor +modified n \hu Unstage hunk under cursor +modified n ]c Next hunk +modified n ]x Next conflict +modified n g? Show keymap help +modified n gc Toggle compact mode +modified n gf Open buffer in previous tab +modified n q Close codediff tab +modified n t Toggle diff layout +modified x ih Hunk textobject +modified o ih Hunk textobject +modified v ih Hunk textobject +result n 2do Get hunk from incoming (2do) +result n 3do Get hunk from current (3do) +result n [c Previous hunk +result n [x Previous conflict +result n \cB Accept ALL both changes +result n \cO Accept ALL current changes +result n \cT Accept ALL incoming changes +result n \cX Discard ALL, reset to base +result n \cb Accept both changes +result n \co Accept current change +result n \ct Accept incoming change +result n \cx Discard changes (keep base) +result n ]c Next hunk +result n ]x Next conflict +result n g? Show keymap help +result n gc Toggle compact mode +result n gf Open buffer in previous tab +result n q Close codediff tab +result n t Toggle diff layout + +## compact side-by-side +original n [c Previous hunk +original n \hr Discard hunk under cursor +original n \hs Stage hunk under cursor +original n \hu Unstage hunk under cursor +original n ]c Next hunk +original n do Get change from other buffer +original n dp Put change to other buffer +original n g? Show keymap help +original n gc Toggle compact mode +original n gf Open buffer in previous tab +original n q Close codediff tab +original n t Toggle diff layout +original n zA codediff: synced fold zA +original n zC codediff: synced fold zC +original n zM codediff: synced fold zM +original n zO codediff: synced fold zO +original n zR codediff: synced fold zR +original n zX codediff: synced fold zX +original n za codediff: synced fold za +original n zc codediff: synced fold zc +original n zo codediff: synced fold zo +original n zv codediff: synced fold zv +original n zx codediff: synced fold zx +original x ih Hunk textobject +original o ih Hunk textobject +original v ih Hunk textobject +modified n [c Previous hunk +modified n \hr Discard hunk under cursor +modified n \hs Stage hunk under cursor +modified n \hu Unstage hunk under cursor +modified n ]c Next hunk +modified n do Get change from other buffer +modified n dp Put change to other buffer +modified n g? Show keymap help +modified n gc Toggle compact mode +modified n gf Open buffer in previous tab +modified n q Close codediff tab +modified n t Toggle diff layout +modified n zA codediff: synced fold zA +modified n zC codediff: synced fold zC +modified n zM codediff: synced fold zM +modified n zO codediff: synced fold zO +modified n zR codediff: synced fold zR +modified n zX codediff: synced fold zX +modified n za codediff: synced fold za +modified n zc codediff: synced fold zc +modified n zo codediff: synced fold zo +modified n zv codediff: synced fold zv +modified n zx codediff: synced fold zx +modified x ih Hunk textobject +modified o ih Hunk textobject +modified v ih Hunk textobject diff --git a/tests/keymap_matrix.lua b/tests/keymap_matrix.lua new file mode 100644 index 00000000..352893db --- /dev/null +++ b/tests/keymap_matrix.lua @@ -0,0 +1,173 @@ +-- Golden keymap matrix harness. +-- +-- Captures every buffer-local mapping installed on a CodeDiff session's +-- buffers, keyed by *role* rather than buffer number, so the result is stable +-- across runs and can be committed as a fixture. +-- +-- Purpose: prove that a refactor of the keymap layer does not change which +-- mappings land on which buffers. Any intended change shows up as a small, +-- reviewable diff of the fixture. + +local M = {} + +-- Modes worth capturing. 'v' is visual+select, 'x' is visual-only; both are +-- listed because codediff binds the hunk textobject via { "o", "x" }. +M.MODES = { "n", "x", "o", "v", "i" } + +--- Resolve the role -> bufnr map for a session. +--- The explorer and history panels share session.explorer, so both are +--- reported under the single "panel" role. +--- @param tabpage number +--- @return table roles +function M.roles(tabpage) + local lifecycle = require("codediff.ui.lifecycle") + local session = lifecycle.get_session(tabpage) + if not session then + return {} + end + + local roles = {} + if session.original_bufnr then + roles.original = session.original_bufnr + end + if session.modified_bufnr then + roles.modified = session.modified_bufnr + end + if session.explorer and session.explorer.bufnr then + roles.panel = session.explorer.bufnr + end + if session.result_bufnr then + roles.result = session.result_bufnr + end + return roles +end + +--- Render a key sequence in a stable, readable form ("hs" not " hs"). +--- Neovim already renders special keys such as `` and `<2-LeftMouse>` in +--- printable form, so those are used verbatim. Only sequences containing a +--- space or a control byte (typically an expanded ``) are translated, +--- which avoids double-encoding an already-readable name into `CR>`. +--- @param map table Entry from nvim_buf_get_keymap +--- @return string +local function display_key(map) + local lhs = map.lhs or "" + if lhs ~= "" and lhs:match("^%g+$") then + return lhs + end + if vim.fn.exists("*keytrans") == 1 then + local ok, translated = pcall(vim.fn.keytrans, lhs) + if ok and translated ~= "" then + return translated + end + end + return lhs +end + +--- Capture all buffer-local mappings for one buffer. +--- +--- Each entry is verified reachable: a mapping can exist in the keymap list +--- yet be unreachable by the key press if its lhs was double-encoded. Querying +--- through `maparg` with the rendered name proves the key really resolves. +--- @param bufnr number +--- @return table mode -> { { lhs, desc }, ... } +function M.capture_buffer(bufnr) + local by_mode = {} + if not bufnr or not vim.api.nvim_buf_is_valid(bufnr) then + return by_mode + end + + for _, mode in ipairs(M.MODES) do + local entries = {} + for _, map in ipairs(vim.api.nvim_buf_get_keymap(bufnr, mode)) do + local shown = display_key(map) + local reachable = vim.api.nvim_buf_call(bufnr, function() + local found = vim.fn.maparg(shown, mode, false, true) + return type(found) == "table" and next(found) ~= nil and found.buffer == 1 + end) + table.insert(entries, { + lhs = shown, + desc = map.desc, + unreachable = not reachable, + }) + end + table.sort(entries, function(a, b) + if a.lhs ~= b.lhs then + return a.lhs < b.lhs + end + return (a.desc or "") < (b.desc or "") + end) + if #entries > 0 then + by_mode[mode] = entries + end + end + + return by_mode +end + +--- Capture the full role x mode x lhs matrix for a session. +--- @param tabpage number +--- @return table matrix +function M.capture(tabpage) + local matrix = {} + for role, bufnr in pairs(M.roles(tabpage)) do + matrix[role] = M.capture_buffer(bufnr) + end + return matrix +end + +-- Deterministic role ordering for rendering. +local ROLE_ORDER = { "original", "modified", "panel", "result" } + +--- Render a captured matrix into stable, diffable text lines. +--- @param label string Scenario name +--- @param matrix table Result of M.capture +--- @return string[] lines +function M.render(label, matrix) + local lines = { "## " .. label } + + for _, role in ipairs(ROLE_ORDER) do + local by_mode = matrix[role] + if by_mode then + for _, mode in ipairs(M.MODES) do + for _, entry in ipairs(by_mode[mode] or {}) do + -- An unreachable entry is a bug: the mapping is registered under a + -- key sequence the user cannot actually press. + local suffix = entry.unreachable and " [UNREACHABLE]" or "" + table.insert(lines, string.format("%-8s %-2s %-14s %s%s", role, mode, entry.lhs, entry.desc or "-", suffix)) + end + end + end + end + + if #lines == 1 then + table.insert(lines, "(no mappings captured)") + end + return lines +end + +--- Capture and render in one step. +--- @param label string +--- @param tabpage number +--- @return string[] lines +function M.snapshot(label, tabpage) + return M.render(label, M.capture(tabpage)) +end + +--- Collect buffer-local mappings for an arbitrary buffer outside a session. +--- Used to assert that pre-existing user mappings survive a session, and that +--- codediff mappings do not outlive it. +--- @param bufnr number +--- @param mode string +--- @return table lhs -> desc ("-" when absent) +function M.map_index(bufnr, mode) + local index = {} + if not bufnr or not vim.api.nvim_buf_is_valid(bufnr) then + return index + end + for _, map in ipairs(vim.api.nvim_buf_get_keymap(bufnr, mode)) do + index[map.lhs] = map.desc or "-" + end + return index +end + +return M diff --git a/tests/ui/keymap/golden_matrix_spec.lua b/tests/ui/keymap/golden_matrix_spec.lua new file mode 100644 index 00000000..23eab992 --- /dev/null +++ b/tests/ui/keymap/golden_matrix_spec.lua @@ -0,0 +1,259 @@ +-- Golden keymap matrix: locks which mappings land on which session buffers. +-- +-- This is the safety net for the keymap registry refactor. It captures the +-- role x mode x lhs x desc matrix for every session shape and compares it +-- against a committed fixture. A refactor that preserves behavior produces an +-- identical fixture; an intended change produces a small, reviewable diff. +-- +-- Regenerate after an intentional change: +-- CODEDIFF_WRITE_KEYMAP_GOLDEN=1 nvim --headless --noplugin -u tests/init.lua \ +-- -c "lua require('plenary.test_harness').test_file('tests/ui/keymap/golden_matrix_spec.lua', { minimal_init = 'tests/init.lua' })" + +local h = dofile("tests/helpers.lua") +local matrix = dofile("tests/keymap_matrix.lua") +local path = require("codediff.core.path") + +h.ensure_plugin_loaded() + +-- Load every module the scenarios need up front. Scenario setup runs git in +-- temp directories, and resolving modules lazily from there is fragile. +local commands = require("codediff.commands") +local view = require("codediff.ui.view") +local lifecycle = require("codediff.ui.lifecycle") +local compact = require("codediff.ui.view.compact") + +local FIXTURE = "tests/fixtures/keymap_matrix.txt" +local WRITE_MODE = vim.env.CODEDIFF_WRITE_KEYMAP_GOLDEN == "1" + +-- Mappings capture at creation time, so the fixture would otherwise +-- depend on whoever ran it. Pin the leader for every scenario. +local LEADER = "\\" + +-- config.setup() merges into the *current* options, so it accumulates across +-- calls. Reset to defaults first so every scenario starts from a known state. +local function reset_config(opts) + vim.g.mapleader = LEADER + local config = require("codediff.config") + config.options = vim.deepcopy(config.defaults) + require("codediff").setup(opts or {}) + require("codediff.ui.highlights").setup() +end + +local function temp_file(suffix, lines) + local file = vim.fn.tempname() .. suffix + vim.fn.writefile(lines, file) + return file +end + +local ORIGINAL_LINES = { "line 1", "line 2", "line 3", "line 4", "line 5" } +local MODIFIED_LINES = { "line 1", "CHANGED 2", "line 3", "line 4", "CHANGED 5" } + +--- Wait until a session on `tabpage` has a computed diff. +local function wait_for_diff(tabpage, timeout_ms) + return vim.wait(timeout_ms or 10000, function() + local session = lifecycle.get_session(tabpage) + return session ~= nil and session.stored_diff_result ~= nil + end, 50) +end + +-- --------------------------------------------------------------------------- +-- Scenario builders. Each returns the tabpage holding the session plus a +-- teardown function. +-- --------------------------------------------------------------------------- + +local function scenario_standalone(layout) + reset_config({ diff = { layout = layout } }) + + local left = temp_file("_golden_left.txt", ORIGINAL_LINES) + local right = temp_file("_golden_right.txt", MODIFIED_LINES) + + view.create({ + mode = "standalone", + git_root = nil, + original = path.make_ref(left, nil), + modified = path.make_ref(right, nil), + original_revision = nil, + modified_revision = nil, + }) + + local tabpage = vim.api.nvim_get_current_tabpage() + assert.is_true(wait_for_diff(tabpage), "standalone " .. layout .. " session should be ready") + + return tabpage, function() + vim.fn.delete(left) + vim.fn.delete(right) + end +end + +local function scenario_explorer(layout) + reset_config({ diff = { layout = layout } }) + + local repo = h.create_temp_git_repo() + repo.write_file("test.txt", ORIGINAL_LINES) + repo.git("add .") + repo.git("commit -m initial") + repo.write_file("test.txt", MODIFIED_LINES) + + vim.cmd("edit " .. vim.fn.fnameescape(repo.path("test.txt"))) + commands.vscode_diff({ fargs = {} }) + + local tabpage + local ready = vim.wait(15000, function() + for _, tp in ipairs(vim.api.nvim_list_tabpages()) do + local session = lifecycle.get_session(tp) + if session and session.explorer and session.explorer.bufnr then + tabpage = tp + return true + end + end + return false + end, 50) + assert.is_true(ready, "explorer session should be created") + + -- Select the changed file so the diff panes hold real buffers. + local explorer = lifecycle.get_session(tabpage).explorer + explorer.on_file_select({ path = "test.txt", group = "unstaged", status = "M", git_root = repo.dir }) + assert.is_true(wait_for_diff(tabpage), "explorer diff should be ready") + + return tabpage, function() + repo.cleanup() + end +end + +local function scenario_conflict() + reset_config({ diff = { layout = "side-by-side" } }) + + local repo = h.create_temp_git_repo() + repo.write_file("conf.txt", ORIGINAL_LINES) + repo.git("add -A") + repo.git("commit -m base") + repo.git("checkout -b feature") + repo.write_file("conf.txt", { "FEATURE", "line 2", "line 3", "line 4", "line 5" }) + repo.git("commit -am feature") + repo.git("checkout main") + repo.write_file("conf.txt", { "MAIN", "line 2", "line 3", "line 4", "line 5" }) + repo.git("commit -am main") + local merge_out = repo.git("merge feature --no-edit") + assert.is_true(merge_out:find("CONFLICT", 1, true) ~= nil, "merge must conflict") + + vim.cmd("edit " .. vim.fn.fnameescape(repo.path("conf.txt"))) + + local ready = false + view.create({ + mode = "standalone", + git_root = repo.dir, + original = path.make_ref("conf.txt", repo.dir), + modified = path.make_ref("conf.txt", repo.dir), + original_revision = ":3", + modified_revision = ":2", + conflict = true, + }, "", function() + ready = true + end) + assert.is_true(vim.wait(15000, function() + return ready + end, 50), "conflict view should become ready") + + local tabpage + for _, tp in ipairs(vim.api.nvim_list_tabpages()) do + local session = lifecycle.get_session(tp) + if session and session.result_bufnr then + tabpage = tp + break + end + end + assert.is_not_nil(tabpage, "conflict session should exist") + + return tabpage, function() + repo.cleanup() + end +end + +local function scenario_compact() + reset_config({ diff = { layout = "side-by-side" } }) + + local left = temp_file("_golden_compact_left.txt", ORIGINAL_LINES) + local right = temp_file("_golden_compact_right.txt", MODIFIED_LINES) + + view.create({ + mode = "standalone", + git_root = nil, + original = path.make_ref(left, nil), + modified = path.make_ref(right, nil), + }) + + local tabpage = vim.api.nvim_get_current_tabpage() + assert.is_true(wait_for_diff(tabpage), "compact base session should be ready") + assert.is_true(compact.enable(tabpage), "compact mode should enable") + + return tabpage, function() + vim.fn.delete(left) + vim.fn.delete(right) + end +end + +-- --------------------------------------------------------------------------- + +describe("keymap golden matrix", function() + after_each(function() + lifecycle.cleanup_all() + h.close_extra_tabs() + end) + + it("matches the committed fixture for every session shape", function() + local scenarios = { + { "standalone side-by-side", function() return scenario_standalone("side-by-side") end }, + { "standalone inline", function() return scenario_standalone("inline") end }, + { "explorer side-by-side", function() return scenario_explorer("side-by-side") end }, + { "explorer inline", function() return scenario_explorer("inline") end }, + { "conflict", scenario_conflict }, + { "compact side-by-side", scenario_compact }, + } + + local lines = { + "# codediff keymap golden matrix", + "# role x mode x lhs x desc for each session shape.", + "# Regenerate with CODEDIFF_WRITE_KEYMAP_GOLDEN=1 after an intended change.", + } + + for _, scenario in ipairs(scenarios) do + local label, build = scenario[1], scenario[2] + local tabpage, teardown = build() + + table.insert(lines, "") + vim.list_extend(lines, matrix.snapshot(label, tabpage)) + + lifecycle.cleanup_all() + h.close_extra_tabs() + teardown() + end + + local rendered = lines + + if WRITE_MODE then + vim.fn.mkdir(vim.fn.fnamemodify(FIXTURE, ":h"), "p") + vim.fn.writefile(rendered, FIXTURE) + print("wrote golden fixture: " .. FIXTURE) + return + end + + assert.is_true(vim.fn.filereadable(FIXTURE) == 1, "missing fixture " .. FIXTURE .. "; regenerate with CODEDIFF_WRITE_KEYMAP_GOLDEN=1") + + local expected = vim.fn.readfile(FIXTURE) + for i = 1, math.max(#expected, #rendered) do + if expected[i] ~= rendered[i] then + assert.is_true( + false, + string.format( + "keymap matrix drifted from %s\nfirst difference at line %d:\n expected: %s\n actual: %s", + FIXTURE, + i, + expected[i] or "", + rendered[i] or "" + ) + ) + end + end + assert.equals(#expected, #rendered, "keymap matrix line count drifted from " .. FIXTURE) + end) +end) diff --git a/tests/ui/keymap/keymap_coverage_spec.lua b/tests/ui/keymap/keymap_coverage_spec.lua new file mode 100644 index 00000000..1ba83f31 --- /dev/null +++ b/tests/ui/keymap/keymap_coverage_spec.lua @@ -0,0 +1,313 @@ +-- Coverage contract: every configured keymap must be *reachable* in every +-- session shape that is supposed to provide it. +-- +-- Reachability is checked through `maparg`, not `nvim_buf_get_keymap`: a +-- mapping can be listed yet unreachable if its lhs was encoded twice, which is +-- exactly the regression that broke <2-LeftMouse>, and . + +local h = dofile("tests/helpers.lua") +local path = require("codediff.core.path") +h.ensure_plugin_loaded() + +local view = require("codediff.ui.view") +local lifecycle = require("codediff.ui.lifecycle") +local commands = require("codediff.commands") +local config = require("codediff.config") + +local function reset_config(opts) + vim.g.mapleader = "\\" + config.options = vim.deepcopy(config.defaults) + require("codediff").setup(opts or {}) + require("codediff.ui.highlights").setup() +end + +local function temp_file(suffix, lines) + local f = vim.fn.tempname() .. suffix + vim.fn.writefile(lines, f) + return f +end + +local function wait_diff(tp) + return vim.wait(10000, function() + local s = lifecycle.get_session(tp) + return s and s.stored_diff_result ~= nil + end, 50) +end + +-- Is `key` reachable in `mode` on any of the session's buffers? +local function reachable(tp, key, modes) + local s = lifecycle.get_session(tp) + -- Build without holes: a nil mid-table would make ipairs stop early and + -- silently skip later roles (this bit me once already). + local bufs = {} + for _, b in pairs({ s.original_bufnr, s.modified_bufnr, s.explorer and s.explorer.bufnr or nil, s.result_bufnr }) do + table.insert(bufs, b) + end + for _, buf in ipairs(bufs) do + if buf and vim.api.nvim_buf_is_valid(buf) then + for _, mode in ipairs(modes) do + local m = vim.api.nvim_buf_call(buf, function() + return vim.fn.maparg(key, mode, false, true) + end) + if type(m) == "table" and next(m) ~= nil and m.buffer == 1 then + return true + end + end + end + end + return false +end + +local NORMAL = { "n" } +local TEXTOBJ = { "o", "x" } + +-- scope -> { config_key -> { expected in these shapes } } +-- "expected" lists the shapes where the mapping MUST be reachable. +local EXPECT = { + view = { + quit = { "standalone", "inline", "explorer", "history", "conflict" }, + next_hunk = { "standalone", "inline", "explorer", "history", "conflict" }, + prev_hunk = { "standalone", "inline", "explorer", "history", "conflict" }, + diff_get = { "standalone", "inline", "explorer", "history" }, + diff_put = { "standalone", "inline", "explorer", "history" }, + open_in_prev_tab = { "standalone", "inline", "explorer", "history", "conflict" }, + toggle_layout = { "standalone", "inline", "explorer", "history", "conflict" }, + toggle_compact = { "standalone", "inline", "explorer", "history", "conflict" }, + show_help = { "standalone", "inline", "explorer", "history", "conflict" }, + stage_hunk = { "standalone", "inline", "explorer", "history", "conflict" }, + unstage_hunk = { "standalone", "inline", "explorer", "history", "conflict" }, + discard_hunk = { "standalone", "inline", "explorer", "history", "conflict" }, + next_file = { "explorer", "history" }, + prev_file = { "explorer", "history" }, + toggle_explorer = { "explorer" }, + focus_explorer = { "explorer" }, + toggle_stage = { "explorer" }, + toggle_staged_view = { "explorer" }, + align_move = { "moves" }, + }, + explorer = { + select = { "explorer" }, + hover = { "explorer" }, + refresh = { "explorer" }, + toggle_view_mode = { "explorer" }, + stage_all = { "explorer" }, + unstage_all = { "explorer" }, + restore = { "explorer" }, + toggle_changes = { "explorer" }, + toggle_staged = { "explorer" }, + fold_open = { "explorer" }, + fold_open_recursive = { "explorer" }, + fold_close = { "explorer" }, + fold_close_recursive = { "explorer" }, + fold_toggle = { "explorer" }, + fold_toggle_recursive = { "explorer" }, + fold_open_all = { "explorer" }, + fold_close_all = { "explorer" }, + }, + history = { + select = { "history" }, + toggle_view_mode = { "history" }, + refresh = { "history" }, + fold_open = { "history" }, + fold_open_recursive = { "history" }, + fold_close = { "history" }, + fold_close_recursive = { "history" }, + fold_toggle = { "history" }, + fold_toggle_recursive = { "history" }, + fold_open_all = { "history" }, + fold_close_all = { "history" }, + }, + conflict = { + accept_incoming = { "conflict" }, + accept_current = { "conflict" }, + accept_both = { "conflict" }, + discard = { "conflict" }, + accept_all_incoming = { "conflict" }, + accept_all_current = { "conflict" }, + accept_all_both = { "conflict" }, + discard_all = { "conflict" }, + next_conflict = { "conflict" }, + prev_conflict = { "conflict" }, + diffget_incoming = { "conflict" }, + diffget_current = { "conflict" }, + }, +} + +local failures = {} +local checked = 0 + +local function run_audit() + local function check_shape(shape, tp) + for scope, entries in pairs(EXPECT) do + for cfg_key, shapes in pairs(entries) do + if vim.tbl_contains(shapes, shape) then + local key = config.options.keymaps[scope][cfg_key] + if type(key) == "string" then + checked = checked + 1 + if not reachable(tp, key, NORMAL) then + table.insert(failures, string.format("%-10s %s.%-22s %-14s NOT REACHABLE", shape, scope, cfg_key, key)) + end + end + end + end + end + -- textobject lives in operator-pending / visual + local ih = config.options.keymaps.view.hunk_textobject + if type(ih) == "string" then + checked = checked + 1 + if not reachable(tp, ih, TEXTOBJ) then + table.insert(failures, string.format("%-10s %-33s %-14s NOT REACHABLE (o/x)", shape, "view.hunk_textobject", ih)) + end + end + -- panel double click + if shape == "explorer" or shape == "history" then + checked = checked + 1 + if not reachable(tp, "<2-LeftMouse>", NORMAL) then + table.insert(failures, string.format("%-10s %-33s %-14s NOT REACHABLE", shape, "panel.double_click", "<2-LeftMouse>")) + end + end + end + + local function finish(tp, teardown) + lifecycle.cleanup_all() + h.close_extra_tabs() + if teardown then + teardown() + end + end + + -- standalone (side-by-side / inline / moves) + for _, spec in ipairs({ + { "standalone", { diff = { layout = "side-by-side" } } }, + { "inline", { diff = { layout = "inline" } } }, + { "moves", { diff = { compute_moves = true } } }, + }) do + local shape, opts = spec[1], spec[2] + reset_config(opts) + local L = temp_file("_x.txt", { "alpha1", "alpha2", "alpha3", "alpha4", "alpha5", "u1", "u2", "u3", "b1", "b2", "b3", "b4", "b5" }) + local R = temp_file("_y.txt", { "u1", "u2", "u3", "b1", "b2", "b3", "b4", "b5", "alpha1", "alpha2", "alpha3", "alpha4", "alpha5" }) + view.create({ mode = "standalone", original = path.make_ref(L, nil), modified = path.make_ref(R, nil) }) + local tp = vim.api.nvim_get_current_tabpage() + wait_diff(tp) + check_shape(shape, tp) + finish(tp, function() + vim.fn.delete(L) + vim.fn.delete(R) + end) + end + + -- explorer + do + reset_config({}) + local repo = h.create_temp_git_repo() + repo.write_file("t.txt", { "a", "b", "c" }) + repo.git("add .") + repo.git("commit -m i") + repo.write_file("t.txt", { "a", "X", "c" }) + vim.cmd("edit " .. vim.fn.fnameescape(repo.path("t.txt"))) + commands.vscode_diff({ fargs = {} }) + local tp + vim.wait(15000, function() + for _, t in ipairs(vim.api.nvim_list_tabpages()) do + local s = lifecycle.get_session(t) + if s and s.explorer and s.explorer.bufnr then + tp = t + return true + end + end + return false + end, 50) + local ex = lifecycle.get_session(tp).explorer + ex.on_file_select({ path = "t.txt", group = "unstaged", status = "M", git_root = repo.dir }) + wait_diff(tp) + check_shape("explorer", tp) + finish(tp, repo.cleanup) + end + + -- history + do + reset_config({}) + local repo = h.create_temp_git_repo() + repo.write_file("t.txt", { "a" }) + repo.git("add .") + repo.git("commit -m one") + repo.write_file("t.txt", { "a", "b" }) + repo.git("commit -am two") + vim.cmd("edit " .. vim.fn.fnameescape(repo.path("t.txt"))) + commands.vscode_diff({ fargs = { "history" } }) + local tp + vim.wait(15000, function() + for _, t in ipairs(vim.api.nvim_list_tabpages()) do + local s = lifecycle.get_session(t) + if s and s.mode == "history" and s.explorer and s.explorer.bufnr then + tp = t + return true + end + end + return false + end, 50) + check_shape("history", tp) + finish(tp, repo.cleanup) + end + + -- conflict + do + reset_config({}) + local repo = h.create_temp_git_repo() + repo.write_file("c.txt", { "l1", "l2", "l3" }) + repo.git("add -A") + repo.git("commit -m base") + repo.git("checkout -b f") + repo.write_file("c.txt", { "F", "l2", "l3" }) + repo.git("commit -am f") + repo.git("checkout main") + repo.write_file("c.txt", { "M", "l2", "l3" }) + repo.git("commit -am m") + repo.git("merge f --no-edit") + vim.cmd("edit " .. vim.fn.fnameescape(repo.path("c.txt"))) + local ready = false + view.create( + { + mode = "standalone", + git_root = repo.dir, + original = path.make_ref("c.txt", repo.dir), + modified = path.make_ref("c.txt", repo.dir), + original_revision = ":3", + modified_revision = ":2", + conflict = true, + }, + "", + function() + ready = true + end + ) + vim.wait(15000, function() + return ready + end, 50) + local tp + for _, t in ipairs(vim.api.nvim_list_tabpages()) do + local s = lifecycle.get_session(t) + if s and s.result_bufnr then + tp = t + break + end + end + check_shape("conflict", tp) + finish(tp, repo.cleanup) + end + + return checked, failures +end + +describe("keymap coverage", function() + after_each(function() + lifecycle.cleanup_all() + h.close_extra_tabs() + end) + + it("makes every configured keymap reachable in every applicable session shape", function() + local count, missing = run_audit() + assert.are.same({}, missing, "these configured keymaps are not reachable") + assert.is_true(count > 100, "audit should exercise the full keymap surface, ran " .. count) + end) +end) diff --git a/tests/ui/keymap/keymap_help_spec.lua b/tests/ui/keymap/keymap_help_spec.lua new file mode 100644 index 00000000..653e8dda --- /dev/null +++ b/tests/ui/keymap/keymap_help_spec.lua @@ -0,0 +1,342 @@ +-- The g? help popup must describe reality. +-- +-- Historically the popup was a hand-maintained list, so it drifted from the +-- mappings actually installed (see #343). These tests pin the contract in both +-- directions: everything shown is really bound, and nothing bound is missing. + +local h = dofile("tests/helpers.lua") +local path = require("codediff.core.path") + +h.ensure_plugin_loaded() + +local view = require("codediff.ui.view") +local lifecycle = require("codediff.ui.lifecycle") +local keymap_help = require("codediff.ui.keymap_help") +local commands = require("codediff.commands") + +local function reset_config(opts) + vim.g.mapleader = "\\" + local config = require("codediff.config") + config.options = vim.deepcopy(config.defaults) + require("codediff").setup(opts or {}) + require("codediff.ui.highlights").setup() +end + +local function temp_file(suffix, lines) + local file = vim.fn.tempname() .. suffix + vim.fn.writefile(lines, file) + return file +end + +local function wait_for_diff(tabpage, timeout_ms) + return vim.wait(timeout_ms or 10000, function() + local session = lifecycle.get_session(tabpage) + return session ~= nil and session.stored_diff_result ~= nil + end, 50) +end + +--- Open the help popup and return its rendered lines, then close it. +--- @param tabpage number +--- @return string[] lines +local function help_lines(tabpage) + keymap_help.toggle(tabpage) + local session = lifecycle.get_session(tabpage) + local win = session and session._help_win + assert.is_true(win ~= nil and vim.api.nvim_win_is_valid(win), "help window should open") + + local lines = vim.api.nvim_buf_get_lines(vim.api.nvim_win_get_buf(win), 0, -1, false) + keymap_help.toggle(tabpage) + return lines +end + +--- Canonicalize a key sequence so help text ("hs") and registry keys +--- ("\hs") can be compared. Both sides go through the same expansion, which +--- also normalizes "" against a raw carriage return. +--- @param key string +--- @return string +local function canonical(key) + local ok, expanded = pcall(vim.api.nvim_replace_termcodes, key, true, true, true) + if ok and expanded ~= "" then + return expanded + end + return key +end + +--- Keys the popup advertises, as a canonical set. The popup renders +--- "", possibly two columns per line. +--- @param lines string[] +--- @return table +local function advertised_keys(lines) + local keys = {} + for _, line in ipairs(lines) do + for key in line:gmatch("(%S+)%s+→") do + keys[canonical(key)] = true + end + end + return keys +end + +--- Keys the session registry expects the popup to document. +--- Already canonical, so these must not be expanded a second time: mouse keys +--- contain raw K_SPECIAL bytes that a second pass would mangle. +--- @param tabpage number +--- @return table +local function documented_keys(tabpage) + return lifecycle.documented_keymaps(tabpage) +end + +--- Every key currently mapped on any of the session's buffers, in any mode. +--- @param tabpage number +--- @return table +local function bound_keys(tabpage) + local session = lifecycle.get_session(tabpage) + local buffers = { + session.original_bufnr, + session.modified_bufnr, + session.explorer and session.explorer.bufnr, + session.result_bufnr, + } + + local keys = {} + for _, bufnr in ipairs(buffers) do + if bufnr and vim.api.nvim_buf_is_valid(bufnr) then + for _, mode in ipairs({ "n", "o", "x", "v" }) do + for _, map in ipairs(vim.api.nvim_buf_get_keymap(bufnr, mode)) do + keys[canonical(map.lhs)] = true + keys[map.lhs] = true + end + end + end + end + return keys +end + +local function open_standalone(opts) + reset_config(opts) + local left = temp_file("_help_left.txt", { "a", "b", "c" }) + local right = temp_file("_help_right.txt", { "a", "X", "c" }) + + view.create({ + mode = "standalone", + git_root = nil, + original = path.make_ref(left, nil), + modified = path.make_ref(right, nil), + }) + + local tabpage = vim.api.nvim_get_current_tabpage() + assert.is_true(wait_for_diff(tabpage), "standalone session should be ready") + + return tabpage, function() + vim.fn.delete(left) + vim.fn.delete(right) + end +end + +local function open_explorer(opts) + reset_config(opts) + local repo = h.create_temp_git_repo() + repo.write_file("t.txt", { "a", "b", "c" }) + repo.git("add .") + repo.git("commit -m initial") + repo.write_file("t.txt", { "a", "X", "c" }) + + vim.cmd("edit " .. vim.fn.fnameescape(repo.path("t.txt"))) + commands.vscode_diff({ fargs = {} }) + + local tabpage + assert.is_true(vim.wait(15000, function() + for _, tp in ipairs(vim.api.nvim_list_tabpages()) do + local session = lifecycle.get_session(tp) + if session and session.explorer and session.explorer.bufnr then + tabpage = tp + return true + end + end + return false + end, 50), "explorer session should be created") + + local explorer = lifecycle.get_session(tabpage).explorer + explorer.on_file_select({ path = "t.txt", group = "unstaged", status = "M", git_root = repo.dir }) + assert.is_true(wait_for_diff(tabpage), "explorer diff should be ready") + + return tabpage, function() + repo.cleanup() + end +end + +local function open_history() + reset_config() + local repo = h.create_temp_git_repo() + repo.write_file("t.txt", { "a" }) + repo.git("add .") + repo.git("commit -m one") + repo.write_file("t.txt", { "a", "b" }) + repo.git("commit -am two") + + vim.cmd("edit " .. vim.fn.fnameescape(repo.path("t.txt"))) + commands.vscode_diff({ fargs = { "history" } }) + + local tabpage + assert.is_true(vim.wait(15000, function() + for _, tp in ipairs(vim.api.nvim_list_tabpages()) do + local session = lifecycle.get_session(tp) + if session and session.mode == "history" and session.explorer and session.explorer.bufnr then + tabpage = tp + return true + end + end + return false + end, 50), "history session should be created") + + return tabpage, function() + repo.cleanup() + end +end + +local function open_conflict() + reset_config() + local repo = h.create_temp_git_repo() + repo.write_file("conf.txt", { "l1", "l2", "l3" }) + repo.git("add -A") + repo.git("commit -m base") + repo.git("checkout -b feature") + repo.write_file("conf.txt", { "FEATURE", "l2", "l3" }) + repo.git("commit -am feature") + repo.git("checkout main") + repo.write_file("conf.txt", { "MAIN", "l2", "l3" }) + repo.git("commit -am main") + assert.is_truthy(repo.git("merge feature --no-edit"):find("CONFLICT", 1, true), "merge must conflict") + + vim.cmd("edit " .. vim.fn.fnameescape(repo.path("conf.txt"))) + + local ready = false + view.create({ + mode = "standalone", + git_root = repo.dir, + original = path.make_ref("conf.txt", repo.dir), + modified = path.make_ref("conf.txt", repo.dir), + original_revision = ":3", + modified_revision = ":2", + conflict = true, + }, "", function() + ready = true + end) + assert.is_true(vim.wait(15000, function() + return ready + end, 50), "conflict view should become ready") + + local tabpage + for _, tp in ipairs(vim.api.nvim_list_tabpages()) do + local session = lifecycle.get_session(tp) + if session and session.result_bufnr then + tabpage = tp + break + end + end + assert.is_not_nil(tabpage, "conflict session should exist") + + return tabpage, function() + repo.cleanup() + end +end + +describe("keymap help popup", function() + after_each(function() + lifecycle.cleanup_all() + h.close_extra_tabs() + end) + + it("advertises only keys that are actually bound", function() + local tabpage, cleanup = open_standalone() + + local advertised = advertised_keys(help_lines(tabpage)) + local bound = bound_keys(tabpage) + + assert.is_true(next(advertised) ~= nil, "help should list something") + for key in pairs(advertised) do + assert.is_true(bound[key] == true, string.format("help advertises %q but nothing is mapped to it", key)) + end + + cleanup() + end) + + it("does not advertise a key disabled in config", function() + local tabpage, cleanup = open_standalone({ keymaps = { view = { quit = false, toggle_compact = false } } }) + + local advertised = advertised_keys(help_lines(tabpage)) + assert.is_nil(advertised[canonical("q")], "quit=false must not appear in help") + assert.is_nil(advertised[canonical("gc")], "toggle_compact=false must not appear in help") + + cleanup() + end) + + it("does not advertise gm when move detection is off", function() + -- compute_moves defaults to false, so align_move is never bound. + local tabpage, cleanup = open_standalone() + + local advertised = advertised_keys(help_lines(tabpage)) + assert.is_nil(advertised[canonical("gm")], "gm is only bound when diff.compute_moves is enabled") + + cleanup() + end) + + it("advertises gm when move detection is on", function() + local tabpage, cleanup = open_standalone({ diff = { compute_moves = true } }) + + local advertised = advertised_keys(help_lines(tabpage)) + assert.is_true(advertised[canonical("gm")] == true, "gm should be listed when compute_moves is enabled") + + cleanup() + end) + + it("does not advertise do/dp in conflict mode", function() + local tabpage, cleanup = open_conflict() + + local advertised = advertised_keys(help_lines(tabpage)) + assert.is_nil(advertised[canonical("do")], "conflict mode replaces do with 2do/3do") + assert.is_nil(advertised[canonical("dp")], "conflict mode has no dp") + assert.is_true(advertised[canonical("2do")] == true, "conflict help should list 2do") + assert.is_true(advertised[canonical("]x")] == true, "conflict help should list conflict navigation") + + cleanup() + end) + + it("documents every mapping the session installs, in every session shape", function() + -- The registry is the source of truth: any mapping claimed with + -- help ~= false must be discoverable through g?. This covers every buffer + -- role and every mode, in each shape codediff can produce. + local shapes = { + { "standalone side-by-side", function() return open_standalone({ diff = { layout = "side-by-side" } }) end }, + { "standalone inline", function() return open_standalone({ diff = { layout = "inline" } }) end }, + { "standalone + compute_moves", function() return open_standalone({ diff = { compute_moves = true } }) end }, + { "standalone + compact", function() + local tabpage, cleanup = open_standalone({ diff = { layout = "side-by-side" } }) + require("codediff.ui.view.compact").enable(tabpage) + return tabpage, cleanup + end }, + { "explorer", function() return open_explorer({}) end }, + { "explorer + auto_open_on_cursor", function() return open_explorer({ explorer = { auto_open_on_cursor = true } }) end }, + { "history", open_history }, + { "conflict", open_conflict }, + } + + for _, shape in ipairs(shapes) do + local label, build = shape[1], shape[2] + local tabpage, cleanup = build() + + local advertised = advertised_keys(help_lines(tabpage)) + local missing = {} + for key in pairs(documented_keys(tabpage)) do + if not advertised[key] then + table.insert(missing, vim.inspect(key)) + end + end + table.sort(missing) + assert.are.same({}, missing, label .. ": these mappings are installed but missing from g?") + + lifecycle.cleanup_all() + h.close_extra_tabs() + cleanup() + end + end) +end) diff --git a/tests/ui/keymap/keymap_lifecycle_spec.lua b/tests/ui/keymap/keymap_lifecycle_spec.lua new file mode 100644 index 00000000..7f5e8775 --- /dev/null +++ b/tests/ui/keymap/keymap_lifecycle_spec.lua @@ -0,0 +1,297 @@ +-- Keymap lifecycle contract. +-- +-- The golden matrix locks *which* mappings get installed. This file locks the +-- other half: ownership and teardown — what happens to mappings that already +-- existed, and whether codediff's own mappings are fully released. +-- +-- Two groups: +-- "invariants" behavior that must not change across the registry refactor. +-- "ownership" the lifecycle contract the registry refactor introduces. +-- These fail against the pre-refactor implementation; that is +-- the point — they are the executable specification. + +local h = dofile("tests/helpers.lua") +local matrix = dofile("tests/keymap_matrix.lua") +local path = require("codediff.core.path") + +h.ensure_plugin_loaded() + +local view = require("codediff.ui.view") +local lifecycle = require("codediff.ui.lifecycle") +local commands = require("codediff.commands") + +local function reset_config(opts) + local config = require("codediff.config") + config.options = vim.deepcopy(config.defaults) + require("codediff").setup(opts or {}) + require("codediff.ui.highlights").setup() +end + +local function temp_file(suffix, lines) + local file = vim.fn.tempname() .. suffix + vim.fn.writefile(lines, file) + return file +end + +local function wait_for_diff(tabpage, timeout_ms) + return vim.wait(timeout_ms or 10000, function() + local session = lifecycle.get_session(tabpage) + return session ~= nil and session.stored_diff_result ~= nil + end, 50) +end + +--- Open a standalone diff between two real files. +--- @return number tabpage, number modified_bufnr, function cleanup +local function open_standalone(original_lines, modified_lines, pre_open) + local left = temp_file("_lifecycle_left.txt", original_lines) + local right = temp_file("_lifecycle_right.txt", modified_lines) + + -- Load the modified side up front so a caller can install its own mappings + -- on the exact buffer codediff will reuse. + local right_buf = vim.fn.bufadd(right) + vim.fn.bufload(right_buf) + if pre_open then + pre_open(right_buf) + end + + view.create({ + mode = "standalone", + git_root = nil, + original = path.make_ref(left, nil), + modified = path.make_ref(right, nil), + }) + + local tabpage = vim.api.nvim_get_current_tabpage() + assert.is_true(wait_for_diff(tabpage), "standalone session should be ready") + + local session = lifecycle.get_session(tabpage) + assert.equals(right_buf, session.modified_bufnr, "codediff should reuse the preloaded buffer") + + return tabpage, right_buf, function() + vim.fn.delete(left) + vim.fn.delete(right) + end +end + +describe("keymap lifecycle", function() + before_each(function() + reset_config() + end) + + after_each(function() + lifecycle.cleanup_all() + h.close_extra_tabs() + end) + + -- ========================================================================= + -- Invariants: must hold before and after the registry refactor. + -- ========================================================================= + describe("invariants", function() + it("installs no mapping for a key configured as false", function() + reset_config({ keymaps = { view = { quit = false, toggle_compact = false } } }) + + local tabpage, mod_buf, cleanup = open_standalone({ "a", "b" }, { "a", "c" }) + local maps = matrix.map_index(mod_buf, "n") + + assert.is_nil(maps["q"], "quit=false must install nothing") + assert.is_nil(maps["gc"], "toggle_compact=false must install nothing") + assert.is_not_nil(maps["]c"], "other mappings must still be installed") + + cleanup() + end) + + it("honors a remapped key and never installs the default", function() + reset_config({ keymaps = { view = { quit = "Q" } } }) + + local tabpage, mod_buf, cleanup = open_standalone({ "a", "b" }, { "a", "c" }) + local maps = matrix.map_index(mod_buf, "n") + + assert.is_not_nil(maps["Q"], "remapped quit should be installed") + assert.is_nil(maps["q"], "default quit must not be installed when remapped") + + cleanup() + end) + + it("emits CodeDiffClose exactly once, before the session is destroyed", function() + -- Note: lifecycle.close() runs `tabclose` first, and the resulting + -- TabLeave already strips view mappings, so CodeDiffClose fires after + -- mappings are gone. What must stay stable is that the event fires once + -- and the session is still queryable while handlers run. + local tabpage, mod_buf, cleanup = open_standalone({ "a", "b" }, { "a", "c" }) + + local fired = 0 + local session_visible_during_event + local autocmd = vim.api.nvim_create_autocmd("User", { + pattern = "CodeDiffClose", + callback = function() + fired = fired + 1 + session_visible_during_event = lifecycle.get_session(tabpage) ~= nil + end, + }) + + lifecycle.close(tabpage) + vim.wait(100) + vim.api.nvim_del_autocmd(autocmd) + + assert.equals(1, fired, "CodeDiffClose must fire exactly once per close") + assert.is_true(session_visible_during_event, "the session must still be queryable while CodeDiffClose handlers run") + assert.is_nil(lifecycle.get_session(tabpage), "the session must be gone after close completes") + + cleanup() + end) + + it("removes view mappings from real buffers when leaving the tab", function() + local tabpage, mod_buf, cleanup = open_standalone({ "a", "b" }, { "a", "c" }) + + assert.is_not_nil(matrix.map_index(mod_buf, "n")["q"], "quit should be mapped while the tab is active") + + vim.cmd("tabnew") + vim.wait(100) + + assert.is_nil(matrix.map_index(mod_buf, "n")["q"], "codediff mappings must not leak into other tabs") + + cleanup() + end) + end) + + -- ========================================================================= + -- Ownership contract: the behavior the registry refactor must deliver. + -- ========================================================================= + describe("ownership", function() + it("restores a pre-existing buffer-local mapping after close", function() + local tabpage, mod_buf, cleanup = open_standalone({ "a", "b" }, { "a", "c" }, function(bufnr) + vim.keymap.set("n", "q", "echo 'user'", { buffer = bufnr, desc = "user-quit" }) + end) + + assert.equals("Close codediff tab", matrix.map_index(mod_buf, "n")["q"], "codediff should own q during the session") + + lifecycle.close(tabpage) + vim.wait(100) + + assert.equals("user-quit", matrix.map_index(mod_buf, "n")["q"], "the user's own q mapping must be restored on close") + + cleanup() + end) + + it("restores a pre-existing hunk-navigation mapping after close (gitsigns case)", function() + local tabpage, mod_buf, cleanup = open_standalone({ "a", "b", "c" }, { "a", "X", "c" }, function(bufnr) + vim.keymap.set("n", "]c", function() end, { buffer = bufnr, desc = "gitsigns-next-hunk" }) + vim.keymap.set("n", "[c", function() end, { buffer = bufnr, desc = "gitsigns-prev-hunk" }) + end) + + lifecycle.close(tabpage) + vim.wait(100) + + local maps = matrix.map_index(mod_buf, "n") + assert.equals("gitsigns-next-hunk", maps["]c"], "a plugin's ]c must survive a codediff session") + assert.equals("gitsigns-prev-hunk", maps["[c"], "a plugin's [c must survive a codediff session") + + cleanup() + end) + + it("releases operator-pending and visual mappings on close", function() + local tabpage, mod_buf, cleanup = open_standalone({ "a", "b", "c" }, { "a", "X", "c" }) + + assert.is_not_nil(matrix.map_index(mod_buf, "o")["ih"], "ih should be mapped during the session") + + lifecycle.close(tabpage) + vim.wait(100) + + assert.is_nil(matrix.map_index(mod_buf, "o")["ih"], "ih must be removed from operator-pending mode on close") + assert.is_nil(matrix.map_index(mod_buf, "x")["ih"], "ih must be removed from visual mode on close") + + cleanup() + end) + + it("releases operator-pending and visual mappings when leaving the tab", function() + local tabpage, mod_buf, cleanup = open_standalone({ "a", "b", "c" }, { "a", "X", "c" }) + + vim.cmd("tabnew") + vim.wait(100) + + assert.is_nil(matrix.map_index(mod_buf, "o")["ih"], "ih must not leak into other tabs in operator-pending mode") + assert.is_nil(matrix.map_index(mod_buf, "x")["ih"], "ih must not leak into other tabs in visual mode") + + cleanup() + end) + + it("detaches mappings from the previous file when the diff switches files", function() + local repo = h.create_temp_git_repo() + repo.write_file("one.txt", { "one", "original" }) + repo.write_file("two.txt", { "two", "original" }) + repo.git("add .") + repo.git("commit -m initial") + repo.write_file("one.txt", { "one", "changed" }) + repo.write_file("two.txt", { "two", "changed" }) + + vim.cmd("edit " .. vim.fn.fnameescape(repo.path("one.txt"))) + commands.vscode_diff({ fargs = {} }) + + local tabpage + assert.is_true(vim.wait(15000, function() + for _, tp in ipairs(vim.api.nvim_list_tabpages()) do + local session = lifecycle.get_session(tp) + if session and session.explorer and session.explorer.bufnr then + tabpage = tp + return true + end + end + return false + end, 50), "explorer session should be created") + + local explorer = lifecycle.get_session(tabpage).explorer + explorer.on_file_select({ path = "one.txt", group = "unstaged", status = "M", git_root = repo.dir }) + assert.is_true(wait_for_diff(tabpage), "first file diff should be ready") + local first_buf = lifecycle.get_session(tabpage).modified_bufnr + assert.is_not_nil(matrix.map_index(first_buf, "n")["q"], "first file should be mapped while displayed") + + explorer.on_file_select({ path = "two.txt", group = "unstaged", status = "M", git_root = repo.dir }) + assert.is_true(vim.wait(10000, function() + local session = lifecycle.get_session(tabpage) + return session and session.modified_bufnr ~= first_buf and session.stored_diff_result ~= nil + end, 50), "second file diff should be ready") + + assert.is_nil(matrix.map_index(first_buf, "n")["q"], "the previous file's buffer must be detached on file switch") + + repo.cleanup() + end) + + it("restores the user's do/dp instead of deleting them in conflict mode", function() + local repo = h.create_temp_git_repo() + repo.write_file("conf.txt", { "l1", "l2", "l3" }) + repo.git("add -A") + repo.git("commit -m base") + repo.git("checkout -b feature") + repo.write_file("conf.txt", { "FEATURE", "l2", "l3" }) + repo.git("commit -am feature") + repo.git("checkout main") + repo.write_file("conf.txt", { "MAIN", "l2", "l3" }) + repo.git("commit -am main") + assert.is_truthy(repo.git("merge feature --no-edit"):find("CONFLICT", 1, true), "merge must conflict") + + vim.cmd("edit " .. vim.fn.fnameescape(repo.path("conf.txt"))) + local conflict_buf = vim.api.nvim_get_current_buf() + vim.keymap.set("n", "do", "echo 'user-do'", { buffer = conflict_buf, desc = "user-do" }) + + local ready = false + view.create({ + mode = "standalone", + git_root = repo.dir, + original = path.make_ref("conf.txt", repo.dir), + modified = path.make_ref("conf.txt", repo.dir), + original_revision = ":3", + modified_revision = ":2", + conflict = true, + }, "", function() + ready = true + end) + assert.is_true(vim.wait(15000, function() + return ready + end, 50), "conflict view should become ready") + + assert.equals("user-do", matrix.map_index(conflict_buf, "n")["do"], "conflict mode must not destroy the user's own do mapping") + + repo.cleanup() + end) + end) +end) diff --git a/tests/ui/keymap/registry_spec.lua b/tests/ui/keymap/registry_spec.lua new file mode 100644 index 00000000..0c2b295b --- /dev/null +++ b/tests/ui/keymap/registry_spec.lua @@ -0,0 +1,379 @@ +-- Unit tests for the keymap slot arbiter and per-session registry. +-- These exercise the machinery directly on scratch buffers, independent of any +-- diff session, so ownership logic can be reviewed in isolation. + +local keymap = require("codediff.keymap") +local slots = require("codediff.keymap.slots") +local normalize = require("codediff.keymap.normalize") + +local function scratch() + return vim.api.nvim_create_buf(false, true) +end + +-- nvim_buf_get_keymap reports lhs in display form (""), while claims are +-- keyed by raw bytes. Query through maparg so both sides agree. +local function map_of(bufnr, mode, lhs) + if not vim.api.nvim_buf_is_valid(bufnr) then + return nil + end + local result = vim.api.nvim_buf_call(bufnr, function() + return vim.fn.maparg(lhs, mode, false, true) + end) + if type(result) ~= "table" or next(result) == nil or result.buffer ~= 1 then + return nil + end + return result +end + +local function noop() end + +describe("keymap registry", function() + local buffers + + before_each(function() + slots.reset() + buffers = {} + end) + + after_each(function() + for _, bufnr in ipairs(buffers) do + if vim.api.nvim_buf_is_valid(bufnr) then + vim.api.nvim_buf_delete(bufnr, { force = true }) + end + end + slots.reset() + end) + + local function new_buf() + local bufnr = scratch() + table.insert(buffers, bufnr) + return bufnr + end + + describe("normalization", function() + it("treats and as the same slot", function() + assert.equals(normalize.canonical(""), normalize.canonical("")) + end) + + it("expands ", function() + local canonical = normalize.canonical("x") + assert.is_not_nil(canonical) + assert.is_not.equal("x", canonical) + end) + + it("treats false, nil and empty string as disabled", function() + assert.is_nil(normalize.resolve(false)) + assert.is_nil(normalize.resolve(nil)) + assert.is_nil(normalize.resolve("")) + assert.equals("q", normalize.resolve("q")) + end) + end) + + describe("claim and release", function() + it("installs a mapping and removes it when no prior mapping existed", function() + local bufnr = new_buf() + local r = keymap.new("test") + + assert.is_true(r:claim(bufnr, "n", "q", noop, { desc = "codediff" })) + assert.equals("codediff", map_of(bufnr, "n", "q").desc) + + r:dispose() + assert.is_nil(map_of(bufnr, "n", "q")) + assert.equals(0, slots.count()) + end) + + it("restores a pre-existing buffer-local mapping", function() + local bufnr = new_buf() + vim.keymap.set("n", "q", noop, { buffer = bufnr, desc = "user" }) + + local r = keymap.new("test") + r:claim(bufnr, "n", "q", noop, { desc = "codediff" }) + assert.equals("codediff", map_of(bufnr, "n", "q").desc) + + r:dispose() + assert.equals("user", map_of(bufnr, "n", "q").desc) + end) + + it("does not recreate a global mapping as buffer-local", function() + local bufnr = new_buf() + vim.keymap.set("n", "codediffGlobalProbe", noop, { desc = "global" }) + + local r = keymap.new("test") + r:claim(bufnr, "n", "codediffGlobalProbe", noop, { desc = "codediff" }) + r:dispose() + + assert.is_nil(map_of(bufnr, "n", "codediffGlobalProbe")) + pcall(vim.keymap.del, "n", "codediffGlobalProbe") + end) + + it("silently ignores a disabled binding", function() + local bufnr = new_buf() + local r = keymap.new("test") + + assert.is_false(r:claim(bufnr, "n", false, noop, {})) + assert.is_false(r:claim(bufnr, "n", nil, noop, {})) + assert.equals(0, r:count()) + end) + + it("claims every requested mode independently", function() + local bufnr = new_buf() + local r = keymap.new("test") + + r:claim(bufnr, { "o", "x" }, "ih", noop, { desc = "textobject" }) + assert.is_not_nil(map_of(bufnr, "o", "ih")) + assert.is_not_nil(map_of(bufnr, "x", "ih")) + + r:dispose() + assert.is_nil(map_of(bufnr, "o", "ih")) + assert.is_nil(map_of(bufnr, "x", "ih")) + end) + + it("replaces its own previous claim when re-registering", function() + local bufnr = new_buf() + local r = keymap.new("test") + + r:claim(bufnr, "n", "q", noop, { desc = "first" }) + r:claim(bufnr, "n", "q", noop, { desc = "second" }) + + assert.equals("second", map_of(bufnr, "n", "q").desc) + assert.equals(1, r:count()) + + r:dispose() + assert.is_nil(map_of(bufnr, "n", "q")) + end) + end) + + describe("multiple owners", function() + it("keeps the other session's mapping when one disposes", function() + local bufnr = new_buf() + vim.keymap.set("n", "q", noop, { buffer = bufnr, desc = "user" }) + + local a = keymap.new("a") + local b = keymap.new("b") + a:claim(bufnr, "n", "q", noop, { desc = "session-a" }) + b:claim(bufnr, "n", "q", noop, { desc = "session-b" }) + + assert.equals("session-b", map_of(bufnr, "n", "q").desc, "newest claim wins") + + b:dispose() + assert.equals("session-a", map_of(bufnr, "n", "q").desc, "falls back to the remaining claim") + + a:dispose() + assert.equals("user", map_of(bufnr, "n", "q").desc, "original returns only after the last release") + end) + + it("restores the original when sessions dispose out of order", function() + local bufnr = new_buf() + vim.keymap.set("n", "q", noop, { buffer = bufnr, desc = "user" }) + + local a = keymap.new("a") + local b = keymap.new("b") + a:claim(bufnr, "n", "q", noop, { desc = "session-a" }) + b:claim(bufnr, "n", "q", noop, { desc = "session-b" }) + + a:dispose() + assert.equals("session-b", map_of(bufnr, "n", "q").desc) + + b:dispose() + assert.equals("user", map_of(bufnr, "n", "q").desc) + end) + + it("honors priority over claim order", function() + local bufnr = new_buf() + local a = keymap.new("a") + local b = keymap.new("b") + + a:claim(bufnr, "n", "q", noop, { desc = "high" }, { priority = 10 }) + b:claim(bufnr, "n", "q", noop, { desc = "low" }, { priority = 1 }) + + assert.equals("high", map_of(bufnr, "n", "q").desc) + + a:dispose() + b:dispose() + end) + end) + + describe("suspend and resume", function() + it("uninstalls and reinstalls suspendable mappings", function() + local bufnr = new_buf() + vim.keymap.set("n", "q", noop, { buffer = bufnr, desc = "user" }) + + local r = keymap.new("test") + r:claim(bufnr, "n", "q", noop, { desc = "codediff" }) + + r:suspend() + assert.equals("user", map_of(bufnr, "n", "q").desc, "suspend must hand the key back") + + r:resume() + assert.equals("codediff", map_of(bufnr, "n", "q").desc, "resume must take it again") + + r:dispose() + assert.equals("user", map_of(bufnr, "n", "q").desc) + end) + + it("leaves non-suspendable mappings installed", function() + local bufnr = new_buf() + local r = keymap.new("test") + r:claim(bufnr, "n", "", noop, { desc = "panel" }, { suspendable = false }) + + r:suspend() + assert.is_not_nil(map_of(bufnr, "n", ""), "panel mappings must survive a tab switch") + + r:dispose() + assert.is_nil(map_of(bufnr, "n", "")) + end) + + it("is idempotent", function() + local bufnr = new_buf() + local r = keymap.new("test") + r:claim(bufnr, "n", "q", noop, { desc = "codediff" }) + + r:suspend() + r:suspend() + r:resume() + r:resume() + assert.equals("codediff", map_of(bufnr, "n", "q").desc) + + r:dispose() + r:dispose() + assert.is_nil(map_of(bufnr, "n", "q")) + end) + end) + + describe("buffer detach", function() + it("releases one buffer without disturbing the others", function() + local old_buf = new_buf() + local new_buffer = new_buf() + vim.keymap.set("n", "q", noop, { buffer = old_buf, desc = "user" }) + + local r = keymap.new("test") + r:claim(old_buf, "n", "q", noop, { desc = "codediff" }) + r:claim(new_buffer, "n", "q", noop, { desc = "codediff" }) + + r:detach_buffer(old_buf) + assert.equals("user", map_of(old_buf, "n", "q").desc, "detached buffer must be restored") + assert.equals("codediff", map_of(new_buffer, "n", "q").desc, "other buffers stay mapped") + + r:dispose() + end) + + it("detach_buffers_except keeps only the listed buffers", function() + local keep_buf = new_buf() + local drop_buf = new_buf() + + local r = keymap.new("test") + r:claim(keep_buf, "n", "q", noop, { desc = "codediff" }) + r:claim(drop_buf, "n", "q", noop, { desc = "codediff" }) + + r:detach_buffers_except({ [keep_buf] = true }) + assert.is_not_nil(map_of(keep_buf, "n", "q")) + assert.is_nil(map_of(drop_buf, "n", "q")) + + r:dispose() + end) + end) + + describe("foreign mappings", function() + it("does not clobber a mapping installed by someone else", function() + local bufnr = new_buf() + local r = keymap.new("test") + r:claim(bufnr, "n", "q", noop, { desc = "codediff" }) + + -- Another plugin takes the key while the session is live. + vim.keymap.set("n", "q", function() end, { buffer = bufnr, desc = "other-plugin" }) + + r:dispose() + local current = map_of(bufnr, "n", "q") + assert.is_not_nil(current, "a foreign mapping must survive codediff teardown") + assert.equals("other-plugin", current.desc, "a foreign mapping must survive codediff teardown") + end) + + it("does not reinstall over a foreign mapping on resume", function() + local bufnr = new_buf() + local r = keymap.new("test") + r:claim(bufnr, "n", "q", noop, { desc = "codediff" }) + + r:suspend() + vim.keymap.set("n", "q", function() end, { buffer = bufnr, desc = "other-plugin" }) + r:resume() + + local current = map_of(bufnr, "n", "q") + assert.is_not_nil(current, "foreign mapping should still be installed") + assert.equals("other-plugin", current.desc) + r:dispose() + end) + end) + + describe("special keys", function() + -- Regression: the slot identity is the canonical byte sequence, but the + -- mapping APIs must be given the original spelling. Passing canonical + -- bytes back to vim.keymap.set re-encodes keys such as <2-LeftMouse> and + -- , leaving a mapping that the real key press can never reach. + local SPECIAL = { "q", "", "", "", "hs", "<2-LeftMouse>", "", "", "", "", "zo", "]c", "2do" } + + it("installs a mapping the key press can actually reach", function() + for _, key in ipairs(SPECIAL) do + local bufnr = new_buf() + local r = keymap.new("test") + assert.is_true(r:claim(bufnr, "n", key, noop, { desc = "codediff" }), key .. " should be claimed") + + local found = map_of(bufnr, "n", key) + assert.is_not_nil(found, string.format("%q is mapped but unreachable via maparg", key)) + assert.equals("codediff", found.desc, key .. " should resolve to the codediff mapping") + + r:dispose() + end + end) + + it("releases special keys on dispose", function() + for _, key in ipairs(SPECIAL) do + local bufnr = new_buf() + local r = keymap.new("test") + r:claim(bufnr, "n", key, noop, { desc = "codediff" }) + r:dispose() + assert.is_nil(map_of(bufnr, "n", key), string.format("%q should be released", key)) + end + end) + + it("restores a pre-existing mapping for special keys", function() + for _, key in ipairs({ "<2-LeftMouse>", "", "" }) do + local bufnr = new_buf() + vim.keymap.set("n", key, noop, { buffer = bufnr, desc = "user" }) + + local r = keymap.new("test") + r:claim(bufnr, "n", key, noop, { desc = "codediff" }) + assert.equals("codediff", map_of(bufnr, "n", key).desc, key .. " should be taken over") + + r:dispose() + local restored = map_of(bufnr, "n", key) + assert.is_not_nil(restored, key .. " should be restored") + assert.equals("user", restored.desc, key .. " should return to the user's mapping") + end + end) + end) + + describe("invalid buffers", function() + it("ignores claims on an invalid buffer", function() + local bufnr = new_buf() + vim.api.nvim_buf_delete(bufnr, { force = true }) + + local r = keymap.new("test") + assert.is_false(r:claim(bufnr, "n", "q", noop, {})) + assert.equals(0, r:count()) + end) + + it("disposes cleanly when a buffer was wiped mid-session", function() + local bufnr = new_buf() + local r = keymap.new("test") + r:claim(bufnr, "n", "q", noop, { desc = "codediff" }) + + vim.api.nvim_buf_delete(bufnr, { force = true }) + keymap.forget_buffer(bufnr) + + assert.has_no.errors(function() + r:dispose() + end) + assert.equals(0, slots.count()) + end) + end) +end) From 14291541b2c9be1ec8fc549d0b9b52d5493e679b Mon Sep 17 00:00:00 2001 From: Yanuo Ma Date: Sat, 1 Aug 2026 02:25:16 -0400 Subject: [PATCH 2/5] fix(keymap): close ownership gaps found by audit An adversarial review of the registry found seven defects. Four of them already existed on main and were simply invisible before mappings became tracked claims; three were gaps in the new machinery itself. Pre-existing, now fixed: - compact's synced-fold wrappers sit on real diff buffers, so they must be released when the user leaves the tab. They were marked non-suspendable, which is only correct for codediff-owned panel buffers. - keymap setup could only add. Mappings from a previous session shape stayed installed: gm after switching to inline, conflict mappings after leaving a merge, and the old key after the quit binding was reconfigured. Setup passes now run inside a named scope and retire whatever the pass did not re-claim; leaving conflict mode retires the conflict scope outright. - a pane wiped while the session was suspended made resume_diff drop the session without disposing its registry, stranding codediff's mappings on whichever pane survived. TabEnter now validates the panes first, and resume_diff disposes before it orphans anything. Gaps in the new machinery: - the slot stored the configured spelling, so changing mapleader mid-session made the installed key unaddressable and the user's mapping was never restored. The API spelling is now frozen with keytrans at claim time, while the canonical bytes remain the slot identity. - "the foreign mapping wins" did not hold in two cases: deleting the restored mapping while codediff was suspended resurrected it on dispose, and re-mapping the same RHS with different options was overwritten on resume. Absence now counts as displacement when a snapshot exists, and mapping identity compares expr, noremap, silent, nowait, script, replace_keycodes and desc rather than the RHS alone. - owns() reported cached state, so g? could advertise a key another plugin had taken over. Ownership queries now verify against the installed mapping. BufWipeout is wired to drop slot bookkeeping for buffers that no longer exist; forget_buffer previously had no caller outside tests. Adds ten regression tests covering the transitions these defects hid in: mapleader change, foreign delete and options change while suspended, displacement, scope retirement, compact tab switch, layout toggle and reconfiguration. Verified against unmodified main, where all four pre-existing defects reproduce. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- lua/codediff/keymap/registry.lua | 71 ++++++++++++- lua/codediff/keymap/slots.lua | 71 +++++++++++-- lua/codediff/ui/conflict/keymaps.lua | 4 + lua/codediff/ui/lifecycle/accessors.lua | 38 +++++++ lua/codediff/ui/lifecycle/cleanup.lua | 16 +++ lua/codediff/ui/lifecycle/init.lua | 3 + lua/codediff/ui/lifecycle/session.lua | 8 ++ lua/codediff/ui/lifecycle/state.lua | 8 +- lua/codediff/ui/view/compact.lua | 17 ++-- lua/codediff/ui/view/keymaps.lua | 7 ++ tests/ui/keymap/keymap_lifecycle_spec.lua | 61 +++++++++++ tests/ui/keymap/registry_spec.lua | 119 ++++++++++++++++++++++ 12 files changed, 402 insertions(+), 21 deletions(-) diff --git a/lua/codediff/keymap/registry.lua b/lua/codediff/keymap/registry.lua index 77c03d73..31b1ae64 100644 --- a/lua/codediff/keymap/registry.lua +++ b/lua/codediff/keymap/registry.lua @@ -33,9 +33,61 @@ function M.new(name) entries = {}, suspended = false, disposed = false, + -- Generation counter per scope, used to retire claims that a later setup + -- pass no longer makes. + scope_gen = {}, + current_scope = nil, }, Registry) end +--- Begin a setup pass for `scope`. +--- +--- Keymap setup is re-run whenever a session changes shape (layout toggle, +--- entering or leaving conflict mode, reconfiguration). Without a scope the +--- pass could only add claims, so mappings from the previous shape survived — +--- `gm` after switching to inline, conflict mappings after leaving a merge. +--- Claims made between begin_scope and end_scope are tagged; end_scope +--- releases anything in that scope the pass did not re-claim. +--- @param scope string +function Registry:begin_scope(scope) + if self.disposed then + return + end + self.scope_gen[scope] = (self.scope_gen[scope] or 0) + 1 + self.current_scope = scope +end + +--- Finish the current setup pass, releasing claims it did not renew. +function Registry:end_scope() + local scope = self.current_scope + self.current_scope = nil + if self.disposed or not scope then + return + end + local generation = self.scope_gen[scope] + for key, entry in pairs(self.entries) do + if entry.scope == scope and entry.generation ~= generation then + slots.release(self, entry.bufnr, entry.mode, entry.lhs) + self.entries[key] = nil + end + end +end + +--- Release every claim belonging to `scope`. +--- Used when a capability goes away entirely, such as leaving conflict mode. +--- @param scope string +function Registry:release_scope(scope) + if self.disposed then + return + end + for key, entry in pairs(self.entries) do + if entry.scope == scope then + slots.release(self, entry.bufnr, entry.mode, entry.lhs) + self.entries[key] = nil + end + end +end + --- Install a mapping owned by this registry. --- @param bufnr number --- @param modes string|string[] @@ -81,6 +133,8 @@ function Registry:claim(bufnr, modes, lhs, rhs, opts, meta) lhs = canonical, suspendable = suspendable, documented = documented, + scope = self.current_scope, + generation = self.current_scope and self.scope_gen[self.current_scope] or nil, } -- A claim added while suspended must not be installed yet. if self.suspended and suspendable then @@ -98,7 +152,7 @@ end function Registry:documented_keys() local keys = {} for _, entry in pairs(self.entries) do - if entry.documented then + if entry.documented and slots.is_live(self, entry.bufnr, entry.mode, entry.lhs) then keys[entry.lhs] = true end end @@ -121,7 +175,9 @@ function Registry:owns(lhs, mode, bufnr) end for _, entry in pairs(self.entries) do if entry.lhs == canonical and (mode == nil or entry.mode == mode) and (bufnr == nil or entry.bufnr == bufnr) then - return true + if slots.is_live(self, entry.bufnr, entry.mode, entry.lhs) then + return true + end end end return false @@ -146,6 +202,17 @@ function Registry:release(bufnr, modes, lhs) end end +--- Drop bookkeeping for a buffer that no longer exists, without calling into +--- Neovim: a wiped buffer already took its mappings with it. +--- @param bufnr number +function Registry:forget_buffer(bufnr) + for key, entry in pairs(self.entries) do + if entry.bufnr == bufnr then + self.entries[key] = nil + end + end +end + --- Release every mapping this registry installed on a buffer. --- Used when a diff pane swaps to a different file, so the buffer that leaves --- the session gets its original mappings back immediately. diff --git a/lua/codediff/keymap/slots.lua b/lua/codediff/keymap/slots.lua index d7e68a94..8e045a8c 100644 --- a/lua/codediff/keymap/slots.lua +++ b/lua/codediff/keymap/slots.lua @@ -111,15 +111,29 @@ local function install(slot, claim) end end ---- Compare two mapping descriptions for identity. +-- Mapping fields that decide whether two mappings are "the same mapping". +-- Options matter: re-mapping the same RHS with different `silent` or `nowait` +-- is a different mapping and must count as foreign. +local COMPARED_FIELDS = { "expr", "noremap", "script", "silent", "nowait", "desc", "replace_keycodes" } + +--- Compare two mapping descriptions for identity, including options. local function same_map(a, b) if not a or not b then return false end if a.callback ~= nil or b.callback ~= nil then - return a.callback == b.callback + if a.callback ~= b.callback then + return false + end + elseif a.rhs ~= b.rhs then + return false + end + for _, field in ipairs(COMPARED_FIELDS) do + if a[field] ~= b[field] then + return false + end end - return a.rhs == b.rhs + return true end --- True when the mapping currently installed is the one this slot applied. @@ -139,12 +153,19 @@ local function installed_is_ours(slot) return current.rhs == expected end ---- True when codediff may take this slot: either nothing buffer-local is ---- installed, or what is installed is the snapshot we previously handed back. +--- True when codediff may take this slot. +--- +--- Only two states are ours to take: nothing buffer-local is installed and +--- there was nothing to begin with, or what is installed is exactly the +--- snapshot we previously handed back. Anything else — including the snapshot +--- having been deleted by someone else while we were suspended — means another +--- party now owns the key. local function slot_is_free_for_us(slot) local current = read_current(slot.bufnr, slot.mode, slot.lhs) if not is_buffer_local(current) then - return true + -- Absence is only "free" when there was no prior mapping to preserve. + -- If we had a snapshot, its disappearance means someone deleted it. + return slot.saved == false end return slot.saved ~= false and same_map(current, slot.saved) end @@ -255,16 +276,29 @@ function M.claim(owner, bufnr, mode, key, lhs, rhs, opts, priority) return false end + -- Freeze the spelling now. `lhs` may contain , which resolves + -- against mapleader at this instant; if the user changes mapleader later, + -- re-reading the configured string would address a different key and we + -- would fail to find, restore or delete what we installed. keytrans gives a + -- stable, API-safe rendering of the canonical bytes. + local frozen = lhs + if vim.fn.exists("*keytrans") == 1 then + local ok, translated = pcall(vim.fn.keytrans, key) + if ok and translated ~= "" then + frozen = translated + end + end + local by_lhs = slot_table(bufnr, mode, true) local slot = by_lhs[key] if not slot then - local current = read_current(bufnr, mode, lhs) + local current = read_current(bufnr, mode, frozen) slot = { bufnr = bufnr, mode = mode, key = key, - lhs = lhs, + lhs = frozen, -- Only buffer-local mappings are ours to restore. A global mapping must -- never be recreated as a buffer-local one. saved = is_buffer_local(current) and current or false, @@ -325,6 +359,27 @@ function M.set_active(owner, bufnr, mode, key, active) reconcile(slot) end +--- True when `owner` has a live, installed claim on this slot. +--- +--- Verifies against the mapping actually installed rather than trusting cached +--- state: displacement is normally detected during reconcile, and a passive +--- query would otherwise still report a key that another plugin has taken +--- over. The help popup relies on this to avoid advertising an action the key +--- no longer invokes. +--- @return boolean +function M.is_live(owner, bufnr, mode, key) + local by_lhs = slot_table(bufnr, mode, false) + local slot = by_lhs and by_lhs[key] + if not slot or slot.displaced then + return false + end + local _, claim = find_claim(slot, owner) + if not claim or not claim.active or slot.applied ~= claim then + return false + end + return installed_is_ours(slot) +end + --- Forget every slot for a buffer without touching Neovim. --- For BufWipeout: the buffer and its mappings are already gone. function M.forget_buffer(bufnr) diff --git a/lua/codediff/ui/conflict/keymaps.lua b/lua/codediff/ui/conflict/keymaps.lua index 8ddc8535..b387b0f3 100644 --- a/lua/codediff/ui/conflict/keymaps.lua +++ b/lua/codediff/ui/conflict/keymaps.lua @@ -43,6 +43,8 @@ function M.setup_keymaps(tabpage) return end + lifecycle.begin_keymap_scope(tabpage, "conflict") + local keymaps = config.options.keymaps.conflict or {} -- Bind to incoming (left), current (right), AND result buffers @@ -89,6 +91,8 @@ function M.setup_keymaps(tabpage) end end end + + lifecycle.end_keymap_scope(tabpage) end return M diff --git a/lua/codediff/ui/lifecycle/accessors.lua b/lua/codediff/ui/lifecycle/accessors.lua index 68b5adb2..67fd1497 100644 --- a/lua/codediff/ui/lifecycle/accessors.lua +++ b/lua/codediff/ui/lifecycle/accessors.lua @@ -373,6 +373,12 @@ function M.set_result(tabpage, result_bufnr, result_win) return false end + -- Leaving conflict mode: retire the conflict mappings so do/dp and the + -- ordinary view mappings can be claimed again on the next setup pass. + if result_bufnr == nil and sess.result_bufnr ~= nil and sess.keymaps then + sess.keymaps:release_scope("conflict") + end + sess.result_bufnr = result_bufnr sess.result_win = result_win @@ -589,6 +595,38 @@ function M.documented_keymaps(tabpage) return sess.keymaps:documented_keys() end +--- Begin a keymap setup pass for `scope` on this session. +--- Claims made until end_keymap_scope are tagged; anything in the scope the +--- pass does not re-claim is released, so a shape change (layout toggle, +--- leaving conflict mode, reconfiguration) cannot leave stale mappings behind. +--- @param tabpage number +--- @param scope string +function M.begin_keymap_scope(tabpage, scope) + local sess = get_active_diffs()[tabpage] + if sess then + registry_for(sess):begin_scope(scope) + end +end + +--- Finish the current keymap setup pass, releasing claims it did not renew. +--- @param tabpage number +function M.end_keymap_scope(tabpage) + local sess = get_active_diffs()[tabpage] + if sess and sess.keymaps then + sess.keymaps:end_scope() + end +end + +--- Release every mapping belonging to `scope`. +--- @param tabpage number +--- @param scope string +function M.release_keymap_scope(tabpage, scope) + local sess = get_active_diffs()[tabpage] + if sess and sess.keymaps then + sess.keymaps:release_scope(scope) + end +end + --- Release a specific mapping the session installed on a buffer. --- @param tabpage number --- @param bufnr number diff --git a/lua/codediff/ui/lifecycle/cleanup.lua b/lua/codediff/ui/lifecycle/cleanup.lua index 74ff5978..40b2d834 100644 --- a/lua/codediff/ui/lifecycle/cleanup.lua +++ b/lua/codediff/ui/lifecycle/cleanup.lua @@ -228,6 +228,22 @@ function M.setup_autocmds() end, }) + -- A wiped buffer takes its mappings with it. Drop the bookkeeping so slots + -- do not accumulate for buffers that no longer exist. + vim.api.nvim_create_autocmd("BufWipeout", { + group = augroup, + callback = function(args) + if args.buf then + require("codediff.keymap").forget_buffer(args.buf) + for _, diff in pairs(session.get_active_diffs()) do + if diff.keymaps then + diff.keymaps:forget_buffer(args.buf) + end + end + end + end, + }) + -- Re-pin panel widths on terminal/tmux resize for every active diff session -- (see issue #346). VimResized is editor-global, so one autocmd handles all -- tabs; layout.arrange() is a no-op for tabs without a session. diff --git a/lua/codediff/ui/lifecycle/init.lua b/lua/codediff/ui/lifecycle/init.lua index 72e613c9..6db33cfa 100644 --- a/lua/codediff/ui/lifecycle/init.lua +++ b/lua/codediff/ui/lifecycle/init.lua @@ -69,6 +69,9 @@ M.set_buf_keymap = accessors.set_buf_keymap M.del_buf_keymap = accessors.del_buf_keymap M.owns_keymap = accessors.owns_keymap M.documented_keymaps = accessors.documented_keymaps +M.begin_keymap_scope = accessors.begin_keymap_scope +M.end_keymap_scope = accessors.end_keymap_scope +M.release_keymap_scope = accessors.release_keymap_scope M.detach_keymap_buffer = accessors.detach_keymap_buffer M.clear_tab_keymaps = accessors.clear_tab_keymaps M.restore_tab_keymaps = accessors.restore_tab_keymaps diff --git a/lua/codediff/ui/lifecycle/session.lua b/lua/codediff/ui/lifecycle/session.lua index 3b538fed..7f93d37b 100644 --- a/lua/codediff/ui/lifecycle/session.lua +++ b/lua/codediff/ui/lifecycle/session.lua @@ -204,6 +204,14 @@ function M.create_session( local current_tab = vim.api.nvim_get_current_tabpage() if current_tab == tabpage and active_diffs[tabpage] then local sess = active_diffs[tabpage] + -- resume_diff tears the session down when a pane was wiped while we + -- were away; let it run first so we never reinstall mappings onto a + -- session that is about to disappear. + local panes_valid = vim.api.nvim_buf_is_valid(sess.original_bufnr) and vim.api.nvim_buf_is_valid(sess.modified_bufnr) + if not panes_valid then + state.resume_diff(tabpage) + return + end accessors.restore_tab_keymaps(tabpage) if sess.reapply_keymaps then pcall(sess.reapply_keymaps) diff --git a/lua/codediff/ui/lifecycle/state.lua b/lua/codediff/ui/lifecycle/state.lua index 6a4b96a1..3560343e 100644 --- a/lua/codediff/ui/lifecycle/state.lua +++ b/lua/codediff/ui/lifecycle/state.lua @@ -119,8 +119,14 @@ local function resume_diff(tabpage) return end - -- Check if buffers still exist + -- Check if buffers still exist. Dropping the session here would otherwise + -- orphan its registry, leaving codediff's mappings installed on whichever + -- pane survived and its saved user mappings unreachable. if not vim.api.nvim_buf_is_valid(diff.original_bufnr) or not vim.api.nvim_buf_is_valid(diff.modified_bufnr) then + if diff.keymaps then + diff.keymaps:dispose() + diff.keymaps = nil + end active_diffs[tabpage] = nil return end diff --git a/lua/codediff/ui/view/compact.lua b/lua/codediff/ui/view/compact.lua index e60507a1..2b342d66 100644 --- a/lua/codediff/ui/view/compact.lua +++ b/lua/codediff/ui/view/compact.lua @@ -123,6 +123,8 @@ local function setup_fold_sync(session, tabpage) { win = session.modified_win, buf = session.modified_bufnr, side = "modified" }, } + lifecycle.begin_keymap_scope(tabpage, "compact") + for _, pane in ipairs(panes) do if pane.win and vim.api.nvim_win_is_valid(pane.win) and pane.buf and vim.api.nvim_buf_is_valid(pane.buf) then @@ -169,10 +171,12 @@ local function setup_fold_sync(session, tabpage) if not ok then vim.notify("[codediff] synced-fold error: " .. tostring(err), vim.log.levels.DEBUG) end - end, { buffer = pane.buf, silent = true, desc = "codediff: synced fold " .. key }, { suspendable = false, help = false }) + end, { buffer = pane.buf, silent = true, desc = "codediff: synced fold " .. key }, { help = false }) end end end + + lifecycle.end_keymap_scope(tabpage) end --- Remove the synced-fold keymap wraps from a session's panes. @@ -180,15 +184,8 @@ end --- explorer paths where buffers persist we need to release them explicitly. --- @param session table --- @param tabpage number -local function teardown_fold_sync(session, tabpage) - local panes = { session.original_bufnr, session.modified_bufnr } - for _, buf in ipairs(panes) do - if buf and vim.api.nvim_buf_is_valid(buf) then - for _, key in ipairs(FOLD_KEYS) do - lifecycle.del_buf_keymap(tabpage, buf, "n", key) - end - end - end +local function teardown_fold_sync(_, tabpage) + lifecycle.release_keymap_scope(tabpage, "compact") end --- The fold-target panes for a session (inline folds only the modified pane). diff --git a/lua/codediff/ui/view/keymaps.lua b/lua/codediff/ui/view/keymaps.lua index 3ef2c632..c0c9c340 100644 --- a/lua/codediff/ui/view/keymaps.lua +++ b/lua/codediff/ui/view/keymaps.lua @@ -26,6 +26,11 @@ end -- Centralized keymap setup for all diff view keymaps -- This function sets up ALL keymaps in one place for better maintainability function M.setup_all_keymaps(tabpage, original_bufnr, modified_bufnr, is_explorer_mode) + -- Scope the pass so mappings from a previous session shape (gm after + -- switching to inline, an old quit key after reconfiguration) are retired + -- rather than left installed alongside the new ones. + lifecycle.begin_keymap_scope(tabpage, "view") + local keymaps = config.options.keymaps.view -- Check mode context @@ -880,6 +885,8 @@ function M.setup_all_keymaps(tabpage, original_bufnr, modified_bufnr, is_explore lifecycle.set_tab_keymap(tabpage, "n", keymaps.align_move, align_move, { desc = "Align moved code block" }) end + lifecycle.end_keymap_scope(tabpage) + -- Keep compact mode in sync when the diff view is (re)built — applies the -- configured default on open and re-folds on file switches (no-op if off). compact.refresh(tabpage) diff --git a/tests/ui/keymap/keymap_lifecycle_spec.lua b/tests/ui/keymap/keymap_lifecycle_spec.lua index 7f5e8775..4421571f 100644 --- a/tests/ui/keymap/keymap_lifecycle_spec.lua +++ b/tests/ui/keymap/keymap_lifecycle_spec.lua @@ -256,6 +256,67 @@ describe("keymap lifecycle", function() repo.cleanup() end) + it("releases compact fold wraps when leaving the tab", function() + -- The wraps live on real diff buffers, so they must not follow those + -- buffers into other tabs. + local tabpage, mod_buf, cleanup = open_standalone({ "a", "b", "c" }, { "a", "X", "c" }) + assert.is_true(require("codediff.ui.view.compact").enable(tabpage), "compact should enable") + assert.is_not_nil(matrix.map_index(mod_buf, "n")["zo"], "zo should be wrapped while compact is active") + + vim.cmd("tabnew") + vim.wait(100) + + assert.is_nil(matrix.map_index(mod_buf, "n")["zo"], "compact fold wraps must not leak into other tabs") + cleanup() + end) + + it("retires layout-specific mappings when the layout changes", function() + reset_config({ diff = { compute_moves = true, layout = "side-by-side" } }) + local left = temp_file("_move_left.txt", { "a1", "a2", "a3", "a4", "a5", "u1", "u2", "u3", "b1", "b2", "b3", "b4", "b5" }) + local right = temp_file("_move_right.txt", { "u1", "u2", "u3", "b1", "b2", "b3", "b4", "b5", "a1", "a2", "a3", "a4", "a5" }) + + view.create({ + mode = "standalone", + git_root = nil, + original = path.make_ref(left, nil), + modified = path.make_ref(right, nil), + }) + local tabpage = vim.api.nvim_get_current_tabpage() + assert.is_true(wait_for_diff(tabpage), "session should be ready") + + local session = lifecycle.get_session(tabpage) + assert.is_not_nil(matrix.map_index(session.modified_bufnr, "n")["gm"], "gm should be bound in side-by-side") + + view.toggle_layout(tabpage) + assert.is_true(vim.wait(10000, function() + local s = lifecycle.get_session(tabpage) + return s and s.layout == "inline" and s.stored_diff_result ~= nil + end, 50), "layout should toggle to inline") + + session = lifecycle.get_session(tabpage) + assert.is_nil(matrix.map_index(session.modified_bufnr, "n")["gm"], "gm applies only to side-by-side and must be retired") + + vim.fn.delete(left) + vim.fn.delete(right) + end) + + it("retires the previous key when the configuration is changed and reapplied", function() + local tabpage, mod_buf, cleanup = open_standalone({ "a", "b" }, { "a", "c" }) + assert.is_not_nil(matrix.map_index(mod_buf, "n")["q"], "default quit should be bound") + + require("codediff").setup({ keymaps = { view = { quit = "Q" } } }) + local session = lifecycle.get_session(tabpage) + if session.reapply_keymaps then + session.reapply_keymaps() + end + vim.wait(200) + + assert.is_not_nil(matrix.map_index(mod_buf, "n")["Q"], "the new quit key should be bound") + assert.is_nil(matrix.map_index(mod_buf, "n")["q"], "the previous quit key must be released") + + cleanup() + end) + it("restores the user's do/dp instead of deleting them in conflict mode", function() local repo = h.create_temp_git_repo() repo.write_file("conf.txt", { "l1", "l2", "l3" }) diff --git a/tests/ui/keymap/registry_spec.lua b/tests/ui/keymap/registry_spec.lua index 0c2b295b..fa53ce8f 100644 --- a/tests/ui/keymap/registry_spec.lua +++ b/tests/ui/keymap/registry_spec.lua @@ -352,6 +352,125 @@ describe("keymap registry", function() end) end) + describe("ownership edge cases", function() + -- Regressions found by an adversarial audit of the registry. + + it("restores the user mapping after mapleader changes mid-session", function() + local bufnr = new_buf() + local saved_leader = vim.g.mapleader + vim.g.mapleader = "\\" + vim.keymap.set("n", "\\x", noop, { buffer = bufnr, desc = "user" }) + + local r = keymap.new("test") + r:claim(bufnr, "n", "x", noop, { desc = "codediff" }) + + -- Changing the leader must not make the installed key unaddressable. + vim.g.mapleader = "," + r:dispose() + vim.g.mapleader = saved_leader + + local restored = map_of(bufnr, "n", "\\x") + assert.is_not_nil(restored, "the user's mapping should be restored") + assert.equals("user", restored.desc) + end) + + it("does not resurrect a mapping the user deleted while suspended", function() + local bufnr = new_buf() + vim.keymap.set("n", "q", noop, { buffer = bufnr, desc = "user" }) + + local r = keymap.new("test") + r:claim(bufnr, "n", "q", noop, { desc = "codediff" }) + r:suspend() + vim.keymap.del("n", "q", { buffer = bufnr }) + r:resume() + + assert.is_nil(map_of(bufnr, "n", "q"), "resume must not reclaim a key the user freed") + r:dispose() + assert.is_nil(map_of(bufnr, "n", "q"), "dispose must not resurrect the deleted mapping") + end) + + it("treats an options-only change as a foreign mapping", function() + local bufnr = new_buf() + vim.keymap.set("n", "q", "echo 1", { buffer = bufnr, desc = "user", silent = true }) + + local r = keymap.new("test") + r:claim(bufnr, "n", "q", noop, { desc = "codediff" }) + r:suspend() + -- Same RHS, different options: a different mapping, and not ours. + vim.keymap.set("n", "q", "echo 1", { buffer = bufnr, desc = "other", silent = false }) + r:resume() + + assert.equals("other", map_of(bufnr, "n", "q").desc, "resume must not overwrite a foreign mapping") + r:dispose() + assert.equals("other", map_of(bufnr, "n", "q").desc, "dispose must not restore over a foreign mapping") + end) + + it("stops reporting ownership once displaced", function() + local bufnr = new_buf() + local r = keymap.new("test") + r:claim(bufnr, "n", "q", noop, { desc = "codediff" }) + assert.is_true(r:owns("q"), "codediff owns the key while installed") + + vim.keymap.set("n", "q", function() end, { buffer = bufnr, desc = "foreign" }) + + assert.is_false(r:owns("q"), "a displaced key is not owned by codediff") + assert.is_nil(r:documented_keys()[normalize.canonical("q")], "a displaced key must not be advertised in help") + r:dispose() + end) + end) + + describe("scopes", function() + it("releases claims a later pass no longer makes", function() + local bufnr = new_buf() + local r = keymap.new("test") + + r:begin_scope("view") + r:claim(bufnr, "n", "q", noop, { desc = "quit" }) + r:claim(bufnr, "n", "gm", noop, { desc = "align move" }) + r:end_scope() + assert.is_not_nil(map_of(bufnr, "n", "gm")) + + -- Second pass omits gm, as happens when switching to inline layout. + r:begin_scope("view") + r:claim(bufnr, "n", "q", noop, { desc = "quit" }) + r:end_scope() + + assert.is_not_nil(map_of(bufnr, "n", "q"), "renewed claims survive") + assert.is_nil(map_of(bufnr, "n", "gm"), "claims not renewed by the pass are released") + r:dispose() + end) + + it("releases a whole scope on demand", function() + local bufnr = new_buf() + local r = keymap.new("test") + + r:begin_scope("view") + r:claim(bufnr, "n", "q", noop, { desc = "quit" }) + r:end_scope() + r:begin_scope("conflict") + r:claim(bufnr, "n", "]x", noop, { desc = "next conflict" }) + r:end_scope() + + r:release_scope("conflict") + assert.is_nil(map_of(bufnr, "n", "]x"), "leaving conflict mode retires its mappings") + assert.is_not_nil(map_of(bufnr, "n", "q"), "other scopes are untouched") + r:dispose() + end) + + it("keeps unscoped claims out of scope retirement", function() + local bufnr = new_buf() + local r = keymap.new("test") + r:claim(bufnr, "n", "K", noop, { desc = "hover" }) + + r:begin_scope("view") + r:claim(bufnr, "n", "q", noop, { desc = "quit" }) + r:end_scope() + + assert.is_not_nil(map_of(bufnr, "n", "K"), "claims made outside a scope are never retired by one") + r:dispose() + end) + end) + describe("invalid buffers", function() it("ignores claims on an invalid buffer", function() local bufnr = new_buf() From 0efe8ac98e52dae7d051332947cc7db359399b1b Mon Sep 17 00:00:00 2001 From: Yanuo Ma Date: Sat, 1 Aug 2026 02:49:53 -0400 Subject: [PATCH 3/5] refactor(view): separate keymap declarations from their actions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ui/view/keymaps.lua carried both jobs: it decided which key binds where, and it implemented what every key does. At 866 lines, 77% of it was action bodies, which made the binding rules hard to read and hid a real hazard — each action closed over tabpage, both buffer numbers and the layout flags, so a mapping installed for one diff could still be holding a buffer the session had already replaced. The action bodies move to ui/view/actions/ unchanged, and take an explicit context instead of capturing it: keymaps.lua 866 -> 214 declarations only actions/hunk.lua 252 hunk lookup, patch building, stage/unstage/ discard, and the ih textobject actions/panes.lua 156 explorer toggle and focus, open-in-previous-tab actions/move.lua 147 gm alignment actions/diffget.lua 118 do / dp actions/stage.lua 70 toggle_stage, toggle_staged_view Making the capture explicit is the point: the values an action depends on are now visible in its signature rather than inherited from whichever setup pass created it. The other keymap modules are left alone. conflict/keymaps.lua already delegates to its own action modules, and the explorer and history panels bind panel-scoped objects that live and die with the panel, so they do not carry the same hazard. Behavior is unchanged: every extracted body is identical to its original modulo the context parameter, and the golden keymap matrix still matches. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- lua/codediff/ui/view/actions/diffget.lua | 118 ++++ lua/codediff/ui/view/actions/hunk.lua | 252 +++++++ lua/codediff/ui/view/actions/move.lua | 147 ++++ lua/codediff/ui/view/actions/panes.lua | 156 +++++ lua/codediff/ui/view/actions/stage.lua | 70 ++ lua/codediff/ui/view/keymaps.lua | 817 ++--------------------- 6 files changed, 811 insertions(+), 749 deletions(-) create mode 100644 lua/codediff/ui/view/actions/diffget.lua create mode 100644 lua/codediff/ui/view/actions/hunk.lua create mode 100644 lua/codediff/ui/view/actions/move.lua create mode 100644 lua/codediff/ui/view/actions/panes.lua create mode 100644 lua/codediff/ui/view/actions/stage.lua diff --git a/lua/codediff/ui/view/actions/diffget.lua b/lua/codediff/ui/view/actions/diffget.lua new file mode 100644 index 00000000..1f7c9466 --- /dev/null +++ b/lua/codediff/ui/view/actions/diffget.lua @@ -0,0 +1,118 @@ +-- Vimdiff-style do/dp: move a hunk between the two panes. +-- +-- In inline layout there is only one pane, so `do` reverts the hunk to the +-- original and `dp` has nothing to do. + +local M = {} + +local lifecycle = require("codediff.ui.lifecycle") +local auto_refresh = require("codediff.ui.auto_refresh") +local hunk_actions = require("codediff.ui.view.actions.hunk") + +function M.diff_get(ctx) + local session = lifecycle.get_session(ctx.tabpage) + if not session then + return + end + + if ctx.is_inline then + -- Inline mode: revert modified lines to original + if not vim.bo[ctx.modified_bufnr].modifiable then + vim.notify("Buffer is not modifiable", vim.log.levels.WARN) + return + end + + local hunk, hunk_idx = hunk_actions.find_hunk_at_cursor(ctx) + if not hunk then + vim.notify("No hunk at cursor position", vim.log.levels.WARN) + return + end + + local orig_lines = vim.api.nvim_buf_get_lines(ctx.original_bufnr, hunk.original.start_line - 1, hunk.original.end_line - 1, false) + vim.api.nvim_buf_set_lines(ctx.modified_bufnr, hunk.modified.start_line - 1, hunk.modified.end_line - 1, false, orig_lines) + auto_refresh.trigger(ctx.modified_bufnr) + vim.api.nvim_echo({ { string.format("Reverted hunk %d", hunk_idx), "None" } }, false, {}) + return + end + + -- Side-by-side mode: copy from other buffer to current + local current_buf = vim.api.nvim_get_current_buf() + local is_original = current_buf == ctx.original_bufnr + local target_buf = current_buf + local source_buf = is_original and ctx.modified_bufnr or ctx.original_bufnr + + -- Check if target buffer is modifiable + if not vim.bo[target_buf].modifiable then + vim.notify("Buffer is not modifiable", vim.log.levels.WARN) + return + end + + local hunk, hunk_idx = hunk_actions.find_hunk_at_cursor(ctx) + if not hunk then + vim.notify("No hunk at cursor position", vim.log.levels.WARN) + return + end + + -- Get source and target ranges + local source_range = is_original and hunk.modified or hunk.original + local target_range = is_original and hunk.original or hunk.modified + + -- Get lines from source buffer + local source_lines = vim.api.nvim_buf_get_lines(source_buf, source_range.start_line - 1, source_range.end_line - 1, false) + + -- Replace lines in target buffer + vim.api.nvim_buf_set_lines(target_buf, target_range.start_line - 1, target_range.end_line - 1, false, source_lines) + + -- Trigger diff refresh to update highlights + auto_refresh.trigger(target_buf) + + vim.api.nvim_echo({ { string.format("Obtained hunk %d", hunk_idx), "None" } }, false, {}) +end + +function M.diff_put(ctx) + local session = lifecycle.get_session(ctx.tabpage) + if not session then + return + end + + if ctx.is_inline then + -- Inline mode: buffer already has modified content, dp is a no-op + vim.notify("Buffer already contains the modified version. Use 'do' to revert to original.", vim.log.levels.INFO) + return + end + + -- Side-by-side mode: copy from current buffer to other + local current_buf = vim.api.nvim_get_current_buf() + local is_original = current_buf == ctx.original_bufnr + local source_buf = current_buf + local target_buf = is_original and ctx.modified_bufnr or ctx.original_bufnr + + -- Check if target buffer is modifiable + if not vim.bo[target_buf].modifiable then + vim.notify("Target buffer is not modifiable", vim.log.levels.WARN) + return + end + + local hunk, hunk_idx = hunk_actions.find_hunk_at_cursor(ctx) + if not hunk then + vim.notify("No hunk at cursor position", vim.log.levels.WARN) + return + end + + -- Get source and target ranges + local source_range = is_original and hunk.original or hunk.modified + local target_range = is_original and hunk.modified or hunk.original + + -- Get lines from source buffer + local source_lines = vim.api.nvim_buf_get_lines(source_buf, source_range.start_line - 1, source_range.end_line - 1, false) + + -- Replace lines in target buffer + vim.api.nvim_buf_set_lines(target_buf, target_range.start_line - 1, target_range.end_line - 1, false, source_lines) + + -- Trigger diff refresh to update highlights + auto_refresh.trigger(target_buf) + + vim.api.nvim_echo({ { string.format("Put hunk %d", hunk_idx), "None" } }, false, {}) +end + +return M diff --git a/lua/codediff/ui/view/actions/hunk.lua b/lua/codediff/ui/view/actions/hunk.lua new file mode 100644 index 00000000..69de9ff5 --- /dev/null +++ b/lua/codediff/ui/view/actions/hunk.lua @@ -0,0 +1,252 @@ +-- Hunk-level actions: locate the hunk under the cursor, and stage, unstage or +-- discard it. +-- +-- Every action takes the session context built by ui/view/keymaps.lua rather +-- than closing over buffers and layout flags, so a mapping installed for one +-- diff cannot act on a stale buffer after a file switch or layout change. + +local M = {} + +local lifecycle = require("codediff.ui.lifecycle") +local auto_refresh = require("codediff.ui.auto_refresh") + +function M.find_hunk_at_cursor(ctx) + local session = lifecycle.get_session(ctx.tabpage) + if not session or not session.stored_diff_result then + return nil, nil + end + local diff_result = session.stored_diff_result + if not diff_result.changes or #diff_result.changes == 0 then + return nil, nil + end + + local current_buf = vim.api.nvim_get_current_buf() + -- In inline mode, always use modified ranges + local is_original = not ctx.is_inline and current_buf == ctx.original_bufnr + local cursor = vim.api.nvim_win_get_cursor(0) + local current_line = cursor[1] + + for i, mapping in ipairs(diff_result.changes) do + local start_line = is_original and mapping.original.start_line or mapping.modified.start_line + local end_line = is_original and mapping.original.end_line or mapping.modified.end_line + -- Check if cursor is within this hunk (end_line is exclusive) + if current_line >= start_line and current_line < end_line then + return mapping, i + end + -- Also match if it's a deletion (empty range) and cursor is at start + if start_line == end_line and current_line == start_line then + return mapping, i + end + end + return nil, nil +end + +local function build_hunk_patch(file_path, orig_lines, mod_lines, orig_start, mod_start) + local orig_count = #orig_lines + local mod_count = #mod_lines + + -- For pure insertions with 0 original lines, git expects start to be + -- the line AFTER which content is inserted (0 if at very start) + local hdr_orig_start = orig_count == 0 and (orig_start > 0 and orig_start - 1 or 0) or orig_start + local hdr_mod_start = mod_count == 0 and (mod_start > 0 and mod_start - 1 or 0) or mod_start + + local parts = { + string.format("--- a/%s", file_path), + string.format("+++ b/%s", file_path), + string.format("@@ -%d,%d +%d,%d @@", hdr_orig_start, orig_count, hdr_mod_start, mod_count), + } + + for _, line in ipairs(orig_lines) do + table.insert(parts, "-" .. line) + end + for _, line in ipairs(mod_lines) do + table.insert(parts, "+" .. line) + end + + -- Patch must end with a newline + return table.concat(parts, "\n") .. "\n" +end + +function M.stage_hunk(ctx) + local session = lifecycle.get_session(ctx.tabpage) + if not session or not session.git_root then + vim.notify("Not in a git repository", vim.log.levels.WARN) + return + end + + -- Only allow staging from unstaged views (working tree changes) + if session.modified_revision ~= nil then + vim.notify("Stage only works on unstaged changes", vim.log.levels.WARN) + return + end + + local hunk, hunk_idx = M.find_hunk_at_cursor(ctx) + if not hunk then + vim.notify("No hunk at cursor position", vim.log.levels.WARN) + return + end + + -- Get the file path relative to git root + local file_path = (session.original.relative ~= "" and session.original.relative) or session.modified.relative + if not file_path or file_path == "" then + vim.notify("No file path for staging", vim.log.levels.WARN) + return + end + + local stage_orig_buf, stage_mod_buf = lifecycle.get_buffers(ctx.tabpage) + if not stage_orig_buf or not stage_mod_buf or not vim.api.nvim_buf_is_valid(stage_orig_buf) or not vim.api.nvim_buf_is_valid(stage_mod_buf) then + vim.notify("Diff buffers are no longer available", vim.log.levels.WARN) + return + end + + -- Read lines from both buffers for this hunk + local orig_lines = vim.api.nvim_buf_get_lines(stage_orig_buf, hunk.original.start_line - 1, hunk.original.end_line - 1, false) + local mod_lines = vim.api.nvim_buf_get_lines(stage_mod_buf, hunk.modified.start_line - 1, hunk.modified.end_line - 1, false) + + local patch = build_hunk_patch(file_path, orig_lines, mod_lines, hunk.original.start_line, hunk.modified.start_line) + + local git = require("codediff.core.git") + git.apply_patch(session.git_root, patch, false, function(err) + if err then + vim.notify("Failed to stage hunk: " .. err, vim.log.levels.ERROR) + return + end + vim.notify(string.format("Staged hunk %d", hunk_idx), vim.log.levels.INFO) + end) +end + +function M.unstage_hunk(ctx) + local session = lifecycle.get_session(ctx.tabpage) + if not session or not session.git_root then + vim.notify("Not in a git repository", vim.log.levels.WARN) + return + end + + -- Only allow unstaging from staged views + if session.modified_revision ~= ":0" then + vim.notify("Unstage only works on staged changes", vim.log.levels.WARN) + return + end + + local hunk, hunk_idx = M.find_hunk_at_cursor(ctx) + if not hunk then + vim.notify("No hunk at cursor position", vim.log.levels.WARN) + return + end + + local file_path = (session.original.relative ~= "" and session.original.relative) or session.modified.relative + if not file_path or file_path == "" then + vim.notify("No file path for unstaging", vim.log.levels.WARN) + return + end + + local unstage_orig_buf, unstage_mod_buf = lifecycle.get_buffers(ctx.tabpage) + if not unstage_orig_buf or not unstage_mod_buf or not vim.api.nvim_buf_is_valid(unstage_orig_buf) or not vim.api.nvim_buf_is_valid(unstage_mod_buf) then + vim.notify("Diff buffers are no longer available", vim.log.levels.WARN) + return + end + + -- Read lines from both buffers for this hunk + local orig_lines = vim.api.nvim_buf_get_lines(unstage_orig_buf, hunk.original.start_line - 1, hunk.original.end_line - 1, false) + local mod_lines = vim.api.nvim_buf_get_lines(unstage_mod_buf, hunk.modified.start_line - 1, hunk.modified.end_line - 1, false) + + local patch = build_hunk_patch(file_path, orig_lines, mod_lines, hunk.original.start_line, hunk.modified.start_line) + + local git = require("codediff.core.git") + git.apply_patch(session.git_root, patch, true, function(err) + if err then + vim.notify("Failed to unstage hunk: " .. err, vim.log.levels.ERROR) + return + end + vim.notify(string.format("Unstaged hunk %d", hunk_idx), vim.log.levels.INFO) + end) +end + +function M.discard_hunk(ctx) + local session = lifecycle.get_session(ctx.tabpage) + if not session or not session.git_root then + vim.notify("Not in a git repository", vim.log.levels.WARN) + return + end + + -- Only allow discarding in unstaged views (working tree changes) + if session.modified_revision ~= nil then + vim.notify("Discard only works on unstaged changes (working tree)", vim.log.levels.WARN) + return + end + + local hunk, hunk_idx = M.find_hunk_at_cursor(ctx) + if not hunk then + vim.notify("No hunk at cursor position", vim.log.levels.WARN) + return + end + + -- Prompt for confirmation before discarding (destructive operation) + local prompt = string.format("Discard hunk %d?", hunk_idx) + local choice = vim.fn.confirm(prompt, "&Discard\n&Cancel", 2, "Warning") + if choice ~= 1 then + return + end + + local discard_orig_buf, discard_mod_buf = lifecycle.get_buffers(ctx.tabpage) + if not discard_orig_buf or not discard_mod_buf or not vim.api.nvim_buf_is_valid(discard_orig_buf) or not vim.api.nvim_buf_is_valid(discard_mod_buf) then + vim.notify("Diff buffers are no longer available", vim.log.levels.WARN) + return + end + + -- Replace the modified hunk range with the original lines. Every other line + -- (including unrelated unsaved edits) stays as-is in the live buffer, so the + -- discarded region falls back to original content and nothing else changes. + local orig_lines = vim.api.nvim_buf_get_lines(discard_orig_buf, hunk.original.start_line - 1, hunk.original.end_line - 1, false) + + local was_modifiable = vim.bo[discard_mod_buf].modifiable + local was_readonly = vim.bo[discard_mod_buf].readonly + + local ok, edit_err = pcall(function() + vim.bo[discard_mod_buf].readonly = false + vim.bo[discard_mod_buf].modifiable = true + vim.api.nvim_buf_set_lines(discard_mod_buf, hunk.modified.start_line - 1, hunk.modified.end_line - 1, false, orig_lines) + -- Persist through the native write path so 'fileformat', 'fileencoding' + -- and 'endofline' are honored. 'noautocmd' keeps format-on-save (and + -- similar BufWritePre hooks) from rewriting lines outside the hunk. + vim.api.nvim_buf_call(discard_mod_buf, function() + vim.cmd("silent noautocmd write!") + end) + end) + + if vim.api.nvim_buf_is_valid(discard_mod_buf) then + vim.bo[discard_mod_buf].modifiable = was_modifiable + vim.bo[discard_mod_buf].readonly = was_readonly + end + + if not ok then + vim.notify("Failed to discard hunk: " .. tostring(edit_err), vim.log.levels.ERROR) + return + end + + auto_refresh.trigger(discard_mod_buf) + vim.notify(string.format("Discarded hunk %d", hunk_idx), vim.log.levels.INFO) +end + +--- Visually select the hunk under the cursor, for the `ih` textobject. +--- @param ctx CodeDiffActionContext +function M.select_hunk(ctx) + local mapping = M.find_hunk_at_cursor(ctx) + if not mapping then + return + end + + local current_buf = vim.api.nvim_get_current_buf() + local is_original = current_buf == ctx.original_bufnr + local start_line = is_original and mapping.original.start_line or mapping.modified.start_line + local end_line = is_original and mapping.original.end_line or mapping.modified.end_line + + -- end_line is exclusive, and empty ranges (deletions) can't be selected + if start_line >= end_line then + return + end + + vim.cmd("normal! " .. start_line .. "GV" .. (end_line - 1) .. "G") +end + +return M diff --git a/lua/codediff/ui/view/actions/move.lua b/lua/codediff/ui/view/actions/move.lua new file mode 100644 index 00000000..9306eb4e --- /dev/null +++ b/lua/codediff/ui/view/actions/move.lua @@ -0,0 +1,147 @@ +-- Temporarily align a moved code block with its counterpart in the other pane, +-- restoring both views once the cursor leaves the block. + +local M = {} + +local lifecycle = require("codediff.ui.lifecycle") + +function M.align_move(ctx) + local session = lifecycle.get_session(ctx.tabpage) + if not session or not session.stored_diff_result or not session.stored_diff_result.moves then + return + end + if ctx.is_inline then + return + end -- Only works in side-by-side + + local moves = session.stored_diff_result.moves + if #moves == 0 then + vim.notify("No moved code blocks in current diff", vim.log.levels.INFO) + return + end + + local current_buf = vim.api.nvim_get_current_buf() + local cursor_line = vim.api.nvim_win_get_cursor(0)[1] + + -- Read current buffers from session (not closure — may have changed via file switch) + local sess_orig_buf = session.original_bufnr + local sess_mod_buf = session.modified_bufnr + + -- Find which move the cursor is in + local current_move = nil + local is_on_original = current_buf == sess_orig_buf + for _, move in ipairs(moves) do + local range = is_on_original and move.original or move.modified + if cursor_line >= range.start_line and cursor_line < range.end_line then + current_move = move + break + end + end + + if not current_move then + vim.notify("Not on a moved code block", vim.log.levels.INFO) + return + end + + local current_win = vim.api.nvim_get_current_win() + local other_win = is_on_original and session.modified_win or session.original_win + if not vim.api.nvim_win_is_valid(other_win) then + return + end + + local my_range = is_on_original and current_move.original or current_move.modified + local other_range = is_on_original and current_move.modified or current_move.original + + -- Save full view state of both windows + local current_view = vim.api.nvim_win_call(current_win, function() + return vim.fn.winsaveview() + end) + local other_view = vim.api.nvim_win_call(other_win, function() + return vim.fn.winsaveview() + end) + local saved_scrolloff_other = vim.wo[other_win].scrolloff + + -- Pause structural scroll-sync while we impose the move alignment. + local scroll = require("codediff.ui.scroll") + scroll.pause(ctx.tabpage) + vim.wo[other_win].scrolloff = 0 + + -- Align using the annotation virt_line as anchor: + -- Both sides have "⇄ moved" above their first moved line. + -- Use winline() to get the actual visual row (accounts for virtual/filler lines). + local my_first = my_range.start_line + local other_first = other_range.start_line + + -- Get actual visual row of the moved block start (accounts for filler virt_lines) + -- Save and restore cursor so the user's position is not disturbed. + local my_visual_row = vim.api.nvim_win_call(current_win, function() + local saved_pos = vim.api.nvim_win_get_cursor(current_win) + vim.api.nvim_win_set_cursor(current_win, { my_first, 0 }) + local row = vim.fn.winline() + vim.api.nvim_win_set_cursor(current_win, saved_pos) + return row + end) + + -- Set other pane: position other_first at the same visual row + -- winline() is 1-based from top of window + vim.api.nvim_win_call(other_win, function() + -- First scroll to the target line at top of window + vim.api.nvim_win_set_cursor(other_win, { other_first, 0 }) + vim.cmd("normal! zt") + -- Now scroll down to match the visual offset (Ctrl-Y scrolls view up, line moves down) + if my_visual_row > 1 then + local keys = vim.api.nvim_replace_termcodes((my_visual_row - 1) .. "", true, false, true) + vim.api.nvim_feedkeys(keys, "nx", false) + end + end) + + -- Restore function — called when cursor leaves moved block or switches window + local augroup = vim.api.nvim_create_augroup("codediff_move_align_" .. ctx.tabpage, { clear = true }) + local restored = false + + local function restore() + if restored then + return + end + restored = true + pcall(vim.api.nvim_del_augroup_by_id, augroup) + if vim.api.nvim_win_is_valid(other_win) then + vim.wo[other_win].scrolloff = saved_scrolloff_other + end + if not vim.api.nvim_win_is_valid(current_win) or not vim.api.nvim_win_is_valid(other_win) then + return + end + -- Restore views first, then resume structural scroll-sync. + vim.api.nvim_win_call(other_win, function() + vim.fn.winrestview(other_view) + end) + vim.api.nvim_win_call(current_win, function() + vim.fn.winrestview(current_view) + end) + scroll.resume(ctx.tabpage) + end + + -- Restore when cursor moves out of the moved block + vim.api.nvim_create_autocmd("CursorMoved", { + group = augroup, + buffer = current_buf, + callback = function() + local new_line = vim.api.nvim_win_get_cursor(0)[1] + if new_line < my_range.start_line or new_line >= my_range.end_line then + restore() + end + end, + }) + + -- Restore when user switches to another window (WinLeave) + -- Use vim.schedule to defer restore until after Neovim finishes + -- the window switch and cursor placement from the click event. + vim.api.nvim_create_autocmd("WinLeave", { + group = augroup, + callback = function() + vim.schedule(restore) + end, + }) +end + +return M diff --git a/lua/codediff/ui/view/actions/panes.lua b/lua/codediff/ui/view/actions/panes.lua new file mode 100644 index 00000000..6b087c46 --- /dev/null +++ b/lua/codediff/ui/view/actions/panes.lua @@ -0,0 +1,156 @@ +-- Actions that move focus or content between codediff's panes and the rest of +-- the editor: explorer visibility and focus, and opening the real file in the +-- previous tab. + +local M = {} + +local lifecycle = require("codediff.ui.lifecycle") +local config = require("codediff.config") + +local function get_explorer_target_file(explorer, session) + local node = explorer.tree and explorer.tree:get_node() + local data = node and node.data + + if not data or data.type == "group" or data.type == "directory" or not data.path or data.path == "" then + return nil + end + + local git_root = data.git_root or explorer.git_root or session.git_root + if not git_root or git_root == "" then + return nil + end + + return vim.fs.joinpath(git_root, data.path) +end + +function M.toggle_explorer(ctx) + local explorer_obj = lifecycle.get_explorer(ctx.tabpage) + if not explorer_obj then + vim.notify("No explorer found for this tab", vim.log.levels.WARN) + return + end + local explorer = require("codediff.ui.explorer") + explorer.toggle_visibility(explorer_obj) +end + +function M.focus_explorer(ctx) + local explorer_obj = lifecycle.get_explorer(ctx.tabpage) + if not explorer_obj then + vim.notify("No explorer found for this tab", vim.log.levels.WARN) + return + end + local split = explorer_obj.split + if not split or not split.winid or not vim.api.nvim_win_is_valid(split.winid) then + -- Explorer is hidden, show it first then focus + local explorer = require("codediff.ui.explorer") + explorer.toggle_visibility(explorer_obj) + end + if split and split.winid and vim.api.nvim_win_is_valid(split.winid) then + vim.api.nvim_set_current_win(split.winid) + end +end + +function M.open_in_prev_tab(ctx) + local session = lifecycle.get_session(ctx.tabpage) + if not session then + return + end + + local current_buf = vim.api.nvim_get_current_buf() + local side = nil + if current_buf == ctx.original_bufnr then + side = "original" + elseif current_buf == ctx.modified_bufnr then + side = "modified" + end + + local explorer = lifecycle.get_explorer(ctx.tabpage) + local is_explorer_buf = explorer and explorer.bufnr and current_buf == explorer.bufnr + + -- Only operate on diff and explorer buffers; ignore history/result silently + if not side and not is_explorer_buf then + return + end + + local is_virtual = (side == "original" and lifecycle.is_original_virtual(ctx.tabpage)) or (side == "modified" and lifecycle.is_modified_virtual(ctx.tabpage)) + + -- Resolve target file path + local target_file + if is_explorer_buf then + target_file = get_explorer_target_file(explorer, session) + if not target_file then + return + end + elseif is_virtual then + local original, modified = lifecycle.get_paths(ctx.tabpage) + local ref = side == "original" and original or modified + if not ref or ref.absolute == "" then + vim.notify("Buffer has no associated file path", vim.log.levels.WARN) + return + end + target_file = ref.absolute + else + target_file = vim.api.nvim_buf_get_name(current_buf) + if target_file == "" then + vim.notify("Buffer has no name; cannot open in previous tab", vim.log.levels.WARN) + return + end + end + + local cursor = side and vim.api.nvim_win_get_cursor(0) or nil + local current_tab = vim.api.nvim_get_current_tabpage() + local tabs = vim.api.nvim_list_tabpages() + + local current_index = nil + for i, tab in ipairs(tabs) do + if tab == current_tab then + current_index = i + break + end + end + + local target_tab + if current_index and current_index > 1 then + target_tab = tabs[current_index - 1] + else + vim.cmd("tabnew") + target_tab = vim.api.nvim_get_current_tabpage() + vim.cmd("tabmove 0") + end + + if vim.api.nvim_get_current_tabpage() ~= target_tab then + vim.api.nvim_set_current_tabpage(target_tab) + end + + local target_win = vim.api.nvim_get_current_win() + if not vim.api.nvim_win_is_valid(target_win) then + vim.notify("No valid window in target tab to open buffer", vim.log.levels.ERROR) + return + end + + local ok, err + if is_virtual or is_explorer_buf then + ok, err = pcall(vim.cmd, "edit " .. vim.fn.fnameescape(target_file)) + else + ok, err = pcall(vim.api.nvim_win_set_buf, target_win, current_buf) + end + if not ok then + vim.notify("Failed to open buffer in previous tab: " .. err, vim.log.levels.ERROR) + return + end + + if cursor then + pcall(vim.api.nvim_win_set_cursor, target_win, cursor) + end + + -- Optionally close codediff after navigating to file + if config.options.keymaps.view.close_on_open_in_prev_tab then + -- Switch back to diff tab and close it + if vim.api.nvim_tabpage_is_valid(current_tab) then + vim.api.nvim_set_current_tabpage(current_tab) + vim.cmd("tabclose") + end + end +end + +return M diff --git a/lua/codediff/ui/view/actions/stage.lua b/lua/codediff/ui/view/actions/stage.lua new file mode 100644 index 00000000..3e20e835 --- /dev/null +++ b/lua/codediff/ui/view/actions/stage.lua @@ -0,0 +1,70 @@ +-- File-level staging actions driven from the explorer or the diff panes. + +local M = {} + +local lifecycle = require("codediff.ui.lifecycle") + +function M.toggle_stage(ctx) + local current_buf = vim.api.nvim_get_current_buf() + local explorer = lifecycle.get_explorer(ctx.tabpage) + local session = lifecycle.get_session(ctx.tabpage) + + if not session then + return + end + + -- Only available in explorer mode with git + if not ctx.is_explorer_mode then + vim.notify("Stage/unstage only available in explorer mode", vim.log.levels.WARN) + return + end + + if not explorer or not explorer.git_root then + vim.notify("Stage/unstage only available in git mode", vim.log.levels.WARN) + return + end + + -- Case 1: Cursor in explorer buffer + if explorer.bufnr and current_buf == explorer.bufnr then + -- Delegate to explorer action (handles files and directories) + local explorer_module = require("codediff.ui.explorer") + explorer_module.toggle_stage_entry(explorer, explorer.tree) + return + end + + -- Case 2: Cursor in diff buffers (original or modified) + if current_buf == ctx.original_bufnr or current_buf == ctx.modified_bufnr then + local file_path = explorer.current_file_path + local group = explorer.current_file_group + + -- Guard: must have a current file selected + if not file_path then + vim.notify("No file selected", vim.log.levels.WARN) + return + end + + -- Guard: file must be stageable + if not group or (group ~= "staged" and group ~= "unstaged" and group ~= "conflicts") then + vim.notify("Current file cannot be staged/unstaged", vim.log.levels.WARN) + return + end + + local explorer_module = require("codediff.ui.explorer") + explorer_module.toggle_stage_file(explorer.git_root, file_path, group) + return + end + + -- Case 3: Other buffers (history, etc.) - do nothing silently +end + +function M.toggle_staged_view(ctx) + local explorer = lifecycle.get_explorer(ctx.tabpage) + if not ctx.is_explorer_mode or not explorer then + vim.notify("Toggle staged view only available in explorer mode", vim.log.levels.WARN) + return + end + local explorer_module = require("codediff.ui.explorer") + explorer_module.toggle_staged_view(explorer) +end + +return M diff --git a/lua/codediff/ui/view/keymaps.lua b/lua/codediff/ui/view/keymaps.lua index c0c9c340..619b4c62 100644 --- a/lua/codediff/ui/view/keymaps.lua +++ b/lua/codediff/ui/view/keymaps.lua @@ -1,27 +1,30 @@ --- Keymaps setup for diff view +-- Keymap declarations for the diff view. +-- +-- This module decides which keys are bound, on which buffers, in which session +-- shapes. The behavior behind each key lives in ui/view/actions/, and receives +-- the session context below rather than closing over buffers and layout flags, +-- so a mapping cannot act on a buffer the session has since replaced. local M = {} local lifecycle = require("codediff.ui.lifecycle") -local auto_refresh = require("codediff.ui.auto_refresh") local config = require("codediff.config") local navigation = require("codediff.ui.view.navigation") local compact = require("codediff.ui.view.compact") -local function get_explorer_target_file(explorer, session) - local node = explorer.tree and explorer.tree:get_node() - local data = node and node.data - - if not data or data.type == "group" or data.type == "directory" or not data.path or data.path == "" then - return nil - end - - local git_root = data.git_root or explorer.git_root or session.git_root - if not git_root or git_root == "" then - return nil - end - - return vim.fs.joinpath(git_root, data.path) -end +local hunk = require("codediff.ui.view.actions.hunk") +local diffget = require("codediff.ui.view.actions.diffget") +local panes = require("codediff.ui.view.actions.panes") +local stage = require("codediff.ui.view.actions.stage") +local move = require("codediff.ui.view.actions.move") + +--- @class CodeDiffActionContext +--- @field tabpage number +--- @field original_bufnr number +--- @field modified_bufnr number +--- @field is_explorer_mode boolean +--- @field is_history_mode boolean +--- @field is_inline boolean +--- @field is_conflict boolean Merge view: a result pane exists -- Centralized keymap setup for all diff view keymaps -- This function sets up ALL keymaps in one place for better maintainability @@ -39,563 +42,18 @@ function M.setup_all_keymaps(tabpage, original_bufnr, modified_bufnr, is_explore local is_inline = session and session.layout == "inline" -- Merge/conflict view: the result pane exists and do/dp are replaced by the -- conflict mappings (see codediff.ui.conflict.keymaps). - local is_conflict = session and session.result_bufnr ~= nil - - -- Helper: Toggle explorer visibility (explorer mode only) - local function toggle_explorer() - local explorer_obj = lifecycle.get_explorer(tabpage) - if not explorer_obj then - vim.notify("No explorer found for this tab", vim.log.levels.WARN) - return - end - local explorer = require("codediff.ui.explorer") - explorer.toggle_visibility(explorer_obj) - end - - -- Helper: Focus explorer panel (explorer mode only) - local function focus_explorer() - local explorer_obj = lifecycle.get_explorer(tabpage) - if not explorer_obj then - vim.notify("No explorer found for this tab", vim.log.levels.WARN) - return - end - local split = explorer_obj.split - if not split or not split.winid or not vim.api.nvim_win_is_valid(split.winid) then - -- Explorer is hidden, show it first then focus - local explorer = require("codediff.ui.explorer") - explorer.toggle_visibility(explorer_obj) - end - if split and split.winid and vim.api.nvim_win_is_valid(split.winid) then - vim.api.nvim_set_current_win(split.winid) - end - end - - -- Helper: Find hunk at cursor position - -- Returns the hunk and its index, or nil if cursor is not in a hunk - local function find_hunk_at_cursor() - local session = lifecycle.get_session(tabpage) - if not session or not session.stored_diff_result then - return nil, nil - end - local diff_result = session.stored_diff_result - if not diff_result.changes or #diff_result.changes == 0 then - return nil, nil - end - - local current_buf = vim.api.nvim_get_current_buf() - -- In inline mode, always use modified ranges - local is_original = not is_inline and current_buf == original_bufnr - local cursor = vim.api.nvim_win_get_cursor(0) - local current_line = cursor[1] - - for i, mapping in ipairs(diff_result.changes) do - local start_line = is_original and mapping.original.start_line or mapping.modified.start_line - local end_line = is_original and mapping.original.end_line or mapping.modified.end_line - -- Check if cursor is within this hunk (end_line is exclusive) - if current_line >= start_line and current_line < end_line then - return mapping, i - end - -- Also match if it's a deletion (empty range) and cursor is at start - if start_line == end_line and current_line == start_line then - return mapping, i - end - end - return nil, nil - end - - -- Helper: Diff get - obtain change from other buffer to current buffer - local function diff_get() - local session = lifecycle.get_session(tabpage) - if not session then - return - end - - if is_inline then - -- Inline mode: revert modified lines to original - if not vim.bo[modified_bufnr].modifiable then - vim.notify("Buffer is not modifiable", vim.log.levels.WARN) - return - end - - local hunk, hunk_idx = find_hunk_at_cursor() - if not hunk then - vim.notify("No hunk at cursor position", vim.log.levels.WARN) - return - end - - local orig_lines = vim.api.nvim_buf_get_lines(original_bufnr, hunk.original.start_line - 1, hunk.original.end_line - 1, false) - vim.api.nvim_buf_set_lines(modified_bufnr, hunk.modified.start_line - 1, hunk.modified.end_line - 1, false, orig_lines) - auto_refresh.trigger(modified_bufnr) - vim.api.nvim_echo({ { string.format("Reverted hunk %d", hunk_idx), "None" } }, false, {}) - return - end - - -- Side-by-side mode: copy from other buffer to current - local current_buf = vim.api.nvim_get_current_buf() - local is_original = current_buf == original_bufnr - local target_buf = current_buf - local source_buf = is_original and modified_bufnr or original_bufnr - - -- Check if target buffer is modifiable - if not vim.bo[target_buf].modifiable then - vim.notify("Buffer is not modifiable", vim.log.levels.WARN) - return - end - - local hunk, hunk_idx = find_hunk_at_cursor() - if not hunk then - vim.notify("No hunk at cursor position", vim.log.levels.WARN) - return - end - - -- Get source and target ranges - local source_range = is_original and hunk.modified or hunk.original - local target_range = is_original and hunk.original or hunk.modified - - -- Get lines from source buffer - local source_lines = vim.api.nvim_buf_get_lines(source_buf, source_range.start_line - 1, source_range.end_line - 1, false) - - -- Replace lines in target buffer - vim.api.nvim_buf_set_lines(target_buf, target_range.start_line - 1, target_range.end_line - 1, false, source_lines) - - -- Trigger diff refresh to update highlights - auto_refresh.trigger(target_buf) - - vim.api.nvim_echo({ { string.format("Obtained hunk %d", hunk_idx), "None" } }, false, {}) - end - - -- Helper: Diff put - put change from current buffer to other buffer - local function diff_put() - local session = lifecycle.get_session(tabpage) - if not session then - return - end - - if is_inline then - -- Inline mode: buffer already has modified content, dp is a no-op - vim.notify("Buffer already contains the modified version. Use 'do' to revert to original.", vim.log.levels.INFO) - return - end - - -- Side-by-side mode: copy from current buffer to other - local current_buf = vim.api.nvim_get_current_buf() - local is_original = current_buf == original_bufnr - local source_buf = current_buf - local target_buf = is_original and modified_bufnr or original_bufnr - - -- Check if target buffer is modifiable - if not vim.bo[target_buf].modifiable then - vim.notify("Target buffer is not modifiable", vim.log.levels.WARN) - return - end - - local hunk, hunk_idx = find_hunk_at_cursor() - if not hunk then - vim.notify("No hunk at cursor position", vim.log.levels.WARN) - return - end - - -- Get source and target ranges - local source_range = is_original and hunk.original or hunk.modified - local target_range = is_original and hunk.modified or hunk.original - - -- Get lines from source buffer - local source_lines = vim.api.nvim_buf_get_lines(source_buf, source_range.start_line - 1, source_range.end_line - 1, false) - - -- Replace lines in target buffer - vim.api.nvim_buf_set_lines(target_buf, target_range.start_line - 1, target_range.end_line - 1, false, source_lines) - - -- Trigger diff refresh to update highlights - auto_refresh.trigger(target_buf) - - vim.api.nvim_echo({ { string.format("Put hunk %d", hunk_idx), "None" } }, false, {}) - end - - -- Helper: Toggle stage/unstage for current file (tab-wide) - -- Works in: explorer buffer, diff buffers (original/modified) - -- Does nothing in: history buffer, other buffers - local function toggle_stage() - local current_buf = vim.api.nvim_get_current_buf() - local explorer = lifecycle.get_explorer(tabpage) - local session = lifecycle.get_session(tabpage) - - if not session then - return - end - - -- Only available in explorer mode with git - if not is_explorer_mode then - vim.notify("Stage/unstage only available in explorer mode", vim.log.levels.WARN) - return - end - - if not explorer or not explorer.git_root then - vim.notify("Stage/unstage only available in git mode", vim.log.levels.WARN) - return - end - - -- Case 1: Cursor in explorer buffer - if explorer.bufnr and current_buf == explorer.bufnr then - -- Delegate to explorer action (handles files and directories) - local explorer_module = require("codediff.ui.explorer") - explorer_module.toggle_stage_entry(explorer, explorer.tree) - return - end - - -- Case 2: Cursor in diff buffers (original or modified) - if current_buf == original_bufnr or current_buf == modified_bufnr then - local file_path = explorer.current_file_path - local group = explorer.current_file_group - - -- Guard: must have a current file selected - if not file_path then - vim.notify("No file selected", vim.log.levels.WARN) - return - end - - -- Guard: file must be stageable - if not group or (group ~= "staged" and group ~= "unstaged" and group ~= "conflicts") then - vim.notify("Current file cannot be staged/unstaged", vim.log.levels.WARN) - return - end - - local explorer_module = require("codediff.ui.explorer") - explorer_module.toggle_stage_file(explorer.git_root, file_path, group) - return - end - - -- Case 3: Other buffers (history, etc.) - do nothing silently - end - - -- Helper: Swap between the staged and unstaged view of the current file (#352). - -- Works from: explorer buffer, diff buffers. Silently no-op elsewhere. - local function toggle_staged_view() - local explorer = lifecycle.get_explorer(tabpage) - if not is_explorer_mode or not explorer then - vim.notify("Toggle staged view only available in explorer mode", vim.log.levels.WARN) - return - end - local explorer_module = require("codediff.ui.explorer") - explorer_module.toggle_staged_view(explorer) - end - - -- Helper: Open the current real buffer in the previous tab (or create one before) - local function open_in_prev_tab() - local session = lifecycle.get_session(tabpage) - if not session then - return - end - - local current_buf = vim.api.nvim_get_current_buf() - local side = nil - if current_buf == original_bufnr then - side = "original" - elseif current_buf == modified_bufnr then - side = "modified" - end - - local explorer = lifecycle.get_explorer(tabpage) - local is_explorer_buf = explorer and explorer.bufnr and current_buf == explorer.bufnr - - -- Only operate on diff and explorer buffers; ignore history/result silently - if not side and not is_explorer_buf then - return - end - - local is_virtual = (side == "original" and lifecycle.is_original_virtual(tabpage)) or (side == "modified" and lifecycle.is_modified_virtual(tabpage)) - - -- Resolve target file path - local target_file - if is_explorer_buf then - target_file = get_explorer_target_file(explorer, session) - if not target_file then - return - end - elseif is_virtual then - local original, modified = lifecycle.get_paths(tabpage) - local ref = side == "original" and original or modified - if not ref or ref.absolute == "" then - vim.notify("Buffer has no associated file path", vim.log.levels.WARN) - return - end - target_file = ref.absolute - else - target_file = vim.api.nvim_buf_get_name(current_buf) - if target_file == "" then - vim.notify("Buffer has no name; cannot open in previous tab", vim.log.levels.WARN) - return - end - end - - local cursor = side and vim.api.nvim_win_get_cursor(0) or nil - local current_tab = vim.api.nvim_get_current_tabpage() - local tabs = vim.api.nvim_list_tabpages() - - local current_index = nil - for i, tab in ipairs(tabs) do - if tab == current_tab then - current_index = i - break - end - end - - local target_tab - if current_index and current_index > 1 then - target_tab = tabs[current_index - 1] - else - vim.cmd("tabnew") - target_tab = vim.api.nvim_get_current_tabpage() - vim.cmd("tabmove 0") - end - - if vim.api.nvim_get_current_tabpage() ~= target_tab then - vim.api.nvim_set_current_tabpage(target_tab) - end - - local target_win = vim.api.nvim_get_current_win() - if not vim.api.nvim_win_is_valid(target_win) then - vim.notify("No valid window in target tab to open buffer", vim.log.levels.ERROR) - return - end - - local ok, err - if is_virtual or is_explorer_buf then - ok, err = pcall(vim.cmd, "edit " .. vim.fn.fnameescape(target_file)) - else - ok, err = pcall(vim.api.nvim_win_set_buf, target_win, current_buf) - end - if not ok then - vim.notify("Failed to open buffer in previous tab: " .. err, vim.log.levels.ERROR) - return - end - - if cursor then - pcall(vim.api.nvim_win_set_cursor, target_win, cursor) - end - - -- Optionally close codediff after navigating to file - if config.options.keymaps.view.close_on_open_in_prev_tab then - -- Switch back to diff tab and close it - if vim.api.nvim_tabpage_is_valid(current_tab) then - vim.api.nvim_set_current_tabpage(current_tab) - vim.cmd("tabclose") - end - end - end - - -- ======================================================================== - -- Hunk-level staging (S, U) - -- Generates a unified diff patch for the hunk under cursor and applies it - -- to the git index via `git apply --cached --unidiff-zero`. - -- Stage (S): applies the hunk's changes to the index (working → staged) - -- Unstage (U): reverse-applies to remove the hunk from the index - -- ======================================================================== - - --- Build a minimal unified diff patch string for a single hunk. - --- The patch has no context lines (used with --unidiff-zero). - --- @param file_path string relative path from git root - --- @param orig_lines string[] lines from the original (HEAD) buffer for this hunk - --- @param mod_lines string[] lines from the modified (working/staged) buffer for this hunk - --- @param orig_start number 1-based start line in original file - --- @param mod_start number 1-based start line in modified file - --- @return string patch valid unified diff patch - local function build_hunk_patch(file_path, orig_lines, mod_lines, orig_start, mod_start) - local orig_count = #orig_lines - local mod_count = #mod_lines - - -- For pure insertions with 0 original lines, git expects start to be - -- the line AFTER which content is inserted (0 if at very start) - local hdr_orig_start = orig_count == 0 and (orig_start > 0 and orig_start - 1 or 0) or orig_start - local hdr_mod_start = mod_count == 0 and (mod_start > 0 and mod_start - 1 or 0) or mod_start - - local parts = { - string.format("--- a/%s", file_path), - string.format("+++ b/%s", file_path), - string.format("@@ -%d,%d +%d,%d @@", hdr_orig_start, orig_count, hdr_mod_start, mod_count), - } - - for _, line in ipairs(orig_lines) do - table.insert(parts, "-" .. line) - end - for _, line in ipairs(mod_lines) do - table.insert(parts, "+" .. line) - end - - -- Patch must end with a newline - return table.concat(parts, "\n") .. "\n" - end - - -- Helper: Stage hunk under cursor to git index - local function stage_hunk() - local session = lifecycle.get_session(tabpage) - if not session or not session.git_root then - vim.notify("Not in a git repository", vim.log.levels.WARN) - return - end - - -- Only allow staging from unstaged views (working tree changes) - if session.modified_revision ~= nil then - vim.notify("Stage only works on unstaged changes", vim.log.levels.WARN) - return - end - - local hunk, hunk_idx = find_hunk_at_cursor() - if not hunk then - vim.notify("No hunk at cursor position", vim.log.levels.WARN) - return - end - - -- Get the file path relative to git root - local file_path = (session.original.relative ~= "" and session.original.relative) or session.modified.relative - if not file_path or file_path == "" then - vim.notify("No file path for staging", vim.log.levels.WARN) - return - end - - local stage_orig_buf, stage_mod_buf = lifecycle.get_buffers(tabpage) - if not stage_orig_buf or not stage_mod_buf or not vim.api.nvim_buf_is_valid(stage_orig_buf) or not vim.api.nvim_buf_is_valid(stage_mod_buf) then - vim.notify("Diff buffers are no longer available", vim.log.levels.WARN) - return - end - - -- Read lines from both buffers for this hunk - local orig_lines = vim.api.nvim_buf_get_lines(stage_orig_buf, hunk.original.start_line - 1, hunk.original.end_line - 1, false) - local mod_lines = vim.api.nvim_buf_get_lines(stage_mod_buf, hunk.modified.start_line - 1, hunk.modified.end_line - 1, false) - - local patch = build_hunk_patch(file_path, orig_lines, mod_lines, hunk.original.start_line, hunk.modified.start_line) - - local git = require("codediff.core.git") - git.apply_patch(session.git_root, patch, false, function(err) - if err then - vim.notify("Failed to stage hunk: " .. err, vim.log.levels.ERROR) - return - end - vim.notify(string.format("Staged hunk %d", hunk_idx), vim.log.levels.INFO) - end) - end - - -- Helper: Unstage hunk under cursor from git index - local function unstage_hunk() - local session = lifecycle.get_session(tabpage) - if not session or not session.git_root then - vim.notify("Not in a git repository", vim.log.levels.WARN) - return - end - - -- Only allow unstaging from staged views - if session.modified_revision ~= ":0" then - vim.notify("Unstage only works on staged changes", vim.log.levels.WARN) - return - end - - local hunk, hunk_idx = find_hunk_at_cursor() - if not hunk then - vim.notify("No hunk at cursor position", vim.log.levels.WARN) - return - end - - local file_path = (session.original.relative ~= "" and session.original.relative) or session.modified.relative - if not file_path or file_path == "" then - vim.notify("No file path for unstaging", vim.log.levels.WARN) - return - end - - local unstage_orig_buf, unstage_mod_buf = lifecycle.get_buffers(tabpage) - if not unstage_orig_buf or not unstage_mod_buf or not vim.api.nvim_buf_is_valid(unstage_orig_buf) or not vim.api.nvim_buf_is_valid(unstage_mod_buf) then - vim.notify("Diff buffers are no longer available", vim.log.levels.WARN) - return - end - - -- Read lines from both buffers for this hunk - local orig_lines = vim.api.nvim_buf_get_lines(unstage_orig_buf, hunk.original.start_line - 1, hunk.original.end_line - 1, false) - local mod_lines = vim.api.nvim_buf_get_lines(unstage_mod_buf, hunk.modified.start_line - 1, hunk.modified.end_line - 1, false) - - local patch = build_hunk_patch(file_path, orig_lines, mod_lines, hunk.original.start_line, hunk.modified.start_line) - - local git = require("codediff.core.git") - git.apply_patch(session.git_root, patch, true, function(err) - if err then - vim.notify("Failed to unstage hunk: " .. err, vim.log.levels.ERROR) - return - end - vim.notify(string.format("Unstaged hunk %d", hunk_idx), vim.log.levels.INFO) - end) - end - - -- Helper: Discard hunk under cursor from working tree. - -- Mirrors VSCode's `revertChange`/`_revertChanges`: revert the hunk range in - -- the in-memory modified buffer (like `applyLineChanges` + `WorkspaceEdit`), - -- then write the buffer to disk (like `modifiedDocument.save()`). No external - -- `git apply` and no file reload, so unrelated buffer content is preserved. - local function discard_hunk() - local session = lifecycle.get_session(tabpage) - if not session or not session.git_root then - vim.notify("Not in a git repository", vim.log.levels.WARN) - return - end - - -- Only allow discarding in unstaged views (working tree changes) - if session.modified_revision ~= nil then - vim.notify("Discard only works on unstaged changes (working tree)", vim.log.levels.WARN) - return - end - - local hunk, hunk_idx = find_hunk_at_cursor() - if not hunk then - vim.notify("No hunk at cursor position", vim.log.levels.WARN) - return - end - - -- Prompt for confirmation before discarding (destructive operation) - local prompt = string.format("Discard hunk %d?", hunk_idx) - local choice = vim.fn.confirm(prompt, "&Discard\n&Cancel", 2, "Warning") - if choice ~= 1 then - return - end - - local discard_orig_buf, discard_mod_buf = lifecycle.get_buffers(tabpage) - if not discard_orig_buf or not discard_mod_buf or not vim.api.nvim_buf_is_valid(discard_orig_buf) or not vim.api.nvim_buf_is_valid(discard_mod_buf) then - vim.notify("Diff buffers are no longer available", vim.log.levels.WARN) - return - end - - -- Replace the modified hunk range with the original lines. Every other line - -- (including unrelated unsaved edits) stays as-is in the live buffer, so the - -- discarded region falls back to original content and nothing else changes. - local orig_lines = vim.api.nvim_buf_get_lines(discard_orig_buf, hunk.original.start_line - 1, hunk.original.end_line - 1, false) - - local was_modifiable = vim.bo[discard_mod_buf].modifiable - local was_readonly = vim.bo[discard_mod_buf].readonly - - local ok, edit_err = pcall(function() - vim.bo[discard_mod_buf].readonly = false - vim.bo[discard_mod_buf].modifiable = true - vim.api.nvim_buf_set_lines(discard_mod_buf, hunk.modified.start_line - 1, hunk.modified.end_line - 1, false, orig_lines) - -- Persist through the native write path so 'fileformat', 'fileencoding' - -- and 'endofline' are honored. 'noautocmd' keeps format-on-save (and - -- similar BufWritePre hooks) from rewriting lines outside the hunk. - vim.api.nvim_buf_call(discard_mod_buf, function() - vim.cmd("silent noautocmd write!") - end) - end) - - if vim.api.nvim_buf_is_valid(discard_mod_buf) then - vim.bo[discard_mod_buf].modifiable = was_modifiable - vim.bo[discard_mod_buf].readonly = was_readonly - end - - if not ok then - vim.notify("Failed to discard hunk: " .. tostring(edit_err), vim.log.levels.ERROR) - return - end - - auto_refresh.trigger(discard_mod_buf) - vim.notify(string.format("Discarded hunk %d", hunk_idx), vim.log.levels.INFO) - end - - -- ======================================================================== - -- Bind all keymaps using unified API (one place for all keymaps!) - -- ======================================================================== + local is_conflict = session and session.result_bufnr ~= nil or false + + --- @type CodeDiffActionContext + local ctx = { + tabpage = tabpage, + original_bufnr = original_bufnr, + modified_bufnr = modified_bufnr, + is_explorer_mode = is_explorer_mode or false, + is_history_mode = is_history_mode or false, + is_inline = is_inline or false, + is_conflict = is_conflict, + } -- Quit keymap (q) if keymaps.quit then @@ -614,10 +72,14 @@ function M.setup_all_keymaps(tabpage, original_bufnr, modified_bufnr, is_explore -- Explorer toggle (e) - only in explorer mode if is_explorer_mode and keymaps.toggle_explorer then - lifecycle.set_tab_keymap(tabpage, "n", keymaps.toggle_explorer, toggle_explorer, { desc = "Toggle explorer visibility" }) + lifecycle.set_tab_keymap(tabpage, "n", keymaps.toggle_explorer, function() + panes.toggle_explorer(ctx) + end, { desc = "Toggle explorer visibility" }) end if is_explorer_mode and keymaps.focus_explorer then - lifecycle.set_tab_keymap(tabpage, "n", keymaps.focus_explorer, focus_explorer, { desc = "Focus explorer panel" }) + lifecycle.set_tab_keymap(tabpage, "n", keymaps.focus_explorer, function() + panes.focus_explorer(ctx) + end, { desc = "Focus explorer panel" }) end -- Diff get/put (do, dp) - layout-aware semantics. @@ -626,14 +88,20 @@ function M.setup_all_keymaps(tabpage, original_bufnr, modified_bufnr, is_explore -- on do/dp is handed back for the duration of the merge, rather than deleted. if keymaps.diff_get and not is_conflict then local desc = is_inline and "Revert hunk to original" or "Get change from other buffer" - lifecycle.set_tab_keymap(tabpage, "n", keymaps.diff_get, diff_get, { desc = desc }) + lifecycle.set_tab_keymap(tabpage, "n", keymaps.diff_get, function() + diffget.diff_get(ctx) + end, { desc = desc }) end if keymaps.diff_put and not is_conflict then local desc = is_inline and "Accept change (no-op in inline)" or "Put change to other buffer" - lifecycle.set_tab_keymap(tabpage, "n", keymaps.diff_put, diff_put, { desc = desc }) + lifecycle.set_tab_keymap(tabpage, "n", keymaps.diff_put, function() + diffget.diff_put(ctx) + end, { desc = desc }) end if keymaps.open_in_prev_tab then - lifecycle.set_tab_keymap(tabpage, "n", keymaps.open_in_prev_tab, open_in_prev_tab, { desc = "Open buffer in previous tab" }) + lifecycle.set_tab_keymap(tabpage, "n", keymaps.open_in_prev_tab, function() + panes.open_in_prev_tab(ctx) + end, { desc = "Open buffer in previous tab" }) end if keymaps.toggle_layout then lifecycle.set_tab_keymap(tabpage, "n", keymaps.toggle_layout, function() @@ -661,11 +129,15 @@ function M.setup_all_keymaps(tabpage, original_bufnr, modified_bufnr, is_explore end if toggle_stage_key then - lifecycle.set_tab_keymap(tabpage, "n", toggle_stage_key, toggle_stage, { desc = "Toggle stage/unstage" }) + lifecycle.set_tab_keymap(tabpage, "n", toggle_stage_key, function() + stage.toggle_stage(ctx) + end, { desc = "Toggle stage/unstage" }) end if keymaps.toggle_staged_view then - lifecycle.set_tab_keymap(tabpage, "n", keymaps.toggle_staged_view, toggle_staged_view, { desc = "Toggle staged/unstaged view for current file" }) + lifecycle.set_tab_keymap(tabpage, "n", keymaps.toggle_staged_view, function() + stage.toggle_staged_view(ctx) + end, { desc = "Toggle staged/unstaged view for current file" }) end end @@ -703,186 +175,33 @@ function M.setup_all_keymaps(tabpage, original_bufnr, modified_bufnr, is_explore end for _, bufnr in ipairs(diff_bufs) do if keymaps.stage_hunk then - lifecycle.set_buf_keymap(tabpage, bufnr, "n", keymaps.stage_hunk, stage_hunk, vim.tbl_extend("force", hunk_opts, { desc = "Stage hunk under cursor" })) + lifecycle.set_buf_keymap(tabpage, bufnr, "n", keymaps.stage_hunk, function() + hunk.stage_hunk(ctx) + end, vim.tbl_extend("force", hunk_opts, { desc = "Stage hunk under cursor" })) end if keymaps.unstage_hunk then - lifecycle.set_buf_keymap(tabpage, bufnr, "n", keymaps.unstage_hunk, unstage_hunk, vim.tbl_extend("force", hunk_opts, { desc = "Unstage hunk under cursor" })) + lifecycle.set_buf_keymap(tabpage, bufnr, "n", keymaps.unstage_hunk, function() + hunk.unstage_hunk(ctx) + end, vim.tbl_extend("force", hunk_opts, { desc = "Unstage hunk under cursor" })) end if keymaps.discard_hunk then - lifecycle.set_buf_keymap(tabpage, bufnr, "n", keymaps.discard_hunk, discard_hunk, vim.tbl_extend("force", hunk_opts, { desc = "Discard hunk under cursor" })) + lifecycle.set_buf_keymap(tabpage, bufnr, "n", keymaps.discard_hunk, function() + hunk.discard_hunk(ctx) + end, vim.tbl_extend("force", hunk_opts, { desc = "Discard hunk under cursor" })) end -- Hunk textobject (ih) - select hunk lines in visual/operator-pending mode if keymaps.hunk_textobject then - local function select_hunk() - local mapping = find_hunk_at_cursor() - if not mapping then - return - end - - local current_buf = vim.api.nvim_get_current_buf() - local is_original = current_buf == original_bufnr - local start_line = is_original and mapping.original.start_line or mapping.modified.start_line - local end_line = is_original and mapping.original.end_line or mapping.modified.end_line - - -- end_line is exclusive, and empty ranges (deletions) can't be selected - if start_line >= end_line then - return - end - - vim.cmd("normal! " .. start_line .. "GV" .. (end_line - 1) .. "G") - end - - lifecycle.set_buf_keymap(tabpage, bufnr, { "o", "x" }, keymaps.hunk_textobject, select_hunk, vim.tbl_extend("force", hunk_opts, { desc = "Hunk textobject" })) + lifecycle.set_buf_keymap(tabpage, bufnr, { "o", "x" }, keymaps.hunk_textobject, function() + hunk.select_hunk(ctx) + end, vim.tbl_extend("force", hunk_opts, { desc = "Hunk textobject" })) end end - -- ======================================================================== - -- Align moved code (gm) - -- Temporarily align other pane to show paired move block - -- ======================================================================== - - local function align_move() - local session = lifecycle.get_session(tabpage) - if not session or not session.stored_diff_result or not session.stored_diff_result.moves then - return - end - if is_inline then - return - end -- Only works in side-by-side - - local moves = session.stored_diff_result.moves - if #moves == 0 then - vim.notify("No moved code blocks in current diff", vim.log.levels.INFO) - return - end - - local current_buf = vim.api.nvim_get_current_buf() - local cursor_line = vim.api.nvim_win_get_cursor(0)[1] - - -- Read current buffers from session (not closure — may have changed via file switch) - local sess_orig_buf = session.original_bufnr - local sess_mod_buf = session.modified_bufnr - - -- Find which move the cursor is in - local current_move = nil - local is_on_original = current_buf == sess_orig_buf - for _, move in ipairs(moves) do - local range = is_on_original and move.original or move.modified - if cursor_line >= range.start_line and cursor_line < range.end_line then - current_move = move - break - end - end - - if not current_move then - vim.notify("Not on a moved code block", vim.log.levels.INFO) - return - end - - local current_win = vim.api.nvim_get_current_win() - local other_win = is_on_original and session.modified_win or session.original_win - if not vim.api.nvim_win_is_valid(other_win) then - return - end - - local my_range = is_on_original and current_move.original or current_move.modified - local other_range = is_on_original and current_move.modified or current_move.original - - -- Save full view state of both windows - local current_view = vim.api.nvim_win_call(current_win, function() - return vim.fn.winsaveview() - end) - local other_view = vim.api.nvim_win_call(other_win, function() - return vim.fn.winsaveview() - end) - local saved_scrolloff_other = vim.wo[other_win].scrolloff - - -- Pause structural scroll-sync while we impose the move alignment. - local scroll = require("codediff.ui.scroll") - scroll.pause(tabpage) - vim.wo[other_win].scrolloff = 0 - - -- Align using the annotation virt_line as anchor: - -- Both sides have "⇄ moved" above their first moved line. - -- Use winline() to get the actual visual row (accounts for virtual/filler lines). - local my_first = my_range.start_line - local other_first = other_range.start_line - - -- Get actual visual row of the moved block start (accounts for filler virt_lines) - -- Save and restore cursor so the user's position is not disturbed. - local my_visual_row = vim.api.nvim_win_call(current_win, function() - local saved_pos = vim.api.nvim_win_get_cursor(current_win) - vim.api.nvim_win_set_cursor(current_win, { my_first, 0 }) - local row = vim.fn.winline() - vim.api.nvim_win_set_cursor(current_win, saved_pos) - return row - end) - - -- Set other pane: position other_first at the same visual row - -- winline() is 1-based from top of window - vim.api.nvim_win_call(other_win, function() - -- First scroll to the target line at top of window - vim.api.nvim_win_set_cursor(other_win, { other_first, 0 }) - vim.cmd("normal! zt") - -- Now scroll down to match the visual offset (Ctrl-Y scrolls view up, line moves down) - if my_visual_row > 1 then - local keys = vim.api.nvim_replace_termcodes((my_visual_row - 1) .. "", true, false, true) - vim.api.nvim_feedkeys(keys, "nx", false) - end - end) - - -- Restore function — called when cursor leaves moved block or switches window - local augroup = vim.api.nvim_create_augroup("codediff_move_align_" .. tabpage, { clear = true }) - local restored = false - - local function restore() - if restored then - return - end - restored = true - pcall(vim.api.nvim_del_augroup_by_id, augroup) - if vim.api.nvim_win_is_valid(other_win) then - vim.wo[other_win].scrolloff = saved_scrolloff_other - end - if not vim.api.nvim_win_is_valid(current_win) or not vim.api.nvim_win_is_valid(other_win) then - return - end - -- Restore views first, then resume structural scroll-sync. - vim.api.nvim_win_call(other_win, function() - vim.fn.winrestview(other_view) - end) - vim.api.nvim_win_call(current_win, function() - vim.fn.winrestview(current_view) - end) - scroll.resume(tabpage) - end - - -- Restore when cursor moves out of the moved block - vim.api.nvim_create_autocmd("CursorMoved", { - group = augroup, - buffer = current_buf, - callback = function() - local new_line = vim.api.nvim_win_get_cursor(0)[1] - if new_line < my_range.start_line or new_line >= my_range.end_line then - restore() - end - end, - }) - - -- Restore when user switches to another window (WinLeave) - -- Use vim.schedule to defer restore until after Neovim finishes - -- the window switch and cursor placement from the click event. - vim.api.nvim_create_autocmd("WinLeave", { - group = augroup, - callback = function() - vim.schedule(restore) - end, - }) - end - if keymaps.align_move and not is_inline and config.options.diff.compute_moves then - lifecycle.set_tab_keymap(tabpage, "n", keymaps.align_move, align_move, { desc = "Align moved code block" }) + lifecycle.set_tab_keymap(tabpage, "n", keymaps.align_move, function() + move.align_move(ctx) + end, { desc = "Align moved code block" }) end lifecycle.end_keymap_scope(tabpage) From 91b6cd9f03728ecb86671e4e78662a043b45f8b6 Mon Sep 17 00:00:00 2001 From: Yanuo Ma Date: Sat, 1 Aug 2026 03:08:18 -0400 Subject: [PATCH 4/5] fix(keymap): correct ownership and scope identity after second audit MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A second adversarial review found two more ways the registry could take a key that was no longer its own, plus a case where releasing one scope destroyed a mapping belonging to another. - installed_is_ours() compared only the callback or right-hand side, so a plugin that re-mapped the same RHS with different options — or reused codediff's own dispatcher — was still treated as ours and deleted on teardown. The slot now records the mapping exactly as Neovim stored it and compares the whole thing, matching what same_map already did elsewhere. - Claims were keyed by owner alone. Two setup passes in one session claiming the same key (a configured `view.toggle_compact = "zo"` alongside compact's own zo wrapper) meant the second replaced the first, and releasing it left the key unmapped instead of revealing the earlier claim. Claim identity is now (owner, scope), so overlapping scopes layer and unwind in order. - Scopes are tracked on a stack rather than a single mutable field, and end_scope takes the scope name so a pass that aborted before closing cannot leave the stack dirty. Nested passes now restore their parent. A claim made after an aborted pass is still attributed to it; that is inherent to an ambient scope and is noted in the code. Adds five regression tests: active remap with changed options, active remap reusing our callback, overlapping scope release, nested scopes, and an aborted pass not growing the stack. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- lua/codediff/keymap/registry.lua | 94 ++++++++++++++++++------- lua/codediff/keymap/slots.lua | 87 +++++++++++++---------- lua/codediff/ui/conflict/keymaps.lua | 2 +- lua/codediff/ui/lifecycle/accessors.lua | 7 +- lua/codediff/ui/view/compact.lua | 2 +- lua/codediff/ui/view/keymaps.lua | 2 +- tests/ui/keymap/registry_spec.lua | 87 +++++++++++++++++++++++ 7 files changed, 211 insertions(+), 70 deletions(-) diff --git a/lua/codediff/keymap/registry.lua b/lua/codediff/keymap/registry.lua index 31b1ae64..0865ded1 100644 --- a/lua/codediff/keymap/registry.lua +++ b/lua/codediff/keymap/registry.lua @@ -21,8 +21,8 @@ local normalize = require("codediff.keymap.normalize") local Registry = {} Registry.__index = Registry -local function entry_key(bufnr, mode, lhs) - return string.format("%d\0%s\0%s", bufnr, mode, lhs) +local function entry_key(bufnr, mode, lhs, scope) + return string.format("%d\0%s\0%s\0%s", bufnr, mode, lhs, scope or "") end --- @param name string Diagnostic label (e.g. "session:3") @@ -34,9 +34,9 @@ function M.new(name) suspended = false, disposed = false, -- Generation counter per scope, used to retire claims that a later setup - -- pass no longer makes. + -- pass no longer makes, plus the stack of passes currently open. scope_gen = {}, - current_scope = nil, + scope_stack = {}, }, Registry) end @@ -53,21 +53,58 @@ function Registry:begin_scope(scope) if self.disposed then return end + -- A pass that aborted before its end_scope would otherwise leave its name on + -- the stack forever, so drop any stale entry for this scope first. Claims + -- made in that window are still attributed to it and will be retired here; + -- that is acceptable because an aborted setup pass is already a failed state. + for i = #self.scope_stack, 1, -1 do + if self.scope_stack[i] == scope then + table.remove(self.scope_stack, i) + end + end self.scope_gen[scope] = (self.scope_gen[scope] or 0) + 1 - self.current_scope = scope + table.insert(self.scope_stack, scope) +end + +--- The pass a claim made right now belongs to. +local function current_scope(self) + return self.scope_stack[#self.scope_stack] end ---- Finish the current setup pass, releasing claims it did not renew. -function Registry:end_scope() - local scope = self.current_scope - self.current_scope = nil - if self.disposed or not scope then +--- Finish a setup pass, releasing claims it did not renew. +--- +--- Unwinds to `scope` when given, so a pass that aborted before its own +--- end_scope cannot leave the stack dirty and silently mis-tag later claims. +--- @param scope string|nil Defaults to the innermost open pass +function Registry:end_scope(scope) + if self.disposed then return end + if scope then + -- Pop until this scope has been closed; anything above it never closed. + local found = false + for i = #self.scope_stack, 1, -1 do + if self.scope_stack[i] == scope then + found = true + for _ = #self.scope_stack, i, -1 do + table.remove(self.scope_stack) + end + break + end + end + if not found then + return + end + else + scope = table.remove(self.scope_stack) + if not scope then + return + end + end local generation = self.scope_gen[scope] for key, entry in pairs(self.entries) do if entry.scope == scope and entry.generation ~= generation then - slots.release(self, entry.bufnr, entry.mode, entry.lhs) + slots.release(self, entry.bufnr, entry.mode, entry.lhs, entry.scope) self.entries[key] = nil end end @@ -82,7 +119,7 @@ function Registry:release_scope(scope) end for key, entry in pairs(self.entries) do if entry.scope == scope then - slots.release(self, entry.bufnr, entry.mode, entry.lhs) + slots.release(self, entry.bufnr, entry.mode, entry.lhs, entry.scope) self.entries[key] = nil end end @@ -125,20 +162,21 @@ function Registry:claim(bufnr, modes, lhs, rhs, opts, meta) -- APIs accept. Passing the canonical bytes to vim.keymap.set would encode -- keys like <2-LeftMouse> and a second time, leaving a mapping the -- real key press can never reach. - if slots.claim(self, bufnr, mode, canonical, resolved, rhs, opts, meta.priority) then - local key = entry_key(bufnr, mode, canonical) + local scope = current_scope(self) + if slots.claim(self, bufnr, mode, canonical, resolved, rhs, opts, meta.priority, scope) then + local key = entry_key(bufnr, mode, canonical, scope) self.entries[key] = { bufnr = bufnr, mode = mode, lhs = canonical, suspendable = suspendable, documented = documented, - scope = self.current_scope, - generation = self.current_scope and self.scope_gen[self.current_scope] or nil, + scope = scope, + generation = scope and self.scope_gen[scope] or nil, } -- A claim added while suspended must not be installed yet. if self.suspended and suspendable then - slots.set_active(self, bufnr, mode, canonical, false) + slots.set_active(self, bufnr, mode, canonical, false, scope) end claimed = true end @@ -152,7 +190,7 @@ end function Registry:documented_keys() local keys = {} for _, entry in pairs(self.entries) do - if entry.documented and slots.is_live(self, entry.bufnr, entry.mode, entry.lhs) then + if entry.documented and slots.is_live(self, entry.bufnr, entry.mode, entry.lhs, entry.scope) then keys[entry.lhs] = true end end @@ -175,7 +213,7 @@ function Registry:owns(lhs, mode, bufnr) end for _, entry in pairs(self.entries) do if entry.lhs == canonical and (mode == nil or entry.mode == mode) and (bufnr == nil or entry.bufnr == bufnr) then - if slots.is_live(self, entry.bufnr, entry.mode, entry.lhs) then + if slots.is_live(self, entry.bufnr, entry.mode, entry.lhs, entry.scope) then return true end end @@ -197,8 +235,14 @@ function Registry:release(bufnr, modes, lhs) return end for _, mode in ipairs(normalize.modes(modes)) do - slots.release(self, bufnr, mode, canonical) - self.entries[entry_key(bufnr, mode, canonical)] = nil + -- Release every scope's claim on this key, since the caller names a + -- mapping rather than a particular setup pass. + for key, entry in pairs(self.entries) do + if entry.bufnr == bufnr and entry.mode == mode and entry.lhs == canonical then + slots.release(self, bufnr, mode, canonical, entry.scope) + self.entries[key] = nil + end + end end end @@ -223,7 +267,7 @@ function Registry:detach_buffer(bufnr) end for key, entry in pairs(self.entries) do if entry.bufnr == bufnr then - slots.release(self, entry.bufnr, entry.mode, entry.lhs) + slots.release(self, entry.bufnr, entry.mode, entry.lhs, entry.scope) self.entries[key] = nil end end @@ -251,7 +295,7 @@ function Registry:suspend() self.suspended = true for _, entry in pairs(self.entries) do if entry.suspendable then - slots.set_active(self, entry.bufnr, entry.mode, entry.lhs, false) + slots.set_active(self, entry.bufnr, entry.mode, entry.lhs, false, entry.scope) end end end @@ -264,7 +308,7 @@ function Registry:resume() self.suspended = false for _, entry in pairs(self.entries) do if entry.suspendable then - slots.set_active(self, entry.bufnr, entry.mode, entry.lhs, true) + slots.set_active(self, entry.bufnr, entry.mode, entry.lhs, true, entry.scope) end end end @@ -276,7 +320,7 @@ function Registry:dispose() end self.disposed = true for key, entry in pairs(self.entries) do - slots.release(self, entry.bufnr, entry.mode, entry.lhs) + slots.release(self, entry.bufnr, entry.mode, entry.lhs, entry.scope) self.entries[key] = nil end end diff --git a/lua/codediff/keymap/slots.lua b/lua/codediff/keymap/slots.lua index 8e045a8c..0565db79 100644 --- a/lua/codediff/keymap/slots.lua +++ b/lua/codediff/keymap/slots.lua @@ -89,28 +89,6 @@ local function drop_slot(slot) end end -local function install(slot, claim) - local rhs = claim.rhs - if type(rhs) == "function" then - -- Wrap in a per-claim dispatcher so the installed mapping is identifiable - -- by function identity even when two claims share the same handler. - if not claim.dispatcher then - claim.dispatcher = function(...) - return claim.rhs(...) - end - end - rhs = claim.dispatcher - end - - local opts = vim.tbl_extend("force", claim.opts or {}, { buffer = slot.bufnr }) - local ok = pcall(vim.keymap.set, slot.mode, slot.lhs, rhs, opts) - if ok then - slot.applied = claim - else - slot.applied = nil - end -end - -- Mapping fields that decide whether two mappings are "the same mapping". -- Options matter: re-mapping the same RHS with different `silent` or `nowait` -- is a different mapping and must count as foreign. @@ -136,21 +114,45 @@ local function same_map(a, b) return true end +local function install(slot, claim) + local rhs = claim.rhs + if type(rhs) == "function" then + -- Wrap in a per-claim dispatcher so the installed mapping is identifiable + -- by function identity even when two claims share the same handler. + if not claim.dispatcher then + claim.dispatcher = function(...) + return claim.rhs(...) + end + end + rhs = claim.dispatcher + end + + local opts = vim.tbl_extend("force", claim.opts or {}, { buffer = slot.bufnr }) + local ok = pcall(vim.keymap.set, slot.mode, slot.lhs, rhs, opts) + if ok then + slot.applied = claim + -- Remember the mapping exactly as Neovim recorded it. Ownership is decided + -- against this snapshot: another plugin may reuse the same RHS, or even the + -- same callback, while changing options, and that is still its mapping. + slot.applied_map = read_current(slot.bufnr, slot.mode, slot.lhs) + else + slot.applied = nil + slot.applied_map = nil + end +end + --- True when the mapping currently installed is the one this slot applied. +--- Compares the whole mapping, not just its right-hand side: a plugin that +--- re-maps the same RHS with different options has replaced ours. local function installed_is_ours(slot) - local claim = slot.applied - if not claim then + if not slot.applied or not slot.applied_map then return false end local current = read_current(slot.bufnr, slot.mode, slot.lhs) if not is_buffer_local(current) then return false end - local expected = claim.dispatcher or claim.rhs - if type(expected) == "function" then - return current.callback == expected - end - return current.rhs == expected + return same_map(current, slot.applied_map) end --- True when codediff may take this slot. @@ -184,6 +186,7 @@ local function restore(slot) pcall(vim.keymap.del, slot.mode, slot.lhs, { buffer = slot.bufnr }) end slot.applied = nil + slot.applied_map = nil end --- Highest-priority active claim; ties resolve to the most recent claim. @@ -246,9 +249,14 @@ local function reconcile(slot) end end -local function find_claim(slot, owner) +--- Claims are identified by owner *and* scope: one registry can hold several +--- claims on the same key from different setup passes (a custom +--- `view.toggle_compact = "zo"` alongside compact's own `zo` wrapper). Keying +--- on the owner alone would make the later pass overwrite the earlier one, and +--- releasing it would leave the key unmapped instead of revealing the first. +local function find_claim(slot, owner, scope) for index, claim in ipairs(slot.claims) do - if claim.owner == owner then + if claim.owner == owner and claim.scope == scope then return index, claim end end @@ -271,7 +279,7 @@ end --- @param opts table|nil Forwarded verbatim to vim.keymap.set --- @param priority integer|nil --- @return boolean claimed -function M.claim(owner, bufnr, mode, key, lhs, rhs, opts, priority) +function M.claim(owner, bufnr, mode, key, lhs, rhs, opts, priority, scope) if not bufnr or not vim.api.nvim_buf_is_valid(bufnr) or not key or not lhs then return false end @@ -307,13 +315,14 @@ function M.claim(owner, bufnr, mode, key, lhs, rhs, opts, priority) by_lhs[key] = slot end - local existing_index = find_claim(slot, owner) + local existing_index = find_claim(slot, owner, scope) if existing_index then table.remove(slot.claims, existing_index) end table.insert(slot.claims, { owner = owner, + scope = scope, rhs = rhs, opts = opts or {}, priority = priority or 0, @@ -326,14 +335,14 @@ end --- Release `owner`'s claim on a slot. --- @param key string Canonical key sequence used as slot identity -function M.release(owner, bufnr, mode, key) +function M.release(owner, bufnr, mode, key, scope) local by_lhs = slot_table(bufnr, mode, false) local slot = by_lhs and by_lhs[key] if not slot then return end - local index = find_claim(slot, owner) + local index = find_claim(slot, owner, scope) if index then table.remove(slot.claims, index) end @@ -343,14 +352,14 @@ end --- Suspend or resume `owner`'s claim without forgetting it. --- @param key string Canonical key sequence used as slot identity -function M.set_active(owner, bufnr, mode, key, active) +function M.set_active(owner, bufnr, mode, key, active, scope) local by_lhs = slot_table(bufnr, mode, false) local slot = by_lhs and by_lhs[key] if not slot then return end - local _, claim = find_claim(slot, owner) + local _, claim = find_claim(slot, owner, scope) if not claim or claim.active == active then return end @@ -367,13 +376,13 @@ end --- over. The help popup relies on this to avoid advertising an action the key --- no longer invokes. --- @return boolean -function M.is_live(owner, bufnr, mode, key) +function M.is_live(owner, bufnr, mode, key, scope) local by_lhs = slot_table(bufnr, mode, false) local slot = by_lhs and by_lhs[key] if not slot or slot.displaced then return false end - local _, claim = find_claim(slot, owner) + local _, claim = find_claim(slot, owner, scope) if not claim or not claim.active or slot.applied ~= claim then return false end diff --git a/lua/codediff/ui/conflict/keymaps.lua b/lua/codediff/ui/conflict/keymaps.lua index b387b0f3..239ebb32 100644 --- a/lua/codediff/ui/conflict/keymaps.lua +++ b/lua/codediff/ui/conflict/keymaps.lua @@ -92,7 +92,7 @@ function M.setup_keymaps(tabpage) end end - lifecycle.end_keymap_scope(tabpage) + lifecycle.end_keymap_scope(tabpage, "conflict") end return M diff --git a/lua/codediff/ui/lifecycle/accessors.lua b/lua/codediff/ui/lifecycle/accessors.lua index 67fd1497..10cea00b 100644 --- a/lua/codediff/ui/lifecycle/accessors.lua +++ b/lua/codediff/ui/lifecycle/accessors.lua @@ -608,12 +608,13 @@ function M.begin_keymap_scope(tabpage, scope) end end ---- Finish the current keymap setup pass, releasing claims it did not renew. +--- Finish a keymap setup pass, releasing claims it did not renew. --- @param tabpage number -function M.end_keymap_scope(tabpage) +--- @param scope string|nil Names the pass to close; defaults to the innermost +function M.end_keymap_scope(tabpage, scope) local sess = get_active_diffs()[tabpage] if sess and sess.keymaps then - sess.keymaps:end_scope() + sess.keymaps:end_scope(scope) end end diff --git a/lua/codediff/ui/view/compact.lua b/lua/codediff/ui/view/compact.lua index 2b342d66..ce60c409 100644 --- a/lua/codediff/ui/view/compact.lua +++ b/lua/codediff/ui/view/compact.lua @@ -176,7 +176,7 @@ local function setup_fold_sync(session, tabpage) end end - lifecycle.end_keymap_scope(tabpage) + lifecycle.end_keymap_scope(tabpage, "compact") end --- Remove the synced-fold keymap wraps from a session's panes. diff --git a/lua/codediff/ui/view/keymaps.lua b/lua/codediff/ui/view/keymaps.lua index 619b4c62..66d60f5e 100644 --- a/lua/codediff/ui/view/keymaps.lua +++ b/lua/codediff/ui/view/keymaps.lua @@ -204,7 +204,7 @@ function M.setup_all_keymaps(tabpage, original_bufnr, modified_bufnr, is_explore end, { desc = "Align moved code block" }) end - lifecycle.end_keymap_scope(tabpage) + lifecycle.end_keymap_scope(tabpage, "view") -- Keep compact mode in sync when the diff view is (re)built — applies the -- configured default on open and re-folds on file switches (no-op if off). diff --git a/tests/ui/keymap/registry_spec.lua b/tests/ui/keymap/registry_spec.lua index fa53ce8f..16fa010a 100644 --- a/tests/ui/keymap/registry_spec.lua +++ b/tests/ui/keymap/registry_spec.lua @@ -405,6 +405,33 @@ describe("keymap registry", function() assert.equals("other", map_of(bufnr, "n", "q").desc, "dispose must not restore over a foreign mapping") end) + it("treats an active remap of the same RHS as foreign", function() + -- A plugin may re-map the same right-hand side with different options. + -- That is its mapping, not ours, and must survive teardown. + local bufnr = new_buf() + local r = keymap.new("test") + r:claim(bufnr, "n", "q", "echo 1", { desc = "codediff", silent = true }) + vim.keymap.set("n", "q", "echo 1", { buffer = bufnr, desc = "foreign", silent = false }) + + r:dispose() + local current = map_of(bufnr, "n", "q") + assert.is_not_nil(current, "the foreign mapping must not be deleted") + assert.equals("foreign", current.desc) + end) + + it("treats an active remap reusing our callback as foreign", function() + local bufnr = new_buf() + local r = keymap.new("test") + r:claim(bufnr, "n", "q", noop, { desc = "codediff" }) + local installed = map_of(bufnr, "n", "q") + vim.keymap.set("n", "q", installed.callback, { buffer = bufnr, desc = "foreign" }) + + r:dispose() + local current = map_of(bufnr, "n", "q") + assert.is_not_nil(current, "the foreign mapping must not be deleted") + assert.equals("foreign", current.desc) + end) + it("stops reporting ownership once displaced", function() local bufnr = new_buf() local r = keymap.new("test") @@ -440,6 +467,30 @@ describe("keymap registry", function() r:dispose() end) + it("reveals the outer scope's claim when an overlapping scope is released", function() + -- A user may configure a view key that collides with a scope-owned one, + -- e.g. view.toggle_compact = "zo" against compact's own zo wrapper. + local bufnr = new_buf() + local r = keymap.new("test") + + r:begin_scope("view") + r:claim(bufnr, "n", "zo", noop, { desc = "toggle compact" }) + r:end_scope("view") + + r:begin_scope("compact") + r:claim(bufnr, "n", "zo", noop, { desc = "synced fold" }) + r:end_scope("compact") + assert.equals("synced fold", map_of(bufnr, "n", "zo").desc, "the newer scope wins while active") + + r:release_scope("compact") + local revealed = map_of(bufnr, "n", "zo") + assert.is_not_nil(revealed, "the view mapping must come back, not vanish") + assert.equals("toggle compact", revealed.desc) + + r:dispose() + assert.is_nil(map_of(bufnr, "n", "zo"), "both scopes released on dispose") + end) + it("releases a whole scope on demand", function() local bufnr = new_buf() local r = keymap.new("test") @@ -457,6 +508,42 @@ describe("keymap registry", function() r:dispose() end) + it("restores the outer pass when scopes nest", function() + local bufnr = new_buf() + local r = keymap.new("test") + + r:begin_scope("view") + r:claim(bufnr, "n", "q", noop, { desc = "quit" }) + -- an inner pass (compact) opening and closing inside the view pass + r:begin_scope("compact") + r:claim(bufnr, "n", "zo", noop, { desc = "fold" }) + r:end_scope("compact") + -- still inside the view pass: this claim must belong to "view" + r:claim(bufnr, "n", "gm", noop, { desc = "align" }) + r:end_scope("view") + + -- a later view pass omits gm; the inner scope's claim must be untouched + r:begin_scope("view") + r:claim(bufnr, "n", "q", noop, { desc = "quit" }) + r:end_scope("view") + + assert.is_nil(map_of(bufnr, "n", "gm"), "claims after a nested scope still belong to the outer pass") + assert.is_not_nil(map_of(bufnr, "n", "zo"), "the inner scope's claims are not retired by the outer pass") + r:dispose() + end) + + it("does not accumulate scopes when a pass never closes", function() + local bufnr = new_buf() + local r = keymap.new("test") + for _ = 1, 20 do + r:begin_scope("view") + r:claim(bufnr, "n", "q", noop, { desc = "quit" }) + -- deliberately no end_scope, as if the pass raised + end + assert.is_true(#r.scope_stack <= 1, "an aborted pass must not grow the scope stack, got " .. #r.scope_stack) + r:dispose() + end) + it("keeps unscoped claims out of scope retirement", function() local bufnr = new_buf() local r = keymap.new("test") From 01ee300da8a2d80d6c4a0f3c8b8393d150eae0de Mon Sep 17 00:00:00 2001 From: Yanuo Ma Date: Sat, 1 Aug 2026 14:32:51 -0400 Subject: [PATCH 5/5] test(keymap): pin the reported issues this refactor closes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds a spec that follows the reproduction steps from each issue as written, so a future change that reopens one fails by issue number rather than as an unexplained assertion somewhere else: #289 / #334 gitsigns [c and ]c stop working after closing CodeDiff #211 / #224 keymaps not restored after navigating files then quitting #394 ih left mapped in operator-pending and visual mode, conflict keys never removed, and the gf escape Both halves of #289/#334 are covered. A global gitsigns mapping already recovered on main, because deleting codediff's buffer-local shadow reveals it; the buffer-local form gitsigns documents for on_attach did not, and that is the half this refactor fixes. The remaining checks are guard rails for issues fixed earlier — #428 close never calls qall, #412 cross-file hunk navigation stays off by default, #276 close_on_open_in_prev_tab stays false, #202 hunk navigation from the explorer, #322 gf from the explorer, #207 mappings keep nowait — so this work cannot silently reopen them. Verified against unmodified origin/main, where 5 of the 16 checks fail. Bumps the minor version: no public API, config key or default changes, but buffer-local mappings codediff overrides are now handed back on teardown, which is a visible behavior change. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- VERSION | 2 +- tests/ui/keymap/issue_regressions_spec.lua | 362 +++++++++++++++++++++ 2 files changed, 363 insertions(+), 1 deletion(-) create mode 100644 tests/ui/keymap/issue_regressions_spec.lua diff --git a/VERSION b/VERSION index aa97c2f1..e604dbd2 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -2.62.2 +2.63.0 diff --git a/tests/ui/keymap/issue_regressions_spec.lua b/tests/ui/keymap/issue_regressions_spec.lua new file mode 100644 index 00000000..6cccf16b --- /dev/null +++ b/tests/ui/keymap/issue_regressions_spec.lua @@ -0,0 +1,362 @@ +-- Regression tests for the keymap-ownership issues this refactor closes. +-- +-- Each check follows the reproduction steps from the GitHub issue as written, +-- so a future change that reopens one of them fails here by name: +-- +-- #289 / #334 gitsigns [c and ]c stop working after closing CodeDiff +-- #211 / #224 keymaps not restored after navigating files then quitting +-- #394 lifecycle teardown hygiene (ih in o/x, conflict keys, gf escape) +-- +-- The remaining checks are guard rails for issues fixed earlier, so this +-- refactor cannot silently reopen them. +local h = dofile("tests/helpers.lua") +h.ensure_plugin_loaded() + +local path = require("codediff.core.path") +local view = require("codediff.ui.view") +local lifecycle = require("codediff.ui.lifecycle") +local commands = require("codediff.commands") +local config = require("codediff.config") + +local results = {} +local function record(issue, name, ok, detail) + table.insert(results, { issue = issue, name = name, ok = ok, detail = detail or "" }) +end + +local function reset(opts) + vim.g.mapleader = "\\" + config.options = vim.deepcopy(config.defaults) + require("codediff").setup(opts or {}) + require("codediff.ui.highlights").setup() +end + +local function wait_diff(tp) + return vim.wait(10000, function() + local s = lifecycle.get_session(tp) + return s and s.stored_diff_result ~= nil + end, 50) +end + +--- Resolve a key as the user would experience it in `bufnr`: buffer-local +--- first, then global. Returns desc plus whether it is buffer-local. +local function effective(bufnr, key, mode) + if not bufnr or not vim.api.nvim_buf_is_valid(bufnr) then + return "", false + end + local m = vim.api.nvim_buf_call(bufnr, function() + return vim.fn.maparg(key, mode or "n", false, true) + end) + if type(m) ~= "table" or next(m) == nil then + return "NONE", false + end + return m.desc or "", m.buffer == 1 +end + +local function temp_pair(a, b) + local L = vim.fn.tempname() .. "_l.txt" + local R = vim.fn.tempname() .. "_r.txt" + vim.fn.writefile(a, L) + vim.fn.writefile(b, R) + return L, R +end + +local function open_standalone(premap) + local L, R = temp_pair({ "a", "b", "c", "d" }, { "a", "X", "c", "Y" }) + local rb = vim.fn.bufadd(R) + vim.fn.bufload(rb) + if premap then + premap(rb) + end + view.create({ mode = "standalone", original = path.make_ref(L, nil), modified = path.make_ref(R, nil) }) + local tp = vim.api.nvim_get_current_tabpage() + wait_diff(tp) + return tp, rb, function() + lifecycle.cleanup_all() + h.close_extra_tabs() + vim.fn.delete(L) + vim.fn.delete(R) + end +end + +local function open_explorer(opts) + reset(opts) + local repo = h.create_temp_git_repo() + repo.write_file("one.txt", { "a", "b", "c" }) + repo.write_file("two.txt", { "p", "q", "r" }) + repo.git("add .") + repo.git("commit -m init") + repo.write_file("one.txt", { "a", "X", "c" }) + repo.write_file("two.txt", { "p", "Y", "r" }) + vim.cmd("edit " .. vim.fn.fnameescape(repo.path("one.txt"))) + commands.vscode_diff({ fargs = {} }) + local tp + vim.wait(15000, function() + for _, t in ipairs(vim.api.nvim_list_tabpages()) do + local s = lifecycle.get_session(t) + if s and s.explorer and s.explorer.bufnr then + tp = t + return true + end + end + return false + end, 50) + return tp, repo +end + +-- =========================================================================== +-- #289 / #334: gitsigns [c and ]c stop working after closing CodeDiff. +-- The reported configs use gitsigns' on_attach map helper, which may produce a +-- global or a buffer-local mapping. Both paths must end up working again. +-- =========================================================================== +reset({ keymaps = { view = { next_hunk = "]c", prev_hunk = "[c" } } }) +do + -- (a) global gitsigns mappings + vim.keymap.set("n", "]c", function() end, { desc = "GITSIGNS-GLOBAL-NEXT" }) + vim.keymap.set("n", "[c", function() end, { desc = "GITSIGNS-GLOBAL-PREV" }) + local tp, rb, done = open_standalone() + local during = effective(rb, "]c") + lifecycle.close(tp) + vim.wait(200) + local after_next = effective(rb, "]c") + local after_prev = effective(rb, "[c") + record("#289/#334", "global gitsigns ]c works again after close", after_next == "GITSIGNS-GLOBAL-NEXT", "during=" .. during .. " after=" .. after_next) + record("#289/#334", "global gitsigns [c works again after close", after_prev == "GITSIGNS-GLOBAL-PREV", after_prev) + done() + pcall(vim.keymap.del, "n", "]c") + pcall(vim.keymap.del, "n", "[c") +end + +reset({ keymaps = { view = { next_hunk = "]c", prev_hunk = "[c" } } }) +do + -- (b) buffer-local gitsigns mappings (gitsigns' documented on_attach form) + local tp, rb, done = open_standalone(function(b) + vim.keymap.set("n", "]c", function() end, { buffer = b, desc = "GITSIGNS-LOCAL-NEXT" }) + vim.keymap.set("n", "[c", function() end, { buffer = b, desc = "GITSIGNS-LOCAL-PREV" }) + end) + lifecycle.close(tp) + vim.wait(200) + record("#289/#334", "buffer-local gitsigns ]c restored after close", effective(rb, "]c") == "GITSIGNS-LOCAL-NEXT", effective(rb, "]c")) + record("#289/#334", "buffer-local gitsigns [c restored after close", effective(rb, "[c") == "GITSIGNS-LOCAL-PREV", effective(rb, "[c")) + done() +end + +-- =========================================================================== +-- #211 / #224: custom keymaps not restored after pressing next-file then q. +-- The issue stresses that step 2 (navigating files) matters, because that is +-- what swaps buffers underneath the session. +-- =========================================================================== +do + local tp, repo = open_explorer({ + keymaps = { view = { next_hunk = "]h", prev_hunk = "[h", next_file = "J", prev_file = "K" }, explorer = { hover = "gk" } }, + }) + local ex = lifecycle.get_session(tp).explorer + + ex.on_file_select({ path = "one.txt", group = "unstaged", status = "M", git_root = repo.dir }) + wait_diff(tp) + local first = lifecycle.get_session(tp).modified_bufnr + + -- Step 2: navigate to the next file, as the issue insists. + ex.on_file_select({ path = "two.txt", group = "unstaged", status = "M", git_root = repo.dir }) + vim.wait(10000, function() + local s = lifecycle.get_session(tp) + return s and s.modified_bufnr ~= first and s.stored_diff_result ~= nil + end, 50) + local second = lifecycle.get_session(tp).modified_bufnr + + -- Step 3: quit. + lifecycle.close(tp) + vim.wait(300) + + local leaked = {} + for _, key in ipairs({ "]h", "[h", "J", "K" }) do + for _, b in ipairs({ first, second }) do + if vim.api.nvim_buf_is_valid(b) then + local d, is_local = effective(b, key) + if is_local then + table.insert(leaked, key .. "@" .. b .. "=" .. d) + end + end + end + end + record("#211/#224", "no codediff mappings survive after next-file then quit", #leaked == 0, table.concat(leaked, " ")) + repo.cleanup() + lifecycle.cleanup_all() + h.close_extra_tabs() +end + +-- =========================================================================== +-- #394: lifecycle teardown hygiene. Two symptoms it calls out specifically: +-- ih leaks because cleanup hard-codes normal mode, and conflict keys are never +-- deleted at all. Plus the gf escape, which it calls the most insidious case. +-- =========================================================================== +reset() +do + local tp, rb, done = open_standalone() + lifecycle.close(tp) + vim.wait(200) + local o = select(2, effective(rb, "ih", "o")) + local x = select(2, effective(rb, "ih", "x")) + record("#394", "ih released from operator-pending and visual", not o and not x, string.format("o_local=%s x_local=%s", o, x)) + done() +end + +reset() +do + local repo = h.create_temp_git_repo() + repo.write_file("c.txt", { "l1", "l2", "l3" }) + repo.git("add -A") + repo.git("commit -m base") + repo.git("checkout -b f") + repo.write_file("c.txt", { "F", "l2", "l3" }) + repo.git("commit -am f") + repo.git("checkout main") + repo.write_file("c.txt", { "M", "l2", "l3" }) + repo.git("commit -am m") + repo.git("merge f --no-edit") + vim.cmd("edit " .. vim.fn.fnameescape(repo.path("c.txt"))) + local cb = vim.api.nvim_get_current_buf() + vim.keymap.set("n", "do", "echo 1", { buffer = cb, desc = "USER-DO" }) + local ready = false + view.create({ + mode = "standalone", + git_root = repo.dir, + original = path.make_ref("c.txt", repo.dir), + modified = path.make_ref("c.txt", repo.dir), + original_revision = ":3", + modified_revision = ":2", + conflict = true, + }, "", function() + ready = true + end) + vim.wait(15000, function() + return ready + end, 50) + record("#394", "conflict mode does not destroy the user's do", effective(cb, "do") == "USER-DO", effective(cb, "do")) + + local tp + for _, t in ipairs(vim.api.nvim_list_tabpages()) do + local s = lifecycle.get_session(t) + if s and s.result_bufnr then + tp = t + break + end + end + if tp then + local s = lifecycle.get_session(tp) + local rbuf, obuf = s.result_bufnr, s.original_bufnr + -- The merge result buffer holds unsaved auto-merged content, so close() + -- prompts. Headless confirm() answers "cancel", which would leave the + -- session open and make this check meaningless. Answer "discard". + local real_confirm = vim.fn.confirm + vim.fn.confirm = function() + return 1 + end + local closed = lifecycle.close(tp) + vim.fn.confirm = real_confirm + vim.wait(300) + record("#394", "conflict session actually closes when confirmed", closed and lifecycle.get_session(tp) == nil, "closed=" .. tostring(closed)) + local left = {} + for _, key in ipairs({ "]x", "[x", "2do", "3do", "\\ct", "\\co" }) do + for _, b in ipairs({ rbuf, obuf }) do + if b and vim.api.nvim_buf_is_valid(b) and select(2, effective(b, key)) then + table.insert(left, key) + end + end + end + record("#394", "conflict mappings removed on close", #left == 0, table.concat(left, " ")) + end + repo.cleanup() + lifecycle.cleanup_all() + h.close_extra_tabs() +end + +do + -- gf escape: the buffer moves to another tab while the session stays alive. + local tp, repo = open_explorer({}) + local ex = lifecycle.get_session(tp).explorer + ex.on_file_select({ path = "one.txt", group = "unstaged", status = "M", git_root = repo.dir }) + wait_diff(tp) + local s = lifecycle.get_session(tp) + local mod = s.modified_bufnr + vim.api.nvim_set_current_win(vim.fn.bufwinid(mod)) + local cb = vim.api.nvim_buf_call(mod, function() + local m = vim.fn.maparg("gf", "n", false, true) + return m and m.callback + end) + if cb then + pcall(cb) + end + vim.wait(500) + local alive = lifecycle.get_session(tp) ~= nil + local still_mapped = select(2, effective(mod, "q")) + record("#394", "gf escape leaves the file clean while session lives", alive and not still_mapped, "session_alive=" .. tostring(alive) .. " q_local=" .. tostring(still_mapped)) + repo.cleanup() + lifecycle.cleanup_all() + h.close_extra_tabs() +end + +-- =========================================================================== +-- Guard rails: previously-fixed issues that must not regress. +-- =========================================================================== +reset() +do + -- #428: ordinary close must never exit Neovim, even as the last codediff tab + local tp, _, done = open_standalone() + vim.cmd("tabonly!") + local qall = false + local realcmd = vim.cmd + vim.cmd = function(c) + if c == "qall" then + qall = true + return + end + return realcmd(c) + end + lifecycle.close(tp) + vim.cmd = realcmd + record("#428", "close does not qall without --exit-on-close", not qall, "qall=" .. tostring(qall)) + done() +end + +reset() +do + -- #412 / #415: cross-file hunk navigation must stay off by default + record("#412", "cycle_hunks_across_files defaults to false", config.options.diff.cycle_hunks_across_files == false, tostring(config.options.diff.cycle_hunks_across_files)) + record("#276", "close_on_open_in_prev_tab defaults to false", config.options.keymaps.view.close_on_open_in_prev_tab == false, tostring(config.options.keymaps.view.close_on_open_in_prev_tab)) +end + +do + -- #202: hunk navigation must be reachable from the explorer panel + local tp, repo = open_explorer({}) + local s = lifecycle.get_session(tp) + local panel = s.explorer.bufnr + record("#202", "hunk navigation reachable from the explorer panel", select(2, effective(panel, "]c")), effective(panel, "]c")) + -- #322 / #67: gf reachable from the explorer panel + record("#322", "gf reachable from the explorer panel", select(2, effective(panel, "gf")), effective(panel, "gf")) + -- #207: codediff mappings keep nowait + local m = vim.api.nvim_buf_call(panel, function() + return vim.fn.maparg("q", "n", false, true) + end) + record("#207", "codediff mappings are nowait", m and m.nowait == 1, "nowait=" .. tostring(m and m.nowait)) + repo.cleanup() + lifecycle.cleanup_all() + h.close_extra_tabs() +end + +describe("keymap issue regressions", function() + after_each(function() + lifecycle.cleanup_all() + h.close_extra_tabs() + end) + + it("keeps every reported keymap issue fixed", function() + local failures = {} + for _, r in ipairs(results) do + if not r.ok then + table.insert(failures, string.format("%s %s (%s)", r.issue, r.name, r.detail)) + end + end + assert.are.same({}, failures, "these reported issues are no longer fixed") + assert.is_true(#results >= 15, "expected the full issue matrix, ran " .. #results) + end) +end)