From 14b26cbea583ef22edcea7d2afe4339b21325837 Mon Sep 17 00:00:00 2001 From: Cole Miller Date: Thu, 20 Feb 2025 15:56:08 -0500 Subject: [PATCH 1/8] Move test --- crates/file_finder/src/file_finder.rs | 108 -------------------- crates/file_finder/src/file_finder_tests.rs | 103 +++++++++++++++++++ 2 files changed, 103 insertions(+), 108 deletions(-) diff --git a/crates/file_finder/src/file_finder.rs b/crates/file_finder/src/file_finder.rs index 74ff2e542f64ac..ee30919c0f169f 100644 --- a/crates/file_finder/src/file_finder.rs +++ b/crates/file_finder/src/file_finder.rs @@ -1344,111 +1344,3 @@ impl PickerDelegate for FileFinderDelegate { ) } } - -#[cfg(test)] -mod tests { - use super::*; - - #[test] - fn test_custom_project_search_ordering_in_file_finder() { - let mut file_finder_sorted_output = vec![ - ProjectPanelOrdMatch(PathMatch { - score: 0.5, - positions: Vec::new(), - worktree_id: 0, - path: Arc::from(Path::new("b0.5")), - path_prefix: Arc::default(), - distance_to_relative_ancestor: 0, - is_dir: false, - }), - ProjectPanelOrdMatch(PathMatch { - score: 1.0, - positions: Vec::new(), - worktree_id: 0, - path: Arc::from(Path::new("c1.0")), - path_prefix: Arc::default(), - distance_to_relative_ancestor: 0, - is_dir: false, - }), - ProjectPanelOrdMatch(PathMatch { - score: 1.0, - positions: Vec::new(), - worktree_id: 0, - path: Arc::from(Path::new("a1.0")), - path_prefix: Arc::default(), - distance_to_relative_ancestor: 0, - is_dir: false, - }), - ProjectPanelOrdMatch(PathMatch { - score: 0.5, - positions: Vec::new(), - worktree_id: 0, - path: Arc::from(Path::new("a0.5")), - path_prefix: Arc::default(), - distance_to_relative_ancestor: 0, - is_dir: false, - }), - ProjectPanelOrdMatch(PathMatch { - score: 1.0, - positions: Vec::new(), - worktree_id: 0, - path: Arc::from(Path::new("b1.0")), - path_prefix: Arc::default(), - distance_to_relative_ancestor: 0, - is_dir: false, - }), - ]; - file_finder_sorted_output.sort_by(|a, b| b.cmp(a)); - - assert_eq!( - file_finder_sorted_output, - vec![ - ProjectPanelOrdMatch(PathMatch { - score: 1.0, - positions: Vec::new(), - worktree_id: 0, - path: Arc::from(Path::new("a1.0")), - path_prefix: Arc::default(), - distance_to_relative_ancestor: 0, - is_dir: false, - }), - ProjectPanelOrdMatch(PathMatch { - score: 1.0, - positions: Vec::new(), - worktree_id: 0, - path: Arc::from(Path::new("b1.0")), - path_prefix: Arc::default(), - distance_to_relative_ancestor: 0, - is_dir: false, - }), - ProjectPanelOrdMatch(PathMatch { - score: 1.0, - positions: Vec::new(), - worktree_id: 0, - path: Arc::from(Path::new("c1.0")), - path_prefix: Arc::default(), - distance_to_relative_ancestor: 0, - is_dir: false, - }), - ProjectPanelOrdMatch(PathMatch { - score: 0.5, - positions: Vec::new(), - worktree_id: 0, - path: Arc::from(Path::new("a0.5")), - path_prefix: Arc::default(), - distance_to_relative_ancestor: 0, - is_dir: false, - }), - ProjectPanelOrdMatch(PathMatch { - score: 0.5, - positions: Vec::new(), - worktree_id: 0, - path: Arc::from(Path::new("b0.5")), - path_prefix: Arc::default(), - distance_to_relative_ancestor: 0, - is_dir: false, - }), - ] - ); - } -} diff --git a/crates/file_finder/src/file_finder_tests.rs b/crates/file_finder/src/file_finder_tests.rs index f14106d62af035..90e1745fa3fc65 100644 --- a/crates/file_finder/src/file_finder_tests.rs +++ b/crates/file_finder/src/file_finder_tests.rs @@ -16,6 +16,109 @@ fn init_logger() { } } +#[test] +fn test_custom_project_search_ordering_in_file_finder() { + let mut file_finder_sorted_output = vec![ + ProjectPanelOrdMatch(PathMatch { + score: 0.5, + positions: Vec::new(), + worktree_id: 0, + path: Arc::from(Path::new("b0.5")), + path_prefix: Arc::default(), + distance_to_relative_ancestor: 0, + is_dir: false, + }), + ProjectPanelOrdMatch(PathMatch { + score: 1.0, + positions: Vec::new(), + worktree_id: 0, + path: Arc::from(Path::new("c1.0")), + path_prefix: Arc::default(), + distance_to_relative_ancestor: 0, + is_dir: false, + }), + ProjectPanelOrdMatch(PathMatch { + score: 1.0, + positions: Vec::new(), + worktree_id: 0, + path: Arc::from(Path::new("a1.0")), + path_prefix: Arc::default(), + distance_to_relative_ancestor: 0, + is_dir: false, + }), + ProjectPanelOrdMatch(PathMatch { + score: 0.5, + positions: Vec::new(), + worktree_id: 0, + path: Arc::from(Path::new("a0.5")), + path_prefix: Arc::default(), + distance_to_relative_ancestor: 0, + is_dir: false, + }), + ProjectPanelOrdMatch(PathMatch { + score: 1.0, + positions: Vec::new(), + worktree_id: 0, + path: Arc::from(Path::new("b1.0")), + path_prefix: Arc::default(), + distance_to_relative_ancestor: 0, + is_dir: false, + }), + ]; + file_finder_sorted_output.sort_by(|a, b| b.cmp(a)); + + assert_eq!( + file_finder_sorted_output, + vec![ + ProjectPanelOrdMatch(PathMatch { + score: 1.0, + positions: Vec::new(), + worktree_id: 0, + path: Arc::from(Path::new("a1.0")), + path_prefix: Arc::default(), + distance_to_relative_ancestor: 0, + is_dir: false, + }), + ProjectPanelOrdMatch(PathMatch { + score: 1.0, + positions: Vec::new(), + worktree_id: 0, + path: Arc::from(Path::new("b1.0")), + path_prefix: Arc::default(), + distance_to_relative_ancestor: 0, + is_dir: false, + }), + ProjectPanelOrdMatch(PathMatch { + score: 1.0, + positions: Vec::new(), + worktree_id: 0, + path: Arc::from(Path::new("c1.0")), + path_prefix: Arc::default(), + distance_to_relative_ancestor: 0, + is_dir: false, + }), + ProjectPanelOrdMatch(PathMatch { + score: 0.5, + positions: Vec::new(), + worktree_id: 0, + path: Arc::from(Path::new("a0.5")), + path_prefix: Arc::default(), + distance_to_relative_ancestor: 0, + is_dir: false, + }), + ProjectPanelOrdMatch(PathMatch { + score: 0.5, + positions: Vec::new(), + worktree_id: 0, + path: Arc::from(Path::new("b0.5")), + path_prefix: Arc::default(), + distance_to_relative_ancestor: 0, + is_dir: false, + }), + ] + ); +} + #[gpui::test] async fn test_matching_paths(cx: &mut TestAppContext) { let app_state = init_test(cx); From d1ec768b8358ad1fc534e37999241a08daff9aa1 Mon Sep 17 00:00:00 2001 From: Cole Miller Date: Thu, 20 Feb 2025 20:55:54 -0500 Subject: [PATCH 2/8] Checkpoint --- crates/file_finder/src/file_finder.rs | 288 +++++++++++++++----- crates/file_finder/src/file_finder_tests.rs | 32 +++ 2 files changed, 248 insertions(+), 72 deletions(-) diff --git a/crates/file_finder/src/file_finder.rs b/crates/file_finder/src/file_finder.rs index ee30919c0f169f..47040307e54a46 100644 --- a/crates/file_finder/src/file_finder.rs +++ b/crates/file_finder/src/file_finder.rs @@ -24,8 +24,10 @@ use picker::{Picker, PickerDelegate}; use project::{PathMatchCandidateSet, Project, ProjectPath, WorktreeId}; use settings::Settings; use std::{ + borrow::Cow, cmp, - path::{Path, PathBuf}, + ops::Range, + path::{Component, Path, PathBuf}, sync::{ atomic::{self, AtomicBool}, Arc, @@ -36,7 +38,7 @@ use ui::{ prelude::*, ContextMenu, HighlightedLabel, ListItem, ListItemSpacing, PopoverMenu, PopoverMenuHandle, }; -use util::{paths::PathWithPosition, post_inc, ResultExt}; +use util::{maybe, paths::PathWithPosition, post_inc, ResultExt}; use workspace::{ item::PreviewTabsSettings, notifications::NotifyResultExt, pane, ModalView, SplitDirection, Workspace, @@ -805,25 +807,28 @@ impl FileFinderDelegate { fn labels_for_match( &self, path_match: &Match, + window: &mut Window, cx: &App, ix: usize, - ) -> (String, Vec, String, Vec) { - let (file_name, file_name_positions, full_path, full_path_positions) = match &path_match { - Match::History { - path: entry_path, - panel_match, - } => { - let worktree_id = entry_path.project.worktree_id; - let project_relative_path = &entry_path.project.path; - let has_worktree = self - .project - .read(cx) - .worktree_for_id(worktree_id, cx) - .is_some(); - - if !has_worktree { - if let Some(absolute_path) = &entry_path.absolute { - return ( + ) -> (HighlightedLabel, HighlightedLabel) { + let (file_name, file_name_positions, mut full_path, mut full_path_positions) = + match &path_match { + Match::History { + path: entry_path, + panel_match, + } => { + let worktree_id = entry_path.project.worktree_id; + let project_relative_path = &entry_path.project.path; + let has_worktree = self + .project + .read(cx) + .worktree_for_id(worktree_id, cx) + .is_some(); + + if let Some(absolute_path) = + entry_path.absolute.as_ref().filter(|_| !has_worktree) + { + ( absolute_path .file_name() .map_or_else( @@ -834,58 +839,90 @@ impl FileFinderDelegate { Vec::new(), absolute_path.to_string_lossy().to_string(), Vec::new(), - ); - } - } + ) + } else { + let mut path = Arc::clone(project_relative_path); + if project_relative_path.as_ref() == Path::new("") { + if let Some(absolute_path) = &entry_path.absolute { + path = Arc::from(absolute_path.as_path()); + } + } - let mut path = Arc::clone(project_relative_path); - if project_relative_path.as_ref() == Path::new("") { - if let Some(absolute_path) = &entry_path.absolute { - path = Arc::from(absolute_path.as_path()); - } - } + let mut path_match = PathMatch { + score: ix as f64, + positions: Vec::new(), + worktree_id: worktree_id.to_usize(), + path, + is_dir: false, // File finder doesn't support directories + path_prefix: "".into(), + distance_to_relative_ancestor: usize::MAX, + }; + if let Some(found_path_match) = &panel_match { + path_match + .positions + .extend(found_path_match.0.positions.iter()) + } - let mut path_match = PathMatch { - score: ix as f64, - positions: Vec::new(), - worktree_id: worktree_id.to_usize(), - path, - is_dir: false, // File finder doesn't support directories - path_prefix: "".into(), - distance_to_relative_ancestor: usize::MAX, - }; - if let Some(found_path_match) = &panel_match { - path_match - .positions - .extend(found_path_match.0.positions.iter()) + self.labels_for_path_match(&path_match) + } } - - self.labels_for_path_match(&path_match) - } - Match::Search(path_match) => self.labels_for_path_match(&path_match.0), - }; + Match::Search(path_match) => self.labels_for_path_match(&path_match.0), + }; if file_name_positions.is_empty() { if let Some(user_home_path) = std::env::var("HOME").ok() { let user_home_path = user_home_path.trim(); if !user_home_path.is_empty() { if (&full_path).starts_with(user_home_path) { - return ( - file_name, - file_name_positions, - full_path.replace(user_home_path, "~"), - full_path_positions, - ); + full_path.replace_range(0..user_home_path.len(), "~"); + full_path_positions.retain_mut(|pos| { + if *pos >= user_home_path.len() { + *pos -= user_home_path.len(); + *pos += 1; + true + } else { + false + } + }) } } } } + if full_path.is_ascii() { + let file_finder_settings = FileFinderSettings::get_global(cx); + let max_width = + FileFinder::modal_max_width(file_finder_settings.modal_max_width, window); + let (normal_em, small_em) = { + let style = window.text_style(); + let font_id = window.text_system().resolve_font(&style.font()); + let font_size = TextSize::Default.rems(cx).to_pixels(window.rem_size()); + let normal = cx + .text_system() + .em_width(font_id, font_size) + .unwrap_or(px(16.)); + let font_size = TextSize::Small.rems(cx).to_pixels(window.rem_size()); + let small = cx + .text_system() + .em_width(font_id, font_size) + .unwrap_or(px(10.)); + (normal, small) + }; + let estimated_width = + px(file_name.len() as f32) * normal_em + px(full_path.len() as f32) * small_em; + + if estimated_width > max_width && full_path.chars().all(|c| c.is_ascii()) { + let mut components = PathComponentSlice::new(&full_path); + components.elide(&mut full_path_positions); + full_path = std::mem::take(components.path_str.to_mut()); + } + } + ( - file_name, - file_name_positions, - full_path, - full_path_positions, + HighlightedLabel::new(file_name, file_name_positions), + HighlightedLabel::new(full_path, full_path_positions) + .size(LabelSize::Small) + .color(Color::Muted), ) } @@ -1249,7 +1286,7 @@ impl PickerDelegate for FileFinderDelegate { &self, ix: usize, selected: bool, - _: &mut Window, + window: &mut Window, cx: &mut Context>, ) -> Option { let settings = FileFinderSettings::get_global(cx); @@ -1269,16 +1306,16 @@ impl PickerDelegate for FileFinderDelegate { .size(IconSize::Small.rems()) .into_any_element(), }; - let (file_name, file_name_positions, full_path, full_path_positions) = - self.labels_for_match(path_match, cx, ix); + let (file_name_label, full_path_label) = self.labels_for_match(path_match, window, cx, ix); - let file_icon = if settings.file_icons { - FileIcons::get_icon(Path::new(&file_name), cx) - .map(Icon::from_path) - .map(|icon| icon.color(Color::Muted)) - } else { - None - }; + let file_icon = maybe!({ + if !settings.file_icons { + return None; + } + let file_name = path_match.path().file_name()?; + let icon = FileIcons::get_icon(file_name.as_ref(), cx)?; + Some(Icon::from_path(icon).color(Color::Muted)) + }); Some( ListItem::new(ix) @@ -1291,12 +1328,8 @@ impl PickerDelegate for FileFinderDelegate { h_flex() .gap_2() .py_px() - .child(HighlightedLabel::new(file_name, file_name_positions)) - .child( - HighlightedLabel::new(full_path, full_path_positions) - .size(LabelSize::Small) - .color(Color::Muted), - ), + .child(file_name_label) + .child(full_path_label), ), ) } @@ -1344,3 +1377,114 @@ impl PickerDelegate for FileFinderDelegate { ) } } + +#[derive(Clone, Debug, PartialEq, Eq)] +struct PathComponentSlice<'a> { + path: Cow<'a, Path>, + path_str: Cow<'a, str>, + component_ranges: Vec<(Component<'a>, Range)>, +} + +impl<'a> PathComponentSlice<'a> { + fn new(path: &'a str) -> Self { + let trimmed_path = Path::new(path).components().as_path().as_os_str(); + let mut component_ranges = Vec::new(); + let mut components = Path::new(trimmed_path).components(); + let len = trimmed_path.as_encoded_bytes().len(); + let mut pos = 0; + while let Some(component) = components.next() { + component_ranges.push((component, pos..0)); + pos = len - components.as_path().as_os_str().as_encoded_bytes().len(); + } + for ((_, range), ancestor) in component_ranges + .iter_mut() + .rev() + .zip(Path::new(trimmed_path).ancestors()) + { + range.end = ancestor.as_os_str().as_encoded_bytes().len(); + } + Self { + path: Cow::Borrowed(Path::new(path)), + path_str: Cow::Borrowed(path), + component_ranges, + } + } + + fn elide(&mut self, matches: &mut Vec) { + let eligible_range = { + assert!(matches.windows(2).all(|w| w[0] <= w[1])); + let mut matches = matches.iter().copied().peekable(); + let mut longest: Option> = None; + let mut cur = 0..0; + let mut seen_normal = false; + for (i, (component, range)) in self.component_ranges.iter().enumerate() { + let is_normal = matches!(component, Component::Normal(_)); + let is_first_normal = is_normal && !seen_normal; + seen_normal |= is_normal; + let is_last = i == self.component_ranges.len() - 1; + let contains_match = matches.peek().is_some_and(|mat| range.contains(mat)); + if contains_match { + matches.next(); + } + if is_first_normal || is_last || !is_normal || contains_match { + if !longest + .as_ref() + .is_some_and(|old| old.end - old.start > cur.end - cur.start) + { + longest = Some(cur); + } + cur = i + 1..i + 1; + } else { + cur.end = i + 1; + } + } + if !longest + .as_ref() + .is_some_and(|old| old.end - old.start > cur.end - cur.start) + { + longest = Some(cur); + } + longest + }; + + let Some(eligible_range) = eligible_range else { + return; + }; + assert!(eligible_range.start <= eligible_range.end); + if eligible_range.is_empty() { + return; + } + let elided_component_range = eligible_range.start..=eligible_range.end - 1; + let eligible_path_range = self.component_ranges[*elided_component_range.start()] + .1 + .start + ..self.component_ranges[*elided_component_range.end()].1.end; + + let elided_len = eligible_path_range.end - eligible_path_range.start; + let placeholder = "…"; + self.component_ranges.retain_mut(|(_, range)| { + if range.start >= eligible_path_range.end { + range.start -= elided_len; + range.start += placeholder.len(); + range.end -= elided_len; + range.end += placeholder.len(); + } else if range.end >= eligible_path_range.start { + return false; + } + true + }); + matches.retain_mut(|mat| { + if *mat >= eligible_path_range.end { + *mat -= elided_len; + *mat += placeholder.len(); + } else if *mat >= eligible_path_range.start { + return false; + } + true + }); + self.path_str + .to_mut() + .replace_range(eligible_path_range, placeholder); + self.path = Path::new(self.path_str.as_ref()).to_owned().into(); + } +} diff --git a/crates/file_finder/src/file_finder_tests.rs b/crates/file_finder/src/file_finder_tests.rs index 90e1745fa3fc65..fde6a5aa994253 100644 --- a/crates/file_finder/src/file_finder_tests.rs +++ b/crates/file_finder/src/file_finder_tests.rs @@ -16,6 +16,38 @@ fn init_logger() { } } +#[test] +fn test_path_component_slice() { + #[track_caller] + fn check( + path: &str, + matches: impl IntoIterator, + expected: &str, + expected_matches: impl IntoIterator, + ) { + let mut slice = PathComponentSlice::new(path); + let mut matches = Vec::from_iter(matches); + slice.elide(&mut matches); + assert_eq!(slice.path_str, expected); + assert_eq!(matches, Vec::from_iter(expected_matches)); + } + + check("p/a/b/c/d/", [], "p/…/d/", []); + check("p/a/b/c/d/", [2, 4, 6], "p/a/b/c/d/", [2, 4, 6]); + check("p/a/b/c/d/", [2, 6], "p/a/…/c/d/", [2, 8]); + check("p/a/b/c/d/", [6], "p/…/c/d/", [6]); + + check("p/a/b/c/d", [], "p/…/d", []); + check("p/a/b/c/d", [2, 4, 6], "p/a/b/c/d", [2, 4, 6]); + check("p/a/b/c/d", [2, 6], "p/a/…/c/d", [2, 8]); + check("p/a/b/c/d", [6], "p/…/c/d", [6]); + + check("/p/a/b/c/d/", [], "/p/…/d/", []); + check("/p/a/b/c/d/", [3, 5, 7], "/p/a/b/c/d/", [3, 5, 7]); + check("/p/a/b/c/d/", [3, 7], "/p/a/…/c/d/", [3, 9]); + check("/p/a/b/c/d/", [7], "/p/…/c/d/", [7]); +} + #[test] fn test_custom_project_search_ordering_in_file_finder() { let mut file_finder_sorted_output = vec![ From 86f2dc52e20e9ed50b23710cdba488e455d6e582 Mon Sep 17 00:00:00 2001 From: Cole Miller Date: Thu, 20 Feb 2025 21:13:58 -0500 Subject: [PATCH 3/8] Em widths --- crates/file_finder/src/file_finder.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/file_finder/src/file_finder.rs b/crates/file_finder/src/file_finder.rs index 47040307e54a46..67c69fc3c044dd 100644 --- a/crates/file_finder/src/file_finder.rs +++ b/crates/file_finder/src/file_finder.rs @@ -908,8 +908,8 @@ impl FileFinderDelegate { .unwrap_or(px(10.)); (normal, small) }; - let estimated_width = - px(file_name.len() as f32) * normal_em + px(full_path.len() as f32) * small_em; + let estimated_width = px(0.8) + * (px(file_name.len() as f32) * normal_em + px(full_path.len() as f32) * small_em); if estimated_width > max_width && full_path.chars().all(|c| c.is_ascii()) { let mut components = PathComponentSlice::new(&full_path); From 4ec69188394536c92ccccc84324fe364dd1b0f55 Mon Sep 17 00:00:00 2001 From: Cole Miller Date: Thu, 20 Feb 2025 21:27:35 -0500 Subject: [PATCH 4/8] Refactor --- crates/file_finder/src/file_finder.rs | 56 ++++++++------------- crates/file_finder/src/file_finder_tests.rs | 41 +++++++-------- 2 files changed, 39 insertions(+), 58 deletions(-) diff --git a/crates/file_finder/src/file_finder.rs b/crates/file_finder/src/file_finder.rs index 67c69fc3c044dd..59400bc5605200 100644 --- a/crates/file_finder/src/file_finder.rs +++ b/crates/file_finder/src/file_finder.rs @@ -912,9 +912,21 @@ impl FileFinderDelegate { * (px(file_name.len() as f32) * normal_em + px(full_path.len() as f32) * small_em); if estimated_width > max_width && full_path.chars().all(|c| c.is_ascii()) { - let mut components = PathComponentSlice::new(&full_path); - components.elide(&mut full_path_positions); - full_path = std::mem::take(components.path_str.to_mut()); + let components = PathComponentSlice::new(&full_path); + if let Some(elided_range) = components.elision_range(&full_path_positions) { + let elided_len = elided_range.end - elided_range.start; + let placeholder = "…"; + full_path_positions.retain_mut(|mat| { + if *mat >= elided_range.end { + *mat -= elided_len; + *mat += placeholder.len(); + } else if *mat >= elided_range.start { + return false; + } + true + }); + full_path.replace_range(elided_range, placeholder); + } } } @@ -1410,7 +1422,7 @@ impl<'a> PathComponentSlice<'a> { } } - fn elide(&mut self, matches: &mut Vec) { + fn elision_range(&self, matches: &[usize]) -> Option> { let eligible_range = { assert!(matches.windows(2).all(|w| w[0] <= w[1])); let mut matches = matches.iter().copied().peekable(); @@ -1447,44 +1459,16 @@ impl<'a> PathComponentSlice<'a> { longest }; - let Some(eligible_range) = eligible_range else { - return; - }; + let eligible_range = eligible_range?; assert!(eligible_range.start <= eligible_range.end); if eligible_range.is_empty() { - return; + return None; } let elided_component_range = eligible_range.start..=eligible_range.end - 1; - let eligible_path_range = self.component_ranges[*elided_component_range.start()] + let elided_range = self.component_ranges[*elided_component_range.start()] .1 .start ..self.component_ranges[*elided_component_range.end()].1.end; - - let elided_len = eligible_path_range.end - eligible_path_range.start; - let placeholder = "…"; - self.component_ranges.retain_mut(|(_, range)| { - if range.start >= eligible_path_range.end { - range.start -= elided_len; - range.start += placeholder.len(); - range.end -= elided_len; - range.end += placeholder.len(); - } else if range.end >= eligible_path_range.start { - return false; - } - true - }); - matches.retain_mut(|mat| { - if *mat >= eligible_path_range.end { - *mat -= elided_len; - *mat += placeholder.len(); - } else if *mat >= eligible_path_range.start { - return false; - } - true - }); - self.path_str - .to_mut() - .replace_range(eligible_path_range, placeholder); - self.path = Path::new(self.path_str.as_ref()).to_owned().into(); + Some(elided_range) } } diff --git a/crates/file_finder/src/file_finder_tests.rs b/crates/file_finder/src/file_finder_tests.rs index fde6a5aa994253..5b58dfadc720a1 100644 --- a/crates/file_finder/src/file_finder_tests.rs +++ b/crates/file_finder/src/file_finder_tests.rs @@ -19,33 +19,30 @@ fn init_logger() { #[test] fn test_path_component_slice() { #[track_caller] - fn check( - path: &str, - matches: impl IntoIterator, - expected: &str, - expected_matches: impl IntoIterator, - ) { - let mut slice = PathComponentSlice::new(path); + fn check(path: &str, matches: impl IntoIterator, expected: &str) { + let mut path = path.to_owned(); + let slice = PathComponentSlice::new(&path); let mut matches = Vec::from_iter(matches); - slice.elide(&mut matches); - assert_eq!(slice.path_str, expected); - assert_eq!(matches, Vec::from_iter(expected_matches)); + if let Some(range) = slice.elision_range(&mut matches) { + path.replace_range(range, "…"); + } + assert_eq!(path, expected); } - check("p/a/b/c/d/", [], "p/…/d/", []); - check("p/a/b/c/d/", [2, 4, 6], "p/a/b/c/d/", [2, 4, 6]); - check("p/a/b/c/d/", [2, 6], "p/a/…/c/d/", [2, 8]); - check("p/a/b/c/d/", [6], "p/…/c/d/", [6]); + check("p/a/b/c/d/", [], "p/…/d/"); + check("p/a/b/c/d/", [2, 4, 6], "p/a/b/c/d/"); + check("p/a/b/c/d/", [2, 6], "p/a/…/c/d/"); + check("p/a/b/c/d/", [6], "p/…/c/d/"); - check("p/a/b/c/d", [], "p/…/d", []); - check("p/a/b/c/d", [2, 4, 6], "p/a/b/c/d", [2, 4, 6]); - check("p/a/b/c/d", [2, 6], "p/a/…/c/d", [2, 8]); - check("p/a/b/c/d", [6], "p/…/c/d", [6]); + check("p/a/b/c/d", [], "p/…/d"); + check("p/a/b/c/d", [2, 4, 6], "p/a/b/c/d"); + check("p/a/b/c/d", [2, 6], "p/a/…/c/d"); + check("p/a/b/c/d", [6], "p/…/c/d"); - check("/p/a/b/c/d/", [], "/p/…/d/", []); - check("/p/a/b/c/d/", [3, 5, 7], "/p/a/b/c/d/", [3, 5, 7]); - check("/p/a/b/c/d/", [3, 7], "/p/a/…/c/d/", [3, 9]); - check("/p/a/b/c/d/", [7], "/p/…/c/d/", [7]); + check("/p/a/b/c/d/", [], "/p/…/d/"); + check("/p/a/b/c/d/", [3, 5, 7], "/p/a/b/c/d/"); + check("/p/a/b/c/d/", [3, 7], "/p/a/…/c/d/"); + check("/p/a/b/c/d/", [7], "/p/…/c/d/"); } #[test] From 3efdf56f7e6b4ac5d660a80ae8463584e9d1b7a9 Mon Sep 17 00:00:00 2001 From: Cole Miller Date: Thu, 20 Feb 2025 21:30:19 -0500 Subject: [PATCH 5/8] Don't elide unless it works --- crates/file_finder/src/file_finder.rs | 27 ++++++++++++++++----------- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/crates/file_finder/src/file_finder.rs b/crates/file_finder/src/file_finder.rs index 59400bc5605200..04919ef6ef45d9 100644 --- a/crates/file_finder/src/file_finder.rs +++ b/crates/file_finder/src/file_finder.rs @@ -915,17 +915,22 @@ impl FileFinderDelegate { let components = PathComponentSlice::new(&full_path); if let Some(elided_range) = components.elision_range(&full_path_positions) { let elided_len = elided_range.end - elided_range.start; - let placeholder = "…"; - full_path_positions.retain_mut(|mat| { - if *mat >= elided_range.end { - *mat -= elided_len; - *mat += placeholder.len(); - } else if *mat >= elided_range.start { - return false; - } - true - }); - full_path.replace_range(elided_range, placeholder); + let estimated_width = px(0.8) + * (px(file_name.len() as f32) * normal_em + + px((full_path.len() - elided_len) as f32) * small_em); + if estimated_width <= max_width { + let placeholder = "…"; + full_path_positions.retain_mut(|mat| { + if *mat >= elided_range.end { + *mat -= elided_len; + *mat += placeholder.len(); + } else if *mat >= elided_range.start { + return false; + } + true + }); + full_path.replace_range(elided_range, placeholder); + } } } } From 2379029288b7ca7868ef593c56ffa9d48aefd604 Mon Sep 17 00:00:00 2001 From: Cole Miller Date: Thu, 20 Feb 2025 22:27:36 -0500 Subject: [PATCH 6/8] Elide no more components than needed --- crates/file_finder/src/file_finder.rs | 91 ++++++++++++++------- crates/file_finder/src/file_finder_tests.rs | 5 +- 2 files changed, 66 insertions(+), 30 deletions(-) diff --git a/crates/file_finder/src/file_finder.rs b/crates/file_finder/src/file_finder.rs index 04919ef6ef45d9..931561ac7315cf 100644 --- a/crates/file_finder/src/file_finder.rs +++ b/crates/file_finder/src/file_finder.rs @@ -908,29 +908,24 @@ impl FileFinderDelegate { .unwrap_or(px(10.)); (normal, small) }; - let estimated_width = px(0.8) - * (px(file_name.len() as f32) * normal_em + px(full_path.len() as f32) * small_em); - - if estimated_width > max_width && full_path.chars().all(|c| c.is_ascii()) { + let budget = full_path_budget(&file_name, normal_em, small_em, max_width); + if full_path.len() > budget { let components = PathComponentSlice::new(&full_path); - if let Some(elided_range) = components.elision_range(&full_path_positions) { + if let Some(elided_range) = + components.elision_range(budget - 1, &full_path_positions) + { let elided_len = elided_range.end - elided_range.start; - let estimated_width = px(0.8) - * (px(file_name.len() as f32) * normal_em - + px((full_path.len() - elided_len) as f32) * small_em); - if estimated_width <= max_width { - let placeholder = "…"; - full_path_positions.retain_mut(|mat| { - if *mat >= elided_range.end { - *mat -= elided_len; - *mat += placeholder.len(); - } else if *mat >= elided_range.start { - return false; - } - true - }); - full_path.replace_range(elided_range, placeholder); - } + let placeholder = "…"; + full_path_positions.retain_mut(|mat| { + if *mat >= elided_range.end { + *mat -= elided_len; + *mat += placeholder.len(); + } else if *mat >= elided_range.start { + return false; + } + true + }); + full_path.replace_range(elided_range, placeholder); } } } @@ -1058,6 +1053,15 @@ impl FileFinderDelegate { } } +fn full_path_budget( + file_name: &str, + normal_em: Pixels, + small_em: Pixels, + max_width: Pixels, +) -> usize { + ((px(max_width / px(0.8)) - px(file_name.len() as f32) * normal_em) / small_em) as usize +} + impl PickerDelegate for FileFinderDelegate { type ListItem = ListItem; @@ -1427,7 +1431,7 @@ impl<'a> PathComponentSlice<'a> { } } - fn elision_range(&self, matches: &[usize]) -> Option> { + fn elision_range(&self, budget: usize, matches: &[usize]) -> Option> { let eligible_range = { assert!(matches.windows(2).all(|w| w[0] <= w[1])); let mut matches = matches.iter().copied().peekable(); @@ -1469,11 +1473,42 @@ impl<'a> PathComponentSlice<'a> { if eligible_range.is_empty() { return None; } - let elided_component_range = eligible_range.start..=eligible_range.end - 1; - let elided_range = self.component_ranges[*elided_component_range.start()] - .1 - .start - ..self.component_ranges[*elided_component_range.end()].1.end; - Some(elided_range) + + let elided_range: Range = { + let byte_range = self.component_ranges[eligible_range.start].1.start + ..self.component_ranges[eligible_range.end - 1].1.end; + let midpoint = self.path_str.len() / 2; + let distance_from_start = byte_range.start.abs_diff(midpoint); + let distance_from_end = byte_range.end.abs_diff(midpoint); + let pick_from_end = distance_from_start > distance_from_end; + let mut len_with_elision = self.path_str.len(); + let mut i = eligible_range.start; + while i < eligible_range.end { + if pick_from_end { + i = eligible_range.end - i + eligible_range.start - 1; + } + len_with_elision -= self.component_ranges[i] + .0 + .as_os_str() + .as_encoded_bytes() + .len() + + 1; + if len_with_elision <= budget { + break; + } + i += 1; + } + if len_with_elision > budget { + return None; + } else if pick_from_end { + i..eligible_range.end + } else { + eligible_range.start..i + 1 + } + }; + + let byte_range = self.component_ranges[elided_range.start].1.start + ..self.component_ranges[elided_range.end - 1].1.end; + Some(byte_range) } } diff --git a/crates/file_finder/src/file_finder_tests.rs b/crates/file_finder/src/file_finder_tests.rs index 5b58dfadc720a1..9efd5418dbe426 100644 --- a/crates/file_finder/src/file_finder_tests.rs +++ b/crates/file_finder/src/file_finder_tests.rs @@ -22,8 +22,9 @@ fn test_path_component_slice() { fn check(path: &str, matches: impl IntoIterator, expected: &str) { let mut path = path.to_owned(); let slice = PathComponentSlice::new(&path); - let mut matches = Vec::from_iter(matches); - if let Some(range) = slice.elision_range(&mut matches) { + let matches = Vec::from_iter(matches); + // FIXME make it work again + if let Some(range) = slice.elision_range(0, &matches) { path.replace_range(range, "…"); } assert_eq!(path, expected); From 45d56255a5926ae8a737bc43ac36703c62b2044a Mon Sep 17 00:00:00 2001 From: Cole Miller Date: Thu, 20 Feb 2025 22:42:47 -0500 Subject: [PATCH 7/8] Fix --- crates/file_finder/src/file_finder.rs | 16 +++++++----- crates/file_finder/src/file_finder_tests.rs | 29 ++++++++++----------- 2 files changed, 24 insertions(+), 21 deletions(-) diff --git a/crates/file_finder/src/file_finder.rs b/crates/file_finder/src/file_finder.rs index 931561ac7315cf..984cf953f66cde 100644 --- a/crates/file_finder/src/file_finder.rs +++ b/crates/file_finder/src/file_finder.rs @@ -1484,10 +1484,12 @@ impl<'a> PathComponentSlice<'a> { let mut len_with_elision = self.path_str.len(); let mut i = eligible_range.start; while i < eligible_range.end { - if pick_from_end { - i = eligible_range.end - i + eligible_range.start - 1; - } - len_with_elision -= self.component_ranges[i] + let x = if pick_from_end { + eligible_range.end - i + eligible_range.start - 1 + } else { + i + }; + len_with_elision -= self.component_ranges[x] .0 .as_os_str() .as_encoded_bytes() @@ -1501,9 +1503,11 @@ impl<'a> PathComponentSlice<'a> { if len_with_elision > budget { return None; } else if pick_from_end { - i..eligible_range.end + let x = eligible_range.end - i + eligible_range.start - 1; + x..eligible_range.end } else { - eligible_range.start..i + 1 + let x = i; + eligible_range.start..x + 1 } }; diff --git a/crates/file_finder/src/file_finder_tests.rs b/crates/file_finder/src/file_finder_tests.rs index 9efd5418dbe426..f54150df4bcb4e 100644 --- a/crates/file_finder/src/file_finder_tests.rs +++ b/crates/file_finder/src/file_finder_tests.rs @@ -19,31 +19,30 @@ fn init_logger() { #[test] fn test_path_component_slice() { #[track_caller] - fn check(path: &str, matches: impl IntoIterator, expected: &str) { + fn check(path: &str, budget: usize, matches: impl IntoIterator, expected: &str) { let mut path = path.to_owned(); let slice = PathComponentSlice::new(&path); let matches = Vec::from_iter(matches); - // FIXME make it work again - if let Some(range) = slice.elision_range(0, &matches) { + if let Some(range) = slice.elision_range(budget - 1, &matches) { path.replace_range(range, "…"); } assert_eq!(path, expected); } - check("p/a/b/c/d/", [], "p/…/d/"); - check("p/a/b/c/d/", [2, 4, 6], "p/a/b/c/d/"); - check("p/a/b/c/d/", [2, 6], "p/a/…/c/d/"); - check("p/a/b/c/d/", [6], "p/…/c/d/"); + check("p/a/b/c/d/", 6, [], "p/…/d/"); + check("p/a/b/c/d/", 1, [2, 4, 6], "p/a/b/c/d/"); + check("p/a/b/c/d/", 10, [2, 6], "p/a/…/c/d/"); + check("p/a/b/c/d/", 8, [6], "p/…/c/d/"); - check("p/a/b/c/d", [], "p/…/d"); - check("p/a/b/c/d", [2, 4, 6], "p/a/b/c/d"); - check("p/a/b/c/d", [2, 6], "p/a/…/c/d"); - check("p/a/b/c/d", [6], "p/…/c/d"); + check("p/a/b/c/d", 5, [], "p/…/d"); + check("p/a/b/c/d", 9, [2, 4, 6], "p/a/b/c/d"); + check("p/a/b/c/d", 9, [2, 6], "p/a/…/c/d"); + check("p/a/b/c/d", 7, [6], "p/…/c/d"); - check("/p/a/b/c/d/", [], "/p/…/d/"); - check("/p/a/b/c/d/", [3, 5, 7], "/p/a/b/c/d/"); - check("/p/a/b/c/d/", [3, 7], "/p/a/…/c/d/"); - check("/p/a/b/c/d/", [7], "/p/…/c/d/"); + check("/p/a/b/c/d/", 7, [], "/p/…/d/"); + check("/p/a/b/c/d/", 11, [3, 5, 7], "/p/a/b/c/d/"); + check("/p/a/b/c/d/", 11, [3, 7], "/p/a/…/c/d/"); + check("/p/a/b/c/d/", 9, [7], "/p/…/c/d/"); } #[test] From 9a014da7da6179d07a5abac9c7c3234111d39bef Mon Sep 17 00:00:00 2001 From: Cole Miller Date: Thu, 20 Feb 2025 22:57:10 -0500 Subject: [PATCH 8/8] More test --- crates/file_finder/src/file_finder_tests.rs | 28 ++++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-) diff --git a/crates/file_finder/src/file_finder_tests.rs b/crates/file_finder/src/file_finder_tests.rs index f54150df4bcb4e..1dd7815956d8aa 100644 --- a/crates/file_finder/src/file_finder_tests.rs +++ b/crates/file_finder/src/file_finder_tests.rs @@ -17,7 +17,7 @@ fn init_logger() { } #[test] -fn test_path_component_slice() { +fn test_path_elision() { #[track_caller] fn check(path: &str, budget: usize, matches: impl IntoIterator, expected: &str) { let mut path = path.to_owned(); @@ -29,6 +29,7 @@ fn test_path_component_slice() { assert_eq!(path, expected); } + // Simple cases, mostly to check that different path shapes are handled gracefully. check("p/a/b/c/d/", 6, [], "p/…/d/"); check("p/a/b/c/d/", 1, [2, 4, 6], "p/a/b/c/d/"); check("p/a/b/c/d/", 10, [2, 6], "p/a/…/c/d/"); @@ -43,6 +44,31 @@ fn test_path_component_slice() { check("/p/a/b/c/d/", 11, [3, 5, 7], "/p/a/b/c/d/"); check("/p/a/b/c/d/", 11, [3, 7], "/p/a/…/c/d/"); check("/p/a/b/c/d/", 9, [7], "/p/…/c/d/"); + + // If the budget can't be met, no elision is done. + check( + "project/dir/child/grandchild", + 5, + [], + "project/dir/child/grandchild", + ); + + // The longest unmatched segment is picked for elision. + check( + "project/one/two/X/three/sub", + 21, + [16], + "project/…/X/three/sub", + ); + + // Elision stops when the budget is met, even though there are more components in the chosen segment. + // It proceeds from the end of the unmatched segment that is closer to the midpoint of the path. + check( + "project/one/two/three/X/sub", + 21, + [22], + "project/…/three/X/sub", + ) } #[test]