#673: Closing a tab activates its neighbour, not the last-used tab — tab_mru is maintained but never consulted (and is index-based, so reorder corrupts it) - #682
Merged
JDonaghy merged 2 commits intoAug 27, 2026
Conversation
Engine::close_tab picked the next active tab by index arithmetic alone — whatever tab shifted into the closed slot became active — even though tab_mru was maintained on every activation. It was never consulted. - Re-key tab_mru to (GroupId, TabId) instead of a positional index, matching tab_nav_history, so reorder/move can never silently repoint an MRU entry at the wrong tab. Deletes the index-fixup loop that used to run on every close. - Add Engine::successor_tab_after_close(group, closed_tab_id), the one place that reads tab_mru to pick a close successor, falling back to positional adjacency only when the MRU has nothing live. Route close_tab and close_tab_at's active-tab-closing paths through it. - close_tab_at no longer switches focus before closing a background tab: closing a tab that isn't its group's active tab now just removes it in place, leaving active_tab/global focus untouched. Closing the active tab of an unfocused group picks its own MRU-successor without stealing global focus either. Tests (src/core/engine/tests.rs): the non-adjacent repro from the issue (open two tabs on top of a tab that isn't their neighbour, close both, land back on the original tab); background-tab close leaving the active tab alone; a drag/reorder that moves a different tab into the tracked tab's old index slot; and successor selection staying scoped per editor group. A TuiDriver test in src/tui_main/shell_app.rs pins the same non-adjacent repro at the black-box tier, reading the rendered tab bar's active-tab background style (the only way the active tab is distinguished visually) rather than engine state. All five engine tests and the driver test were confirmed to fail against the pre-fix code before the fix landed. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Blocking review finding: close_active_tab_of_background_group (the branch close_tab_at takes when tab_idx is the *active* tab of an *unfocused* split group — the real right-click "Close" path) had zero test coverage. Every existing close_tab_at test, including the ones added by this PR, only ever closed a background tab inside the *focused* group. Add two engine-level tests: - test_close_tab_at_active_tab_of_unfocused_group_uses_own_mru_successor: opens a split, replays the non-adjacent MRU repro in the unfocused group, and asserts global focus and the focused group's own active tab never move while the background group's successor comes from its own MRU stack. - test_close_tab_at_last_tab_of_unfocused_group_removes_whole_group: covers the group.tabs.len() <= 1 fallback that tears down the whole background group, and asserts its tab_mru/tab_nav_history entries are pruned. Also address a non-blocking review note: comment the four bulk-close paths (close_other_tabs, close_tabs_to_right, close_tabs_to_left, close_saved_tabs) to make explicit that their final tab_mru_touch() call must stay after the active-tab is pinned back, since it's what overwrites the transient MRU entries their internal close_tab() loop iterations wrote for tabs never meant to end up active. Confirmed both new tests pass against the fixed code; ran the full close_tab*/context_menu test surface plus cargo build/fmt/clippy clean.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Closes #673
Automated PR opened by coordinator for review of issue #673.