file_finder: Remove common segments of long paths in search results (#25049)
This PR makes progress on #7711 by identifying any common prefix of the paths in the file finder's search results, and replacing the "interior" of that prefix---every path segment but the first and last---with `...`, when a heuristic indicates that the longest path would otherwise overflow the modal. The elision is not applied to any segment that contains a match for the search query. There may be more work to do on #7711 in the case of long result paths that do not share a significant common prefix. Release Notes: - Improved display of long paths in the file finder modal Co-authored-by: Max <max@zed.dev>
This commit is contained in:
Cole Miller
GitHub
parent
80458ffb96
commit
9ef0501853
49 changed files with 207 additions and 157 deletions
|
|
@ -25,6 +25,7 @@ use project::{PathMatchCandidateSet, Project, ProjectPath, WorktreeId};
|
|||
use settings::Settings;
|
||||
use std::{
|
||||
cmp,
|
||||
ops::Range,
|
||||
path::{Path, PathBuf},
|
||||
sync::{
|
||||
atomic::{self, AtomicBool},
|
||||
|
|
@ -381,6 +382,7 @@ impl PartialOrd for ProjectPanelOrdMatch {
|
|||
struct Matches {
|
||||
separate_history: bool,
|
||||
matches: Vec<Match>,
|
||||
elided_byte_range: Option<Range<usize>>,
|
||||
}
|
||||
|
||||
#[derive(Debug, PartialEq, Eq, PartialOrd, Ord, Clone)]
|
||||
|
|
@ -392,6 +394,35 @@ enum Match {
|
|||
Search(ProjectPanelOrdMatch),
|
||||
}
|
||||
|
||||
struct MatchLabels {
|
||||
path: String,
|
||||
path_positions: Vec<usize>,
|
||||
file_name: String,
|
||||
file_name_positions: Vec<usize>,
|
||||
}
|
||||
|
||||
#[derive(Clone, Copy, Debug)]
|
||||
struct EmWidths {
|
||||
normal: Pixels,
|
||||
small: Pixels,
|
||||
}
|
||||
|
||||
fn em_widths(window: &mut Window, cx: &mut App) -> EmWidths {
|
||||
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.));
|
||||
EmWidths { normal, small }
|
||||
}
|
||||
|
||||
impl Match {
|
||||
fn path(&self) -> &Arc<Path> {
|
||||
match self {
|
||||
|
|
@ -406,6 +437,43 @@ impl Match {
|
|||
Match::Search(panel_match) => Some(&panel_match),
|
||||
}
|
||||
}
|
||||
|
||||
fn common_prefix<'a>(&self, other: &'a Path) -> &'a Path {
|
||||
let mut prefix = other;
|
||||
let path_positions = match self {
|
||||
Match::History {
|
||||
panel_match: Some(mat),
|
||||
..
|
||||
}
|
||||
| Match::Search(mat) => mat.0.positions.as_slice(),
|
||||
Match::History {
|
||||
panel_match: None, ..
|
||||
} => &[],
|
||||
};
|
||||
let first_path_position = *path_positions.iter().min().unwrap_or(&0);
|
||||
while self.path().strip_prefix(prefix).is_err()
|
||||
|| prefix.to_string_lossy().len() > first_path_position
|
||||
{
|
||||
let Some(parent) = prefix.parent() else {
|
||||
break;
|
||||
};
|
||||
prefix = parent;
|
||||
}
|
||||
prefix
|
||||
}
|
||||
|
||||
fn approx_width(&self, em_widths: EmWidths) -> Pixels {
|
||||
let file_name = self.path().file_name().map_or_else(
|
||||
|| self.path().to_string_lossy(),
|
||||
|file_name| file_name.to_string_lossy(),
|
||||
);
|
||||
let parent = self
|
||||
.path()
|
||||
.parent()
|
||||
.map_or_else(|| "".into(), |parent| parent.to_string_lossy());
|
||||
px(file_name.chars().count() as f32) * em_widths.normal
|
||||
+ px(parent.chars().count() as f32) * em_widths.small
|
||||
}
|
||||
}
|
||||
|
||||
impl Matches {
|
||||
|
|
@ -451,7 +519,11 @@ impl Matches {
|
|||
query: Option<&FileSearchQuery>,
|
||||
new_search_matches: impl Iterator<Item = ProjectPanelOrdMatch>,
|
||||
extend_old_matches: bool,
|
||||
em_widths: EmWidths,
|
||||
max_width: Pixels,
|
||||
) {
|
||||
self.elided_byte_range = None;
|
||||
|
||||
let Some(query) = query else {
|
||||
// assuming that if there's no query, then there's no search matches.
|
||||
self.matches.clear();
|
||||
|
|
@ -507,6 +579,31 @@ impl Matches {
|
|||
}
|
||||
}
|
||||
}
|
||||
|
||||
let Some((first, rest)) = self.matches.split_first() else {
|
||||
return;
|
||||
};
|
||||
let mut prefix = first.path().as_ref();
|
||||
let mut widest_match = first.approx_width(em_widths);
|
||||
for mat in rest {
|
||||
widest_match = widest_match.max(mat.approx_width(em_widths));
|
||||
prefix = mat.common_prefix(prefix);
|
||||
}
|
||||
|
||||
if widest_match > max_width {
|
||||
let components = prefix.components().collect::<Vec<_>>();
|
||||
let prefix = prefix.to_string_lossy();
|
||||
if components.len() > 3 {
|
||||
let after_first = components[1..].iter().collect::<PathBuf>();
|
||||
let after_first = after_first.to_string_lossy();
|
||||
let start = prefix.len() - after_first.len();
|
||||
let after_first_before_last = components[1..components.len() - 2]
|
||||
.iter()
|
||||
.collect::<PathBuf>();
|
||||
let after_first_before_last = after_first_before_last.to_string_lossy();
|
||||
self.elided_byte_range = Some(start..start + after_first_before_last.len());
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// If a < b, then a is a worse match, aligning with the `ProjectPanelOrdMatch` ordering.
|
||||
|
|
@ -535,6 +632,25 @@ impl Matches {
|
|||
}
|
||||
}
|
||||
|
||||
impl MatchLabels {
|
||||
fn check(&self) {
|
||||
let file_name = &self.file_name;
|
||||
for i in &self.file_name_positions {
|
||||
assert!(
|
||||
self.file_name.is_char_boundary(*i),
|
||||
"{i} is not a valid char boundary in file name {file_name:?}"
|
||||
);
|
||||
}
|
||||
let path = &self.path;
|
||||
for i in &self.path_positions {
|
||||
assert!(
|
||||
self.path.is_char_boundary(*i),
|
||||
"{i} is not a valid char boundary in path {path:?}"
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn matching_history_items<'a>(
|
||||
history_items: impl IntoIterator<Item = &'a FoundPath>,
|
||||
currently_opened: Option<&'a FoundPath>,
|
||||
|
|
@ -644,7 +760,6 @@ impl FileSearchQuery {
|
|||
}
|
||||
|
||||
impl FileFinderDelegate {
|
||||
#[allow(clippy::too_many_arguments)]
|
||||
fn new(
|
||||
file_finder: WeakEntity<FileFinder>,
|
||||
workspace: WeakEntity<Workspace>,
|
||||
|
|
@ -745,10 +860,10 @@ impl FileFinderDelegate {
|
|||
.map(ProjectPanelOrdMatch);
|
||||
let did_cancel = cancel_flag.load(atomic::Ordering::Relaxed);
|
||||
picker
|
||||
.update(&mut cx, |picker, cx| {
|
||||
.update_in(&mut cx, |picker, window, cx| {
|
||||
picker
|
||||
.delegate
|
||||
.set_search_matches(search_id, did_cancel, query, matches, cx)
|
||||
.set_search_matches(search_id, did_cancel, query, matches, window, cx)
|
||||
})
|
||||
.log_err();
|
||||
})
|
||||
|
|
@ -760,9 +875,13 @@ impl FileFinderDelegate {
|
|||
did_cancel: bool,
|
||||
query: FileSearchQuery,
|
||||
matches: impl IntoIterator<Item = ProjectPanelOrdMatch>,
|
||||
|
||||
window: &mut Window,
|
||||
cx: &mut Context<Picker<Self>>,
|
||||
) {
|
||||
let em_widths = em_widths(window, cx);
|
||||
let file_finder_settings = FileFinderSettings::get_global(cx);
|
||||
let max_width = FileFinder::modal_max_width(file_finder_settings.modal_max_width, window);
|
||||
|
||||
if search_id >= self.latest_search_id {
|
||||
self.latest_search_id = search_id;
|
||||
let query_changed = Some(query.path_query())
|
||||
|
|
@ -784,6 +903,8 @@ impl FileFinderDelegate {
|
|||
Some(&query),
|
||||
matches.into_iter(),
|
||||
extend_old_matches,
|
||||
em_widths,
|
||||
max_width,
|
||||
);
|
||||
|
||||
self.selected_index = selected_match.map_or_else(
|
||||
|
|
@ -802,13 +923,8 @@ impl FileFinderDelegate {
|
|||
}
|
||||
}
|
||||
|
||||
fn labels_for_match(
|
||||
&self,
|
||||
path_match: &Match,
|
||||
cx: &App,
|
||||
ix: usize,
|
||||
) -> (String, Vec<usize>, String, Vec<usize>) {
|
||||
let (file_name, file_name_positions, full_path, full_path_positions) = match &path_match {
|
||||
fn labels_for_match(&self, path_match: &Match, cx: &App, ix: usize) -> MatchLabels {
|
||||
let mut labels = match &path_match {
|
||||
Match::History {
|
||||
path: entry_path,
|
||||
panel_match,
|
||||
|
|
@ -823,18 +939,18 @@ impl FileFinderDelegate {
|
|||
|
||||
if !has_worktree {
|
||||
if let Some(absolute_path) = &entry_path.absolute {
|
||||
return (
|
||||
absolute_path
|
||||
return MatchLabels {
|
||||
file_name: absolute_path
|
||||
.file_name()
|
||||
.map_or_else(
|
||||
|| project_relative_path.to_string_lossy(),
|
||||
|file_name| file_name.to_string_lossy(),
|
||||
)
|
||||
.to_string(),
|
||||
Vec::new(),
|
||||
absolute_path.to_string_lossy().to_string(),
|
||||
Vec::new(),
|
||||
);
|
||||
file_name_positions: Vec::new(),
|
||||
path: absolute_path.to_string_lossy().to_string(),
|
||||
path_positions: Vec::new(),
|
||||
};
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -865,44 +981,39 @@ impl FileFinderDelegate {
|
|||
Match::Search(path_match) => self.labels_for_path_match(&path_match.0),
|
||||
};
|
||||
|
||||
if file_name_positions.is_empty() {
|
||||
if labels.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,
|
||||
);
|
||||
if labels.path.starts_with(user_home_path) {
|
||||
labels.path.replace_range(0..user_home_path.len(), "~");
|
||||
labels.path_positions.retain_mut(|position| {
|
||||
if *position >= user_home_path.len() {
|
||||
*position -= user_home_path.len();
|
||||
*position += 1;
|
||||
true
|
||||
} else {
|
||||
false
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
(
|
||||
file_name,
|
||||
file_name_positions,
|
||||
full_path,
|
||||
full_path_positions,
|
||||
)
|
||||
labels.check();
|
||||
labels
|
||||
}
|
||||
|
||||
fn labels_for_path_match(
|
||||
&self,
|
||||
path_match: &PathMatch,
|
||||
) -> (String, Vec<usize>, String, Vec<usize>) {
|
||||
let path = &path_match.path;
|
||||
let path_string = path.to_string_lossy();
|
||||
let full_path = [path_match.path_prefix.as_ref(), path_string.as_ref()].join("");
|
||||
fn labels_for_path_match(&self, path_match: &PathMatch) -> MatchLabels {
|
||||
let mut path_positions = path_match.positions.clone();
|
||||
|
||||
let file_name = path.file_name().map_or_else(
|
||||
let file_name = path_match.path.file_name().map_or_else(
|
||||
|| path_match.path_prefix.to_string(),
|
||||
|file_name| file_name.to_string_lossy().to_string(),
|
||||
);
|
||||
let file_name_start = path_match.path_prefix.len() + path_string.len() - file_name.len();
|
||||
let mut path = path_match.path.to_string_lossy().to_string();
|
||||
let file_name_start = path_match.path_prefix.len() + path.len() - file_name.len();
|
||||
let file_name_positions = path_positions
|
||||
.iter()
|
||||
.filter_map(|pos| {
|
||||
|
|
@ -914,10 +1025,31 @@ impl FileFinderDelegate {
|
|||
})
|
||||
.collect();
|
||||
|
||||
let full_path = full_path.trim_end_matches(&file_name).to_string();
|
||||
path_positions.retain(|idx| *idx < full_path.len());
|
||||
path.drain(file_name_start.saturating_sub(path_match.path_prefix.len())..);
|
||||
path_positions.retain_mut(|idx| {
|
||||
if *idx < path.len() {
|
||||
if let Some(elided_range) = &self.matches.elided_byte_range {
|
||||
if *idx >= elided_range.end {
|
||||
*idx += '…'.len_utf8();
|
||||
*idx -= elided_range.len();
|
||||
}
|
||||
}
|
||||
true
|
||||
} else {
|
||||
false
|
||||
}
|
||||
});
|
||||
|
||||
(file_name, file_name_positions, full_path, path_positions)
|
||||
if let Some(elided_range) = &self.matches.elided_byte_range {
|
||||
path.replace_range(elided_range.clone(), "…");
|
||||
}
|
||||
|
||||
MatchLabels {
|
||||
file_name,
|
||||
file_name_positions,
|
||||
path,
|
||||
path_positions,
|
||||
}
|
||||
}
|
||||
|
||||
fn lookup_absolute_path(
|
||||
|
|
@ -969,10 +1101,17 @@ impl FileFinderDelegate {
|
|||
}
|
||||
|
||||
picker
|
||||
.update_in(&mut cx, |picker, _, cx| {
|
||||
.update_in(&mut cx, |picker, window, cx| {
|
||||
let picker_delegate = &mut picker.delegate;
|
||||
let search_id = util::post_inc(&mut picker_delegate.search_count);
|
||||
picker_delegate.set_search_matches(search_id, false, query, path_matches, cx);
|
||||
picker_delegate.set_search_matches(
|
||||
search_id,
|
||||
false,
|
||||
query,
|
||||
path_matches,
|
||||
window,
|
||||
cx,
|
||||
);
|
||||
|
||||
anyhow::Ok(())
|
||||
})
|
||||
|
|
@ -1049,6 +1188,10 @@ impl PickerDelegate for FileFinderDelegate {
|
|||
window: &mut Window,
|
||||
cx: &mut Context<Picker<Self>>,
|
||||
) -> Task<()> {
|
||||
let em_widths = em_widths(window, cx);
|
||||
let file_finder_settings = FileFinderSettings::get_global(cx);
|
||||
let max_width = FileFinder::modal_max_width(file_finder_settings.modal_max_width, window);
|
||||
|
||||
let raw_query = raw_query.replace(' ', "");
|
||||
let raw_query = raw_query.trim();
|
||||
if raw_query.is_empty() {
|
||||
|
|
@ -1076,6 +1219,8 @@ impl PickerDelegate for FileFinderDelegate {
|
|||
None,
|
||||
None.into_iter(),
|
||||
false,
|
||||
em_widths,
|
||||
max_width,
|
||||
);
|
||||
|
||||
self.first_update = false;
|
||||
|
|
@ -1269,11 +1414,10 @@ 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 labels = self.labels_for_match(path_match, cx, ix);
|
||||
|
||||
let file_icon = if settings.file_icons {
|
||||
FileIcons::get_icon(Path::new(&file_name), cx)
|
||||
FileIcons::get_icon(Path::new(&labels.file_name), cx)
|
||||
.map(Icon::from_path)
|
||||
.map(|icon| icon.color(Color::Muted))
|
||||
} else {
|
||||
|
|
@ -1291,9 +1435,12 @@ impl PickerDelegate for FileFinderDelegate {
|
|||
h_flex()
|
||||
.gap_2()
|
||||
.py_px()
|
||||
.child(HighlightedLabel::new(file_name, file_name_positions))
|
||||
.child(HighlightedLabel::new(
|
||||
labels.file_name,
|
||||
labels.file_name_positions,
|
||||
))
|
||||
.child(
|
||||
HighlightedLabel::new(full_path, full_path_positions)
|
||||
HighlightedLabel::new(labels.path, labels.path_positions)
|
||||
.size(LabelSize::Small)
|
||||
.color(Color::Muted),
|
||||
),
|
||||
|
|
|
|||
|
|
@ -384,6 +384,7 @@ async fn test_matching_cancellation(cx: &mut TestAppContext) {
|
|||
ProjectPanelOrdMatch(matches[1].clone()),
|
||||
ProjectPanelOrdMatch(matches[3].clone()),
|
||||
],
|
||||
window,
|
||||
cx,
|
||||
);
|
||||
|
||||
|
|
@ -398,6 +399,7 @@ async fn test_matching_cancellation(cx: &mut TestAppContext) {
|
|||
ProjectPanelOrdMatch(matches[2].clone()),
|
||||
ProjectPanelOrdMatch(matches[3].clone()),
|
||||
],
|
||||
window,
|
||||
cx,
|
||||
);
|
||||
|
||||
|
|
@ -492,12 +494,11 @@ async fn test_single_file_worktrees(cx: &mut TestAppContext) {
|
|||
let matches = collect_search_matches(picker).search_matches_only();
|
||||
assert_eq!(matches.len(), 1);
|
||||
|
||||
let (file_name, file_name_positions, full_path, full_path_positions) =
|
||||
delegate.labels_for_path_match(&matches[0]);
|
||||
assert_eq!(file_name, "the-file");
|
||||
assert_eq!(file_name_positions, &[0, 1, 4]);
|
||||
assert_eq!(full_path, "");
|
||||
assert_eq!(full_path_positions, &[0; 0]);
|
||||
let labels = delegate.labels_for_path_match(&matches[0]);
|
||||
assert_eq!(labels.file_name, "the-file");
|
||||
assert_eq!(labels.file_name_positions, &[0, 1, 4]);
|
||||
assert_eq!(labels.path, "");
|
||||
assert_eq!(labels.path_positions, &[0; 0]);
|
||||
});
|
||||
|
||||
// Since the worktree root is a file, searching for its name followed by a slash does
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue