From f9160984706630fa747a3b91d63e1d9d4b65374f Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Thu, 25 Jun 2026 15:18:09 +0000 Subject: [PATCH] fix(diff): emit lifecycle events for unified layout Signed-off-by: Thomas Kosiewski --- lua/claudecode/diff.lua | 19 ++ lua/claudecode/diff_inline.lua | 246 +++++++++++++++----- tests/unit/diff_auto_resize_events_spec.lua | 160 +++++++++++++ 3 files changed, 361 insertions(+), 64 deletions(-) diff --git a/lua/claudecode/diff.lua b/lua/claudecode/diff.lua index 4708ee59..25b9cdad 100644 --- a/lua/claudecode/diff.lua +++ b/lua/claudecode/diff.lua @@ -1171,6 +1171,11 @@ function M._cleanup_diff_state(tab_name, reason) local inline = require("claudecode.diff_inline") inline.cleanup_inline_diff(tab_name, diff_data) active_diffs[tab_name] = nil + fire_diff_event("ClaudeCodeDiffClosed", { + tab_name = tab_name, + file_path = diff_data.old_file_path, + reason = reason, + }) return end @@ -1319,6 +1324,20 @@ function M._setup_blocking_diff(params, resolution_callback) if config and config.diff_opts and config.diff_opts.layout == "unified" then local inline = require("claudecode.diff_inline") inline.setup_inline_diff(params, resolution_callback, config) + + local state = active_diffs[tab_name] + if state then + fire_diff_event("ClaudeCodeDiffOpened", { + tab_name = tab_name, + file_path = params.old_file_path, + new_file_path = params.new_file_path, + is_new_file = state.is_new_file, + diff_window = state.new_window, + target_window = state.target_window, + terminal_window = state.terminal_win_in_new_tab or find_claudecode_terminal_window(), + tab_number = state.created_new_tab and state.new_tab_number or nil, + }) + end return end diff --git a/lua/claudecode/diff_inline.lua b/lua/claudecode/diff_inline.lua index 9a8919f0..cd327791 100644 --- a/lua/claudecode/diff_inline.lua +++ b/lua/claudecode/diff_inline.lua @@ -142,6 +142,84 @@ function M.render_diff_buffer(buf, lines, line_types) end end +--- Create a scratch editor window when the current tab only has terminal/sidebar windows. +---@return number win_id Window ID of the created scratch editor window +local function create_fallback_editor_window() + vim.cmd("rightbelow vsplit") + local fallback_win = vim.api.nvim_get_current_win() + + local ok, err = pcall(function() + local scratch_buf = vim.api.nvim_create_buf(false, true) + if scratch_buf == 0 then + error({ code = -32000, message = "Buffer creation failed", data = "Could not create fallback editor buffer" }) + end + + vim.api.nvim_buf_set_option(scratch_buf, "bufhidden", "wipe") + vim.api.nvim_win_set_buf(fallback_win, scratch_buf) + end) + + if not ok then + if fallback_win and vim.api.nvim_win_is_valid(fallback_win) then + pcall(vim.api.nvim_win_close, fallback_win, true) + end + error(err) + end + + return fallback_win +end + +--- Clean up inline diff resources if setup fails before state registration. +---@param tab_name string Diff identifier +---@param partial table Partially created resources +local function cleanup_unregistered_inline_setup(tab_name, partial) + local diff = require("claudecode.diff") + + if diff._get_active_diffs()[tab_name] then + diff._cleanup_diff_state(tab_name, "setup failed") + return + end + + for _, autocmd_id in ipairs(partial.autocmd_ids or {}) do + pcall(vim.api.nvim_del_autocmd, autocmd_id) + end + + local original_tab = partial.original_tab_number + local stranded_tab = partial.new_tab_number + if not (stranded_tab and vim.api.nvim_tabpage_is_valid(stranded_tab)) then + local current_tab = vim.api.nvim_get_current_tabpage() + if original_tab and vim.api.nvim_tabpage_is_valid(original_tab) and current_tab ~= original_tab then + stranded_tab = current_tab + end + end + + if stranded_tab and vim.api.nvim_tabpage_is_valid(stranded_tab) and stranded_tab ~= original_tab then + pcall(vim.api.nvim_set_current_tabpage, stranded_tab) + pcall(vim.cmd, "tabclose") + if original_tab and vim.api.nvim_tabpage_is_valid(original_tab) then + pcall(vim.api.nvim_set_current_tabpage, original_tab) + end + else + if partial.diff_window and vim.api.nvim_win_is_valid(partial.diff_window) then + pcall(vim.api.nvim_win_close, partial.diff_window, true) + end + + if partial.fallback_window and vim.api.nvim_win_is_valid(partial.fallback_window) then + pcall(vim.api.nvim_win_close, partial.fallback_window, true) + end + end + + if partial.new_buffer and vim.api.nvim_buf_is_valid(partial.new_buffer) then + pcall(vim.api.nvim_buf_delete, partial.new_buffer, { force = true }) + end + + if partial.had_terminal_in_original then + local terminal_ok, terminal_module = pcall(require, "claudecode.terminal") + if terminal_ok then + pcall(terminal_module.ensure_visible) + end + end +end + -- ── Setup ───────────────────────────────────────────────────────── --- Set up an inline diff view for the given parameters. @@ -222,9 +300,31 @@ function M.setup_inline_diff(params, resolution_callback, config) local new_tab_handle = nil local had_terminal_in_original = false + local diff_win + local autocmd_ids = {} + local partial_setup = { + new_buffer = buf, + original_tab_number = original_tab_number, + autocmd_ids = autocmd_ids, + } + local function guard_inline_setup(action) + local results = { pcall(action) } + if not results[1] then + cleanup_unregistered_inline_setup(tab_name, partial_setup) + error(results[2]) + end + return unpack(results, 2) + end + if config and config.diff_opts and config.diff_opts.open_in_new_tab then - original_tab_number, terminal_win_in_new_tab, had_terminal_in_original, new_tab_handle = - diff._display_terminal_in_new_tab() + original_tab_number, terminal_win_in_new_tab, had_terminal_in_original, new_tab_handle = guard_inline_setup( + function() + return diff._display_terminal_in_new_tab() + end + ) + partial_setup.original_tab_number = original_tab_number + partial_setup.new_tab_number = new_tab_handle + partial_setup.had_terminal_in_original = had_terminal_in_original created_new_tab = true end @@ -239,6 +339,7 @@ function M.setup_inline_diff(params, resolution_callback, config) -- When in a new tab, use a window from the current tab rather than the global -- search which could return a window from the original tab local editor_win + local fallback_window if created_new_tab then local tab_wins = vim.api.nvim_tabpage_list_wins(0) for _, w in ipairs(tab_wins) do @@ -253,19 +354,28 @@ function M.setup_inline_diff(params, resolution_callback, config) end else editor_win = diff._find_main_editor_window() + if not editor_win then + fallback_window = guard_inline_setup(create_fallback_editor_window) + partial_setup.fallback_window = fallback_window + editor_win = fallback_window + end end - if editor_win then + guard_inline_setup(function() + assert(editor_win and vim.api.nvim_win_is_valid(editor_win), "ClaudeCode unified diff target window must be valid") vim.api.nvim_set_current_win(editor_win) - end - vim.cmd("rightbelow vsplit") - local diff_win = vim.api.nvim_get_current_win() - vim.api.nvim_win_set_buf(diff_win, buf) + vim.cmd("rightbelow vsplit") + diff_win = vim.api.nvim_get_current_win() + partial_setup.diff_window = diff_win + vim.api.nvim_win_set_buf(diff_win, buf) + end) -- Configure window for sign column display pcall(vim.api.nvim_set_option_value, "signcolumn", "yes", { win = diff_win }) -- Equalize window widths - vim.cmd("wincmd =") + guard_inline_setup(function() + vim.cmd("wincmd =") + end) -- Scroll to first change for i, lt in ipairs(line_types) do @@ -293,71 +403,75 @@ function M.setup_inline_diff(params, resolution_callback, config) end -- Restore terminal width after opening the split - if terminal_win_in_new_tab and vim.api.nvim_win_is_valid(terminal_win_in_new_tab) then - local terminal_config = config.terminal or {} - local split_width = terminal_config.split_width_percentage or 0.30 - local total_width = vim.o.columns - local terminal_width = math.floor(total_width * split_width) - vim.api.nvim_win_set_width(terminal_win_in_new_tab, terminal_width) - elseif term_win and vim.api.nvim_win_is_valid(term_win) then - local win_config = vim.api.nvim_win_get_config(term_win) - local is_floating = win_config.relative and win_config.relative ~= "" - if not is_floating and term_width then - pcall(vim.api.nvim_win_set_width, term_win, term_width) + guard_inline_setup(function() + if terminal_win_in_new_tab and vim.api.nvim_win_is_valid(terminal_win_in_new_tab) then + local terminal_config = config.terminal or {} + local split_width = terminal_config.split_width_percentage or 0.30 + local total_width = vim.o.columns + local terminal_width = math.floor(total_width * split_width) + vim.api.nvim_win_set_width(terminal_win_in_new_tab, terminal_width) + elseif term_win and vim.api.nvim_win_is_valid(term_win) then + local win_config = vim.api.nvim_win_get_config(term_win) + local is_floating = win_config.relative and win_config.relative ~= "" + if not is_floating and term_width then + pcall(vim.api.nvim_win_set_width, term_win, term_width) + end end - end + end) - -- Register autocmds - local aug = diff._get_autocmd_group() - local autocmd_ids = {} + -- Register autocmds and state with layout = "unified" + guard_inline_setup(function() + local aug = diff._get_autocmd_group() - autocmd_ids[#autocmd_ids + 1] = vim.api.nvim_create_autocmd("BufWriteCmd", { - group = aug, - buffer = buf, - callback = function() - diff._resolve_diff_as_saved(tab_name, buf) - return true -- prevent actual write - end, - }) - - for _, ev in ipairs({ "BufDelete", "BufUnload", "BufWipeout" }) do - autocmd_ids[#autocmd_ids + 1] = vim.api.nvim_create_autocmd(ev, { + autocmd_ids[#autocmd_ids + 1] = vim.api.nvim_create_autocmd("BufWriteCmd", { group = aug, buffer = buf, callback = function() - diff._resolve_diff_as_rejected(tab_name) + diff._resolve_diff_as_saved(tab_name, buf) + return true -- prevent actual write end, }) - end - -- Register state with layout = "unified" - diff._register_diff_state(tab_name, { - old_file_path = params.old_file_path, - new_file_path = params.new_file_path, - new_file_contents = params.new_file_contents, - new_buffer = buf, - new_window = diff_win, - lines = lines, - line_types = line_types, - is_new_file = is_new_file, - autocmd_ids = autocmd_ids, - created_at = vim.fn.localtime(), - status = "pending", - resolution_callback = resolution_callback, - result_content = nil, - layout = "unified", - -- Track the originating MCP client so close_diffs_for_client can tear this - -- diff down if that client disconnects (parity with the native path, #261). - client_id = params.client_id, - -- Tab/window tracking - original_tab_number = original_tab_number, - created_new_tab = created_new_tab, - new_tab_number = new_tab_handle, - had_terminal_in_original = had_terminal_in_original, - terminal_win_in_new_tab = terminal_win_in_new_tab, - term_win = term_win, - term_width = term_width, - }) + for _, ev in ipairs({ "BufDelete", "BufUnload", "BufWipeout" }) do + autocmd_ids[#autocmd_ids + 1] = vim.api.nvim_create_autocmd(ev, { + group = aug, + buffer = buf, + callback = function() + diff._resolve_diff_as_rejected(tab_name) + end, + }) + end + + diff._register_diff_state(tab_name, { + old_file_path = params.old_file_path, + new_file_path = params.new_file_path, + new_file_contents = params.new_file_contents, + new_buffer = buf, + new_window = diff_win, + target_window = editor_win, + fallback_window = fallback_window, + lines = lines, + line_types = line_types, + is_new_file = is_new_file, + autocmd_ids = autocmd_ids, + created_at = vim.fn.localtime(), + status = "pending", + resolution_callback = resolution_callback, + result_content = nil, + layout = "unified", + -- Track the originating MCP client so close_diffs_for_client can tear this + -- diff down if that client disconnects (parity with the native path, #261). + client_id = params.client_id, + -- Tab/window tracking + original_tab_number = original_tab_number, + created_new_tab = created_new_tab, + new_tab_number = new_tab_handle, + had_terminal_in_original = had_terminal_in_original, + terminal_win_in_new_tab = terminal_win_in_new_tab, + term_win = term_win, + term_width = term_width, + }) + end) end -- ── Resolution functions ────────────────────────────────────────── @@ -458,6 +572,10 @@ function M.cleanup_inline_diff(tab_name, diff_data) pcall(vim.api.nvim_win_close, diff_data.new_window, true) end + if diff_data.fallback_window and vim.api.nvim_win_is_valid(diff_data.fallback_window) then + pcall(vim.api.nvim_win_close, diff_data.fallback_window, false) + end + -- Restore terminal width if diff_data.term_win and vim.api.nvim_win_is_valid(diff_data.term_win) then local win_config = vim.api.nvim_win_get_config(diff_data.term_win) diff --git a/tests/unit/diff_auto_resize_events_spec.lua b/tests/unit/diff_auto_resize_events_spec.lua index b65dff68..7a757c43 100644 --- a/tests/unit/diff_auto_resize_events_spec.lua +++ b/tests/unit/diff_auto_resize_events_spec.lua @@ -137,6 +137,16 @@ describe("Diff lifecycle User events", function() return captured end + local function find_events(captured, pattern) + local events = {} + for _, e in ipairs(captured) do + if e.event == "User" and e.opts and e.opts.pattern == pattern then + events[#events + 1] = e + end + end + return events + end + local function find_event(captured, pattern) for i = #captured, 1, -1 do local e = captured[i] @@ -147,6 +157,38 @@ describe("Diff lifecycle User events", function() return nil end + local function reset_to_terminal_only() + assert(vim and vim._mock and vim._mock.reset, "expected vim mock with _mock.reset()") + + vim._mock.reset() + vim._tabs = { [1] = true } + vim._current_tabpage = 1 + vim._current_window = 1000 + vim._next_winid = 1001 + + vim._mock.add_buffer(1, "term://fake/claude", "", { buftype = "terminal", modified = false }) + vim._mock.add_window(1000, 1, { 1, 0 }) + vim._win_tab[1000] = 1 + vim._tab_windows[1] = { 1000 } + end + + local function reset_to_default_editor_window() + if not (vim and vim._mock and vim._mock.reset) then + return + end + + vim._mock.reset() + vim._tabs = { [1] = true } + vim._current_tabpage = 1 + vim._current_window = 1000 + vim._next_winid = 1001 + + vim._mock.add_buffer(1, "/home/user/project/test.lua", "local test = {}\nreturn test") + vim._mock.add_window(1000, 1, { 1, 0 }) + vim._win_tab[1000] = 1 + vim._tab_windows[1] = { 1000 } + end + before_each(function() local f = io.open(test_old_file, "w") f:write("local a = 1\nlocal b = 2\n") @@ -163,8 +205,10 @@ describe("Diff lifecycle User events", function() end) after_each(function() + diff._cleanup_all_active_diffs("test_cleanup") package.loaded["claudecode.terminal"] = nil os.remove(test_old_file) + reset_to_default_editor_window() end) it("emits ClaudeCodeDiffOpened with the full payload when a diff opens", function() @@ -200,6 +244,122 @@ describe("Diff lifecycle User events", function() end) end) + it("emits ClaudeCodeDiffOpened and ClaudeCodeDiffClosed for unified layout", function() + diff.setup({ terminal = {}, diff_opts = { layout = "unified" } }) + + local co = coroutine.create(function() + open_diff_tool.handler({ + old_file_path = test_old_file, + new_file_path = test_old_file, + new_file_contents = "local a = 1\nlocal b = 42\nlocal c = 3\n", + tab_name = "opened-unified-tab", + }) + end) + local opened_captured = capture_events(function() + local ok, err = coroutine.resume(co) + assert(ok, tostring(err)) + end) + + local opened_events = find_events(opened_captured, "ClaudeCodeDiffOpened") + assert.are.equal(1, #opened_events) + local opened_data = opened_events[1].opts.data + assert.are.equal("opened-unified-tab", opened_data.tab_name) + assert.are.equal(test_old_file, opened_data.file_path) + assert.are.equal(test_old_file, opened_data.new_file_path) + assert.is_false(opened_data.is_new_file) + assert.is_not_nil(opened_data.diff_window) + assert.is_true(vim.api.nvim_win_is_valid(opened_data.diff_window)) + assert.is_not_nil(opened_data.target_window) + assert.is_true(vim.api.nvim_win_is_valid(opened_data.target_window)) + assert.is_false(opened_events[1].opts.modeline) + + local closed_captured = capture_events(function() + diff._cleanup_diff_state("opened-unified-tab", "diff rejected") + end) + local closed_events = find_events(closed_captured, "ClaudeCodeDiffClosed") + assert.are.equal(1, #closed_events) + local closed_data = closed_events[1].opts.data + assert.are.equal("opened-unified-tab", closed_data.tab_name) + assert.are.equal(test_old_file, closed_data.file_path) + assert.are.equal("diff rejected", closed_data.reason) + end) + + it("emits unified ClaudeCodeDiffOpened with a valid target_window from a terminal-only tab", function() + reset_to_terminal_only() + assert.is_nil(diff._find_main_editor_window()) + diff.setup({ terminal = {}, diff_opts = { layout = "unified" } }) + + local co = coroutine.create(function() + open_diff_tool.handler({ + old_file_path = test_old_file, + new_file_path = test_old_file, + new_file_contents = "local a = 1\nlocal b = 43\nlocal c = 3\n", + tab_name = "opened-unified-terminal-only-tab", + }) + end) + local opened_captured = capture_events(function() + local ok, err = coroutine.resume(co) + assert(ok, tostring(err)) + end) + + local opened_events = find_events(opened_captured, "ClaudeCodeDiffOpened") + assert.are.equal(1, #opened_events) + local opened_data = opened_events[1].opts.data + assert.is_not_nil(opened_data.diff_window) + assert.is_true(vim.api.nvim_win_is_valid(opened_data.diff_window)) + assert.is_not_nil(opened_data.target_window) + assert.is_true(vim.api.nvim_win_is_valid(opened_data.target_window)) + assert.are_not.equal(1000, opened_data.target_window) + local target_buf = vim.api.nvim_win_get_buf(opened_data.target_window) + assert.are_not.equal("terminal", vim.api.nvim_buf_get_option(target_buf, "buftype")) + + local state = diff._get_active_diffs()["opened-unified-terminal-only-tab"] + assert.is_table(state) + assert.are.equal(opened_data.target_window, state.fallback_window) + + diff._cleanup_diff_state("opened-unified-terminal-only-tab", "diff rejected") + assert.is_false(vim.api.nvim_win_is_valid(opened_data.target_window)) + assert.is_true(vim.api.nvim_win_is_valid(1000)) + reset_to_default_editor_window() + end) + + it("cleans up a terminal-only unified fallback when setup fails before state registration", function() + reset_to_terminal_only() + assert.is_nil(diff._find_main_editor_window()) + diff.setup({ terminal = {}, diff_opts = { layout = "unified" } }) + + local original_cmd = vim.cmd + local vsplit_count = 0 + vim.cmd = function(command) + if command == "rightbelow vsplit" then + vsplit_count = vsplit_count + 1 + if vsplit_count == 2 then + error("forced unified setup failure after fallback split") + end + end + return original_cmd(command) + end + + local co = coroutine.create(function() + open_diff_tool.handler({ + old_file_path = test_old_file, + new_file_path = test_old_file, + new_file_contents = "local a = 1\nlocal b = 44\nlocal c = 3\n", + tab_name = "failed-unified-terminal-only-tab", + }) + end) + local ok = coroutine.resume(co) + vim.cmd = original_cmd + + assert.is_false(ok) + assert.are.equal(2, vsplit_count) + assert.is_nil(diff._get_active_diffs()["failed-unified-terminal-only-tab"]) + assert.is_true(vim.api.nvim_win_is_valid(1000)) + local wins = vim.api.nvim_list_wins() + assert.are.equal(1, #wins) + assert.are.equal(1000, wins[1]) + end) + it("emits ClaudeCodeDiffClosed with tab_name/file_path/reason on cleanup", function() diff._register_diff_state("events-tab", { old_file_path = test_old_file,