fix(diff): close plugin-created split on cleanup #175
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #155.
This PR prevents leftover diff splits by tracking which windows are created by the diff UI and cleaning them up deterministically when
close_tabis invoked.Summary
open_diff_blocking()is called with an existingtab_name, resolve/cleanup the prior diff state to avoid leaked windows.diff_optskeys and clarify legacy aliases.Testing
make check/make testcould not be run in this environment becauseluacheck/bustedare not installed.lua/andtests/.📋 Implementation Plan
Plan: Fix issue #155 (diff splits left behind)
Context / Why
Issue #155 reports that during Claude-driven edits the plugin frequently opens a 2-pane diff (original + proposed) as vertical splits, but after accepting the change one split remains. Over time, repeated diffs accumulate these leftover windows, forcing users to manually
:qthem.Goal: make diff UI cleanup deterministic so that, when a diff is closed (via the
close_tabMCP tool), the plugin closes any windows it created for the diff and does not leak extra splits.Evidence (what I inspected)
ehaynes99, 2025-11-14).lua/claudecode/diff.lua_create_diff_view_from_window()callschoose_original_window(...)and, whendecision == "split", runscreate_split()and uses that new window for the original side (around lines ~933–941)._cleanup_diff_state()closesdiff_data.new_windowbut only runs:diffoffindiff_data.target_window(around lines ~1029–1039). Iftarget_windowwas created bycreate_split(), it stays open.open_diff_blocking()calls_resolve_diff_as_rejected()when an active diff with the sametab_namealready exists, but does not clean up its windows (around ~1262–1265).lua/claudecode/config.lua: legacy optionsvertical_splitandopen_in_current_tabare mapped onto moderndiff_opts.layoutanddiff_opts.open_in_new_tab(lines ~205–215).README.mdstill documents legacy keys likevertical_split/open_in_current_taband mentionsauto_close_on_accept(which is validated but not implemented).CLAUDE.mddocumentsopen_in_new_tabbut includes an example that also sets legacy keys, which overrides the new setting.Plan
1) Track which diff windows were created by the plugin
Why: cleanup must differentiate between windows that were part of the user’s pre-existing layout vs windows created solely to render the diff.
target_window_created_by_plugin: boolean(ororiginal_window_created_by_plugin) to the object returned by_create_diff_view_from_window().active_diffs[tab_name]in_setup_blocking_diff().choose_original_window(...).decision == "split", thentarget_window_created_by_plugin = truebecause we just createdoriginal_windowviacreate_split().false(reused an existing window).Optional hardening (if you want to cover more layouts)
There is another window-creation path when
_create_diff_view_from_window()is called withtarget_window == niland it needs tocreate_split()away from a terminal/sidebar buffer (around ~908–927). If this path is contributing to “mystery splits”, consider tracking a small list likecreated_windows = { ... }instead of a single boolean, so cleanup can close all plugin-created windows deterministically.2) Close the plugin-created original-side window during cleanup
_cleanup_diff_state(tab_name, reason)(non-new-tab branch):diff_data.new_window(proposed side).diff_data.target_window:diff_data.target_window_created_by_plugin == trueand the window is still valid, close it withnvim_win_close(..., true).vim.cmd("diffoff")) so we don’t destroy a user’s pre-existing window.Safety: only close windows we explicitly created.
3) Clean up when replacing an active diff with the same
tab_nameWhy: avoids accumulating stale diff windows/state when the CLI requests a new diff before the old one is fully closed.
open_diff_blocking():active_diffs[tab_name]exists, call_resolve_diff_as_rejected(tab_name)and then_cleanup_diff_state(tab_name, "replaced by new diff")before starting the new diff.close_tabtool already treats “diff not found” as success.4) Add/extend unit tests to prevent regressions
Add a focused spec that reproduces issue #155 using the existing vim mock (which simulates
:vsplitand window closing).Test: closes plugin-created original window after accept
choose_original_windowmustsplit.diff._setup_blocking_diff(...).target_window+new_windowfromdiff._get_active_diffs()[tab_name].diff._resolve_diff_as_saved(tab_name, new_buffer)thendiff.close_diff_by_tab_name(tab_name).new_windowandtarget_windoware no longer valid, and the original user window is still valid.Test: does not close reused existing window
decision == "reuse".new_window, and keeps the reused window alive.5) Documentation cleanup (reduce user confusion)
README.mddiff section to document modern keys:diff_opts.layout = "vertical"|"horizontal"diff_opts.open_in_new_tabdiff_opts.keep_terminal_focus,diff_opts.hide_terminal_in_new_tab,diff_opts.on_new_file_rejectvertical_split/open_in_current_tabas legacy aliases (or remove them from examples).CLAUDE.mdexample so it doesn’t set bothopen_in_new_taband legacyopen_in_current_tabat the same time.auto_close_on_accept/show_diff_statsas legacy/no-op until implemented.Acceptance Criteria
make test).Generated with
mux• Model:openai:gpt-5.2• Thinking:xhigh