From ccaf3268f8dec1270599c720f3d81c4b9a04e6dd Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Fri, 11 Oct 2024 12:58:49 +0300 Subject: [PATCH] Check paths for FS existence before parsing them as paths with line numbers (#19057) Closes https://github.com/zed-industries/zed/issues/18268 Release Notes: - Fixed Zed not being open filenames with special combination of brackets ([#18268](https://github.com/zed-industries/zed/issues/18268)) --- crates/cli/src/main.rs | 43 ++++++++++--------- crates/util/src/paths.rs | 44 +++++++++----------- crates/zed/src/main.rs | 17 ++++---- crates/zed/src/zed/open_listener.rs | 64 +++++++++++++++-------------- 4 files changed, 87 insertions(+), 81 deletions(-) diff --git a/crates/cli/src/main.rs b/crates/cli/src/main.rs index a09deaaf94..e69183d1ea 100644 --- a/crates/cli/src/main.rs +++ b/crates/cli/src/main.rs @@ -58,27 +58,32 @@ struct Args { dev_server_token: Option, } -fn parse_path_with_position(argument_str: &str) -> Result { - let path = PathWithPosition::parse_str(argument_str); - let curdir = env::current_dir()?; - - let canonicalized = path.map_path(|path| match fs::canonicalize(&path) { - Ok(path) => Ok(path), - Err(e) => { - if let Some(mut parent) = path.parent() { - if parent == Path::new("") { - parent = &curdir +fn parse_path_with_position(argument_str: &str) -> anyhow::Result { + let canonicalized = match Path::new(argument_str).canonicalize() { + Ok(existing_path) => PathWithPosition::from_path(existing_path), + Err(_) => { + let path = PathWithPosition::parse_str(argument_str); + let curdir = env::current_dir().context("reteiving current directory")?; + path.map_path(|path| match fs::canonicalize(&path) { + Ok(path) => Ok(path), + Err(e) => { + if let Some(mut parent) = path.parent() { + if parent == Path::new("") { + parent = &curdir + } + match fs::canonicalize(parent) { + Ok(parent) => Ok(parent.join(path.file_name().unwrap())), + Err(_) => Err(e), + } + } else { + Err(e) + } } - match fs::canonicalize(parent) { - Ok(parent) => Ok(parent.join(path.file_name().unwrap())), - Err(_) => Err(e), - } - } else { - Err(e) - } + }) } - })?; - Ok(canonicalized.to_string(|path| path.display().to_string())) + .with_context(|| format!("parsing as path with position {argument_str}"))?, + }; + Ok(canonicalized.to_string(|path| path.to_string_lossy().to_string())) } fn main() -> Result<()> { diff --git a/crates/util/src/paths.rs b/crates/util/src/paths.rs index f4ecfefc52..90b95f2c7b 100644 --- a/crates/util/src/paths.rs +++ b/crates/util/src/paths.rs @@ -221,11 +221,7 @@ impl PathWithPosition { pub fn parse_str(s: &str) -> Self { let trimmed = s.trim(); let path = Path::new(trimmed); - let maybe_file_name_with_row_col = path - .file_name() - .unwrap_or_default() - .to_str() - .unwrap_or_default(); + let maybe_file_name_with_row_col = path.file_name().unwrap_or_default().to_string_lossy(); if maybe_file_name_with_row_col.is_empty() { return Self { path: Path::new(s).to_path_buf(), @@ -240,7 +236,7 @@ impl PathWithPosition { static SUFFIX_RE: LazyLock = LazyLock::new(|| Regex::new(ROW_COL_CAPTURE_REGEX).unwrap()); match SUFFIX_RE - .captures(maybe_file_name_with_row_col) + .captures(&maybe_file_name_with_row_col) .map(|caps| caps.extract()) { Some((_, [file_name, maybe_row, maybe_column])) => { @@ -361,26 +357,26 @@ pub fn compare_paths( let b_is_file = components_b.peek().is_none() && b_is_file; let ordering = a_is_file.cmp(&b_is_file).then_with(|| { let path_a = Path::new(component_a.as_os_str()); - let num_and_remainder_a = NumericPrefixWithSuffix::from_numeric_prefixed_str( - if a_is_file { - path_a.file_stem() - } else { - path_a.file_name() - } - .and_then(|s| s.to_str()) - .unwrap_or_default(), - ); + let path_string_a = if a_is_file { + path_a.file_stem() + } else { + path_a.file_name() + } + .map(|s| s.to_string_lossy()); + let num_and_remainder_a = path_string_a + .as_deref() + .map(NumericPrefixWithSuffix::from_numeric_prefixed_str); let path_b = Path::new(component_b.as_os_str()); - let num_and_remainder_b = NumericPrefixWithSuffix::from_numeric_prefixed_str( - if b_is_file { - path_b.file_stem() - } else { - path_b.file_name() - } - .and_then(|s| s.to_str()) - .unwrap_or_default(), - ); + let path_string_b = if b_is_file { + path_b.file_stem() + } else { + path_b.file_name() + } + .map(|s| s.to_string_lossy()); + let num_and_remainder_b = path_string_b + .as_deref() + .map(NumericPrefixWithSuffix::from_numeric_prefixed_str); num_and_remainder_a.cmp(&num_and_remainder_b) }); diff --git a/crates/zed/src/main.rs b/crates/zed/src/main.rs index 9857c60491..cd245dc12c 100644 --- a/crates/zed/src/main.rs +++ b/crates/zed/src/main.rs @@ -58,8 +58,9 @@ use workspace::{ AppState, WorkspaceSettings, WorkspaceStore, }; use zed::{ - app_menus, build_window_options, handle_cli_connection, handle_keymap_file_changes, - initialize_workspace, open_paths_with_positions, OpenListener, OpenRequest, + app_menus, build_window_options, derive_paths_with_position, handle_cli_connection, + handle_keymap_file_changes, initialize_workspace, open_paths_with_positions, OpenListener, + OpenRequest, }; use crate::zed::inline_completion_registry; @@ -712,13 +713,11 @@ fn handle_open_request( if let Some(connection_info) = request.ssh_connection { cx.spawn(|mut cx| async move { + let paths_with_position = + derive_paths_with_position(app_state.fs.as_ref(), request.open_paths).await; open_ssh_project( connection_info, - request - .open_paths - .into_iter() - .map(|path| path.path) - .collect::>(), + paths_with_position.into_iter().map(|p| p.path).collect(), app_state, workspace::OpenOptions::default(), &mut cx, @@ -733,8 +732,10 @@ fn handle_open_request( if !request.open_paths.is_empty() { let app_state = app_state.clone(); task = Some(cx.spawn(|mut cx| async move { + let paths_with_position = + derive_paths_with_position(app_state.fs.as_ref(), request.open_paths).await; let (_window, results) = open_paths_with_positions( - &request.open_paths, + &paths_with_position, app_state, workspace::OpenOptions::default(), &mut cx, diff --git a/crates/zed/src/zed/open_listener.rs b/crates/zed/src/zed/open_listener.rs index 7746337df1..c40876ad7a 100644 --- a/crates/zed/src/zed/open_listener.rs +++ b/crates/zed/src/zed/open_listener.rs @@ -9,12 +9,15 @@ use collections::HashMap; use db::kvp::KEY_VALUE_STORE; use editor::scroll::Autoscroll; use editor::Editor; +use fs::Fs; use futures::channel::mpsc::{UnboundedReceiver, UnboundedSender}; use futures::channel::{mpsc, oneshot}; +use futures::future::join_all; use futures::{FutureExt, SinkExt, StreamExt}; use gpui::{AppContext, AsyncAppContext, Global, WindowHandle}; use language::{Bias, Point}; use remote::SshConnectionOptions; +use std::path::Path; use std::sync::Arc; use std::time::Duration; use std::{process, thread}; @@ -27,7 +30,7 @@ use workspace::{AppState, OpenOptions, Workspace}; #[derive(Default, Debug)] pub struct OpenRequest { pub cli_connection: Option<(mpsc::Receiver, IpcSender)>, - pub open_paths: Vec, + pub open_paths: Vec, pub open_channel_notes: Vec<(u64, Option)>, pub join_channel: Option, pub ssh_connection: Option, @@ -57,8 +60,7 @@ impl OpenRequest { fn parse_file_path(&mut self, file: &str) { if let Some(decoded) = urlencoding::decode(file).log_err() { - let path_buf = PathWithPosition::parse_str(&decoded); - self.open_paths.push(path_buf) + self.open_paths.push(decoded.into_owned()) } } @@ -369,26 +371,15 @@ async fn open_workspaces( location .paths() .iter() - .map(|path| PathWithPosition { - path: path.clone(), - row: None, - column: None, - }) - .collect::>() + .map(|path| path.to_string_lossy().to_string()) + .collect() }) .collect::>() }) .collect() } } else { - // If paths are provided, parse them (they include positions) - let paths_with_position = paths - .into_iter() - .map(|path_with_position_string| { - PathWithPosition::parse_str(&path_with_position_string) - }) - .collect(); - vec![paths_with_position] + vec![paths] }; if grouped_paths.is_empty() { @@ -441,7 +432,7 @@ async fn open_workspaces( } async fn open_workspace( - workspace_paths: Vec, + workspace_paths: Vec, open_new_workspace: Option, wait: bool, responses: &IpcSender, @@ -451,8 +442,10 @@ async fn open_workspace( ) -> bool { let mut errored = false; + let paths_with_position = + derive_paths_with_position(app_state.fs.as_ref(), workspace_paths).await; match open_paths_with_positions( - &workspace_paths, + &paths_with_position, app_state.clone(), workspace::OpenOptions { open_new_workspace, @@ -466,7 +459,7 @@ async fn open_workspace( Ok((workspace, items)) => { let mut item_release_futures = Vec::new(); - for (item, path) in items.into_iter().zip(&workspace_paths) { + for (item, path) in items.into_iter().zip(&paths_with_position) { match item { Some(Ok(item)) => { cx.update(|cx| { @@ -497,7 +490,7 @@ async fn open_workspace( if wait { let background = cx.background_executor().clone(); let wait = async move { - if workspace_paths.is_empty() { + if paths_with_position.is_empty() { let (done_tx, done_rx) = oneshot::channel(); let _subscription = workspace.update(cx, |_, cx| { cx.on_release(move |_, _, _| { @@ -532,7 +525,7 @@ async fn open_workspace( errored = true; responses .send(CliResponse::Stderr { - message: format!("error opening {workspace_paths:?}: {error}"), + message: format!("error opening {paths_with_position:?}: {error}"), }) .log_err(); } @@ -540,9 +533,26 @@ async fn open_workspace( errored } +pub async fn derive_paths_with_position( + fs: &dyn Fs, + path_strings: impl IntoIterator>, +) -> Vec { + join_all(path_strings.into_iter().map(|path_str| async move { + let canonicalized = fs.canonicalize(Path::new(path_str.as_ref())).await; + (path_str, canonicalized) + })) + .await + .into_iter() + .map(|(original, canonicalized)| match canonicalized { + Ok(canonicalized) => PathWithPosition::from_path(canonicalized), + Err(_) => PathWithPosition::parse_str(original.as_ref()), + }) + .collect() +} + #[cfg(test)] mod tests { - use std::{path::PathBuf, sync::Arc}; + use std::sync::Arc; use cli::{ ipc::{self}, @@ -551,7 +561,6 @@ mod tests { use editor::Editor; use gpui::TestAppContext; use serde_json::json; - use util::paths::PathWithPosition; use workspace::{AppState, Workspace}; use crate::zed::{open_listener::open_workspace, tests::init_test}; @@ -665,12 +674,7 @@ mod tests { ) { let (response_tx, _) = ipc::channel::().unwrap(); - let path = PathBuf::from(path); - let workspace_paths = vec![PathWithPosition { - path, - row: None, - column: None, - }]; + let workspace_paths = vec![path.to_owned()]; let errored = cx .spawn(|mut cx| async move {