From fe653a77aef9cda5521958ad5b58d3d33c6aff20 Mon Sep 17 00:00:00 2001 From: Anthony Eid <56899983+Anthony-Eid@users.noreply.github.com> Date: Fri, 30 Jan 2026 17:21:15 -0500 Subject: [PATCH] settings_ui: Fix editable project settings not being updated when a new workspace is created (#47992) The bug occurred because `cx.observe_new::` would pass a mutable `Window` that was also the new workspace's window. Later, `fetch_files` would fail to read the newly created workspace from `WorkspaceStore` because its window already had a mutable reference to it. The fix always passes the Settings UI's window handle to the `fetch_files` function, and uses a `settings_window_handle.defer` callback to call `fetch_files` after the newly created workspace's window no longer has any mutable references to it. Release Notes: - settings_ui: Fixed editable project settings not being updated when a new workspace is created --- crates/settings_ui/src/settings_ui.rs | 375 +++++++++++++++++++++++++- 1 file changed, 362 insertions(+), 13 deletions(-) diff --git a/crates/settings_ui/src/settings_ui.rs b/crates/settings_ui/src/settings_ui.rs index 9d6ec1fa4aa373bb808a00bf563ab15150fcf4ce..14367749879d95de1195bce4c22b7316528da9c3 100644 --- a/crates/settings_ui/src/settings_ui.rs +++ b/crates/settings_ui/src/settings_ui.rs @@ -1544,19 +1544,32 @@ impl SettingsWindow { }) .detach(); - cx.observe_new::(move |_, window, cx| { - let workspace = cx.entity(); - let Some(window) = window else { - return; - }; + let handle = window.window_handle(); + cx.observe_new::(move |workspace, _, cx| { + let project = workspace.project().clone(); + let this_weak = this_weak.clone(); - this_weak - .update(cx, |this, cx| { - this.fetch_files(window, cx); - cx.observe_release_in(&workspace, window, |this, _, window, cx| { - this.fetch_files(window, cx) - }) - .detach(); + // We defer on the settings window (via `handle`) rather than using + // the workspace's window from observe_new. When window.defer() runs + // its callback, it calls handle.update() which temporarily removes + // that window from cx.windows. If we deferred on the workspace's + // window, then when fetch_files() tries to read ALL workspaces from + // the store (including the newly created one), it would fail with + // "window not found" because that workspace's window would be + // temporarily removed from cx.windows for the duration of our callback. + handle + .update(cx, move |_, window, cx| { + window.defer(cx, move |window, cx| { + this_weak + .update(cx, |this, cx| { + this.fetch_files(window, cx); + cx.observe_release_in(&project, window, |this, _, window, cx| { + this.fetch_files(window, cx) + }) + .detach(); + }) + .ok(); + }); }) .ok(); }) @@ -3587,7 +3600,8 @@ impl Render for SettingsWindow { fn all_projects( window: Option<&WindowHandle>, cx: &App, -) -> impl Iterator> { +) -> impl Iterator> { + let mut seen_project_ids = std::collections::HashSet::new(); workspace::AppState::global(cx) .upgrade() .map(|app_state| { @@ -3600,6 +3614,7 @@ fn all_projects( .chain( window.and_then(|workspace| Some(workspace.read(cx).ok()?.project().clone())), ) + .filter(move |project| seen_project_ids.insert(project.entity_id())) }) .into_iter() .flatten() @@ -4492,6 +4507,340 @@ pub mod test { > Appearance & Behavior " ); + + #[gpui::test] + async fn test_settings_window_shows_worktrees_from_multiple_workspaces( + cx: &mut gpui::TestAppContext, + ) { + use project::Project; + use serde_json::json; + + cx.update(|cx| { + register_settings(cx); + }); + + let app_state = cx.update(|cx| { + let app_state = AppState::test(cx); + AppState::set_global(Arc::downgrade(&app_state), cx); + app_state + }); + + let fake_fs = app_state.fs.as_fake(); + + fake_fs + .insert_tree( + "/workspace1", + json!({ + "worktree_a": { + "file1.rs": "fn main() {}" + }, + "worktree_b": { + "file2.rs": "fn test() {}" + } + }), + ) + .await; + + fake_fs + .insert_tree( + "/workspace2", + json!({ + "worktree_c": { + "file3.rs": "fn foo() {}" + } + }), + ) + .await; + + let project1 = cx.update(|cx| { + Project::local( + app_state.client.clone(), + app_state.node_runtime.clone(), + app_state.user_store.clone(), + app_state.languages.clone(), + app_state.fs.clone(), + None, + project::LocalProjectFlags::default(), + cx, + ) + }); + + project1 + .update(cx, |project, cx| { + project.find_or_create_worktree("/workspace1/worktree_a", true, cx) + }) + .await + .expect("Failed to create worktree_a"); + project1 + .update(cx, |project, cx| { + project.find_or_create_worktree("/workspace1/worktree_b", true, cx) + }) + .await + .expect("Failed to create worktree_b"); + + let project2 = cx.update(|cx| { + Project::local( + app_state.client.clone(), + app_state.node_runtime.clone(), + app_state.user_store.clone(), + app_state.languages.clone(), + app_state.fs.clone(), + None, + project::LocalProjectFlags::default(), + cx, + ) + }); + + project2 + .update(cx, |project, cx| { + project.find_or_create_worktree("/workspace2/worktree_c", true, cx) + }) + .await + .expect("Failed to create worktree_c"); + + let (_workspace1, cx) = cx.add_window_view(|window, cx| { + Workspace::new( + Default::default(), + project1.clone(), + app_state.clone(), + window, + cx, + ) + }); + + let _workspace1_handle = cx.window_handle().downcast::().unwrap(); + + let (_workspace2, cx) = cx.add_window_view(|window, cx| { + Workspace::new( + Default::default(), + project2.clone(), + app_state.clone(), + window, + cx, + ) + }); + + let workspace2_handle = cx.window_handle().downcast::().unwrap(); + + cx.run_until_parked(); + + let (settings_window, cx) = cx + .add_window_view(|window, cx| SettingsWindow::new(Some(workspace2_handle), window, cx)); + + cx.run_until_parked(); + + settings_window.read_with(cx, |settings_window, _| { + let worktree_names: Vec<_> = settings_window + .worktree_root_dirs + .values() + .cloned() + .collect(); + + assert!( + worktree_names.iter().any(|name| name == "worktree_a"), + "Should contain worktree_a from workspace1, but found: {:?}", + worktree_names + ); + assert!( + worktree_names.iter().any(|name| name == "worktree_b"), + "Should contain worktree_b from workspace1, but found: {:?}", + worktree_names + ); + assert!( + worktree_names.iter().any(|name| name == "worktree_c"), + "Should contain worktree_c from workspace2, but found: {:?}", + worktree_names + ); + + assert_eq!( + worktree_names.len(), + 3, + "Should have exactly 3 worktrees from both workspaces, but found: {:?}", + worktree_names + ); + + let project_files: Vec<_> = settings_window + .files + .iter() + .filter_map(|(f, _)| match f { + SettingsUiFile::Project((worktree_id, _)) => Some(*worktree_id), + _ => None, + }) + .collect(); + + let unique_project_files: std::collections::HashSet<_> = project_files.iter().collect(); + assert_eq!( + project_files.len(), + unique_project_files.len(), + "Should have no duplicate project files, but found duplicates. All files: {:?}", + project_files + ); + }); + } + + #[gpui::test] + async fn test_settings_window_updates_when_new_workspace_created( + cx: &mut gpui::TestAppContext, + ) { + use project::Project; + use serde_json::json; + + cx.update(|cx| { + register_settings(cx); + }); + + let app_state = cx.update(|cx| { + let app_state = AppState::test(cx); + AppState::set_global(Arc::downgrade(&app_state), cx); + app_state + }); + + let fake_fs = app_state.fs.as_fake(); + + fake_fs + .insert_tree( + "/workspace1", + json!({ + "worktree_a": { + "file1.rs": "fn main() {}" + } + }), + ) + .await; + + fake_fs + .insert_tree( + "/workspace2", + json!({ + "worktree_b": { + "file2.rs": "fn test() {}" + } + }), + ) + .await; + + let project1 = cx.update(|cx| { + Project::local( + app_state.client.clone(), + app_state.node_runtime.clone(), + app_state.user_store.clone(), + app_state.languages.clone(), + app_state.fs.clone(), + None, + project::LocalProjectFlags::default(), + cx, + ) + }); + + project1 + .update(cx, |project, cx| { + project.find_or_create_worktree("/workspace1/worktree_a", true, cx) + }) + .await + .expect("Failed to create worktree_a"); + + let (_workspace1, cx) = cx.add_window_view(|window, cx| { + Workspace::new( + Default::default(), + project1.clone(), + app_state.clone(), + window, + cx, + ) + }); + + let workspace1_handle = cx.window_handle().downcast::().unwrap(); + + cx.run_until_parked(); + + let (settings_window, cx) = cx + .add_window_view(|window, cx| SettingsWindow::new(Some(workspace1_handle), window, cx)); + + cx.run_until_parked(); + + settings_window.read_with(cx, |settings_window, _| { + assert_eq!( + settings_window.worktree_root_dirs.len(), + 1, + "Should have 1 worktree initially" + ); + }); + + let project2 = cx.update(|_, cx| { + Project::local( + app_state.client.clone(), + app_state.node_runtime.clone(), + app_state.user_store.clone(), + app_state.languages.clone(), + app_state.fs.clone(), + None, + project::LocalProjectFlags::default(), + cx, + ) + }); + + project2 + .update(&mut cx.cx, |project, cx| { + project.find_or_create_worktree("/workspace2/worktree_b", true, cx) + }) + .await + .expect("Failed to create worktree_b"); + + let (_workspace2, cx) = cx.add_window_view(|window, cx| { + Workspace::new( + Default::default(), + project2.clone(), + app_state.clone(), + window, + cx, + ) + }); + + cx.run_until_parked(); + + settings_window.read_with(cx, |settings_window, _| { + let worktree_names: Vec<_> = settings_window + .worktree_root_dirs + .values() + .cloned() + .collect(); + + assert!( + worktree_names.iter().any(|name| name == "worktree_a"), + "Should contain worktree_a, but found: {:?}", + worktree_names + ); + assert!( + worktree_names.iter().any(|name| name == "worktree_b"), + "Should contain worktree_b from newly created workspace, but found: {:?}", + worktree_names + ); + + assert_eq!( + worktree_names.len(), + 2, + "Should have 2 worktrees after new workspace created, but found: {:?}", + worktree_names + ); + + let project_files: Vec<_> = settings_window + .files + .iter() + .filter_map(|(f, _)| match f { + SettingsUiFile::Project((worktree_id, _)) => Some(*worktree_id), + _ => None, + }) + .collect(); + + let unique_project_files: std::collections::HashSet<_> = project_files.iter().collect(); + assert_eq!( + project_files.len(), + unique_project_files.len(), + "Should have no duplicate project files, but found duplicates. All files: {:?}", + project_files + ); + }); + } } #[cfg(test)]