Fix panic in open urls (#9032)

Co-Authored-By: Nathan <nathan@zed.dev>

Release Notes:

- N/A

Co-authored-by: Nathan <nathan@zed.dev>
This commit is contained in:
Conrad Irwin 2024-03-07 13:52:50 -07:00 committed by GitHub
parent 9068911eb4
commit 284a57d4d1
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
9 changed files with 176 additions and 225 deletions

View file

@ -249,7 +249,7 @@ pub fn init(cx: &mut AppContext) {
cx.on_action(move |_: &workspace::NewFile, cx| {
let app_state = workspace::AppState::global(cx);
if let Some(app_state) = app_state.upgrade() {
workspace::open_new(&app_state, cx, |workspace, cx| {
workspace::open_new(app_state, cx, |workspace, cx| {
Editor::new_file(workspace, &Default::default(), cx)
})
.detach();
@ -258,7 +258,7 @@ pub fn init(cx: &mut AppContext) {
cx.on_action(move |_: &workspace::NewWindow, cx| {
let app_state = workspace::AppState::global(cx);
if let Some(app_state) = app_state.upgrade() {
workspace::open_new(&app_state, cx, |workspace, cx| {
workspace::open_new(app_state, cx, |workspace, cx| {
Editor::new_file(workspace, &Default::default(), cx)
})
.detach();

View file

@ -150,14 +150,9 @@ impl App {
/// to open one or more URLs.
pub fn on_open_urls<F>(&self, mut callback: F) -> &Self
where
F: 'static + FnMut(Vec<String>, &mut AppContext),
F: 'static + FnMut(Vec<String>),
{
let this = Rc::downgrade(&self.0);
self.0.borrow().platform.on_open_urls(Box::new(move |urls| {
if let Some(app) = this.upgrade() {
callback(urls, &mut app.borrow_mut());
}
}));
self.0.borrow().platform.on_open_urls(Box::new(callback));
self
}

View file

@ -102,7 +102,7 @@ pub fn new_journal_entry(app_state: Arc<AppState>, cx: &mut WindowContext) {
cx.spawn(|mut cx| async move {
let (journal_dir, entry_path) = create_entry.await?;
let (workspace, _) = cx
.update(|cx| workspace::open_paths(&[journal_dir], &app_state, None, cx))?
.update(|cx| workspace::open_paths(&[journal_dir], app_state, None, cx))?
.await?;
let opened = workspace

View file

@ -493,7 +493,7 @@ mod tests {
}),
)
.await;
cx.update(|cx| open_paths(&[PathBuf::from("/dir/main.ts")], &app_state, None, cx))
cx.update(|cx| open_paths(&[PathBuf::from("/dir/main.ts")], app_state, None, cx))
.await
.unwrap();
assert_eq!(cx.update(|cx| cx.windows().len()), 1);

View file

@ -37,8 +37,8 @@ pub fn init(cx: &mut AppContext) {
base_keymap_picker::init(cx);
}
pub fn show_welcome_view(app_state: &Arc<AppState>, cx: &mut AppContext) {
open_new(&app_state, cx, |workspace, cx| {
pub fn show_welcome_view(app_state: Arc<AppState>, cx: &mut AppContext) {
open_new(app_state, cx, |workspace, cx| {
workspace.toggle_dock(DockPosition::Left, cx);
let welcome_page = WelcomePage::new(workspace, cx);
workspace.add_item_to_center(Box::new(welcome_page.clone()), cx);

View file

@ -262,7 +262,7 @@ pub fn init(app_state: Arc<AppState>, cx: &mut AppContext) {
cx.spawn(move |cx| async move {
if let Some(paths) = paths.await.log_err().flatten() {
cx.update(|cx| {
open_paths(&paths, &app_state, None, cx).detach_and_log_err(cx)
open_paths(&paths, app_state, None, cx).detach_and_log_err(cx)
})
.ok();
}
@ -1414,7 +1414,7 @@ impl Workspace {
let app_state = self.app_state.clone();
cx.spawn(|_, mut cx| async move {
cx.update(|cx| open_paths(&paths, &app_state, window_to_replace, cx))?
cx.update(|cx| open_paths(&paths, app_state, window_to_replace, cx))?
.await?;
Ok(())
})
@ -4381,7 +4381,7 @@ fn activate_any_workspace_window(cx: &mut AsyncAppContext) -> Option<WindowHandl
#[allow(clippy::type_complexity)]
pub fn open_paths(
abs_paths: &[PathBuf],
app_state: &Arc<AppState>,
app_state: Arc<AppState>,
requesting_window: Option<WindowHandle<Workspace>>,
cx: &mut AppContext,
) -> Task<
@ -4390,7 +4390,6 @@ pub fn open_paths(
Vec<Option<Result<Box<dyn ItemHandle>, anyhow::Error>>>,
)>,
> {
let app_state = app_state.clone();
let abs_paths = abs_paths.to_vec();
// Open paths in existing workspace if possible
let existing = activate_workspace_for_project(cx, {
@ -4417,11 +4416,11 @@ pub fn open_paths(
}
pub fn open_new(
app_state: &Arc<AppState>,
app_state: Arc<AppState>,
cx: &mut AppContext,
init: impl FnOnce(&mut Workspace, &mut ViewContext<Workspace>) + 'static + Send,
) -> Task<()> {
let task = Workspace::new_local(Vec::new(), app_state.clone(), None, cx);
let task = Workspace::new_local(Vec::new(), app_state, None, cx);
cx.spawn(|mut cx| async move {
if let Some((workspace, opened_paths)) = task.await.log_err() {
workspace

View file

@ -106,11 +106,11 @@ fn main() {
let (listener, mut open_rx) = OpenListener::new();
let listener = Arc::new(listener);
let open_listener = listener.clone();
app.on_open_urls(move |urls, cx| open_listener.open_urls(&urls, cx));
app.on_open_urls(move |urls| open_listener.open_urls(urls));
app.on_reopen(move |cx| {
if let Some(app_state) = AppState::try_global(cx).and_then(|app_state| app_state.upgrade())
{
workspace::open_new(&app_state, cx, |workspace, cx| {
workspace::open_new(app_state, cx, |workspace, cx| {
Editor::new_file(workspace, &Default::default(), cx)
})
.detach();
@ -273,7 +273,7 @@ fn main() {
cx.activate(true);
let urls = collect_url_args(cx);
if !urls.is_empty() {
listener.open_urls(&urls, cx)
listener.open_urls(urls)
}
} else {
upload_panics_and_crashes(http.clone(), cx);
@ -282,141 +282,38 @@ fn main() {
if std::env::var(FORCE_CLI_MODE_ENV_VAR_NAME).ok().is_some()
&& !listener.triggered.load(Ordering::Acquire)
{
listener.open_urls(&collect_url_args(cx), cx)
listener.open_urls(collect_url_args(cx))
}
}
let mut triggered_authentication = false;
fn open_paths_and_log_errs(
paths: &[PathBuf],
app_state: &Arc<AppState>,
cx: &mut AppContext,
) {
let task = workspace::open_paths(&paths, &app_state, None, cx);
cx.spawn(|_| async move {
if let Some((_window, results)) = task.await.log_err() {
for result in results.into_iter().flatten() {
if let Err(err) = result {
log::error!("Error opening path: {err}",);
}
}
}
})
.detach();
}
match open_rx.try_next() {
Ok(Some(OpenRequest::Paths { paths })) => {
open_paths_and_log_errs(&paths, &app_state, cx)
match open_rx
.try_next()
.ok()
.flatten()
.and_then(|urls| OpenRequest::parse(urls, cx).log_err())
{
Some(request) => {
triggered_authentication = handle_open_request(request, app_state.clone(), cx)
}
Ok(Some(OpenRequest::CliConnection { connection })) => {
let app_state = app_state.clone();
cx.spawn(move |cx| handle_cli_connection(connection, app_state, cx))
.detach();
}
Ok(Some(OpenRequest::JoinChannel { channel_id })) => {
triggered_authentication = true;
let app_state = app_state.clone();
let client = client.clone();
cx.spawn(|cx| async move {
// ignore errors here, we'll show a generic "not signed in"
let _ = authenticate(client, &cx).await;
cx.update(|cx| {
workspace::join_channel(client::ChannelId(channel_id), app_state, None, cx)
})?
.await?;
anyhow::Ok(())
})
.detach_and_log_err(cx);
}
Ok(Some(OpenRequest::OpenChannelNotes {
channel_id,
heading,
})) => {
triggered_authentication = true;
let app_state = app_state.clone();
let client = client.clone();
cx.spawn(|mut cx| async move {
// ignore errors here, we'll show a generic "not signed in"
let _ = authenticate(client, &cx).await;
let workspace_window =
workspace::get_any_active_workspace(app_state, cx.clone()).await?;
let workspace = workspace_window.root_view(&cx)?;
cx.update_window(workspace_window.into(), |_, cx| {
ChannelView::open(client::ChannelId(channel_id), heading, workspace, cx)
})?
.await?;
anyhow::Ok(())
})
.detach_and_log_err(cx);
}
Ok(None) | Err(_) => cx
None => cx
.spawn({
let app_state = app_state.clone();
|cx| async move { restore_or_create_workspace(&app_state, cx).await }
|cx| async move { restore_or_create_workspace(app_state, cx).await }
})
.detach(),
}
let app_state = app_state.clone();
cx.spawn(move |cx| async move {
while let Some(request) = open_rx.next().await {
match request {
OpenRequest::Paths { paths } => {
cx.update(|cx| open_paths_and_log_errs(&paths, &app_state, cx))
.ok();
while let Some(urls) = open_rx.next().await {
cx.update(|cx| {
if let Some(request) = OpenRequest::parse(urls, cx).log_err() {
handle_open_request(request, app_state.clone(), cx);
}
OpenRequest::CliConnection { connection } => {
let app_state = app_state.clone();
cx.spawn(move |cx| {
handle_cli_connection(connection, app_state.clone(), cx)
})
.detach();
}
OpenRequest::JoinChannel { channel_id } => {
let app_state = app_state.clone();
cx.update(|mut cx| {
cx.spawn(|cx| async move {
cx.update(|cx| {
workspace::join_channel(
client::ChannelId(channel_id),
app_state,
None,
cx,
)
})?
.await?;
anyhow::Ok(())
})
.detach_and_log_err(&mut cx);
})
.log_err();
}
OpenRequest::OpenChannelNotes {
channel_id,
heading,
} => {
let app_state = app_state.clone();
let open_notes_task = cx.spawn(|mut cx| async move {
let workspace_window =
workspace::get_any_active_workspace(app_state, cx.clone()).await?;
let workspace = workspace_window.root_view(&cx)?;
cx.update_window(workspace_window.into(), |_, cx| {
ChannelView::open(
client::ChannelId(channel_id),
heading,
workspace,
cx,
)
})?
.await?;
anyhow::Ok(())
});
cx.update(|cx| open_notes_task.detach_and_log_err(cx))
.log_err();
}
}
})
.ok();
}
})
.detach();
@ -428,6 +325,70 @@ fn main() {
});
}
fn open_paths_and_log_errs(paths: &[PathBuf], app_state: Arc<AppState>, cx: &mut AppContext) {
let task = workspace::open_paths(&paths, app_state, None, cx);
cx.spawn(|_| async move {
if let Some((_window, results)) = task.await.log_err() {
for result in results.into_iter().flatten() {
if let Err(err) = result {
log::error!("Error opening path: {err}",);
}
}
}
})
.detach();
}
fn handle_open_request(
request: OpenRequest,
app_state: Arc<AppState>,
cx: &mut AppContext,
) -> bool {
let mut triggered_authentication = false;
match request {
OpenRequest::Paths { paths } => open_paths_and_log_errs(&paths, app_state, cx),
OpenRequest::CliConnection { connection } => {
let app_state = app_state.clone();
cx.spawn(move |cx| handle_cli_connection(connection, app_state, cx))
.detach();
}
OpenRequest::JoinChannel { channel_id } => {
triggered_authentication = true;
cx.spawn(|cx| async move {
// ignore errors here, we'll show a generic "not signed in"
let _ = authenticate(app_state.client.clone(), &cx).await;
cx.update(|cx| {
workspace::join_channel(client::ChannelId(channel_id), app_state, None, cx)
})?
.await?;
anyhow::Ok(())
})
.detach_and_log_err(cx);
}
OpenRequest::OpenChannelNotes {
channel_id,
heading,
} => {
triggered_authentication = true;
let client = app_state.client.clone();
cx.spawn(|mut cx| async move {
// ignore errors here, we'll show a generic "not signed in"
let _ = authenticate(client, &cx).await;
let workspace_window =
workspace::get_any_active_workspace(app_state, cx.clone()).await?;
let workspace = workspace_window.root_view(&cx)?;
cx.update_window(workspace_window.into(), |_, cx| {
ChannelView::open(client::ChannelId(channel_id), heading, workspace, cx)
})?
.await?;
anyhow::Ok(())
})
.detach_and_log_err(cx);
}
}
triggered_authentication
}
async fn authenticate(client: Arc<Client>, cx: &AsyncAppContext) -> Result<()> {
if stdout_is_a_pty() {
if client::IMPERSONATE_LOGIN.is_some() {
@ -465,7 +426,7 @@ async fn installation_id() -> Result<(String, bool)> {
Ok((installation_id, false))
}
async fn restore_or_create_workspace(app_state: &Arc<AppState>, cx: AsyncAppContext) {
async fn restore_or_create_workspace(app_state: Arc<AppState>, cx: AsyncAppContext) {
async_maybe!({
if let Some(location) = workspace::last_opened_workspace_paths().await {
cx.update(|cx| workspace::open_paths(location.paths().as_ref(), app_state, None, cx))?

View file

@ -37,8 +37,66 @@ pub enum OpenRequest {
},
}
impl OpenRequest {
pub fn parse(urls: Vec<String>, cx: &AppContext) -> Result<Self> {
if let Some(server_name) = urls.first().and_then(|url| url.strip_prefix("zed-cli://")) {
Self::parse_cli_connection(server_name)
} else if let Some(request_path) = urls.first().and_then(|url| parse_zed_link(url, cx)) {
Self::parse_zed_url(request_path)
} else {
Ok(Self::parse_file_urls(urls))
}
}
fn parse_cli_connection(server_name: &str) -> Result<OpenRequest> {
let connection = connect_to_cli(server_name)?;
Ok(OpenRequest::CliConnection { connection })
}
fn parse_zed_url(request_path: &str) -> Result<OpenRequest> {
let mut parts = request_path.split('/');
if parts.next() == Some("channel") {
if let Some(slug) = parts.next() {
if let Some(id_str) = slug.split('-').last() {
if let Ok(channel_id) = id_str.parse::<u64>() {
let Some(next) = parts.next() else {
return Ok(OpenRequest::JoinChannel { channel_id });
};
if let Some(heading) = next.strip_prefix("notes#") {
return Ok(OpenRequest::OpenChannelNotes {
channel_id,
heading: Some([heading].into_iter().chain(parts).join("/")),
});
} else if next == "notes" {
return Ok(OpenRequest::OpenChannelNotes {
channel_id,
heading: None,
});
}
}
}
}
}
Err(anyhow!("invalid zed url: {}", request_path))
}
fn parse_file_urls(urls: Vec<String>) -> OpenRequest {
let paths: Vec<_> = urls
.iter()
.flat_map(|url| url.strip_prefix("file://"))
.flat_map(|url| {
let decoded = urlencoding::decode_binary(url.as_bytes());
PathBuf::try_from_bytes(decoded.as_ref()).log_err()
})
.collect();
OpenRequest::Paths { paths }
}
}
pub struct OpenListener {
tx: UnboundedSender<OpenRequest>,
tx: UnboundedSender<Vec<String>>,
pub triggered: AtomicBool,
}
@ -55,7 +113,7 @@ impl OpenListener {
cx.set_global(GlobalOpenListener(listener))
}
pub fn new() -> (Self, UnboundedReceiver<OpenRequest>) {
pub fn new() -> (Self, UnboundedReceiver<Vec<String>>) {
let (tx, rx) = mpsc::unbounded();
(
OpenListener {
@ -66,74 +124,12 @@ impl OpenListener {
)
}
pub fn open_urls(&self, urls: &[String], cx: &AppContext) {
pub fn open_urls(&self, urls: Vec<String>) {
self.triggered.store(true, Ordering::Release);
let request = if let Some(server_name) =
urls.first().and_then(|url| url.strip_prefix("zed-cli://"))
{
self.handle_cli_connection(server_name)
} else if let Some(request_path) = urls.first().and_then(|url| parse_zed_link(url, cx)) {
self.handle_zed_url_scheme(request_path)
} else {
self.handle_file_urls(urls)
};
if let Some(request) = request {
self.tx
.unbounded_send(request)
.map_err(|_| anyhow!("no listener for open requests"))
.log_err();
}
}
fn handle_cli_connection(&self, server_name: &str) -> Option<OpenRequest> {
if let Some(connection) = connect_to_cli(server_name).log_err() {
return Some(OpenRequest::CliConnection { connection });
}
None
}
fn handle_zed_url_scheme(&self, request_path: &str) -> Option<OpenRequest> {
let mut parts = request_path.split('/');
if parts.next() == Some("channel") {
if let Some(slug) = parts.next() {
if let Some(id_str) = slug.split('-').last() {
if let Ok(channel_id) = id_str.parse::<u64>() {
let Some(next) = parts.next() else {
return Some(OpenRequest::JoinChannel { channel_id });
};
if let Some(heading) = next.strip_prefix("notes#") {
return Some(OpenRequest::OpenChannelNotes {
channel_id,
heading: Some([heading].into_iter().chain(parts).join("/")),
});
} else if next == "notes" {
return Some(OpenRequest::OpenChannelNotes {
channel_id,
heading: None,
});
}
}
}
}
}
log::error!("invalid zed url: {}", request_path);
None
}
fn handle_file_urls(&self, urls: &[String]) -> Option<OpenRequest> {
let paths: Vec<_> = urls
.iter()
.flat_map(|url| url.strip_prefix("file://"))
.flat_map(|url| {
let decoded = urlencoding::decode_binary(url.as_bytes());
PathBuf::try_from_bytes(decoded.as_ref()).log_err()
})
.collect();
Some(OpenRequest::Paths { paths })
self.tx
.unbounded_send(urls)
.map_err(|_| anyhow!("no listener for open requests"))
.log_err();
}
}
@ -210,7 +206,7 @@ pub async fn handle_cli_connection(
let mut errored = false;
match cx.update(|cx| workspace::open_paths(&paths, &app_state, None, cx)) {
match cx.update(|cx| workspace::open_paths(&paths, app_state, None, cx)) {
Ok(task) => match task.await {
Ok((workspace, items)) => {
let mut item_release_futures = Vec::new();

View file

@ -233,7 +233,7 @@ pub fn initialize_workspace(app_state: Arc<AppState>, cx: &mut AppContext) {
cx.toggle_full_screen();
})
.register_action(|_, action: &OpenZedUrl, cx| {
OpenListener::global(cx).open_urls(&[action.url.clone()], cx)
OpenListener::global(cx).open_urls(vec![action.url.clone()])
})
.register_action(|_, action: &OpenBrowser, cx| cx.open_url(&action.url))
.register_action(move |_, _: &IncreaseBufferFontSize, cx| {
@ -399,7 +399,7 @@ pub fn initialize_workspace(app_state: Arc<AppState>, cx: &mut AppContext) {
let app_state = Arc::downgrade(&app_state);
move |_, _: &NewWindow, cx| {
if let Some(app_state) = app_state.upgrade() {
open_new(&app_state, cx, |workspace, cx| {
open_new(app_state, cx, |workspace, cx| {
Editor::new_file(workspace, &Default::default(), cx)
})
.detach();
@ -410,7 +410,7 @@ pub fn initialize_workspace(app_state: Arc<AppState>, cx: &mut AppContext) {
let app_state = Arc::downgrade(&app_state);
move |_, _: &NewFile, cx| {
if let Some(app_state) = app_state.upgrade() {
open_new(&app_state, cx, |workspace, cx| {
open_new(app_state, cx, |workspace, cx| {
Editor::new_file(workspace, &Default::default(), cx)
})
.detach();
@ -911,7 +911,7 @@ mod tests {
cx.update(|cx| {
open_paths(
&[PathBuf::from("/root/a"), PathBuf::from("/root/b")],
&app_state,
app_state.clone(),
None,
cx,
)
@ -920,7 +920,7 @@ mod tests {
.unwrap();
assert_eq!(cx.read(|cx| cx.windows().len()), 1);
cx.update(|cx| open_paths(&[PathBuf::from("/root/a")], &app_state, None, cx))
cx.update(|cx| open_paths(&[PathBuf::from("/root/a")], app_state.clone(), None, cx))
.await
.unwrap();
assert_eq!(cx.read(|cx| cx.windows().len()), 1);
@ -942,7 +942,7 @@ mod tests {
cx.update(|cx| {
open_paths(
&[PathBuf::from("/root/b"), PathBuf::from("/root/c")],
&app_state,
app_state.clone(),
None,
cx,
)
@ -958,7 +958,7 @@ mod tests {
cx.update(|cx| {
open_paths(
&[PathBuf::from("/root/c"), PathBuf::from("/root/d")],
&app_state,
app_state,
Some(window),
cx,
)
@ -995,7 +995,7 @@ mod tests {
.insert_tree("/root", json!({"a": "hey"}))
.await;
cx.update(|cx| open_paths(&[PathBuf::from("/root/a")], &app_state, None, cx))
cx.update(|cx| open_paths(&[PathBuf::from("/root/a")], app_state.clone(), None, cx))
.await
.unwrap();
assert_eq!(cx.update(|cx| cx.windows().len()), 1);
@ -1062,7 +1062,7 @@ mod tests {
assert!(!window_is_edited(window, cx));
// Opening the buffer again doesn't impact the window's edited state.
cx.update(|cx| open_paths(&[PathBuf::from("/root/a")], &app_state, None, cx))
cx.update(|cx| open_paths(&[PathBuf::from("/root/a")], app_state, None, cx))
.await
.unwrap();
let editor = window
@ -1100,7 +1100,7 @@ mod tests {
async fn test_new_empty_workspace(cx: &mut TestAppContext) {
let app_state = init_test(cx);
cx.update(|cx| {
open_new(&app_state, cx, |workspace, cx| {
open_new(app_state.clone(), cx, |workspace, cx| {
Editor::new_file(workspace, &Default::default(), cx)
})
})
@ -1291,7 +1291,7 @@ mod tests {
)
.await;
cx.update(|cx| open_paths(&[PathBuf::from("/dir1/")], &app_state, None, cx))
cx.update(|cx| open_paths(&[PathBuf::from("/dir1/")], app_state, None, cx))
.await
.unwrap();
assert_eq!(cx.update(|cx| cx.windows().len()), 1);
@ -1525,7 +1525,7 @@ mod tests {
Path::new("/root/excluded_dir/ignored_subdir").to_path_buf(),
];
let (opened_workspace, new_items) = cx
.update(|cx| workspace::open_paths(&paths_to_open, &app_state, None, cx))
.update(|cx| workspace::open_paths(&paths_to_open, app_state, None, cx))
.await
.unwrap();