settings_ui: Fix editable project settings not being updated when a new workspace is created (#47992)
The bug occurred because `cx.observe_new::<Workspace>` 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
This commit is contained in:
Anthony Eid authored and GitHub committed
1 parent
1238939bff
commit
df052bf818
1 file changed
+362
-13
@@ -1544,19 +1544,32 @@ impl SettingsWindow {
|
||||
})
|
||||
.detach();
|
||||
|
||||
cx.observe_new::<Workspace>(move |_, window, cx| {
|
||||
let workspace = cx.entity();
|
||||
let Some(window) = window else {
|
||||
return;
|
||||
};
|
||||
let handle = window.window_handle();
|
||||
cx.observe_new::<Workspace>(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<Workspace>>,
|
||||
cx: &App,
|
||||
) -> impl Iterator<Item = Entity<project::Project>> {
|
||||
) -> impl Iterator<Item = Entity<Project>> {
|
||||
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::<Workspace>().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::<Workspace>().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::<Workspace>().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)]
|
||||
|
||||
Reference in new issue
Block a user