mirror of
https://github.com/zed-industries/zed.git
synced 2026-08-10 17:49:15 +00:00
workspace: Return the created workspace when opening a remote project (#62028)
Some checks are pending
Congratsbot / check-author (push) Waiting to run
Congratsbot / congrats (push) Blocked by required conditions
deploy_nightly_docs / deploy_docs (push) Waiting to run
run_tests / orchestrate (push) Waiting to run
run_tests / check_style (push) Waiting to run
run_tests / clippy_windows (push) Blocked by required conditions
run_tests / clippy_linux (push) Blocked by required conditions
run_tests / clippy_mac (push) Blocked by required conditions
run_tests / clippy_mac_x86_64 (push) Blocked by required conditions
run_tests / run_tests_windows (push) Blocked by required conditions
run_tests / run_tests_linux (push) Blocked by required conditions
run_tests / run_tests_mac (push) Blocked by required conditions
run_tests / miri_scheduler (push) Blocked by required conditions
run_tests / doctests (push) Blocked by required conditions
run_tests / check_workspace_binaries (push) Blocked by required conditions
run_tests / build_visual_tests_binary (push) Blocked by required conditions
run_tests / check_wasm (push) Blocked by required conditions
run_tests / check_dependencies (push) Blocked by required conditions
run_tests / check_docs (push) Blocked by required conditions
run_tests / check_licenses (push) Blocked by required conditions
run_tests / check_scripts (push) Blocked by required conditions
run_tests / check_postgres_and_protobuf_migrations (push) Blocked by required conditions
run_tests / extension_tests (push) Blocked by required conditions
run_tests / tests_pass (push) Blocked by required conditions
Some checks are pending
Congratsbot / check-author (push) Waiting to run
Congratsbot / congrats (push) Blocked by required conditions
deploy_nightly_docs / deploy_docs (push) Waiting to run
run_tests / orchestrate (push) Waiting to run
run_tests / check_style (push) Waiting to run
run_tests / clippy_windows (push) Blocked by required conditions
run_tests / clippy_linux (push) Blocked by required conditions
run_tests / clippy_mac (push) Blocked by required conditions
run_tests / clippy_mac_x86_64 (push) Blocked by required conditions
run_tests / run_tests_windows (push) Blocked by required conditions
run_tests / run_tests_linux (push) Blocked by required conditions
run_tests / run_tests_mac (push) Blocked by required conditions
run_tests / miri_scheduler (push) Blocked by required conditions
run_tests / doctests (push) Blocked by required conditions
run_tests / check_workspace_binaries (push) Blocked by required conditions
run_tests / build_visual_tests_binary (push) Blocked by required conditions
run_tests / check_wasm (push) Blocked by required conditions
run_tests / check_dependencies (push) Blocked by required conditions
run_tests / check_docs (push) Blocked by required conditions
run_tests / check_licenses (push) Blocked by required conditions
run_tests / check_scripts (push) Blocked by required conditions
run_tests / check_postgres_and_protobuf_migrations (push) Blocked by required conditions
run_tests / extension_tests (push) Blocked by required conditions
run_tests / tests_pass (push) Blocked by required conditions
# Objective Fix the `fallback workspace must not be one of the workspaces being removed` panic. Fixes FR-148 Fixes ZED-AKR Fixes ZED-97M # Solution The remote path of `find_or_create_workspace` opened a project, then returned the window's *active* workspace instead of the one it created. The open awaits toolchain loading and item restoration, and a save prompt from a concurrent workspace removal can re-activate a workspace being removed during that wait; the stale return value then trips the assert in `MultiWorkspace::remove`. Return the created workspace from `open_remote_project_inner` and use it at the call sites instead of re-reading the active workspace after the await. # Testing Added `test_find_or_create_workspace_returns_the_created_remote_workspace`: opens a mock remote project and re-activates the previous workspace mid-open (standing in for the save prompt), then asserts the open returns the workspace it created. Verified it fails against the pre-fix behavior. ## Self-Review Checklist: - [x] I've reviewed my own diff for quality, security, and reliability - [x] Unsafe blocks (if any) have justifying comments - [x] The content adheres to Zed's UI standards ([UX/UI](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist) and [icon](https://github.com/zed-industries/zed/blob/main/crates/icons/README.md) guidelines) - [x] Tests cover the new/changed behavior - [x] Performance impact has been considered and is acceptable --- Release Notes: - Fixed a crash when a workspace was removed while a remote project was opening in the same window.
This commit is contained in:
parent
b5764581d2
commit
4aad57fd1f
5 changed files with 116 additions and 29 deletions
|
|
@ -246,6 +246,7 @@ pub async fn open_remote_project(
|
|||
(window, workspace)
|
||||
};
|
||||
|
||||
let mut remote_workspace = None;
|
||||
loop {
|
||||
let (cancel_tx, mut cancel_rx) = oneshot::channel();
|
||||
let delegate = window.update(cx, {
|
||||
|
|
@ -348,7 +349,7 @@ pub async fn open_remote_project(
|
|||
let (paths, paths_with_positions) =
|
||||
determine_paths_with_positions(&remote_connection, paths.clone()).await;
|
||||
|
||||
let opened_items = cx
|
||||
let opened = cx
|
||||
.update(|cx| {
|
||||
workspace::open_remote_project_with_new_connection(
|
||||
window,
|
||||
|
|
@ -368,7 +369,7 @@ pub async fn open_remote_project(
|
|||
}
|
||||
});
|
||||
|
||||
match opened_items {
|
||||
match opened {
|
||||
Err(e) => {
|
||||
log::error!("Failed to open project: {e:#}");
|
||||
let response = window
|
||||
|
|
@ -412,7 +413,8 @@ pub async fn open_remote_project(
|
|||
});
|
||||
}
|
||||
|
||||
Ok(items) => {
|
||||
Ok((workspace, items)) => {
|
||||
remote_workspace = workspace;
|
||||
navigate_to_positions(&window, items, &paths_with_positions, cx);
|
||||
}
|
||||
}
|
||||
|
|
@ -420,22 +422,15 @@ pub async fn open_remote_project(
|
|||
break;
|
||||
}
|
||||
|
||||
// Register the remote client with extensions. We use `multi_workspace.workspace()` here
|
||||
// (not `initial_workspace`) because `open_remote_project_inner` activated the new remote
|
||||
// workspace, so the active workspace is now the one with the remote project.
|
||||
window
|
||||
.update(cx, |multi_workspace: &mut MultiWorkspace, _, cx| {
|
||||
let workspace = multi_workspace.workspace().clone();
|
||||
workspace.update(cx, |workspace, cx| {
|
||||
if let Some(client) = workspace.project().read(cx).remote_client() {
|
||||
if let Some(extension_store) = ExtensionStore::try_global(cx) {
|
||||
extension_store
|
||||
.update(cx, |store, cx| store.register_remote_client(client, cx));
|
||||
}
|
||||
}
|
||||
});
|
||||
})
|
||||
.ok();
|
||||
if let Some(remote_workspace) = remote_workspace {
|
||||
remote_workspace.update(cx, |workspace, cx| {
|
||||
if let Some(client) = workspace.project().read(cx).remote_client()
|
||||
&& let Some(extension_store) = ExtensionStore::try_global(cx)
|
||||
{
|
||||
extension_store.update(cx, |store, cx| store.register_remote_client(client, cx));
|
||||
}
|
||||
});
|
||||
}
|
||||
Ok(window)
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -506,7 +506,8 @@ impl ProjectPicker {
|
|||
connection, project, paths, app_state, window, None, None, cx,
|
||||
)
|
||||
.await
|
||||
.log_err();
|
||||
.log_err()
|
||||
.map(|(_workspace, items)| items);
|
||||
|
||||
if let Some(items) = items {
|
||||
for (item, path) in items.into_iter().zip(paths_with_positions) {
|
||||
|
|
|
|||
|
|
@ -14912,3 +14912,91 @@ fn test_split_leading_icon_char() {
|
|||
assert_eq!(trimmed.as_ref(), "abc");
|
||||
assert_eq!(positions, vec![0, 1]);
|
||||
}
|
||||
|
||||
#[gpui::test]
|
||||
async fn test_find_or_create_workspace_returns_the_created_remote_workspace(
|
||||
cx: &mut TestAppContext,
|
||||
server_cx: &mut TestAppContext,
|
||||
) {
|
||||
let local_project = init_test_project("/local", cx).await;
|
||||
cx.update(|cx| {
|
||||
release_channel::init(semver::Version::new(0, 0, 0), cx);
|
||||
});
|
||||
server_cx.update(|cx| {
|
||||
release_channel::init(semver::Version::new(0, 0, 0), cx);
|
||||
});
|
||||
let (multi_workspace, cx) =
|
||||
cx.add_window_view(|window, cx| MultiWorkspace::test_new(local_project, window, cx));
|
||||
let local_workspace = multi_workspace.read_with(cx, |mw, _| mw.workspace().clone());
|
||||
|
||||
let server_fs = FakeFs::new(server_cx.executor());
|
||||
server_fs
|
||||
.insert_tree("/remote-project", serde_json::json!({ "src": {} }))
|
||||
.await;
|
||||
let (opts, server_session, _) = remote::RemoteClient::fake_server(cx, server_cx);
|
||||
server_cx.update(remote_server::HeadlessProject::init);
|
||||
let server_executor = server_cx.executor();
|
||||
let _headless = server_cx.new(|cx| {
|
||||
remote_server::HeadlessProject::new(
|
||||
remote_server::HeadlessAppState {
|
||||
session: server_session,
|
||||
fs: server_fs.clone(),
|
||||
http_client: Arc::new(http_client::BlockedHttpClient),
|
||||
node_runtime: node_runtime::NodeRuntime::unavailable(),
|
||||
languages: Arc::new(language::LanguageRegistry::new(server_executor)),
|
||||
extension_host_proxy: Arc::new(extension::ExtensionHostProxy::new()),
|
||||
startup_time: std::time::Instant::now(),
|
||||
},
|
||||
false,
|
||||
cx,
|
||||
)
|
||||
});
|
||||
let remote_client = remote::RemoteClient::connect_mock(opts.clone(), cx).await;
|
||||
|
||||
// Stand in for the save prompt from a concurrent workspace removal: as
|
||||
// soon as the remote workspace is activated mid-open, activate the local
|
||||
// workspace again. The open must still return the workspace it created,
|
||||
// not whichever workspace is active once it finishes.
|
||||
multi_workspace.update_in(cx, |_, window, cx| {
|
||||
let local_workspace = local_workspace.clone();
|
||||
cx.subscribe_in(&cx.entity(), window, move |this, _, event, window, cx| {
|
||||
if matches!(event, MultiWorkspaceEvent::WorkspaceAdded(_)) {
|
||||
this.activate(local_workspace.clone(), None, window, cx);
|
||||
}
|
||||
})
|
||||
.detach();
|
||||
});
|
||||
|
||||
let created = multi_workspace
|
||||
.update_in(cx, |mw, window, cx| {
|
||||
let key = ProjectGroupKey::new(
|
||||
Some(opts.clone()),
|
||||
PathList::new(&[PathBuf::from("/remote-project")]),
|
||||
);
|
||||
mw.find_or_create_workspace(
|
||||
PathList::new(&[PathBuf::from("/remote-project")]),
|
||||
Some(opts),
|
||||
Some(key),
|
||||
move |_, _, _| Task::ready(Ok(Some(remote_client))),
|
||||
&[],
|
||||
None,
|
||||
workspace::OpenMode::Activate,
|
||||
window,
|
||||
cx,
|
||||
)
|
||||
})
|
||||
.await
|
||||
.expect("opening the remote project should succeed");
|
||||
cx.run_until_parked();
|
||||
|
||||
assert_eq!(
|
||||
created.read_with(cx, |workspace, cx| PathList::new(&workspace.root_paths(cx))),
|
||||
PathList::new(&[PathBuf::from("/remote-project")]),
|
||||
"the returned workspace should be the remote workspace that was created"
|
||||
);
|
||||
assert_eq!(
|
||||
multi_workspace.read_with(cx, |mw, _| mw.workspace().clone()),
|
||||
local_workspace,
|
||||
"the local workspace should have re-activated during the open"
|
||||
);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1321,7 +1321,7 @@ impl MultiWorkspace {
|
|||
let window_handle =
|
||||
window_handle.ok_or_else(|| anyhow::anyhow!("Window is not a MultiWorkspace"))?;
|
||||
|
||||
open_remote_project_with_existing_connection(
|
||||
let (workspace, _items) = open_remote_project_with_existing_connection(
|
||||
connection_options,
|
||||
new_project,
|
||||
effective_paths_vec,
|
||||
|
|
@ -1334,7 +1334,6 @@ impl MultiWorkspace {
|
|||
.await?;
|
||||
|
||||
window_handle.update(cx, |multi_workspace, window, cx| {
|
||||
let workspace = multi_workspace.workspace().clone();
|
||||
multi_workspace.add(workspace.clone(), window, cx);
|
||||
workspace
|
||||
})
|
||||
|
|
|
|||
|
|
@ -10739,7 +10739,7 @@ pub fn open_remote_project_with_new_connection(
|
|||
app_state: Arc<AppState>,
|
||||
paths: Vec<PathBuf>,
|
||||
cx: &mut App,
|
||||
) -> Task<Result<Vec<Option<Box<dyn ItemHandle>>>>> {
|
||||
) -> Task<Result<(Option<Entity<Workspace>>, Vec<Option<Box<dyn ItemHandle>>>)>> {
|
||||
cx.spawn(async move |cx| {
|
||||
let (workspace_id, serialized_workspace) =
|
||||
deserialize_remote_project(remote_connection.connection_options(), paths.clone(), cx)
|
||||
|
|
@ -10758,7 +10758,7 @@ pub fn open_remote_project_with_new_connection(
|
|||
.await?
|
||||
{
|
||||
Some(result) => result,
|
||||
None => return Ok(Vec::new()),
|
||||
None => return Ok((None, Vec::new())),
|
||||
};
|
||||
|
||||
let project = cx.update(|cx| {
|
||||
|
|
@ -10774,7 +10774,7 @@ pub fn open_remote_project_with_new_connection(
|
|||
)
|
||||
});
|
||||
|
||||
open_remote_project_inner(
|
||||
let (workspace, items) = open_remote_project_inner(
|
||||
project,
|
||||
paths,
|
||||
workspace_id,
|
||||
|
|
@ -10785,7 +10785,8 @@ pub fn open_remote_project_with_new_connection(
|
|||
None,
|
||||
cx,
|
||||
)
|
||||
.await
|
||||
.await?;
|
||||
Ok((Some(workspace), items))
|
||||
})
|
||||
}
|
||||
|
||||
|
|
@ -10798,7 +10799,7 @@ pub fn open_remote_project_with_existing_connection(
|
|||
provisional_project_group_key: Option<ProjectGroupKey>,
|
||||
source_workspace: Option<WeakEntity<Workspace>>,
|
||||
cx: &mut AsyncApp,
|
||||
) -> Task<Result<Vec<Option<Box<dyn ItemHandle>>>>> {
|
||||
) -> Task<Result<(Entity<Workspace>, Vec<Option<Box<dyn ItemHandle>>>)>> {
|
||||
cx.spawn(async move |cx| {
|
||||
let (workspace_id, serialized_workspace) =
|
||||
deserialize_remote_project(connection_options.clone(), paths.clone(), cx).await?;
|
||||
|
|
@ -10828,7 +10829,7 @@ async fn open_remote_project_inner(
|
|||
provisional_project_group_key: Option<ProjectGroupKey>,
|
||||
source_workspace: Option<WeakEntity<Workspace>>,
|
||||
cx: &mut AsyncApp,
|
||||
) -> Result<Vec<Option<Box<dyn ItemHandle>>>> {
|
||||
) -> Result<(Entity<Workspace>, Vec<Option<Box<dyn ItemHandle>>>)> {
|
||||
let mut project_paths_to_open = vec![];
|
||||
let mut project_path_errors = vec![];
|
||||
|
||||
|
|
@ -10927,7 +10928,10 @@ async fn open_remote_project_inner(
|
|||
}
|
||||
});
|
||||
|
||||
Ok(items.into_iter().map(|item| item?.ok()).collect())
|
||||
Ok((
|
||||
workspace,
|
||||
items.into_iter().map(|item| item?.ok()).collect(),
|
||||
))
|
||||
}
|
||||
|
||||
fn deserialize_remote_project(
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue