From d79fd746a96ddb5642939c9727baefce642d78e6 Mon Sep 17 00:00:00 2001 From: akbash Date: Tue, 25 Aug 2026 23:44:56 +0300 Subject: fix: require explicit workspace group close (#3206) refs #2874 Co-authored-by: akbash-bot <300245827+akbash-bot@users.noreply.github.com> --- src/app/input/modal.rs | 65 ++++++++++++++++++++++++++++++++++++++++------- src/app/input/mouse.rs | 2 +- src/app/input/navigate.rs | 25 +++++++++++++++--- 3 files changed, 78 insertions(+), 14 deletions(-) (limited to 'src/app/input') diff --git a/src/app/input/modal.rs b/src/app/input/modal.rs index 343fa99c..e710459b 100644 --- a/src/app/input/modal.rs +++ b/src/app/input/modal.rs @@ -733,12 +733,15 @@ pub(crate) fn handle_resize_key(state: &mut AppState, raw_key: TerminalKey) { } pub(super) fn open_confirm_close(state: &mut AppState) { - state.mode = Mode::ConfirmClose; + state.begin_workspace_close_confirmation(state.selected); } #[cfg(test)] pub(super) fn confirm_close_accept(state: &mut AppState) { - state.close_selected_workspace(); + if let Some(ws_idx) = state.take_confirmed_workspace_close_index() { + state.selected = ws_idx; + state.close_selected_workspace(); + } if state.workspaces.is_empty() { state.mode = Mode::Navigate; } else { @@ -747,6 +750,7 @@ pub(super) fn confirm_close_accept(state: &mut AppState) { } pub(super) fn confirm_close_cancel(state: &mut AppState) { + state.confirm_close_workspace_id = None; state.mode = Mode::Navigate; } @@ -1113,9 +1117,8 @@ impl App { } pub(super) fn confirm_close_accept_via_api(&mut self) { - let ws_idx = self.state.selected; - if ws_idx < self.state.workspaces.len() { - self.close_workspace_idx_via_api(ws_idx); + if let Some(ws_idx) = self.state.take_confirmed_workspace_close_index() { + self.close_workspace_idx_with_group_via_api(ws_idx); } self.state.mode = if self.state.active.is_some() { Mode::Terminal @@ -1245,7 +1248,7 @@ impl App { if self.state.confirm_close { open_confirm_close(&mut self.state); } else { - self.close_workspace_idx_via_api(ws_idx); + self.close_workspace_idx_with_group_via_api(ws_idx); self.state.mode = Mode::Navigate; } } @@ -2139,8 +2142,8 @@ mod tests { #[test] fn confirm_close_keyboard_actions_are_direct_not_focused() { let mut state = state_with_workspaces(&["a", "b"]); - state.mode = Mode::ConfirmClose; state.selected = 1; + open_confirm_close(&mut state); handle_confirm_close_key( &mut state, @@ -2149,7 +2152,7 @@ mod tests { assert_eq!(state.mode, Mode::Navigate); assert_eq!(state.workspaces.len(), 2); - state.mode = Mode::ConfirmClose; + open_confirm_close(&mut state); handle_confirm_close_key( &mut state, KeyEvent::new(KeyCode::Enter, KeyModifiers::empty()), @@ -2160,7 +2163,6 @@ mod tests { #[test] fn confirm_close_for_linked_worktree_closes_workspace_only() { let mut state = state_with_workspaces(&["main", "issue"]); - state.mode = Mode::ConfirmClose; state.selected = 1; state.workspaces[1].worktree_space = Some(crate::workspace::WorktreeSpaceMembership { key: "repo-key".into(), @@ -2170,6 +2172,7 @@ mod tests { is_linked_worktree: true, }); + open_confirm_close(&mut state); handle_confirm_close_key( &mut state, KeyEvent::new(KeyCode::Enter, KeyModifiers::empty()), @@ -2304,6 +2307,50 @@ mod tests { assert_eq!(state.workspaces.len(), 2); } + #[test] + fn api_confirm_close_accept_closes_parent_worktree_group() { + let mut app = app_with_test_workspaces(&["main", "issue"]); + mark_worktree_space_member(&mut app.state, 0, "repo-key"); + mark_worktree_space_member(&mut app.state, 1, "repo-key"); + app.state.selected = 0; + open_confirm_close(&mut app.state); + + app.handle_confirm_close_key_via_api(KeyEvent::new(KeyCode::Enter, KeyModifiers::empty())); + + assert!(app.state.workspaces.is_empty()); + assert_eq!(app.state.mode, Mode::Navigate); + assert_eq!(app.event_hub.events_after(0).len(), 2); + } + + #[test] + fn api_confirm_close_accept_keeps_the_original_workspace_target() { + let mut app = app_with_test_workspaces(&["main", "issue", "other"]); + mark_worktree_space_member(&mut app.state, 0, "repo-key"); + mark_worktree_space_member(&mut app.state, 1, "repo-key"); + app.state.selected = 0; + open_confirm_close(&mut app.state); + + app.focus_workspace_idx_via_api(2); + assert_eq!(app.state.selected, 2); + assert_eq!(app.state.mode, Mode::ConfirmClose); + + app.handle_confirm_close_key_via_api(KeyEvent::new(KeyCode::Enter, KeyModifiers::empty())); + + assert_eq!(app.state.workspaces.len(), 1); + assert_eq!(app.state.workspaces[0].display_name(), "other"); + assert_eq!( + app.event_hub + .events_after(0) + .iter() + .filter(|(_, event)| matches!( + event.event, + crate::api::schema::EventKind::WorkspaceClosed + )) + .count(), + 2 + ); + } + #[test] fn api_context_menu_close_tab_last_parent_group_workspace_keeps_confirmation_mode() { let mut app = app_with_test_workspaces(&["main", "issue"]); diff --git a/src/app/input/mouse.rs b/src/app/input/mouse.rs index 3c6c86f4..bf82126b 100644 --- a/src/app/input/mouse.rs +++ b/src/app/input/mouse.rs @@ -3394,7 +3394,7 @@ mod tests { app.state.workspaces = vec![Workspace::test_new("a"), Workspace::test_new("b")]; app.state.active = Some(0); app.state.selected = 1; - app.state.mode = Mode::ConfirmClose; + app.state.begin_workspace_close_confirmation(1); let popup = app.state.confirm_close_rect(); let inner = Rect::new( diff --git a/src/app/input/navigate.rs b/src/app/input/navigate.rs index 8e3d7d6e..517e267d 100644 --- a/src/app/input/navigate.rs +++ b/src/app/input/navigate.rs @@ -214,7 +214,7 @@ impl App { if self.state.confirm_close { super::modal::open_confirm_close(&mut self.state); } else { - self.close_workspace_idx_via_api(ws_idx); + self.close_workspace_idx_with_group_via_api(ws_idx); leave_navigate_mode(&mut self.state); } } @@ -443,9 +443,9 @@ impl App { self.runtime_workspace_focus("tui.workspace.focus", workspace_id); } - pub(crate) fn close_workspace_idx_via_api(&mut self, ws_idx: usize) { + pub(crate) fn close_workspace_idx_with_group_via_api(&mut self, ws_idx: usize) { let workspace_id = self.public_workspace_id(ws_idx); - self.runtime_workspace_close("tui.workspace.close", workspace_id); + self.runtime_workspace_close_group("tui.workspace.close", workspace_id); } pub(crate) fn move_workspace_via_api(&mut self, source_ws_idx: usize, insert_idx: usize) { @@ -489,7 +489,7 @@ impl App { if self.state.confirm_implicit_worktree_group_close(ws_idx) { return true; } - self.close_workspace_idx_via_api(ws_idx); + self.close_workspace_idx_with_group_via_api(ws_idx); return false; } let tab_idx = self.state.workspaces[ws_idx].active_tab_index(); @@ -3517,6 +3517,23 @@ navigate_pane_down = "ctrl+j" assert_eq!(state.mode, Mode::Terminal); } + #[test] + fn tui_close_parent_group_closes_immediately_when_confirmation_disabled() { + let mut app = app_with_test_workspaces(&["main", "issue"]); + mark_worktree_space_member(&mut app.state, 0, "repo-key"); + mark_worktree_space_member(&mut app.state, 1, "repo-key"); + app.state.active = Some(0); + app.state.selected = 0; + app.state.mode = Mode::Navigate; + app.state.confirm_close = false; + + app.execute_tui_navigate_action(NavigateAction::CloseWorkspace, ActionContext::Navigate); + + assert!(app.state.workspaces.is_empty()); + assert_eq!(app.state.mode, Mode::Navigate); + assert_eq!(app.event_hub.events_after(0).len(), 2); + } + #[test] fn prefix_close_pane_last_parent_group_pane_opens_confirmation() { let mut state = state_with_workspaces(&["main", "issue"]); -- cgit v1.2.3