diff --git a/crates/agent_ui/src/agent_panel.rs b/crates/agent_ui/src/agent_panel.rs index 5cacdb93af8623683ceb5ea4e86001d4a84be8b8..3504e3f0718b02bbde9a4fe09001e9abc20d481e 100644 --- a/crates/agent_ui/src/agent_panel.rs +++ b/crates/agent_ui/src/agent_panel.rs @@ -5171,7 +5171,7 @@ mod tests { multi_workspace .read_with(cx, |multi_workspace, _cx| { assert_eq!( - multi_workspace.workspaces().len(), + multi_workspace.len(), 1, "LocalProject should not create a new workspace" ); @@ -5534,16 +5534,15 @@ mod tests { .read_with(cx, |multi_workspace, cx| { // There should be more than one workspace now (the original + the new worktree). assert!( - multi_workspace.workspaces().len() > 1, + multi_workspace.len() > 1, "expected a new workspace to have been created, found {}", - multi_workspace.workspaces().len(), + multi_workspace.len(), ); // Check the newest workspace's panel for the correct agent. - let workspaces = multi_workspace.workspaces(); - let new_workspace = workspaces - .iter() - .find(|ws| ws.entity_id() != workspace.entity_id()) + let new_workspace = multi_workspace + .workspaces() + .find(|ws| *ws != workspace) .expect("should find the new workspace"); let new_panel = new_workspace .read(cx) diff --git a/crates/agent_ui/src/conversation_view.rs b/crates/agent_ui/src/conversation_view.rs index 1b9d364e9ce03702b47c63e8a856f0ba4b8aba87..b8fb1d392a1cd07720b1b6e0114a1f00b1902897 100644 --- a/crates/agent_ui/src/conversation_view.rs +++ b/crates/agent_ui/src/conversation_view.rs @@ -3406,7 +3406,6 @@ pub(crate) mod tests { // Verify workspace1 is no longer the active workspace multi_workspace_handle .read_with(cx, |mw, _cx| { - assert_eq!(mw.active_workspace_index(), 1); assert_ne!(mw.workspace(), &workspace1); }) .unwrap(); diff --git a/crates/recent_projects/src/recent_projects.rs b/crates/recent_projects/src/recent_projects.rs index b3f918e204c5600193cd01a0f7569888d333edd9..ae193c99f1b2135f55069ef670a7ee058ba15fb3 100644 --- a/crates/recent_projects/src/recent_projects.rs +++ b/crates/recent_projects/src/recent_projects.rs @@ -357,7 +357,6 @@ pub fn init(cx: &mut App) { .update(cx, |multi_workspace, window, cx| { let sibling_workspace_ids: HashSet = multi_workspace .workspaces() - .iter() .filter_map(|ws| ws.read(cx).database_id()) .collect(); @@ -1113,9 +1112,7 @@ impl PickerDelegate for RecentProjectsDelegate { .update(cx, |multi_workspace, window, cx| { let workspace = multi_workspace .workspaces() - .iter() - .find(|ws| ws.read(cx).database_id() == Some(workspace_id)) - .cloned(); + .find(|ws| ws.read(cx).database_id() == Some(workspace_id)); if let Some(workspace) = workspace { multi_workspace.activate(workspace, window, cx); } @@ -1932,9 +1929,7 @@ impl RecentProjectsDelegate { .update(cx, |multi_workspace, window, cx| { let workspace = multi_workspace .workspaces() - .iter() - .find(|ws| ws.read(cx).database_id() == Some(workspace_id)) - .cloned(); + .find(|ws| ws.read(cx).database_id() == Some(workspace_id)); if let Some(workspace) = workspace { multi_workspace.remove(&workspace, window, cx); } @@ -2143,7 +2138,9 @@ mod tests { ); assert!( - !multi_workspace.workspaces().contains(&dirty_workspace), + !multi_workspace + .workspaces() + .any(|workspace| workspace == dirty_workspace), "The original dirty workspace should have been replaced" ); diff --git a/crates/settings_ui/src/settings_ui.rs b/crates/settings_ui/src/settings_ui.rs index 634db0e247fdc370c479df0ed4f6d1f84a5284f6..4c7a98f6c0fa94e659a6db4e00aa28e2b4516e13 100644 --- a/crates/settings_ui/src/settings_ui.rs +++ b/crates/settings_ui/src/settings_ui.rs @@ -3753,7 +3753,6 @@ fn all_projects( .flat_map(|multi_workspace| { multi_workspace .workspaces() - .iter() .map(|workspace| workspace.read(cx).project().clone()) .collect::>() }), diff --git a/crates/sidebar/src/project_group_builder.rs b/crates/sidebar/src/project_group_builder.rs index 378db285a90479690bb40c327da33e63b3cd0807..b77c1b708148c77b92451681b06ce4af527b954a 100644 --- a/crates/sidebar/src/project_group_builder.rs +++ b/crates/sidebar/src/project_group_builder.rs @@ -15,7 +15,6 @@ use std::{ }; use gpui::{App, Entity}; -use ui::SharedString; use workspace::{MultiWorkspace, PathList, ProjectGroupKey, Workspace}; #[derive(Default)] diff --git a/crates/sidebar/src/sidebar.rs b/crates/sidebar/src/sidebar.rs index f0f04c1e6843b77bad63f30542e6d5a62dd40d35..d6ffe5aed70ff434818bf3ee9e3541eb350a9723 100644 --- a/crates/sidebar/src/sidebar.rs +++ b/crates/sidebar/src/sidebar.rs @@ -443,7 +443,7 @@ impl Sidebar { }) .detach(); - let workspaces = multi_workspace.read(cx).workspaces().to_vec(); + let workspaces = multi_workspace.read(cx).workspaces().collect::>(); cx.defer_in(window, move |this, window, cx| { for workspace in &workspaces { this.subscribe_to_workspace(workspace, window, cx); @@ -697,7 +697,7 @@ impl Sidebar { return; }; let mw = multi_workspace.read(cx); - let workspaces = mw.workspaces().to_vec(); + let workspaces = mw.workspaces().collect::>(); let active_workspace = Some(mw.active_workspace()); let agent_server_store = workspaces @@ -1633,7 +1633,7 @@ impl Sidebar { let workspace_count = multi_workspace .upgrade() - .map_or(0, |mw| mw.read(cx).workspaces().len()); + .map_or(0, |mw| mw.read(cx).workspaces().count()); let menu = if workspace_count > 1 { let workspace_for_move = workspace.clone(); let multi_workspace_for_move = multi_workspace.clone(); @@ -2018,9 +2018,7 @@ impl Sidebar { let workspace = window.read(cx).ok().and_then(|multi_workspace| { multi_workspace .workspaces() - .iter() .find(|workspace| predicate(workspace, cx)) - .cloned() })?; Some((window, workspace)) }) @@ -2035,9 +2033,7 @@ impl Sidebar { multi_workspace .read(cx) .workspaces() - .iter() .find(|workspace| predicate(workspace, cx)) - .cloned() }) } @@ -2368,7 +2364,7 @@ impl Sidebar { return; }; - let workspaces = multi_workspace.read(cx).workspaces().to_vec(); + let workspaces = multi_workspace.read(cx).workspaces().collect::>(); for workspace in workspaces { if let Some(agent_panel) = workspace.read(cx).panel::(cx) { let cancelled = @@ -2962,7 +2958,6 @@ impl Sidebar { .map(|mw| { mw.read(cx) .workspaces() - .iter() .filter_map(|ws| ws.read(cx).database_id()) .collect() }) @@ -3401,12 +3396,9 @@ impl Sidebar { } fn active_workspace(&self, cx: &App) -> Option> { - self.multi_workspace.upgrade().and_then(|w| { - w.read(cx) - .workspaces() - .get(w.read(cx).active_workspace_index()) - .cloned() - }) + self.multi_workspace + .upgrade() + .map(|w| w.read(cx).active_workspace()) } fn show_thread_import_modal(&mut self, window: &mut Window, cx: &mut Context) { @@ -3514,12 +3506,11 @@ impl Sidebar { } fn show_archive(&mut self, window: &mut Window, cx: &mut Context) { - let Some(active_workspace) = self.multi_workspace.upgrade().and_then(|w| { - w.read(cx) - .workspaces() - .get(w.read(cx).active_workspace_index()) - .cloned() - }) else { + let Some(active_workspace) = self + .multi_workspace + .upgrade() + .map(|w| w.read(cx).active_workspace()) + else { return; }; let Some(agent_panel) = active_workspace.read(cx).panel::(cx) else { @@ -3821,21 +3812,18 @@ pub fn dump_workspace_info( let multi_workspace = workspace.multi_workspace().and_then(|weak| weak.upgrade()); let workspaces: Vec> = match &multi_workspace { - Some(mw) => mw.read(cx).workspaces().to_vec(), + Some(mw) => mw.read(cx).workspaces().collect::>(), None => vec![this_entity.clone()], }; - let active_index = multi_workspace + let active_workspace = multi_workspace .as_ref() - .map(|mw| mw.read(cx).active_workspace_index()); + .map(|mw| mw.read(cx).active_workspace()); writeln!(output, "MultiWorkspace: {} workspace(s)", workspaces.len()).ok(); - if let Some(index) = active_index { - writeln!(output, "Active workspace index: {index}").ok(); - } writeln!(output).ok(); for (index, ws) in workspaces.iter().enumerate() { - let is_active = active_index == Some(index); + let is_active = active_workspace.as_ref() == Some(ws); writeln!( output, "--- Workspace {index}{} ---", diff --git a/crates/sidebar/src/sidebar_tests.rs b/crates/sidebar/src/sidebar_tests.rs index 1499fc48a9fd094b07d181701866ab941c5968f3..c60a8ab42706e008f29eddbb356d52b70e32650e 100644 --- a/crates/sidebar/src/sidebar_tests.rs +++ b/crates/sidebar/src/sidebar_tests.rs @@ -515,7 +515,7 @@ async fn test_workspace_lifecycle(cx: &mut TestAppContext) { // Remove the second workspace multi_workspace.update_in(cx, |mw, window, cx| { - let workspace = mw.workspaces()[1].clone(); + let workspace = mw.workspaces().nth(1).unwrap().clone(); mw.remove(&workspace, window, cx); }); cx.run_until_parked(); @@ -1870,13 +1870,13 @@ async fn test_confirm_on_historical_thread_activates_workspace(cx: &mut TestAppC // Switch to workspace 1 so we can verify the confirm switches back. multi_workspace.update_in(cx, |mw, window, cx| { - let workspace = mw.workspaces()[1].clone(); + let workspace = mw.workspaces().nth(1).unwrap().clone(); mw.activate(workspace, window, cx); }); cx.run_until_parked(); assert_eq!( - multi_workspace.read_with(cx, |mw, _| mw.active_workspace_index()), - 1 + multi_workspace.read_with(cx, |mw, _| mw.active_workspace()), + multi_workspace.read_with(cx, |mw, _| mw.workspaces().nth(1).unwrap()) ); // Confirm on the historical (non-live) thread at index 1. @@ -1890,8 +1890,8 @@ async fn test_confirm_on_historical_thread_activates_workspace(cx: &mut TestAppC cx.run_until_parked(); assert_eq!( - multi_workspace.read_with(cx, |mw, _| mw.active_workspace_index()), - 0 + multi_workspace.read_with(cx, |mw, _| mw.active_workspace()), + multi_workspace.read_with(cx, |mw, _| mw.workspaces().nth(0).unwrap()) ); } @@ -2038,7 +2038,8 @@ async fn test_focused_thread_tracks_user_intent(cx: &mut TestAppContext) { let panel_b = add_agent_panel(&workspace_b, cx); cx.run_until_parked(); - let workspace_a = multi_workspace.read_with(cx, |mw, _cx| mw.workspaces()[0].clone()); + let workspace_a = + multi_workspace.read_with(cx, |mw, _cx| mw.workspaces().nth(0).unwrap().clone()); // ── 1. Initial state: focused thread derived from active panel ───── sidebar.read_with(cx, |sidebar, _cx| { @@ -2137,7 +2138,7 @@ async fn test_focused_thread_tracks_user_intent(cx: &mut TestAppContext) { }); multi_workspace.update_in(cx, |mw, window, cx| { - let workspace = mw.workspaces()[0].clone(); + let workspace = mw.workspaces().nth(0).unwrap().clone(); mw.activate(workspace, window, cx); }); cx.run_until_parked(); @@ -2192,8 +2193,8 @@ async fn test_focused_thread_tracks_user_intent(cx: &mut TestAppContext) { // Switching workspaces via the multi_workspace (simulates clicking // a workspace header) should clear focused_thread. multi_workspace.update_in(cx, |mw, window, cx| { - if let Some(index) = mw.workspaces().iter().position(|w| w == &workspace_b) { - let workspace = mw.workspaces()[index].clone(); + let workspace = mw.workspaces().find(|w| w == &workspace_b); + if let Some(workspace) = workspace { mw.activate(workspace, window, cx); } }); @@ -2488,7 +2489,7 @@ async fn test_cmd_n_shows_new_thread_entry_in_absorbed_worktree(cx: &mut TestApp // Switch to the worktree workspace. multi_workspace.update_in(cx, |mw, window, cx| { - let workspace = mw.workspaces()[1].clone(); + let workspace = mw.workspaces().nth(1).unwrap().clone(); mw.activate(workspace, window, cx); }); @@ -3055,7 +3056,7 @@ async fn test_absorbed_worktree_running_thread_shows_live_status(cx: &mut TestAp // Switch back to the main workspace before setting up the sidebar. multi_workspace.update_in(cx, |mw, window, cx| { - let workspace = mw.workspaces()[0].clone(); + let workspace = mw.workspaces().nth(0).unwrap().clone(); mw.activate(workspace, window, cx); }); @@ -3153,7 +3154,7 @@ async fn test_absorbed_worktree_completion_triggers_notification(cx: &mut TestAp let worktree_panel = add_agent_panel(&worktree_workspace, cx); multi_workspace.update_in(cx, |mw, window, cx| { - let workspace = mw.workspaces()[0].clone(); + let workspace = mw.workspaces().nth(0).unwrap().clone(); mw.activate(workspace, window, cx); }); @@ -3255,7 +3256,7 @@ async fn test_clicking_worktree_thread_opens_workspace_when_none_exists(cx: &mut // Only 1 workspace should exist. assert_eq!( - multi_workspace.read_with(cx, |mw, _| mw.workspaces().len()), + multi_workspace.read_with(cx, |mw, _| mw.workspaces().count()), 1, ); @@ -3272,11 +3273,11 @@ async fn test_clicking_worktree_thread_opens_workspace_when_none_exists(cx: &mut // A new workspace should have been created for the worktree path. let new_workspace = multi_workspace.read_with(cx, |mw, _| { assert_eq!( - mw.workspaces().len(), + mw.workspaces().count(), 2, "confirming a worktree thread without a workspace should open one", ); - mw.workspaces()[1].clone() + mw.workspaces().nth(1).unwrap().clone() }); let new_path_list = @@ -3474,7 +3475,7 @@ async fn test_clicking_absorbed_worktree_thread_activates_worktree_workspace( // Activate the main workspace before setting up the sidebar. multi_workspace.update_in(cx, |mw, window, cx| { - let workspace = mw.workspaces()[0].clone(); + let workspace = mw.workspaces().nth(0).unwrap().clone(); mw.activate(workspace, window, cx); }); @@ -3501,8 +3502,8 @@ async fn test_clicking_absorbed_worktree_thread_activates_worktree_workspace( .expect("should find the worktree thread entry"); assert_eq!( - multi_workspace.read_with(cx, |mw, _| mw.active_workspace_index()), - 0, + multi_workspace.read_with(cx, |mw, _| mw.active_workspace()), + multi_workspace.read_with(cx, |mw, _| mw.workspaces().nth(0).unwrap()), "main workspace should be active initially" ); @@ -3517,9 +3518,7 @@ async fn test_clicking_absorbed_worktree_thread_activates_worktree_workspace( cx.run_until_parked(); // The worktree workspace should now be active, not the main one. - let active_workspace = multi_workspace.read_with(cx, |mw, _| { - mw.workspaces()[mw.active_workspace_index()].clone() - }); + let active_workspace = multi_workspace.read_with(cx, |mw, _| mw.active_workspace().clone()); assert_eq!( active_workspace, worktree_workspace, "clicking an absorbed worktree thread should activate the worktree workspace" @@ -3559,13 +3558,13 @@ async fn test_activate_archived_thread_with_saved_paths_activates_matching_works // Ensure workspace A is active. multi_workspace.update_in(cx, |mw, window, cx| { - let workspace = mw.workspaces()[0].clone(); + let workspace = mw.workspaces().nth(0).unwrap().clone(); mw.activate(workspace, window, cx); }); cx.run_until_parked(); assert_eq!( - multi_workspace.read_with(cx, |mw, _| mw.active_workspace_index()), - 0 + multi_workspace.read_with(cx, |mw, _| mw.active_workspace()), + multi_workspace.read_with(cx, |mw, _| mw.workspaces().nth(0).unwrap()) ); // Call activate_archived_thread – should resolve saved paths and @@ -3589,8 +3588,8 @@ async fn test_activate_archived_thread_with_saved_paths_activates_matching_works cx.run_until_parked(); assert_eq!( - multi_workspace.read_with(cx, |mw, _| mw.active_workspace_index()), - 1, + multi_workspace.read_with(cx, |mw, _| mw.active_workspace()), + multi_workspace.read_with(cx, |mw, _| mw.workspaces().nth(1).unwrap()), "should have activated the workspace matching the saved path_list" ); } @@ -3623,13 +3622,13 @@ async fn test_activate_archived_thread_cwd_fallback_with_matching_workspace( // Start with workspace A active. multi_workspace.update_in(cx, |mw, window, cx| { - let workspace = mw.workspaces()[0].clone(); + let workspace = mw.workspaces().nth(0).unwrap().clone(); mw.activate(workspace, window, cx); }); cx.run_until_parked(); assert_eq!( - multi_workspace.read_with(cx, |mw, _| mw.active_workspace_index()), - 0 + multi_workspace.read_with(cx, |mw, _| mw.active_workspace()), + multi_workspace.read_with(cx, |mw, _| mw.workspaces().nth(0).unwrap()) ); // No thread saved to the store – cwd is the only path hint. @@ -3652,8 +3651,8 @@ async fn test_activate_archived_thread_cwd_fallback_with_matching_workspace( cx.run_until_parked(); assert_eq!( - multi_workspace.read_with(cx, |mw, _| mw.active_workspace_index()), - 1, + multi_workspace.read_with(cx, |mw, _| mw.active_workspace()), + multi_workspace.read_with(cx, |mw, _| mw.workspaces().nth(1).unwrap()), "should have activated the workspace matching the cwd" ); } @@ -3686,13 +3685,13 @@ async fn test_activate_archived_thread_no_paths_no_cwd_uses_active_workspace( // Activate workspace B (index 1) to make it the active one. multi_workspace.update_in(cx, |mw, window, cx| { - let workspace = mw.workspaces()[1].clone(); + let workspace = mw.workspaces().nth(1).unwrap().clone(); mw.activate(workspace, window, cx); }); cx.run_until_parked(); assert_eq!( - multi_workspace.read_with(cx, |mw, _| mw.active_workspace_index()), - 1 + multi_workspace.read_with(cx, |mw, _| mw.active_workspace()), + multi_workspace.read_with(cx, |mw, _| mw.workspaces().nth(1).unwrap()) ); // No saved thread, no cwd – should fall back to the active workspace. @@ -3715,8 +3714,8 @@ async fn test_activate_archived_thread_no_paths_no_cwd_uses_active_workspace( cx.run_until_parked(); assert_eq!( - multi_workspace.read_with(cx, |mw, _| mw.active_workspace_index()), - 1, + multi_workspace.read_with(cx, |mw, _| mw.active_workspace()), + multi_workspace.read_with(cx, |mw, _| mw.workspaces().nth(1).unwrap()), "should have stayed on the active workspace when no path info is available" ); } @@ -3746,7 +3745,7 @@ async fn test_activate_archived_thread_saved_paths_opens_new_workspace(cx: &mut let session_id = acp::SessionId::new(Arc::from("archived-new-ws")); assert_eq!( - multi_workspace.read_with(cx, |mw, _| mw.workspaces().len()), + multi_workspace.read_with(cx, |mw, _| mw.workspaces().count()), 1, "should start with one workspace" ); @@ -3770,7 +3769,7 @@ async fn test_activate_archived_thread_saved_paths_opens_new_workspace(cx: &mut cx.run_until_parked(); assert_eq!( - multi_workspace.read_with(cx, |mw, _| mw.workspaces().len()), + multi_workspace.read_with(cx, |mw, _| mw.workspaces().count()), 2, "should have opened a second workspace for the archived thread's saved paths" ); @@ -3821,14 +3820,14 @@ async fn test_activate_archived_thread_reuses_workspace_in_another_window(cx: &m assert_eq!( multi_workspace_a - .read_with(cx_a, |mw, _| mw.workspaces().len()) + .read_with(cx_a, |mw, _| mw.workspaces().count()) .unwrap(), 1, "should not add the other window's workspace into the current window" ); assert_eq!( multi_workspace_b - .read_with(cx_a, |mw, _| mw.workspaces().len()) + .read_with(cx_a, |mw, _| mw.workspaces().count()) .unwrap(), 1, "should reuse the existing workspace in the other window" @@ -3898,14 +3897,14 @@ async fn test_activate_archived_thread_reuses_workspace_in_another_window_with_t assert_eq!( multi_workspace_a - .read_with(cx_a, |mw, _| mw.workspaces().len()) + .read_with(cx_a, |mw, _| mw.workspaces().count()) .unwrap(), 1, "should not add the other window's workspace into the current window" ); assert_eq!( multi_workspace_b - .read_with(cx_a, |mw, _| mw.workspaces().len()) + .read_with(cx_a, |mw, _| mw.workspaces().count()) .unwrap(), 1, "should reuse the existing workspace in the other window" @@ -3985,14 +3984,14 @@ async fn test_activate_archived_thread_prefers_current_window_for_matching_paths }); assert_eq!( multi_workspace_a - .read_with(cx_a, |mw, _| mw.workspaces().len()) + .read_with(cx_a, |mw, _| mw.workspaces().count()) .unwrap(), 1, "current window should continue reusing its existing workspace" ); assert_eq!( multi_workspace_b - .read_with(cx_a, |mw, _| mw.workspaces().len()) + .read_with(cx_a, |mw, _| mw.workspaces().count()) .unwrap(), 1, "other windows should not be activated just because they also match the saved paths" @@ -4062,13 +4061,14 @@ async fn test_archive_thread_uses_next_threads_own_workspace(cx: &mut TestAppCon // Activate main workspace so the sidebar tracks the main panel. multi_workspace.update_in(cx, |mw, window, cx| { - let workspace = mw.workspaces()[0].clone(); + let workspace = mw.workspaces().nth(0).unwrap().clone(); mw.activate(workspace, window, cx); }); let sidebar = setup_sidebar(&multi_workspace, cx); - let main_workspace = multi_workspace.read_with(cx, |mw, _| mw.workspaces()[0].clone()); + let main_workspace = + multi_workspace.read_with(cx, |mw, _| mw.workspaces().nth(0).unwrap().clone()); let main_panel = add_agent_panel(&main_workspace, cx); let _worktree_panel = add_agent_panel(&worktree_workspace, cx); @@ -4970,8 +4970,9 @@ mod property_test { ) { match operation { Operation::SaveThread { workspace_index } => { - let workspace = - multi_workspace.read_with(cx, |mw, _| mw.workspaces()[workspace_index].clone()); + let workspace = multi_workspace.read_with(cx, |mw, _| { + mw.workspaces().nth(workspace_index).unwrap().clone() + }); let path_list = workspace .read_with(cx, |workspace, cx| PathList::new(&workspace.root_paths(cx))); save_thread_to_path(state, path_list, cx); @@ -5059,7 +5060,7 @@ mod property_test { } Operation::RemoveWorkspace { index } => { let removed = multi_workspace.update_in(cx, |mw, window, cx| { - let workspace = mw.workspaces()[index].clone(); + let workspace = mw.workspaces().nth(index).unwrap().clone(); mw.remove(&workspace, window, cx) }); if removed { @@ -5073,8 +5074,8 @@ mod property_test { } } Operation::SwitchWorkspace { index } => { - let workspace = - multi_workspace.read_with(cx, |mw, _| mw.workspaces()[index].clone()); + let workspace = multi_workspace + .read_with(cx, |mw, _| mw.workspaces().nth(index).unwrap().clone()); multi_workspace.update_in(cx, |mw, window, cx| { mw.activate(workspace, window, cx); }); @@ -5124,8 +5125,9 @@ mod property_test { .await; // Re-scan the main workspace's project so it discovers the new worktree. - let main_workspace = - multi_workspace.read_with(cx, |mw, _| mw.workspaces()[workspace_index].clone()); + let main_workspace = multi_workspace.read_with(cx, |mw, _| { + mw.workspaces().nth(workspace_index).unwrap().clone() + }); let main_project = main_workspace.read_with(cx, |ws, _| ws.project().clone()); main_project .update(cx, |p, cx| p.git_scans_complete(cx)) @@ -5173,7 +5175,7 @@ mod property_test { anyhow::bail!("sidebar should still have an associated multi-workspace"); }; - let workspaces = multi_workspace.read(cx).workspaces().to_vec(); + let workspaces = multi_workspace.read(cx).workspaces().collect::>(); // Workspaces with no root paths are not shown because the // sidebar skips empty path lists. All other workspaces should @@ -5211,7 +5213,7 @@ mod property_test { let Some(multi_workspace) = sidebar.multi_workspace.upgrade() else { anyhow::bail!("sidebar should still have an associated multi-workspace"); }; - let workspaces = multi_workspace.read(cx).workspaces().to_vec(); + let workspaces = multi_workspace.read(cx).workspaces().collect::>(); let thread_store = ThreadMetadataStore::global(cx); let sidebar_thread_ids: HashSet = sidebar diff --git a/crates/title_bar/src/title_bar.rs b/crates/title_bar/src/title_bar.rs index 440249907adb6d29602ad8e950d0fd26a2d1c31d..dfcd933dc20df9a6f6643402719f2ec1143cc7fe 100644 --- a/crates/title_bar/src/title_bar.rs +++ b/crates/title_bar/src/title_bar.rs @@ -740,7 +740,6 @@ impl TitleBar { .map(|mw| { mw.read(cx) .workspaces() - .iter() .filter_map(|ws| ws.read(cx).database_id()) .collect() }) @@ -803,7 +802,6 @@ impl TitleBar { .map(|mw| { mw.read(cx) .workspaces() - .iter() .filter_map(|ws| ws.read(cx).database_id()) .collect() }) diff --git a/crates/workspace/src/multi_workspace.rs b/crates/workspace/src/multi_workspace.rs index 7268a1aebe1a03f3bb5f5bf32ce45e57fce5f580..e6e5d51c3c24bf512380d836a5ca95c1d09bb8dc 100644 --- a/crates/workspace/src/multi_workspace.rs +++ b/crates/workspace/src/multi_workspace.rs @@ -15,7 +15,7 @@ use std::future::Future; use std::path::PathBuf; use std::sync::Arc; use ui::prelude::*; -use util::{ResultExt, debug_panic, path_list::PathList}; +use util::{ResultExt, path_list::PathList}; use zed_actions::agents_sidebar::{MoveWorkspaceToNewWindow, ToggleThreadSwitcher}; use agent_settings::AgentSettings; @@ -435,11 +435,10 @@ impl MultiWorkspace { } } - pub fn workspaces(&self) -> Vec> { + pub fn workspaces(&self) -> impl Iterator> + '_ { self.project_groups .iter() .flat_map(|group| group.workspaces.iter().cloned()) - .collect() } pub fn open_sidebar(&mut self, cx: &mut Context) { @@ -470,8 +469,9 @@ impl MultiWorkspace { pub fn close_window(&mut self, _: &CloseWindow, window: &mut Window, cx: &mut Context) { cx.spawn_in(window, async move |this, cx| { - let workspaces = - this.update(cx, |multi_workspace, _cx| multi_workspace.workspaces())?; + let workspaces: Vec<_> = this.update(cx, |multi_workspace, _cx| { + multi_workspace.workspaces().collect() + })?; for workspace in workspaces { let should_continue = workspace @@ -514,16 +514,6 @@ impl MultiWorkspace { self.active_workspace.clone() } - pub fn active_workspace_index(&self) -> usize { - self.workspaces() - .iter() - .position(|workspace| workspace == &self.active_workspace) - .unwrap_or_else(|| { - debug_panic!("active workspace was not present in project groups"); - 0 - }) - } - /// Adds a workspace to this window without changing which workspace is /// active. pub fn add(&mut self, workspace: Entity, window: &Window, cx: &mut Context) { @@ -535,6 +525,13 @@ impl MultiWorkspace { self.insert_workspace(workspace, window, cx); } + /// Returns the number of workspaces in this multiworkspace + pub fn len(&self) -> usize { + self.project_groups + .iter() + .fold(0, |acc, group| acc + group.workspaces.len()) + } + /// Ensures the workspace is in the multiworkspace and makes it the active one. pub fn activate( &mut self, @@ -571,9 +568,10 @@ impl MultiWorkspace { return; } - if let Some(index) = self.workspaces().iter().position(|w| *w == workspace) { - let changed = self.active_workspace_index() != index; - self.active_workspace = self.workspaces()[index].clone(); + let exists = self.workspaces().any(|w| w == workspace); + if exists { + let changed = self.active_workspace != workspace; + self.active_workspace = workspace; if changed { cx.emit(MultiWorkspaceEvent::ActiveWorkspaceChanged); self.serialize(cx); @@ -611,7 +609,7 @@ impl MultiWorkspace { window: &Window, cx: &mut Context, ) { - if self.workspaces().iter().any(|w| *w == workspace) { + if self.workspaces().any(|w| w == workspace) { return; } @@ -668,15 +666,16 @@ impl MultiWorkspace { } fn cycle_workspace(&mut self, delta: isize, window: &mut Window, cx: &mut Context) { - let workspaces = self.workspaces(); - let count = workspaces.len() as isize; - if count <= 1 { - return; + let count = self.len(); + let current_ix = self + .workspaces() + .position(|w| w == self.active_workspace) + .unwrap_or(0); + let next_ix = ((current_ix as isize + delta).rem_euclid(count as isize)) as usize; + let next_workspace = self.workspaces().nth(next_ix); + if let Some(next_workspace) = next_workspace { + self.activate(next_workspace, window, cx); } - let current = self.active_workspace_index() as isize; - let next = ((current + delta).rem_euclid(count)) as usize; - let workspace = workspaces[next].clone(); - self.activate(workspace, window, cx); } fn next_workspace(&mut self, _: &NextWorkspace, window: &mut Window, cx: &mut Context) { @@ -909,7 +908,7 @@ impl MultiWorkspace { window: &mut Window, cx: &mut Context, ) -> bool { - let all_workspaces = self.workspaces(); + let all_workspaces: Vec<_> = self.workspaces().collect(); if all_workspaces.len() <= 1 { return false; } @@ -928,11 +927,11 @@ impl MultiWorkspace { // If we removed the active workspace, pick a new one. if self.active_workspace == *workspace { - self.active_workspace = self + let workspace = self .workspaces() - .into_iter() .next() .expect("there is always at least one workspace after the len() > 1 check"); + self.active_workspace = workspace; } self.detach_workspace(workspace, cx); diff --git a/crates/workspace/src/multi_workspace_tests.rs b/crates/workspace/src/multi_workspace_tests.rs index 50161121719ec7b2835fd11e389f24860e57d8f5..3726a3c5f57a28527d5f26159ff495ac88cc0e52 100644 --- a/crates/workspace/src/multi_workspace_tests.rs +++ b/crates/workspace/src/multi_workspace_tests.rs @@ -98,7 +98,12 @@ async fn test_replace(cx: &mut TestAppContext) { let (multi_workspace, cx) = cx.add_window_view(|window, cx| MultiWorkspace::test_new(project_a.clone(), window, cx)); - let workspace_a_id = multi_workspace.read_with(cx, |mw, _cx| mw.workspaces()[0].entity_id()); + let workspace_a_id = multi_workspace.read_with(cx, |mw, _cx| { + mw.workspaces() + .next() + .expect("workspace should exist") + .entity_id() + }); // Replace the only workspace (single-workspace case). let workspace_b = multi_workspace.update_in(cx, |mw, window, cx| { @@ -108,14 +113,20 @@ async fn test_replace(cx: &mut TestAppContext) { }); multi_workspace.read_with(cx, |mw, _cx| { - assert_eq!(mw.workspaces().len(), 1); + assert_eq!(mw.len(), 1); assert_eq!( - mw.workspaces()[0].entity_id(), + mw.workspaces() + .next() + .expect("workspace should exist") + .entity_id(), workspace_b.entity_id(), "slot should now be project_b" ); assert_ne!( - mw.workspaces()[0].entity_id(), + mw.workspaces() + .next() + .expect("workspace should exist") + .entity_id(), workspace_a_id, "project_a should be gone" ); @@ -127,8 +138,12 @@ async fn test_replace(cx: &mut TestAppContext) { }); multi_workspace.read_with(cx, |mw, _cx| { - assert_eq!(mw.workspaces().len(), 2); - assert_eq!(mw.active_workspace_index(), 1); + assert_eq!(mw.len(), 2); + assert_eq!( + mw.workspaces() + .position(|workspace| workspace == mw.active_workspace()), + Some(1) + ); }); let workspace_d = multi_workspace.update_in(cx, |mw, window, cx| { @@ -138,15 +153,25 @@ async fn test_replace(cx: &mut TestAppContext) { }); multi_workspace.read_with(cx, |mw, _cx| { - assert_eq!(mw.workspaces().len(), 2, "should still have 2 workspaces"); - assert_eq!(mw.active_workspace_index(), 1); + assert_eq!(mw.len(), 2, "should still have 2 workspaces"); assert_eq!( - mw.workspaces()[1].entity_id(), + mw.workspaces() + .position(|workspace| workspace == mw.active_workspace()), + Some(1) + ); + assert_eq!( + mw.workspaces() + .nth(1) + .expect("no workspace at index 1") + .entity_id(), workspace_d.entity_id(), "active slot should now be project_d" ); assert_ne!( - mw.workspaces()[1].entity_id(), + mw.workspaces() + .nth(1) + .expect("no workspace at index 1") + .entity_id(), workspace_c.entity_id(), "project_c should be gone" ); @@ -158,14 +183,11 @@ async fn test_replace(cx: &mut TestAppContext) { }); multi_workspace.read_with(cx, |mw, _cx| { + assert_eq!(mw.len(), 2, "no workspace should be added or removed"); assert_eq!( - mw.workspaces().len(), - 2, - "no workspace should be added or removed" - ); - assert_eq!( - mw.active_workspace_index(), - 0, + mw.workspaces() + .position(|workspace| workspace == mw.active_workspace()), + Some(0), "should have switched to workspace_b" ); }); diff --git a/crates/workspace/src/persistence.rs b/crates/workspace/src/persistence.rs index d38602ea768e8edc4f3de1ec439e67f0ee432a63..2ef6e7a0d0e15282963029cbe0a867b8edaf7b64 100644 --- a/crates/workspace/src/persistence.rs +++ b/crates/workspace/src/persistence.rs @@ -2556,7 +2556,11 @@ mod tests { // --- Remove the second workspace (index 1) --- multi_workspace.update_in(cx, |mw, window, cx| { - let ws = mw.workspaces()[1].clone(); + let ws = mw + .workspaces() + .nth(1) + .expect("no workspace at index 1") + .clone(); mw.remove(&ws, window, cx); }); @@ -4239,7 +4243,11 @@ mod tests { // Remove workspace at index 1 (the second workspace). multi_workspace.update_in(cx, |mw, window, cx| { - let ws = mw.workspaces()[1].clone(); + let ws = mw + .workspaces() + .nth(1) + .expect("no workspace at index 1") + .clone(); mw.remove(&ws, window, cx); }); @@ -4350,7 +4358,11 @@ mod tests { // Remove workspace2 (index 1). multi_workspace.update_in(cx, |mw, window, cx| { - let ws = mw.workspaces()[1].clone(); + let ws = mw + .workspaces() + .nth(1) + .expect("no workspace at index 1") + .clone(); mw.remove(&ws, window, cx); }); @@ -4434,7 +4446,11 @@ mod tests { // Remove workspace2 — this pushes a task to pending_removal_tasks. multi_workspace.update_in(cx, |mw, window, cx| { - let ws = mw.workspaces()[1].clone(); + let ws = mw + .workspaces() + .nth(1) + .expect("no workspace at index 1") + .clone(); mw.remove(&ws, window, cx); }); @@ -4443,7 +4459,6 @@ mod tests { let all_tasks = multi_workspace.update_in(cx, |mw, window, cx| { let mut tasks: Vec> = mw .workspaces() - .iter() .map(|workspace| { workspace.update(cx, |workspace, cx| { workspace.flush_serialization(window, cx) diff --git a/crates/workspace/src/workspace.rs b/crates/workspace/src/workspace.rs index 460eba45d9faf70a16749a84aa4679a75a85a4b0..5b1cdc38642ff83411f91343545d5e43dc1db90a 100644 --- a/crates/workspace/src/workspace.rs +++ b/crates/workspace/src/workspace.rs @@ -8718,12 +8718,10 @@ pub async fn restore_multiworkspace( if let Some(target_id) = state.active_workspace_id { window_handle .update(cx, |multi_workspace, window, cx| { - let target_index = multi_workspace + let target_workspace = multi_workspace .workspaces() - .iter() - .position(|ws| ws.read(cx).database_id() == Some(target_id)); - let index = target_index.unwrap_or(0); - if let Some(workspace) = multi_workspace.workspaces().get(index).cloned() { + .find(|ws| ws.read(cx).database_id() == Some(target_id)); + if let Some(workspace) = target_workspace { multi_workspace.activate(workspace, window, cx); } }) @@ -8731,7 +8729,8 @@ pub async fn restore_multiworkspace( } else { window_handle .update(cx, |multi_workspace, window, cx| { - if let Some(workspace) = multi_workspace.workspaces().first().cloned() { + let first_workspace = multi_workspace.workspaces().next(); + if let Some(workspace) = first_workspace { multi_workspace.activate(workspace, window, cx); } }) @@ -9136,7 +9135,7 @@ pub fn workspace_windows_for_location( }; multi_workspace.read(cx).is_ok_and(|multi_workspace| { - multi_workspace.workspaces().iter().any(|workspace| { + multi_workspace.workspaces().any(|workspace| { match workspace.read(cx).workspace_location(cx) { WorkspaceLocation::Location(location, _) => { match (&location, serialized_location) { @@ -10783,7 +10782,11 @@ mod tests { // Activate workspace A multi_workspace_handle .update(cx, |mw, window, cx| { - let workspace = mw.workspaces()[0].clone(); + let workspace = mw + .workspaces() + .nth(0) + .expect("no workspace at index 0") + .clone(); mw.activate(workspace, window, cx); }) .unwrap(); @@ -10805,7 +10808,11 @@ mod tests { // Verify workspace A is active multi_workspace_handle .read_with(cx, |mw, _| { - assert_eq!(mw.active_workspace_index(), 0); + assert_eq!( + mw.workspaces() + .position(|workspace| workspace == mw.active_workspace()), + Some(0) + ); }) .unwrap(); @@ -10821,8 +10828,9 @@ mod tests { multi_workspace_handle .read_with(cx, |mw, _| { assert_eq!( - mw.active_workspace_index(), - 1, + mw.workspaces() + .position(|workspace| workspace == mw.active_workspace()), + Some(1), "workspace B should be activated when it prompts" ); }) @@ -14553,7 +14561,7 @@ mod tests { // Switch to workspace A multi_workspace_handle .update(cx, |mw, window, cx| { - let workspace = mw.workspaces()[0].clone(); + let workspace = mw.workspaces().next().expect("no workspace").clone(); mw.activate(workspace, window, cx); }) .unwrap(); @@ -14599,7 +14607,11 @@ mod tests { // Switch to workspace B multi_workspace_handle .update(cx, |mw, window, cx| { - let workspace = mw.workspaces()[1].clone(); + let workspace = mw + .workspaces() + .nth(1) + .expect("no workspace at index 1") + .clone(); mw.activate(workspace, window, cx); }) .unwrap(); @@ -14608,7 +14620,11 @@ mod tests { // Switch back to workspace A multi_workspace_handle .update(cx, |mw, window, cx| { - let workspace = mw.workspaces()[0].clone(); + let workspace = mw + .workspaces() + .nth(0) + .expect("no workspace at index 0") + .clone(); mw.activate(workspace, window, cx); }) .unwrap(); diff --git a/crates/zed/src/visual_test_runner.rs b/crates/zed/src/visual_test_runner.rs index 7e081c15a564cb996f176345ee3330f00ee6b6f3..7f81bf2a58ae6749ccef2a744a939c7ec1bbfd68 100644 --- a/crates/zed/src/visual_test_runner.rs +++ b/crates/zed/src/visual_test_runner.rs @@ -73,7 +73,8 @@ fn main() { // Create test files in the real filesystem create_test_files(&project_path); - let test_result = std::panic::catch_unwind(|| run_visual_tests(project_path, update_baseline)); + let test_result = + std::panic::catch_unwind(|| run_visual_tests(project_path.into(), update_baseline)); // Note: We don't delete temp_path here because background worktree tasks may still // be running. The directory will be cleaned up when the process exits or by the OS. diff --git a/crates/zed/src/zed.rs b/crates/zed/src/zed.rs index fbebb37985c2ebd76a63db5b4b807a8a7e0203ce..3efd0b6dfe70f6702c606d6008799f3f218bfa5a 100644 --- a/crates/zed/src/zed.rs +++ b/crates/zed/src/zed.rs @@ -1521,7 +1521,7 @@ fn quit(_: &Quit, cx: &mut App) { let window = *window; let workspaces = window .update(cx, |multi_workspace, _, _| { - multi_workspace.workspaces().to_vec() + multi_workspace.workspaces().collect::>() }) .log_err(); @@ -2455,7 +2455,6 @@ mod tests { .update(cx, |multi_workspace, window, cx| { let mut tasks = multi_workspace .workspaces() - .iter() .map(|workspace| { workspace.update(cx, |workspace, cx| { workspace.flush_serialization(window, cx) @@ -5529,7 +5528,7 @@ mod tests { let workspace1 = window .read_with(cx, |multi_workspace, _| { - multi_workspace.workspaces()[0].clone() + multi_workspace.workspaces().nth(0).unwrap().clone() }) .unwrap(); @@ -5539,7 +5538,8 @@ mod tests { multi_workspace.activate(workspace3.clone(), window, cx); // Switch back to workspace1 for test setup multi_workspace.activate(workspace1, window, cx); - assert_eq!(multi_workspace.active_workspace_index(), 0); + let workspaces: Vec<_> = multi_workspace.workspaces().collect(); + assert_eq!(multi_workspace.workspace(), &workspaces[0]); }) .unwrap(); @@ -5548,8 +5548,9 @@ mod tests { // Verify setup: 3 workspaces, workspace 0 active, still 1 window window .read_with(cx, |multi_workspace, _| { - assert_eq!(multi_workspace.workspaces().len(), 3); - assert_eq!(multi_workspace.active_workspace_index(), 0); + let workspaces: Vec<_> = multi_workspace.workspaces().collect(); + assert_eq!(workspaces.len(), 3); + assert_eq!(multi_workspace.workspace(), &workspaces[0]); }) .unwrap(); assert_eq!(cx.windows().len(), 1); @@ -5571,9 +5572,10 @@ mod tests { // Verify workspace 2 is active and file opened there window .read_with(cx, |multi_workspace, cx| { + let workspaces: Vec<_> = multi_workspace.workspaces().collect(); assert_eq!( - multi_workspace.active_workspace_index(), - 2, + multi_workspace.workspace(), + &workspaces[2], "Should have switched to workspace 3 which contains /dir3" ); let active_item = multi_workspace @@ -5605,9 +5607,10 @@ mod tests { // Verify workspace 1 is active and file opened there window .read_with(cx, |multi_workspace, cx| { + let workspaces: Vec<_> = multi_workspace.workspaces().collect(); assert_eq!( - multi_workspace.active_workspace_index(), - 1, + multi_workspace.workspace(), + &workspaces[1], "Should have switched to workspace 2 which contains /dir2" ); let active_item = multi_workspace @@ -5654,9 +5657,10 @@ mod tests { // Verify workspace 0 is active and file opened there window .read_with(cx, |multi_workspace, cx| { + let workspaces: Vec<_> = multi_workspace.workspaces().collect(); assert_eq!( - multi_workspace.active_workspace_index(), - 0, + multi_workspace.workspace(), + &workspaces[0], "Should have switched back to workspace 0 which contains /dir1" ); let active_item = multi_workspace @@ -5766,7 +5770,8 @@ mod tests { // Verify workspace1_1 is active window1 .read_with(cx, |multi_workspace, _| { - assert_eq!(multi_workspace.active_workspace_index(), 0); + let workspaces: Vec<_> = multi_workspace.workspaces().collect(); + assert_eq!(multi_workspace.workspace(), &workspaces[0]); }) .unwrap(); @@ -5832,7 +5837,12 @@ mod tests { // Verify workspace1_1 is still active (not workspace1_2 with dirty item) window1 .read_with(cx, |multi_workspace, _| { - assert_eq!(multi_workspace.active_workspace_index(), 0); + assert_eq!( + multi_workspace + .workspaces() + .position(|workspace| workspace == multi_workspace.active_workspace()), + Some(0) + ); }) .unwrap(); @@ -5843,8 +5853,10 @@ mod tests { window1 .read_with(cx, |multi_workspace, _| { assert_eq!( - multi_workspace.active_workspace_index(), - 1, + multi_workspace + .workspaces() + .position(|workspace| workspace == multi_workspace.active_workspace()), + Some(1), "Case 2: Non-active workspace should be activated when it has dirty item" ); }) @@ -6027,7 +6039,11 @@ mod tests { // still be active rather than whichever workspace happened to restore last. window_a .update(cx, |multi_workspace, window, cx| { - let workspace = multi_workspace.workspaces()[0].clone(); + let workspace = multi_workspace + .workspaces() + .nth(0) + .expect("no workspace at index 0") + .clone(); multi_workspace.activate(workspace, window, cx); }) .unwrap(); @@ -6124,7 +6140,9 @@ mod tests { .iter() .map(|window| { window - .read_with(cx, |multi_workspace, _| multi_workspace.workspaces().len()) + .read_with(cx, |multi_workspace, _| { + multi_workspace.workspaces().count() + }) .unwrap() }) .collect(); @@ -6147,7 +6165,6 @@ mod tests { .read_with(cx, |multi_workspace, cx| { multi_workspace .workspaces() - .iter() .map(|ws| ws.read(cx).root_paths(cx)) .collect() }) @@ -6184,7 +6201,7 @@ mod tests { let active = multi_workspace.workspace(); ( active.read(cx).root_paths(cx), - multi_workspace.workspaces().len(), + multi_workspace.workspaces().count(), ) }) .unwrap();