Change PathLikeWithPosition<P> 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
This commit is contained in:
Santeri Salmijärvi 2024-07-30 16:39:33 +03:00 committed by GitHub
parent 41c550cbe1
commit 13dcb42c1c
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
8 changed files with 184 additions and 270 deletions

View file

@ -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<PathLikeWithPosition<FileSearchQuery>>,
latest_search_query: Option<FileSearchQuery>,
currently_opened_path: Option<FoundPath>,
matches: Matches,
selected_index: usize,
@ -226,7 +226,7 @@ impl Matches {
&'a mut self,
history_items: impl IntoIterator<Item = &'a FoundPath> + Clone,
currently_opened: Option<&'a FoundPath>,
query: Option<&PathLikeWithPosition<FileSearchQuery>>,
query: Option<&FileSearchQuery>,
new_search_matches: impl Iterator<Item = ProjectPanelOrdMatch>,
extend_old_matches: bool,
) {
@ -303,7 +303,7 @@ impl Matches {
fn matching_history_item_paths<'a>(
history_items: impl IntoIterator<Item = &'a FoundPath>,
currently_opened: Option<&'a FoundPath>,
query: Option<&PathLikeWithPosition<FileSearchQuery>>,
query: Option<&FileSearchQuery>,
) -> HashMap<Arc<Path>, Option<ProjectPanelOrdMatch>> {
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<usize>,
path_position: PathWithPosition,
}
impl FileSearchQuery {
@ -456,7 +457,7 @@ impl FileFinderDelegate {
fn spawn_search(
&mut self,
query: PathLikeWithPosition<FileSearchQuery>,
query: FileSearchQuery,
cx: &mut ViewContext<Picker<Self>>,
) -> 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<FileSearchQuery>,
query: FileSearchQuery,
matches: impl IntoIterator<Item = ProjectPanelOrdMatch>,
cx: &mut ViewContext<Picker<Self>>,
) {
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<FileSearchQuery>,
query: FileSearchQuery,
cx: &mut ViewContext<'_, Picker<Self>>,
) -> 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();

View file

@ -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<AppState> {
})
}
fn test_path_like(test_str: &str) -> PathLikeWithPosition<FileSearchQuery> {
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(