From 13dcb42c1c62effb064568280e55720aa3da78d9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Santeri=20Salmij=C3=A4rvi?= Date: Tue, 30 Jul 2024 16:39:33 +0300 Subject: [PATCH] Change PathLikeWithPosition

into a non-generic type and replace ad-hoc Windows path parsing (#15373) This simplifies `PathWithPosition` by making the common use case concrete and removing the manual, incomplete Windows path parsing. Windows paths also don't get '/'s replaced by '\\'s anymore to limit the responsibility of the code to just parsing out the suffix and creating `PathBuf` from the rest. `Path::file_name()` is now used to extract the filename and potential suffix instead of manual parsing from the full input. This way e.g. Windows paths that begin with a drive letter are handled correctly without platform-specific hacks. Release Notes: - N/A --- crates/cli/src/main.rs | 10 +- crates/file_finder/src/file_finder.rs | 55 ++-- crates/file_finder/src/file_finder_tests.rs | 56 ++-- crates/recent_projects/src/dev_servers.rs | 4 +- crates/recent_projects/src/ssh_connections.rs | 6 +- crates/terminal_view/src/terminal_view.rs | 23 +- crates/util/src/paths.rs | 262 +++++++----------- crates/zed/src/zed/open_listener.rs | 38 +-- 8 files changed, 184 insertions(+), 270 deletions(-) diff --git a/crates/cli/src/main.rs b/crates/cli/src/main.rs index cd4c0b3868..6421b0876d 100644 --- a/crates/cli/src/main.rs +++ b/crates/cli/src/main.rs @@ -5,14 +5,13 @@ use clap::Parser; use cli::{ipc::IpcOneShotServer, CliRequest, CliResponse, IpcHandshake}; use parking_lot::Mutex; use std::{ - convert::Infallible, env, fs, io, path::{Path, PathBuf}, process::ExitStatus, sync::Arc, thread::{self, JoinHandle}, }; -use util::paths::PathLikeWithPosition; +use util::paths::PathWithPosition; struct Detect; @@ -54,13 +53,10 @@ struct Args { } fn parse_path_with_position(argument_str: &str) -> Result { - let path_like = PathLikeWithPosition::parse_str::(argument_str, |_, path_str| { - Ok(Path::new(path_str).to_path_buf()) - }) - .unwrap(); + let path = PathWithPosition::parse_str(argument_str); let curdir = env::current_dir()?; - let canonicalized = path_like.map_path_like(|path| match fs::canonicalize(&path) { + let canonicalized = path.map_path(|path| match fs::canonicalize(&path) { Ok(path) => Ok(path), Err(e) => { if let Some(mut parent) = path.parent() { diff --git a/crates/file_finder/src/file_finder.rs b/crates/file_finder/src/file_finder.rs index 9f7b163a31..54732d4bcb 100644 --- a/crates/file_finder/src/file_finder.rs +++ b/crates/file_finder/src/file_finder.rs @@ -28,7 +28,7 @@ use std::{ }; use text::Point; use ui::{prelude::*, HighlightedLabel, ListItem, ListItemSpacing}; -use util::{paths::PathLikeWithPosition, post_inc, ResultExt}; +use util::{paths::PathWithPosition, post_inc, ResultExt}; use workspace::{item::PreviewTabsSettings, ModalView, Workspace}; actions!(file_finder, [SelectPrev]); @@ -158,7 +158,7 @@ pub struct FileFinderDelegate { search_count: usize, latest_search_id: usize, latest_search_did_cancel: bool, - latest_search_query: Option>, + latest_search_query: Option, currently_opened_path: Option, matches: Matches, selected_index: usize, @@ -226,7 +226,7 @@ impl Matches { &'a mut self, history_items: impl IntoIterator + Clone, currently_opened: Option<&'a FoundPath>, - query: Option<&PathLikeWithPosition>, + query: Option<&FileSearchQuery>, new_search_matches: impl Iterator, extend_old_matches: bool, ) { @@ -303,7 +303,7 @@ impl Matches { fn matching_history_item_paths<'a>( history_items: impl IntoIterator, currently_opened: Option<&'a FoundPath>, - query: Option<&PathLikeWithPosition>, + query: Option<&FileSearchQuery>, ) -> HashMap, Option> { let Some(query) = query else { return history_items @@ -351,7 +351,7 @@ fn matching_history_item_paths<'a>( fuzzy::match_fixed_path_set( candidates, worktree.to_usize(), - query.path_like.path_query(), + query.path_query(), false, max_results, ) @@ -400,6 +400,7 @@ pub enum Event { struct FileSearchQuery { raw_query: String, file_query_end: Option, + path_position: PathWithPosition, } impl FileSearchQuery { @@ -456,7 +457,7 @@ impl FileFinderDelegate { fn spawn_search( &mut self, - query: PathLikeWithPosition, + query: FileSearchQuery, cx: &mut ViewContext>, ) -> Task<()> { let relative_to = self @@ -491,7 +492,7 @@ impl FileFinderDelegate { cx.spawn(|picker, mut cx| async move { let matches = fuzzy::match_path_sets( candidate_sets.as_slice(), - query.path_like.path_query(), + query.path_query(), relative_to, false, 100, @@ -516,18 +517,18 @@ impl FileFinderDelegate { &mut self, search_id: usize, did_cancel: bool, - query: PathLikeWithPosition, + query: FileSearchQuery, matches: impl IntoIterator, cx: &mut ViewContext>, ) { if search_id >= self.latest_search_id { self.latest_search_id = search_id; let extend_old_matches = self.latest_search_did_cancel - && Some(query.path_like.path_query()) + && Some(query.path_query()) == self .latest_search_query .as_ref() - .map(|query| query.path_like.path_query()); + .map(|query| query.path_query()); self.matches.push_new_matches( &self.history_items, self.currently_opened_path.as_ref(), @@ -658,7 +659,7 @@ impl FileFinderDelegate { fn lookup_absolute_path( &self, - query: PathLikeWithPosition, + query: FileSearchQuery, cx: &mut ViewContext<'_, Picker>, ) -> Task<()> { cx.spawn(|picker, mut cx| async move { @@ -672,7 +673,7 @@ impl FileFinderDelegate { return; }; - let query_path = Path::new(query.path_like.path_query()); + let query_path = Path::new(query.path_query()); let mut path_matches = Vec::new(); match fs.metadata(query_path).await.log_err() { Some(Some(_metadata)) => { @@ -796,20 +797,20 @@ impl PickerDelegate for FileFinderDelegate { cx.notify(); Task::ready(()) } else { - let query = - PathLikeWithPosition::parse_str(&raw_query, |normalized_query, path_like_str| { - Ok::<_, std::convert::Infallible>(FileSearchQuery { - raw_query: normalized_query.to_owned(), - file_query_end: if path_like_str == raw_query { - None - } else { - Some(path_like_str.len()) - }, - }) - }) - .expect("infallible"); + let path_position = PathWithPosition::parse_str(&raw_query); - if Path::new(query.path_like.path_query()).is_absolute() { + let query = FileSearchQuery { + raw_query: raw_query.trim().to_owned(), + file_query_end: if path_position.path.to_str().unwrap_or(raw_query) == raw_query { + None + } else { + // Safe to unwrap as we won't get here when the unwrap in if fails + Some(path_position.path.to_str().unwrap().len()) + }, + path_position, + }; + + if Path::new(query.path_query()).is_absolute() { self.lookup_absolute_path(query, cx) } else { self.spawn_search(query, cx) @@ -898,12 +899,12 @@ impl PickerDelegate for FileFinderDelegate { let row = self .latest_search_query .as_ref() - .and_then(|query| query.row) + .and_then(|query| query.path_position.row) .map(|row| row.saturating_sub(1)); let col = self .latest_search_query .as_ref() - .and_then(|query| query.column) + .and_then(|query| query.path_position.column) .unwrap_or(0) .saturating_sub(1); let finder = self.file_finder.clone(); diff --git a/crates/file_finder/src/file_finder_tests.rs b/crates/file_finder/src/file_finder_tests.rs index e2056f3831..c6ce27d4f4 100644 --- a/crates/file_finder/src/file_finder_tests.rs +++ b/crates/file_finder/src/file_finder_tests.rs @@ -226,13 +226,13 @@ async fn test_row_column_numbers_query_inside_file(cx: &mut TestAppContext) { .latest_search_query .as_ref() .expect("Finder should have a query after the update_matches call"); - assert_eq!(latest_search_query.path_like.raw_query, query_inside_file); + assert_eq!(latest_search_query.raw_query, query_inside_file); + assert_eq!(latest_search_query.file_query_end, Some(file_query.len())); + assert_eq!(latest_search_query.path_position.row, Some(file_row)); assert_eq!( - latest_search_query.path_like.file_query_end, - Some(file_query.len()) + latest_search_query.path_position.column, + Some(file_column as u32) ); - assert_eq!(latest_search_query.row, Some(file_row)); - assert_eq!(latest_search_query.column, Some(file_column as u32)); }); cx.dispatch_action(SelectNext); @@ -301,13 +301,13 @@ async fn test_row_column_numbers_query_outside_file(cx: &mut TestAppContext) { .latest_search_query .as_ref() .expect("Finder should have a query after the update_matches call"); - assert_eq!(latest_search_query.path_like.raw_query, query_outside_file); + assert_eq!(latest_search_query.raw_query, query_outside_file); + assert_eq!(latest_search_query.file_query_end, Some(file_query.len())); + assert_eq!(latest_search_query.path_position.row, Some(file_row)); assert_eq!( - latest_search_query.path_like.file_query_end, - Some(file_query.len()) + latest_search_query.path_position.column, + Some(file_column as u32) ); - assert_eq!(latest_search_query.row, Some(file_row)); - assert_eq!(latest_search_query.column, Some(file_column as u32)); }); cx.dispatch_action(SelectNext); @@ -357,7 +357,7 @@ async fn test_matching_cancellation(cx: &mut TestAppContext) { let (picker, _, cx) = build_find_picker(project, cx); - let query = test_path_like("hi"); + let query = test_path_position("hi"); picker .update(cx, |picker, cx| { picker.delegate.spawn_search(query.clone(), cx) @@ -450,7 +450,7 @@ async fn test_ignored_root(cx: &mut TestAppContext) { picker .update(cx, |picker, cx| { - picker.delegate.spawn_search(test_path_like("hi"), cx) + picker.delegate.spawn_search(test_path_position("hi"), cx) }) .await; picker.update(cx, |picker, _| assert_eq!(picker.delegate.matches.len(), 7)); @@ -478,7 +478,7 @@ async fn test_single_file_worktrees(cx: &mut TestAppContext) { // is included in the matching, because the worktree is a single file. picker .update(cx, |picker, cx| { - picker.delegate.spawn_search(test_path_like("thf"), cx) + picker.delegate.spawn_search(test_path_position("thf"), cx) }) .await; cx.read(|cx| { @@ -499,7 +499,7 @@ async fn test_single_file_worktrees(cx: &mut TestAppContext) { // not match anything. picker .update(cx, |f, cx| { - f.delegate.spawn_search(test_path_like("thf/"), cx) + f.delegate.spawn_search(test_path_position("thf/"), cx) }) .await; picker.update(cx, |f, _| assert_eq!(f.delegate.matches.len(), 0)); @@ -548,7 +548,7 @@ async fn test_path_distance_ordering(cx: &mut TestAppContext) { let finder = open_file_picker(&workspace, cx); finder .update(cx, |f, cx| { - f.delegate.spawn_search(test_path_like("a.txt"), cx) + f.delegate.spawn_search(test_path_position("a.txt"), cx) }) .await; @@ -581,7 +581,7 @@ async fn test_search_worktree_without_files(cx: &mut TestAppContext) { picker .update(cx, |f, cx| { - f.delegate.spawn_search(test_path_like("dir"), cx) + f.delegate.spawn_search(test_path_position("dir"), cx) }) .await; cx.read(|cx| { @@ -1854,18 +1854,18 @@ fn init_test(cx: &mut TestAppContext) -> Arc { }) } -fn test_path_like(test_str: &str) -> PathLikeWithPosition { - PathLikeWithPosition::parse_str(test_str, |normalized_query, path_like_str| { - Ok::<_, std::convert::Infallible>(FileSearchQuery { - raw_query: normalized_query.to_owned(), - file_query_end: if path_like_str == test_str { - None - } else { - Some(path_like_str.len()) - }, - }) - }) - .unwrap() +fn test_path_position(test_str: &str) -> FileSearchQuery { + let path_position = PathWithPosition::parse_str(&test_str); + + FileSearchQuery { + raw_query: test_str.to_owned(), + file_query_end: if path_position.path.to_str().unwrap() == test_str { + None + } else { + Some(path_position.path.to_str().unwrap().len()) + }, + path_position, + } } fn build_find_picker( diff --git a/crates/recent_projects/src/dev_servers.rs b/crates/recent_projects/src/dev_servers.rs index 4de3073195..b324e8e856 100644 --- a/crates/recent_projects/src/dev_servers.rs +++ b/crates/recent_projects/src/dev_servers.rs @@ -39,7 +39,7 @@ use ui::{ RadioWithLabel, Tooltip, }; use ui_input::{FieldLabelLayout, TextField}; -use util::paths::PathLikeWithPosition; +use util::paths::PathWithPosition; use util::ResultExt; use workspace::notifications::NotifyResultExt; use workspace::OpenOptions; @@ -991,7 +991,7 @@ impl DevServerProjects { project .paths .into_iter() - .map(|path| PathLikeWithPosition::from_path(PathBuf::from(path))) + .map(|path| PathWithPosition::from_path(PathBuf::from(path))) .collect(), app_state, OpenOptions::default(), diff --git a/crates/recent_projects/src/ssh_connections.rs b/crates/recent_projects/src/ssh_connections.rs index 55e823d1a0..3057096738 100644 --- a/crates/recent_projects/src/ssh_connections.rs +++ b/crates/recent_projects/src/ssh_connections.rs @@ -19,7 +19,7 @@ use ui::{ h_flex, v_flex, FluentBuilder as _, Icon, IconName, IconSize, InteractiveElement, IntoElement, Label, LabelCommon, Styled, StyledExt as _, ViewContext, VisualContext, WindowContext, }; -use util::paths::PathLikeWithPosition; +use util::paths::PathWithPosition; use workspace::{AppState, ModalView, Workspace}; #[derive(Deserialize)] @@ -345,7 +345,7 @@ pub fn connect_over_ssh( pub async fn open_ssh_project( connection_options: SshConnectionOptions, - paths: Vec>, + paths: Vec, app_state: Arc, _open_options: workspace::OpenOptions, cx: &mut AsyncAppContext, @@ -398,7 +398,7 @@ pub async fn open_ssh_project( for path in paths { project .update(cx, |project, cx| { - project.find_or_create_worktree(&path.path_like, true, cx) + project.find_or_create_worktree(&path.path, true, cx) })? .await?; } diff --git a/crates/terminal_view/src/terminal_view.rs b/crates/terminal_view/src/terminal_view.rs index e01585c4cc..a724d674da 100644 --- a/crates/terminal_view/src/terminal_view.rs +++ b/crates/terminal_view/src/terminal_view.rs @@ -26,7 +26,7 @@ use terminal::{ }; use terminal_element::{is_blank, TerminalElement}; use ui::{h_flex, prelude::*, ContextMenu, Icon, IconName, Label, Tooltip}; -use util::{paths::PathLikeWithPosition, ResultExt}; +use util::{paths::PathWithPosition, ResultExt}; use workspace::{ item::{BreadcrumbText, Item, ItemEvent, SerializableItem, TabContentParams}, notifications::NotifyResultExt, @@ -672,7 +672,7 @@ fn subscribe_for_terminal_events( .await; let paths_to_open = valid_files_to_open .iter() - .map(|(p, _)| p.path_like.clone()) + .map(|(p, _)| p.path.clone()) .collect(); let opened_items = task_workspace .update(&mut cx, |workspace, cx| { @@ -746,7 +746,7 @@ fn possible_open_paths_metadata( column: Option, potential_paths: HashSet, cx: &mut ViewContext, -) -> Task, Metadata)>> { +) -> Task> { cx.background_executor().spawn(async move { let mut paths_with_metadata = Vec::with_capacity(potential_paths.len()); @@ -755,8 +755,8 @@ fn possible_open_paths_metadata( .map(|potential_path| async { let metadata = fs.metadata(&potential_path).await.ok().flatten(); ( - PathLikeWithPosition { - path_like: potential_path, + PathWithPosition { + path: potential_path, row, column, }, @@ -781,14 +781,11 @@ fn possible_open_targets( cwd: &Option, maybe_path: &String, cx: &mut ViewContext, -) -> Task, Metadata)>> { - let path_like = PathLikeWithPosition::parse_str(maybe_path.as_str(), |_, path_str| { - Ok::<_, std::convert::Infallible>(Path::new(path_str).to_path_buf()) - }) - .expect("infallible"); - let row = path_like.row; - let column = path_like.column; - let maybe_path = path_like.path_like; +) -> Task> { + let path_position = PathWithPosition::parse_str(maybe_path.as_str()); + let row = path_position.row; + let column = path_position.column; + let maybe_path = path_position.path; let potential_abs_paths = if maybe_path.is_absolute() { HashSet::from_iter([maybe_path]) } else if maybe_path.starts_with("~") { diff --git a/crates/util/src/paths.rs b/crates/util/src/paths.rs index a038161ca4..5f6b808a69 100644 --- a/crates/util/src/paths.rs +++ b/crates/util/src/paths.rs @@ -96,59 +96,56 @@ pub const FILE_ROW_COLUMN_DELIMITER: char = ':'; /// A representation of a path-like string with optional row and column numbers. /// Matching values example: `te`, `test.rs:22`, `te:22:5`, etc. #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, Hash)] -pub struct PathLikeWithPosition

{ - pub path_like: P, +pub struct PathWithPosition { + pub path: PathBuf, pub row: Option, // Absent if row is absent. pub column: Option, } -impl

PathLikeWithPosition

{ - /// Returns a PathLikeWithPosition from a path. - pub fn from_path(path: P) -> Self { +impl PathWithPosition { + /// Returns a PathWithPosition from a path. + pub fn from_path(path: PathBuf) -> Self { Self { - path_like: path, + path, row: None, column: None, } } /// Parses a string that possibly has `:row:column` suffix. /// Ignores trailing `:`s, so `test.rs:22:` is parsed as `test.rs:22`. - /// If any of the row/column component parsing fails, the whole string is then parsed as a path like. - /// If on Windows, `s` will replace `/` with `\` for compatibility. - pub fn parse_str( - s: &str, - parse_path_like_str: impl Fn(&str, &str) -> Result, - ) -> Result { - #[cfg(target_os = "windows")] - let s = &s.replace('/', "\\"); - - let fallback = |fallback_str| { - Ok(Self { - path_like: parse_path_like_str(s, fallback_str)?, - row: None, - column: None, - }) + /// If the suffix parsing fails, the whole string is parsed as a path. + pub fn parse_str(s: &str) -> Self { + let fallback = |fallback_str| Self { + path: Path::new(fallback_str).to_path_buf(), + row: None, + column: None, }; let trimmed = s.trim(); - - #[cfg(target_os = "windows")] - { - let is_absolute = trimmed.starts_with(r"\\?\"); - if is_absolute { - return Self::parse_absolute_path(trimmed, |p| parse_path_like_str(s, p)); - } + let path = Path::new(trimmed); + let maybe_file_name_with_row_col = path + .file_name() + .unwrap_or_default() + .to_str() + .unwrap_or_default(); + if maybe_file_name_with_row_col.is_empty() { + return fallback(s); } - match trimmed.split_once(FILE_ROW_COLUMN_DELIMITER) { - Some((path_like_str, maybe_row_and_col_str)) => { - let path_like_str = path_like_str.trim(); + match maybe_file_name_with_row_col.split_once(FILE_ROW_COLUMN_DELIMITER) { + Some((file_name, maybe_row_and_col_str)) => { + let file_name = file_name.trim(); let maybe_row_and_col_str = maybe_row_and_col_str.trim(); - if path_like_str.is_empty() { - fallback(s) - } else if maybe_row_and_col_str.is_empty() { - fallback(path_like_str) + if file_name.is_empty() { + return fallback(s); + } + + let suffix_length = maybe_row_and_col_str.len() + 1; + let path_without_suffix = &trimmed[..trimmed.len() - suffix_length]; + + if maybe_row_and_col_str.is_empty() { + fallback(path_without_suffix) } else { let (row_parse_result, maybe_col_str) = match maybe_row_and_col_str.split_once(FILE_ROW_COLUMN_DELIMITER) { @@ -158,36 +155,38 @@ impl

PathLikeWithPosition

{ None => (maybe_row_and_col_str.parse::(), ""), }; + let path = Path::new(path_without_suffix).to_path_buf(); + match row_parse_result { Ok(row) => { if maybe_col_str.is_empty() { - Ok(Self { - path_like: parse_path_like_str(s, path_like_str)?, + Self { + path, row: Some(row), column: None, - }) + } } else { let (maybe_col_str, _) = maybe_col_str.split_once(':').unwrap_or((maybe_col_str, "")); match maybe_col_str.parse::() { - Ok(col) => Ok(Self { - path_like: parse_path_like_str(s, path_like_str)?, + Ok(col) => Self { + path, row: Some(row), column: Some(col), - }), - Err(_) => Ok(Self { - path_like: parse_path_like_str(s, path_like_str)?, + }, + Err(_) => Self { + path, row: Some(row), column: None, - }), + }, } } } - Err(_) => Ok(Self { - path_like: parse_path_like_str(s, path_like_str)?, + Err(_) => Self { + path, row: None, column: None, - }), + }, } } } @@ -195,79 +194,27 @@ impl

PathLikeWithPosition

{ } } - /// This helper function is used for parsing absolute paths on Windows. It exists because absolute paths on Windows are quite different from other platforms. See [this page](https://learn.microsoft.com/en-us/dotnet/standard/io/file-path-formats#dos-device-paths) for more information. - #[cfg(target_os = "windows")] - fn parse_absolute_path( - s: &str, - parse_path_like_str: impl Fn(&str) -> Result, - ) -> Result { - let fallback = |fallback_str| { - Ok(Self { - path_like: parse_path_like_str(fallback_str)?, - row: None, - column: None, - }) - }; - - let mut iterator = s.split(FILE_ROW_COLUMN_DELIMITER); - - let drive_prefix = iterator.next().unwrap_or_default(); - let file_path = iterator.next().unwrap_or_default(); - - // TODO: How to handle drives without a letter? UNC paths? - let complete_path = drive_prefix.replace("\\\\?\\", "") + ":" + &file_path; - - if let Some(row_str) = iterator.next() { - if let Some(column_str) = iterator.next() { - match row_str.parse::() { - Ok(row) => match column_str.parse::() { - Ok(col) => { - return Ok(Self { - path_like: parse_path_like_str(&complete_path)?, - row: Some(row), - column: Some(col), - }); - } - - Err(_) => { - return Ok(Self { - path_like: parse_path_like_str(&complete_path)?, - row: Some(row), - column: None, - }); - } - }, - - Err(_) => { - return fallback(&complete_path); - } - } - } - } - return fallback(&complete_path); - } - - pub fn map_path_like( + pub fn map_path( self, - mapping: impl FnOnce(P) -> Result, - ) -> Result, E> { - Ok(PathLikeWithPosition { - path_like: mapping(self.path_like)?, + mapping: impl FnOnce(PathBuf) -> Result, + ) -> Result { + Ok(PathWithPosition { + path: mapping(self.path)?, row: self.row, column: self.column, }) } - pub fn to_string(&self, path_like_to_string: impl Fn(&P) -> String) -> String { - let path_like_string = path_like_to_string(&self.path_like); + pub fn to_string(&self, path_to_string: impl Fn(&PathBuf) -> String) -> String { + let path_string = path_to_string(&self.path); if let Some(row) = self.row { if let Some(column) = self.column { - format!("{path_like_string}:{row}:{column}") + format!("{path_string}:{row}:{column}") } else { - format!("{path_like_string}:{row}") + format!("{path_string}:{row}") } } else { - path_like_string + path_string } } } @@ -335,38 +282,29 @@ impl PathMatcher { mod tests { use super::*; - type TestPath = PathLikeWithPosition<(String, String)>; - - fn parse_str(s: &str) -> TestPath { - TestPath::parse_str(s, |normalized, s| { - Ok::<_, std::convert::Infallible>((normalized.to_string(), s.to_string())) - }) - .expect("infallible") - } - #[test] fn path_with_position_parsing_positive() { let input_and_expected = [ ( "test_file.rs", - PathLikeWithPosition { - path_like: ("test_file.rs".to_string(), "test_file.rs".to_string()), + PathWithPosition { + path: PathBuf::from("test_file.rs"), row: None, column: None, }, ), ( "test_file.rs:1", - PathLikeWithPosition { - path_like: ("test_file.rs:1".to_string(), "test_file.rs".to_string()), + PathWithPosition { + path: PathBuf::from("test_file.rs"), row: Some(1), column: None, }, ), ( "test_file.rs:1:2", - PathLikeWithPosition { - path_like: ("test_file.rs:1:2".to_string(), "test_file.rs".to_string()), + PathWithPosition { + path: PathBuf::from("test_file.rs"), row: Some(1), column: Some(2), }, @@ -374,7 +312,7 @@ mod tests { ]; for (input, expected) in input_and_expected { - let actual = parse_str(input); + let actual = PathWithPosition::parse_str(input); assert_eq!( actual, expected, "For positive case input str '{input}', got a parse mismatch" @@ -394,11 +332,11 @@ mod tests { ("test_file.rs:1::2", Some(1), None), ("test_file.rs:1:2:3", Some(1), Some(2)), ] { - let actual = parse_str(input); + let actual = PathWithPosition::parse_str(input); assert_eq!( actual, - PathLikeWithPosition { - path_like: (input.to_string(), "test_file.rs".to_string()), + PathWithPosition { + path: PathBuf::from("test_file.rs"), row, column, }, @@ -414,27 +352,24 @@ mod tests { let input_and_expected = [ ( "test_file.rs:", - PathLikeWithPosition { - path_like: ("test_file.rs:".to_string(), "test_file.rs".to_string()), + PathWithPosition { + path: PathBuf::from("test_file.rs"), row: None, column: None, }, ), ( "test_file.rs:1:", - PathLikeWithPosition { - path_like: ("test_file.rs:1:".to_string(), "test_file.rs".to_string()), + PathWithPosition { + path: PathBuf::from("test_file.rs"), row: Some(1), column: None, }, ), ( "crates/file_finder/src/file_finder.rs:1902:13:", - PathLikeWithPosition { - path_like: ( - "crates/file_finder/src/file_finder.rs:1902:13:".to_string(), - "crates/file_finder/src/file_finder.rs".to_string(), - ), + PathWithPosition { + path: PathBuf::from("crates/file_finder/src/file_finder.rs"), row: Some(1902), column: Some(13), }, @@ -445,71 +380,64 @@ mod tests { let input_and_expected = [ ( "test_file.rs:", - PathLikeWithPosition { - path_like: ("test_file.rs:".to_string(), "test_file.rs".to_string()), + PathWithPosition { + path: PathBuf::from("test_file.rs"), row: None, column: None, }, ), ( "test_file.rs:1:", - PathLikeWithPosition { - path_like: ("test_file.rs:1:".to_string(), "test_file.rs".to_string()), + PathWithPosition { + path: PathBuf::from("test_file.rs"), row: Some(1), column: None, }, ), ( "\\\\?\\C:\\Users\\someone\\test_file.rs:1902:13:", - PathLikeWithPosition { - path_like: ( - "\\\\?\\C:\\Users\\someone\\test_file.rs:1902:13:".to_string(), - "C:\\Users\\someone\\test_file.rs".to_string(), - ), + PathWithPosition { + path: PathBuf::from("\\\\?\\C:\\Users\\someone\\test_file.rs"), row: Some(1902), column: Some(13), }, ), ( "\\\\?\\C:\\Users\\someone\\test_file.rs:1902:13:15:", - PathLikeWithPosition { - path_like: ( - "\\\\?\\C:\\Users\\someone\\test_file.rs:1902:13:15:".to_string(), - "C:\\Users\\someone\\test_file.rs".to_string(), - ), + PathWithPosition { + path: PathBuf::from("\\\\?\\C:\\Users\\someone\\test_file.rs"), row: Some(1902), column: Some(13), }, ), ( "\\\\?\\C:\\Users\\someone\\test_file.rs:1902:::15:", - PathLikeWithPosition { - path_like: ( - "\\\\?\\C:\\Users\\someone\\test_file.rs:1902:::15:".to_string(), - "C:\\Users\\someone\\test_file.rs".to_string(), - ), + PathWithPosition { + path: PathBuf::from("\\\\?\\C:\\Users\\someone\\test_file.rs"), row: Some(1902), column: None, }, ), + ( + "C:\\Users\\someone\\test_file.rs:1902:13:", + PathWithPosition { + path: PathBuf::from("C:\\Users\\someone\\test_file.rs"), + row: Some(1902), + column: Some(13), + }, + ), ( "crates/utils/paths.rs", - PathLikeWithPosition { - path_like: ( - "crates\\utils\\paths.rs".to_string(), - "crates\\utils\\paths.rs".to_string(), - ), + PathWithPosition { + path: PathBuf::from("crates\\utils\\paths.rs"), row: None, column: None, }, ), ( "crates/utils/paths.rs:101", - PathLikeWithPosition { - path_like: ( - "crates\\utils\\paths.rs:101".to_string(), - "crates\\utils\\paths.rs".to_string(), - ), + PathWithPosition { + path: PathBuf::from("crates\\utils\\paths.rs"), row: Some(101), column: None, }, @@ -517,7 +445,7 @@ mod tests { ]; for (input, expected) in input_and_expected { - let actual = parse_str(input); + let actual = PathWithPosition::parse_str(input); assert_eq!( actual, expected, "For special case input str '{input}', got a parse mismatch" diff --git a/crates/zed/src/zed/open_listener.rs b/crates/zed/src/zed/open_listener.rs index 307dd3294c..fb94f2430e 100644 --- a/crates/zed/src/zed/open_listener.rs +++ b/crates/zed/src/zed/open_listener.rs @@ -14,12 +14,10 @@ use futures::{FutureExt, SinkExt, StreamExt}; use gpui::{AppContext, AsyncAppContext, Global, WindowHandle}; use language::{Bias, Point}; use remote::SshConnectionOptions; -use std::path::Path; -use std::path::PathBuf; use std::sync::Arc; use std::time::Duration; use std::{process, thread}; -use util::paths::PathLikeWithPosition; +use util::paths::PathWithPosition; use util::ResultExt; use welcome::{show_welcome_view, FIRST_OPEN}; use workspace::item::ItemHandle; @@ -28,7 +26,7 @@ use workspace::{AppState, 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, @@ -58,11 +56,8 @@ impl OpenRequest { fn parse_file_path(&mut self, file: &str) { if let Some(decoded) = urlencoding::decode(file).log_err() { - if let Some(path_buf) = - PathLikeWithPosition::parse_str(&decoded, |_, s| PathBuf::try_from(s)).log_err() - { - self.open_paths.push(path_buf) - } + let path_buf = PathWithPosition::parse_str(&decoded); + self.open_paths.push(path_buf) } } @@ -193,7 +188,7 @@ fn connect_to_cli( } pub async fn open_paths_with_positions( - path_likes: &Vec>, + path_positions: &Vec, app_state: Arc, open_options: workspace::OpenOptions, cx: &mut AsyncAppContext, @@ -203,10 +198,10 @@ pub async fn open_paths_with_positions( )> { let mut caret_positions = HashMap::default(); - let paths = path_likes + let paths = path_positions .iter() .map(|path_with_position| { - let path = path_with_position.path_like.clone(); + let path = path_with_position.path.clone(); if let Some(row) = path_with_position.row { if path.is_file() { let row = row.saturating_sub(1); @@ -364,8 +359,8 @@ async fn open_workspaces( location .paths() .iter() - .map(|path| PathLikeWithPosition { - path_like: path.clone(), + .map(|path| PathWithPosition { + path: path.clone(), row: None, column: None, }) @@ -380,10 +375,7 @@ async fn open_workspaces( let paths_with_position = paths .into_iter() .map(|path_with_position_string| { - PathLikeWithPosition::parse_str(&path_with_position_string, |_, path_str| { - Ok::<_, std::convert::Infallible>(Path::new(path_str).to_path_buf()) - }) - .expect("Infallible") + PathWithPosition::parse_str(&path_with_position_string) }) .collect(); vec![paths_with_position] @@ -434,7 +426,7 @@ async fn open_workspaces( } async fn open_workspace( - workspace_paths: Vec>, + workspace_paths: Vec, open_new_workspace: Option, wait: bool, responses: &IpcSender, @@ -542,7 +534,7 @@ mod tests { use editor::Editor; use gpui::TestAppContext; use serde_json::json; - use util::paths::PathLikeWithPosition; + use util::paths::PathWithPosition; use workspace::{AppState, Workspace}; use crate::zed::{open_listener::open_workspace, tests::init_test}; @@ -656,9 +648,9 @@ mod tests { ) { let (response_tx, _) = ipc::channel::().unwrap(); - let path_like = PathBuf::from(path); - let workspace_paths = vec![PathLikeWithPosition { - path_like, + let path = PathBuf::from(path); + let workspace_paths = vec![PathWithPosition { + path, row: None, column: None, }];