mirror of
https://github.com/zed-industries/zed.git
synced 2026-06-01 03:14:56 +07:00
cli: Respect --new when opening URLs from the CLI (#54408)
`zed --new ssh://host/path/to/file` only created a new window on the first call. Subsequent invocations silently reused the existing SSH workspace for the same host, which then tried to open the new path against the existing worktree and surfaced a `DevServerProjectPathDoesNotExist` popup when the path didn't belong to any open worktree. The CLI correctly translated `--new` to `OpenBehavior::AlwaysNew`, but the URL branch of `handle_cli_connection` dropped the `open_behavior`. `handle_open_request` then called `open_remote_project` (and `open_paths_with_positions` for `file://`) with `OpenOptions::default()`, which is `WorkspaceMatching::MatchExact`, so any existing SSH window for the same host won the match. The issue applies to `file://` URLs and `-a` / `-e` / `--reuse` for URL-shaped arguments in general; `--new` was just the most visible symptom. The fix is making sure the translation of `OpenRequest` is uniform across plain paths and URL-shaped arguments. Closes #52679. Release Notes: - Fixed `zed --new ssh://host/path` reusing an existing SSH window instead of opening a new one. This also applied to other URL-shaped path arguments.
This commit is contained in:
parent
068d64edd6
commit
0f6ebdd269
2 changed files with 155 additions and 39 deletions
|
|
@ -909,6 +909,7 @@ fn main() {
|
|||
wsl,
|
||||
diff_all: diff_all_mode,
|
||||
dev_container: args.dev_container,
|
||||
..Default::default()
|
||||
})
|
||||
}
|
||||
|
||||
|
|
@ -1264,6 +1265,11 @@ fn handle_open_request(request: OpenRequest, app_state: Arc<AppState>, cx: &mut
|
|||
});
|
||||
}
|
||||
OpenRequestKind::GitCommit { sha } => {
|
||||
let base_open_options = zed::open_options_for_request(
|
||||
request.open_behavior,
|
||||
&workspace::SerializedWorkspaceLocation::Local,
|
||||
cx,
|
||||
);
|
||||
cx.spawn(async move |cx| {
|
||||
let paths_with_position =
|
||||
derive_paths_with_position(app_state.fs.as_ref(), request.open_paths).await;
|
||||
|
|
@ -1272,7 +1278,7 @@ fn handle_open_request(request: OpenRequest, app_state: Arc<AppState>, cx: &mut
|
|||
&[],
|
||||
false,
|
||||
app_state,
|
||||
workspace::OpenOptions::default(),
|
||||
base_open_options,
|
||||
cx,
|
||||
)
|
||||
.await?;
|
||||
|
|
@ -1314,16 +1320,12 @@ fn handle_open_request(request: OpenRequest, app_state: Arc<AppState>, cx: &mut
|
|||
}
|
||||
|
||||
if let Some(connection_options) = request.remote_connection {
|
||||
let open_behavior = request.open_behavior;
|
||||
let location = workspace::SerializedWorkspaceLocation::Remote(connection_options.clone());
|
||||
let base_open_options = zed::open_options_for_request(open_behavior, &location, cx);
|
||||
cx.spawn(async move |cx| {
|
||||
let paths: Vec<PathBuf> = request.open_paths.into_iter().map(PathBuf::from).collect();
|
||||
open_remote_project(
|
||||
connection_options,
|
||||
paths,
|
||||
app_state,
|
||||
workspace::OpenOptions::default(),
|
||||
cx,
|
||||
)
|
||||
.await
|
||||
open_remote_project(connection_options, paths, app_state, base_open_options, cx).await
|
||||
})
|
||||
.detach_and_log_err(cx);
|
||||
return;
|
||||
|
|
@ -1333,6 +1335,11 @@ fn handle_open_request(request: OpenRequest, app_state: Arc<AppState>, cx: &mut
|
|||
let dev_container = request.dev_container;
|
||||
if !request.open_paths.is_empty() || !request.diff_paths.is_empty() {
|
||||
let app_state = app_state.clone();
|
||||
let base_open_options = zed::open_options_for_request(
|
||||
request.open_behavior,
|
||||
&workspace::SerializedWorkspaceLocation::Local,
|
||||
cx,
|
||||
);
|
||||
task = Some(cx.spawn(async move |cx| {
|
||||
let paths_with_position =
|
||||
derive_paths_with_position(app_state.fs.as_ref(), request.open_paths).await;
|
||||
|
|
@ -1343,7 +1350,7 @@ fn handle_open_request(request: OpenRequest, app_state: Arc<AppState>, cx: &mut
|
|||
app_state,
|
||||
workspace::OpenOptions {
|
||||
open_in_dev_container: dev_container,
|
||||
..Default::default()
|
||||
..base_open_options
|
||||
},
|
||||
cx,
|
||||
)
|
||||
|
|
|
|||
|
|
@ -42,6 +42,7 @@ pub struct OpenRequest {
|
|||
pub open_channel_notes: Vec<(u64, Option<String>)>,
|
||||
pub join_channel: Option<u64>,
|
||||
pub remote_connection: Option<RemoteConnectionOptions>,
|
||||
pub open_behavior: Option<cli::OpenBehavior>,
|
||||
}
|
||||
|
||||
pub enum OpenRequestKind {
|
||||
|
|
@ -135,6 +136,7 @@ impl OpenRequest {
|
|||
this.diff_paths = request.diff_paths;
|
||||
this.diff_all = request.diff_all;
|
||||
this.dev_container = request.dev_container;
|
||||
this.open_behavior = request.open_behavior;
|
||||
if let Some(wsl) = request.wsl {
|
||||
let (user, distro_name) = if let Some((user, distro)) = wsl.split_once('@') {
|
||||
if user.is_empty() {
|
||||
|
|
@ -382,6 +384,7 @@ pub struct RawOpenRequest {
|
|||
pub diff_all: bool,
|
||||
pub dev_container: bool,
|
||||
pub wsl: Option<String>,
|
||||
pub open_behavior: Option<cli::OpenBehavior>,
|
||||
}
|
||||
|
||||
impl Global for OpenListener {}
|
||||
|
|
@ -571,6 +574,7 @@ pub async fn handle_cli_connection(
|
|||
diff_all,
|
||||
dev_container,
|
||||
wsl,
|
||||
open_behavior: Some(open_behavior),
|
||||
},
|
||||
cx,
|
||||
) {
|
||||
|
|
@ -735,6 +739,52 @@ async fn resolve_open_behavior(
|
|||
None
|
||||
}
|
||||
|
||||
pub(crate) fn open_options_for_request(
|
||||
open_behavior: Option<cli::OpenBehavior>,
|
||||
location: &SerializedWorkspaceLocation,
|
||||
cx: &App,
|
||||
) -> workspace::OpenOptions {
|
||||
open_behavior.map_or_else(workspace::OpenOptions::default, |open_behavior| {
|
||||
open_options_for_behavior(open_behavior, location, cx)
|
||||
})
|
||||
}
|
||||
|
||||
pub(crate) fn open_options_for_behavior(
|
||||
open_behavior: cli::OpenBehavior,
|
||||
location: &SerializedWorkspaceLocation,
|
||||
cx: &App,
|
||||
) -> workspace::OpenOptions {
|
||||
// If reuse flag is passed, open a new workspace in an existing window.
|
||||
let requesting_window = if open_behavior == cli::OpenBehavior::Reuse {
|
||||
workspace::workspace_windows_for_location(location, cx)
|
||||
.into_iter()
|
||||
.next()
|
||||
} else {
|
||||
None
|
||||
};
|
||||
workspace::OpenOptions {
|
||||
workspace_matching: match open_behavior {
|
||||
cli::OpenBehavior::AlwaysNew | cli::OpenBehavior::Reuse => {
|
||||
workspace::WorkspaceMatching::None
|
||||
}
|
||||
cli::OpenBehavior::Add => workspace::WorkspaceMatching::MatchSubdirectory,
|
||||
_ => workspace::WorkspaceMatching::MatchExact,
|
||||
},
|
||||
add_dirs_to_sidebar: match open_behavior {
|
||||
cli::OpenBehavior::ExistingWindow => true,
|
||||
// For the default value, we consult the settings to decide
|
||||
// whether to open in a new window or existing window.
|
||||
cli::OpenBehavior::Default => {
|
||||
workspace::WorkspaceSettings::get_global(cx).cli_default_open_behavior
|
||||
== settings::CliDefaultOpenBehavior::ExistingWindow
|
||||
}
|
||||
_ => false,
|
||||
},
|
||||
requesting_window,
|
||||
..Default::default()
|
||||
}
|
||||
}
|
||||
|
||||
async fn open_workspaces(
|
||||
paths: Vec<String>,
|
||||
diff_paths: Vec<[String; 2]>,
|
||||
|
|
@ -787,39 +837,13 @@ async fn open_workspaces(
|
|||
let mut errored = false;
|
||||
|
||||
for (location, workspace_paths) in grouped_locations {
|
||||
// If reuse flag is passed, open a new workspace in an existing window.
|
||||
let replace_window = if open_behavior == cli::OpenBehavior::Reuse {
|
||||
cx.update(|cx| {
|
||||
workspace::workspace_windows_for_location(&location, cx)
|
||||
.into_iter()
|
||||
.next()
|
||||
})
|
||||
} else {
|
||||
None
|
||||
};
|
||||
let base_open_options =
|
||||
cx.update(|cx| open_options_for_behavior(open_behavior, &location, cx));
|
||||
let open_options = workspace::OpenOptions {
|
||||
workspace_matching: match open_behavior {
|
||||
cli::OpenBehavior::AlwaysNew | cli::OpenBehavior::Reuse => {
|
||||
workspace::WorkspaceMatching::None
|
||||
}
|
||||
cli::OpenBehavior::Add => workspace::WorkspaceMatching::MatchSubdirectory,
|
||||
_ => workspace::WorkspaceMatching::MatchExact,
|
||||
},
|
||||
add_dirs_to_sidebar: match open_behavior {
|
||||
cli::OpenBehavior::ExistingWindow => true,
|
||||
// For the default value, we consult the settings to decide
|
||||
// whether to open in a new window or existing window.
|
||||
cli::OpenBehavior::Default => cx.update(|cx| {
|
||||
workspace::WorkspaceSettings::get_global(cx).cli_default_open_behavior
|
||||
== settings::CliDefaultOpenBehavior::ExistingWindow
|
||||
}),
|
||||
_ => false,
|
||||
},
|
||||
requesting_window: replace_window,
|
||||
wait,
|
||||
env: env.clone(),
|
||||
open_in_dev_container: dev_container,
|
||||
..Default::default()
|
||||
..base_open_options
|
||||
};
|
||||
|
||||
match location {
|
||||
|
|
@ -1160,6 +1184,25 @@ mod tests {
|
|||
}
|
||||
}
|
||||
|
||||
#[gpui::test]
|
||||
fn test_parse_ssh_url_preserves_open_behavior(cx: &mut TestAppContext) {
|
||||
let _app_state = init_test(cx);
|
||||
|
||||
let request = cx.update(|cx| {
|
||||
OpenRequest::parse(
|
||||
RawOpenRequest {
|
||||
urls: vec!["ssh://me@host:/".into()],
|
||||
open_behavior: Some(cli::OpenBehavior::AlwaysNew),
|
||||
..Default::default()
|
||||
},
|
||||
cx,
|
||||
)
|
||||
.unwrap()
|
||||
});
|
||||
|
||||
assert_eq!(request.open_behavior, Some(cli::OpenBehavior::AlwaysNew));
|
||||
}
|
||||
|
||||
#[gpui::test]
|
||||
fn test_reject_ssh_urls(cx: &mut TestAppContext) {
|
||||
let _app_state = init_test(cx);
|
||||
|
|
@ -1182,6 +1225,24 @@ mod tests {
|
|||
}
|
||||
}
|
||||
|
||||
#[gpui::test]
|
||||
fn test_open_options_for_behavior_always_new(cx: &mut TestAppContext) {
|
||||
let _app_state = init_test(cx);
|
||||
let options = cx.update(|cx| {
|
||||
open_options_for_behavior(
|
||||
cli::OpenBehavior::AlwaysNew,
|
||||
&SerializedWorkspaceLocation::Local,
|
||||
cx,
|
||||
)
|
||||
});
|
||||
assert_eq!(
|
||||
options.workspace_matching,
|
||||
workspace::WorkspaceMatching::None
|
||||
);
|
||||
assert!(!options.add_dirs_to_sidebar);
|
||||
assert!(options.requesting_window.is_none());
|
||||
}
|
||||
|
||||
#[gpui::test]
|
||||
fn test_parse_agent_url(cx: &mut TestAppContext) {
|
||||
let _app_state = init_test(cx);
|
||||
|
|
@ -2166,6 +2227,25 @@ mod tests {
|
|||
}
|
||||
}
|
||||
|
||||
fn make_cli_url_open_request(
|
||||
urls: Vec<String>,
|
||||
open_behavior: cli::OpenBehavior,
|
||||
) -> CliRequest {
|
||||
CliRequest::Open {
|
||||
paths: vec![],
|
||||
urls,
|
||||
diff_paths: vec![],
|
||||
diff_all: false,
|
||||
wsl: None,
|
||||
wait: false,
|
||||
open_behavior,
|
||||
env: None,
|
||||
user_data_dir: None,
|
||||
dev_container: false,
|
||||
cwd: None,
|
||||
}
|
||||
}
|
||||
|
||||
/// Runs the real [`cli::run_cli_response_loop`] on an OS thread against
|
||||
/// the Zed-side `handle_cli_connection` on the GPUI foreground executor,
|
||||
/// using `allow_parking` so the test scheduler tolerates cross-thread
|
||||
|
|
@ -2472,6 +2552,35 @@ mod tests {
|
|||
assert_eq!(cx.windows().len(), 2);
|
||||
}
|
||||
|
||||
#[gpui::test]
|
||||
async fn test_e2e_explicit_new_flag_with_file_url_opens_new_window(cx: &mut TestAppContext) {
|
||||
let app_state = init_test(cx);
|
||||
|
||||
app_state
|
||||
.fs
|
||||
.as_fake()
|
||||
.insert_tree(path!("/project"), json!({ "file.txt": "content" }))
|
||||
.await;
|
||||
|
||||
open_workspace_file(path!("/project"), Default::default(), app_state.clone(), cx).await;
|
||||
assert_eq!(cx.windows().len(), 1);
|
||||
|
||||
let file_url = format!(
|
||||
"file://{}",
|
||||
urlencoding::encode(path!("/project/file.txt")).into_owned()
|
||||
);
|
||||
let (status, prompt_shown) = run_cli_with_zed_handler(
|
||||
cx,
|
||||
app_state,
|
||||
make_cli_url_open_request(vec![file_url], cli::OpenBehavior::AlwaysNew),
|
||||
None,
|
||||
);
|
||||
|
||||
assert_eq!(status, 0);
|
||||
assert!(!prompt_shown, "no prompt should be shown with -n flag");
|
||||
assert_eq!(cx.windows().len(), 2);
|
||||
}
|
||||
|
||||
#[gpui::test]
|
||||
async fn test_e2e_paths_in_existing_workspace_no_prompt(cx: &mut TestAppContext) {
|
||||
let app_state = init_test(cx);
|
||||
|
|
|
|||
Loading…
Reference in a new issue