From 9e73922bde20ef97ba5d7758a22f16add117d40a Mon Sep 17 00:00:00 2001 From: Yanuo Ma Date: Wed, 4 Feb 2026 21:27:32 -0500 Subject: [PATCH 1/3] Revert "fix: restore user keymaps after closing CodeDiff (#215)" This reverts commit 9a18715d88e7f844b36408f6288da2d775ad99a2, reversing changes made to b83645ba125c70ea0ae87f2d45cce8525cac9dde. --- lua/codediff/ui/lifecycle/accessors.lua | 116 ++--------- tests/keymap_restore_spec.lua | 253 ------------------------ 2 files changed, 19 insertions(+), 350 deletions(-) delete mode 100644 tests/keymap_restore_spec.lua diff --git a/lua/codediff/ui/lifecycle/accessors.lua b/lua/codediff/ui/lifecycle/accessors.lua index a0c1f251..9c4c7660 100644 --- a/lua/codediff/ui/lifecycle/accessors.lua +++ b/lua/codediff/ui/lifecycle/accessors.lua @@ -399,34 +399,6 @@ function M.confirm_close_with_unsaved(tabpage) end end ---- Find and save an existing buffer-local keymap before overwriting ---- @param bufnr number Buffer number ---- @param mode string Keymap mode ---- @param lhs string Left-hand side of the keymap ---- @return table|nil The saved keymap data, or nil if no existing keymap -local function save_existing_keymap(bufnr, mode, lhs) - if not vim.api.nvim_buf_is_valid(bufnr) then - return nil - end - local keymaps = vim.api.nvim_buf_get_keymap(bufnr, mode) - for _, map in ipairs(keymaps) do - if map.lhs == lhs then - return { - lhs = map.lhs, - rhs = map.rhs, - callback = map.callback, - expr = map.expr == 1, - noremap = map.noremap == 1, - nowait = map.nowait == 1, - silent = map.silent == 1, - script = map.script == 1, - desc = map.desc, - } - end - end - return nil -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 @@ -442,43 +414,30 @@ function M.set_tab_keymap(tabpage, mode, lhs, rhs, opts) return false end - -- Initialize saved keymaps storage if needed - sess.saved_keymaps = sess.saved_keymaps or {} - opts = opts or {} local base_opts = { noremap = true, silent = true, nowait = true } - -- Helper to save existing keymap before setting new one - local function set_with_save(bufnr) - if not vim.api.nvim_buf_is_valid(bufnr) then - return - end - -- Save existing keymap (only if not already saved for this buffer/lhs) - local key = bufnr .. ":" .. mode .. ":" .. lhs - if sess.saved_keymaps[key] == nil then - local existing = save_existing_keymap(bufnr, mode, lhs) - -- Store even if nil (marks that we've checked this keymap) - sess.saved_keymaps[key] = existing or false - end - vim.keymap.set(mode, lhs, rhs, vim.tbl_extend("force", base_opts, opts, { buffer = bufnr })) + 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 })) end - set_with_save(sess.original_bufnr) - set_with_save(sess.modified_bufnr) + 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 })) + end local explorer = sess.explorer - if explorer and explorer.bufnr then - set_with_save(explorer.bufnr) + 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 })) end - if sess.result_bufnr then - set_with_save(sess.result_bufnr) + 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 })) end return true end ---- Remove codediff keymaps from a session's buffers and restore original keymaps +--- Remove codediff keymaps from a session's buffers function M.clear_tab_keymaps(tabpage) local active_diffs = session.get_active_diffs() local sess = active_diffs[tabpage] @@ -486,65 +445,28 @@ function M.clear_tab_keymaps(tabpage) return end - -- Guard: If already cleared, don't run again (prevents double-cleanup from deleting restored keymaps) - if not sess.saved_keymaps then - return - end - - local saved_keymaps = sess.saved_keymaps - - -- Helper to restore a single keymap - local function restore_keymap(bufnr, mode, lhs) + local function del_buf_keymaps(bufnr, keys) if not vim.api.nvim_buf_is_valid(bufnr) then return end - local key = bufnr .. ":" .. mode .. ":" .. lhs - local saved = saved_keymaps[key] - - -- First, delete the codediff keymap - pcall(vim.keymap.del, mode, lhs, { buffer = bufnr }) - - -- Then restore the original keymap if one existed - if saved and saved ~= false then - local restore_opts = { - buffer = bufnr, - expr = saved.expr, - noremap = saved.noremap, - nowait = saved.nowait, - silent = saved.silent, - desc = saved.desc, - } - local rhs = saved.callback or saved.rhs or "" - pcall(vim.keymap.set, mode, lhs, rhs, restore_opts) - end - end - - -- Helper to restore keymaps for a buffer from a keymap config table - local function restore_buf_keymaps(bufnr, keys) - if not vim.api.nvim_buf_is_valid(bufnr) then - return - end - for _, lhs in pairs(keys) do - if lhs then - restore_keymap(bufnr, "n", lhs) + for _, key in pairs(keys) do + if key then + pcall(vim.keymap.del, "n", key, { buffer = bufnr }) end end end - restore_buf_keymaps(sess.original_bufnr, config.options.keymaps.view) - restore_buf_keymaps(sess.modified_bufnr, config.options.keymaps.view) + del_buf_keymaps(sess.original_bufnr, config.options.keymaps.view) + del_buf_keymaps(sess.modified_bufnr, config.options.keymaps.view) if sess.explorer and sess.explorer.bufnr then - restore_buf_keymaps(sess.explorer.bufnr, config.options.keymaps.view) - restore_buf_keymaps(sess.explorer.bufnr, config.options.keymaps.explorer or {}) + del_buf_keymaps(sess.explorer.bufnr, config.options.keymaps.view) + del_buf_keymaps(sess.explorer.bufnr, config.options.keymaps.explorer or {}) end if sess.result_bufnr then - restore_buf_keymaps(sess.result_bufnr, config.options.keymaps.view) + del_buf_keymaps(sess.result_bufnr, config.options.keymaps.view) end - - -- Clear saved keymaps - sess.saved_keymaps = nil end --- Setup auto-sync on file switch: automatically update diff when user edits a different file in working buffer diff --git a/tests/keymap_restore_spec.lua b/tests/keymap_restore_spec.lua deleted file mode 100644 index 80b595b0..00000000 --- a/tests/keymap_restore_spec.lua +++ /dev/null @@ -1,253 +0,0 @@ --- Test: Keymap Restoration --- Validates that user-defined keymaps are restored after closing CodeDiff --- Addresses: https://github.com/esmuellert/codediff.nvim/issues/211 --- --- Issue scenario: --- 1. User has keymaps like J, K, ]h, [h defined (globally or buffer-local) --- 2. User configures CodeDiff to use same keys: next_file=J, prev_file=K, etc. --- 3. User opens :CodeDiff, presses J (next file), presses q (quit) --- 4. User's original keymaps are lost - -local helpers = require("tests.helpers") -local commands = require("codediff.commands") - --- Setup CodeDiff command for tests -local function setup_command() - vim.api.nvim_create_user_command("CodeDiff", function(opts) - commands.vscode_diff(opts) - end, { - nargs = "*", - bang = true, - complete = function() - return { "file", "install" } - end, - }) -end - -describe("Keymap Restoration (Issue #211)", function() - local repo - local test_bufnr - local original_J_called = false - local original_K_called = false - - before_each(function() - helpers.ensure_plugin_loaded() - setup_command() - - -- Create a temp git repo with changes - repo = helpers.create_temp_git_repo() - - -- Create initial commit - repo.write_file("test.lua", { "line 1", "line 2", "line 3" }) - repo.git("add -A") - repo.git('commit -m "Initial commit"') - - -- Make an uncommitted change so CodeDiff has something to show - repo.write_file("test.lua", { "line 1 modified", "line 2", "line 3" }) - - -- Open the file - vim.cmd("edit " .. repo.path("test.lua")) - test_bufnr = vim.api.nvim_get_current_buf() - - -- Reset tracking flags - original_J_called = false - original_K_called = false - end) - - after_each(function() - -- Clean up tabs - helpers.close_extra_tabs() - vim.wait(200) - - -- Clean up repo - if repo then - repo.cleanup() - end - end) - - it("Restores buffer-local keymaps after closing CodeDiff (exact issue #211 scenario)", function() - -- This test replicates the exact issue: - -- User has buffer-local keymaps on their working file - -- CodeDiff opens and sets its own keymaps on buffers - -- User navigates files (keymaps should be restored on old buffer) - -- After closing, all remaining buffers should have keymaps restored - - -- Create a second file to switch between - repo.write_file("other.lua", { "other line 1", "other line 2" }) - repo.git("add other.lua") - repo.git('commit -m "Add other file"') - repo.write_file("other.lua", { "other line 1 modified", "other line 2" }) - - -- Load the other file buffer and set keymaps on it too - vim.cmd("edit " .. repo.path("other.lua")) - local other_bufnr = vim.api.nvim_get_current_buf() - - vim.keymap.set("n", "J", function() - -- Do nothing, just a marker - end, { buffer = other_bufnr, desc = "User's J on other.lua" }) - - -- Go back to test.lua - vim.cmd("edit " .. repo.path("test.lua")) - assert.equals(test_bufnr, vim.api.nvim_get_current_buf()) - - -- Set up user's custom buffer-local keymaps on test.lua - vim.keymap.set("n", "J", function() - original_J_called = true - end, { buffer = test_bufnr, desc = "User's J keymap" }) - - vim.keymap.set("n", "K", function() - original_K_called = true - end, { buffer = test_bufnr, desc = "User's K keymap" }) - - -- Open CodeDiff (explorer mode) - vim.cmd("CodeDiff") - - -- Wait for CodeDiff to open - local opened = vim.wait(5000, function() - return vim.fn.tabpagenr("$") > 1 - end) - assert.is_true(opened, "Should open CodeDiff in new tab") - - local codediff_tab = vim.api.nvim_get_current_tabpage() - - -- Wait for session to be ready - local ready = helpers.wait_for_session_ready(codediff_tab, 10000) - assert.is_true(ready, "CodeDiff session should be ready") - - -- At this point CodeDiff has set keymaps on the modified buffer - - -- Close CodeDiff - vim.cmd("tabclose") - vim.wait(500) - - -- Verify we're back to original tab - assert.equals(1, vim.fn.tabpagenr("$"), "Should be back to single tab") - - -- Buffer must still be valid for keymap restoration to matter - assert.is_true(vim.api.nvim_buf_is_valid(test_bufnr), "Working file buffer should still be valid") - - -- CRITICAL: Verify the user's keymaps are RESTORED (this was broken before fix) - local keymaps_after = vim.api.nvim_buf_get_keymap(test_bufnr, "n") - local found_J_after = false - local found_K_after = false - for _, map in ipairs(keymaps_after) do - if map.lhs == "J" and map.desc == "User's J keymap" then - found_J_after = true - end - if map.lhs == "K" and map.desc == "User's K keymap" then - found_K_after = true - end - end - - assert.is_true(found_J_after, "User's J keymap should be restored after closing CodeDiff") - assert.is_true(found_K_after, "User's K keymap should be restored after closing CodeDiff") - - -- Verify the restored callbacks actually WORK - vim.api.nvim_set_current_buf(test_bufnr) - vim.api.nvim_feedkeys("J", "x", false) - vim.wait(50) - assert.is_true(original_J_called, "User's J callback should work after restoration") - - vim.api.nvim_feedkeys("K", "x", false) - vim.wait(50) - assert.is_true(original_K_called, "User's K callback should work after restoration") - - -- Also verify other.lua's keymaps are intact - if vim.api.nvim_buf_is_valid(other_bufnr) then - local other_keymaps = vim.api.nvim_buf_get_keymap(other_bufnr, "n") - local found_J_on_other = false - for _, map in ipairs(other_keymaps) do - if map.lhs == "J" and map.desc == "User's J on other.lua" then - found_J_on_other = true - end - end - assert.is_true(found_J_on_other, "User's J keymap on other.lua should be intact") - end - end) - - it("Handles buffers without pre-existing keymaps", function() - -- Don't set any custom keymaps - just open and close CodeDiff - -- This should not error - - -- Open CodeDiff - vim.cmd("CodeDiff") - - -- Wait for CodeDiff to open - local opened = vim.wait(5000, function() - return vim.fn.tabpagenr("$") > 1 - end) - assert.is_true(opened, "Should open CodeDiff in new tab") - - local codediff_tab = vim.api.nvim_get_current_tabpage() - local ready = helpers.wait_for_session_ready(codediff_tab, 10000) - assert.is_true(ready, "CodeDiff session should be ready") - - -- Close CodeDiff - should not error - vim.cmd("tabclose") - vim.wait(500) - - -- Verify no CodeDiff keymaps remain on the buffer - if vim.api.nvim_buf_is_valid(test_bufnr) then - local keymaps = vim.api.nvim_buf_get_keymap(test_bufnr, "n") - for _, map in ipairs(keymaps) do - -- Check that codediff keymaps are removed - assert.is_nil( - map.desc and map.desc:match("codediff") or map.desc and map.desc:match("Next hunk"), - "CodeDiff keymaps should be removed: " .. (map.lhs or "") - ) - end - end - end) - - it("Restores keymaps with different options (expr, silent, etc)", function() - -- Set up a more complex keymap with various options - local expr_result = "test_expr_result" - vim.keymap.set("n", "]h", function() - return expr_result - end, { - buffer = test_bufnr, - expr = true, - silent = true, - desc = "User's expr keymap", - }) - - -- Verify keymap exists with correct options - local keymaps_before = vim.api.nvim_buf_get_keymap(test_bufnr, "n") - local found_before = false - for _, map in ipairs(keymaps_before) do - if map.lhs == "]h" then - found_before = true - assert.equals(1, map.expr, "Should have expr=true before") - assert.equals(1, map.silent, "Should have silent=true before") - end - end - assert.is_true(found_before, "Should have ]h keymap before CodeDiff") - - -- Open and close CodeDiff - vim.cmd("CodeDiff") - local opened = vim.wait(5000, function() - return vim.fn.tabpagenr("$") > 1 - end) - assert.is_true(opened, "Should open CodeDiff") - - local codediff_tab = vim.api.nvim_get_current_tabpage() - helpers.wait_for_session_ready(codediff_tab, 10000) - - vim.cmd("tabclose") - vim.wait(500) - - -- Verify keymap is restored with correct options - if vim.api.nvim_buf_is_valid(test_bufnr) then - local keymaps_after = vim.api.nvim_buf_get_keymap(test_bufnr, "n") - local found_after = false - for _, map in ipairs(keymaps_after) do - if map.lhs == "]h" and map.desc == "User's expr keymap" then - found_after = true - assert.equals(1, map.expr, "Should have expr=true after restore") - assert.equals(1, map.silent, "Should have silent=true after restore") - end - end - assert.is_true(found_after, "User's ]h keymap should be restored with correct options") - end - end) -end) From 0cb5ea3c7e1e4f3ed903a3fc57e88f8ee0ca9f93 Mon Sep 17 00:00:00 2001 From: Yanuo Ma Date: Wed, 4 Feb 2026 21:44:20 -0500 Subject: [PATCH 2/3] fix(keymaps): track all buffers for proper cleanup on close Fixes #211 - Keymaps not restored properly after quitting CodeDiff The issue was that when navigating files with J/K, new buffers would get keymaps set but old buffers weren't tracked. On close, only current buffers had keymaps deleted, leaving orphaned buffer-local keymaps that shadowed user's global keymaps. Solution: - Track all buffers in sess.keymap_buffers when keymaps are set - On close, delete keymaps from ALL tracked buffers This also reverts the previous save/restore approach (commit 9a18715) which was unnecessarily complex and didn't fix the actual issue. --- lua/codediff/ui/lifecycle/accessors.lua | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/lua/codediff/ui/lifecycle/accessors.lua b/lua/codediff/ui/lifecycle/accessors.lua index 9c4c7660..070957d1 100644 --- a/lua/codediff/ui/lifecycle/accessors.lua +++ b/lua/codediff/ui/lifecycle/accessors.lua @@ -414,24 +414,31 @@ 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 base_opts = { noremap = true, silent = true, nowait = true } 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 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 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 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 end return true @@ -456,17 +463,19 @@ function M.clear_tab_keymaps(tabpage) end end - del_buf_keymaps(sess.original_bufnr, config.options.keymaps.view) - del_buf_keymaps(sess.modified_bufnr, config.options.keymaps.view) + -- 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 + end + -- Also handle explorer keymaps separately (explorer buffer uses both view and explorer keymaps) if sess.explorer and sess.explorer.bufnr then - del_buf_keymaps(sess.explorer.bufnr, config.options.keymaps.view) del_buf_keymaps(sess.explorer.bufnr, config.options.keymaps.explorer or {}) end - if sess.result_bufnr then - del_buf_keymaps(sess.result_bufnr, config.options.keymaps.view) - end + sess.keymap_buffers = nil end --- Setup auto-sync on file switch: automatically update diff when user edits a different file in working buffer From 39ff519e88702ad49d502bdb00369d29a0289108 Mon Sep 17 00:00:00 2001 From: Yanuo Ma Date: Wed, 4 Feb 2026 21:47:52 -0500 Subject: [PATCH 3/3] refactor(keymaps): remove duplicate next_file/prev_file definitions These keymaps are already set via view/keymaps.lua:setup_all_keymaps() which uses set_tab_keymap to set them on all buffers including explorer and history panels. No need to set them again in panel-specific keymaps. --- lua/codediff/ui/explorer/keymaps.lua | 15 ++------------- lua/codediff/ui/history/keymaps.lua | 15 ++------------- 2 files changed, 4 insertions(+), 26 deletions(-) diff --git a/lua/codediff/ui/explorer/keymaps.lua b/lua/codediff/ui/explorer/keymaps.lua index 38564557..3a2f7515 100644 --- a/lua/codediff/ui/explorer/keymaps.lua +++ b/lua/codediff/ui/explorer/keymaps.lua @@ -162,19 +162,8 @@ function M.setup(explorer) end, vim.tbl_extend("force", map_options, { buffer = split.bufnr, desc = "Restore/discard changes" })) end - -- Navigate to next file (uses view keymaps) - if config.options.keymaps.view.next_file then - vim.keymap.set("n", config.options.keymaps.view.next_file, function() - render_module.navigate_next(explorer) - end, vim.tbl_extend("force", map_options, { buffer = split.bufnr, desc = "Next file" })) - end - - -- Navigate to previous file (uses view keymaps) - if config.options.keymaps.view.prev_file then - vim.keymap.set("n", config.options.keymaps.view.prev_file, function() - render_module.navigate_prev(explorer) - end, vim.tbl_extend("force", map_options, { buffer = split.bufnr, desc = "Previous file" })) - end + -- 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 explorer end return M diff --git a/lua/codediff/ui/history/keymaps.lua b/lua/codediff/ui/history/keymaps.lua index 66ec89df..b4346f08 100644 --- a/lua/codediff/ui/history/keymaps.lua +++ b/lua/codediff/ui/history/keymaps.lua @@ -85,19 +85,8 @@ function M.setup(history, opts) end end, vim.tbl_extend("force", map_options, { buffer = split.bufnr, desc = "Select file" })) - -- Navigate to next file (uses view keymaps) - if config.options.keymaps.view.next_file then - vim.keymap.set("n", config.options.keymaps.view.next_file, function() - opts.navigate_next(history) - end, vim.tbl_extend("force", map_options, { buffer = split.bufnr, desc = "Next file" })) - end - - -- Navigate to previous file (uses view keymaps) - if config.options.keymaps.view.prev_file then - vim.keymap.set("n", config.options.keymaps.view.prev_file, function() - opts.navigate_prev(history) - end, vim.tbl_extend("force", map_options, { buffer = split.bufnr, desc = "Previous file" })) - end + -- 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