From 9206f0a876dff1316f2f77f8b026c90b55608281 Mon Sep 17 00:00:00 2001 From: Peter Retzlaff Date: Tue, 8 Jul 2025 09:38:31 +0200 Subject: [PATCH 1/3] Add filter_format field for Column in pickers Add the option to provide a formatting function for the filtering of picker columns that is separate from the formatting function for displaying it in the view. Use this new option for the "path" column of the workspace diagnostics picker, so that we are filtering on the full (relative) file path, instead of the truncated value. Fixes #13608 --- helix-term/src/commands/lsp.rs | 10 ++++++++++ helix-term/src/ui/picker.rs | 15 ++++++++++++--- 2 files changed, 22 insertions(+), 3 deletions(-) diff --git a/helix-term/src/commands/lsp.rs b/helix-term/src/commands/lsp.rs index ac9dd6e27..214629323 100644 --- a/helix-term/src/commands/lsp.rs +++ b/helix-term/src/commands/lsp.rs @@ -281,6 +281,16 @@ fn diag_picker( } else { Default::default() } + }) + .with_filter_format(|item: &PickerDiagnostic, _| { + if let Some(path) = item.location.uri.as_path() { + path::get_relative_path(path) + .to_string_lossy() + .to_string() + .into() + } else { + Default::default() + } }), ); primary_column += 1; diff --git a/helix-term/src/ui/picker.rs b/helix-term/src/ui/picker.rs index 3f3aaba2b..ed968a1fa 100644 --- a/helix-term/src/ui/picker.rs +++ b/helix-term/src/ui/picker.rs @@ -137,7 +137,7 @@ fn inject_nucleo_item( ) { injector.push(item, |item, dst| { for (column, text) in columns.iter().filter(|column| column.filter).zip(dst) { - *text = column.format_text(item, editor_data).into() + *text = column.format_filter_text(item, editor_data).into() } }); } @@ -189,6 +189,7 @@ type ColumnFormatFn = for<'a> fn(&'a T, &'a D) -> Cell<'a>; pub struct Column { name: Arc, format: ColumnFormatFn, + filter_format: ColumnFormatFn, /// Whether the column should be passed to nucleo for matching and filtering. /// `DynamicPicker` uses this so that the dynamic column (for example regex in /// global search) is not used for filtering twice. @@ -201,6 +202,7 @@ impl Column { Self { name: name.into(), format, + filter_format: format, filter: true, hidden: false, } @@ -213,6 +215,7 @@ impl Column { Self { name: name.into(), format, + filter_format: format, filter: false, hidden: true, } @@ -223,12 +226,18 @@ impl Column { self } + pub fn with_filter_format(mut self, format_fn: ColumnFormatFn) -> Self { + self.filter_format = format_fn; + self + } + fn format<'a>(&self, item: &'a T, data: &'a D) -> Cell<'a> { (self.format)(item, data) } - fn format_text<'a>(&self, item: &'a T, data: &'a D) -> Cow<'a, str> { - let text: String = self.format(item, data).content.into(); + fn format_filter_text<'a>(&self, item: &'a T, data: &'a D) -> Cow<'a, str> { + let cell = (self.filter_format)(item, data); + let text: String = cell.content.into(); text.into() } } From 7c934855fced340b8ff78c5a2c2bf18120daa5f7 Mon Sep 17 00:00:00 2001 From: Peter Retzlaff Date: Tue, 8 Jul 2025 15:51:31 +0200 Subject: [PATCH 2/3] Revert "Add filter_format field for Column in pickers" This reverts commit 9206f0a876dff1316f2f77f8b026c90b55608281. --- helix-term/src/commands/lsp.rs | 10 ---------- helix-term/src/ui/picker.rs | 15 +++------------ 2 files changed, 3 insertions(+), 22 deletions(-) diff --git a/helix-term/src/commands/lsp.rs b/helix-term/src/commands/lsp.rs index 214629323..ac9dd6e27 100644 --- a/helix-term/src/commands/lsp.rs +++ b/helix-term/src/commands/lsp.rs @@ -281,16 +281,6 @@ fn diag_picker( } else { Default::default() } - }) - .with_filter_format(|item: &PickerDiagnostic, _| { - if let Some(path) = item.location.uri.as_path() { - path::get_relative_path(path) - .to_string_lossy() - .to_string() - .into() - } else { - Default::default() - } }), ); primary_column += 1; diff --git a/helix-term/src/ui/picker.rs b/helix-term/src/ui/picker.rs index ed968a1fa..3f3aaba2b 100644 --- a/helix-term/src/ui/picker.rs +++ b/helix-term/src/ui/picker.rs @@ -137,7 +137,7 @@ fn inject_nucleo_item( ) { injector.push(item, |item, dst| { for (column, text) in columns.iter().filter(|column| column.filter).zip(dst) { - *text = column.format_filter_text(item, editor_data).into() + *text = column.format_text(item, editor_data).into() } }); } @@ -189,7 +189,6 @@ type ColumnFormatFn = for<'a> fn(&'a T, &'a D) -> Cell<'a>; pub struct Column { name: Arc, format: ColumnFormatFn, - filter_format: ColumnFormatFn, /// Whether the column should be passed to nucleo for matching and filtering. /// `DynamicPicker` uses this so that the dynamic column (for example regex in /// global search) is not used for filtering twice. @@ -202,7 +201,6 @@ impl Column { Self { name: name.into(), format, - filter_format: format, filter: true, hidden: false, } @@ -215,7 +213,6 @@ impl Column { Self { name: name.into(), format, - filter_format: format, filter: false, hidden: true, } @@ -226,18 +223,12 @@ impl Column { self } - pub fn with_filter_format(mut self, format_fn: ColumnFormatFn) -> Self { - self.filter_format = format_fn; - self - } - fn format<'a>(&self, item: &'a T, data: &'a D) -> Cell<'a> { (self.format)(item, data) } - fn format_filter_text<'a>(&self, item: &'a T, data: &'a D) -> Cow<'a, str> { - let cell = (self.filter_format)(item, data); - let text: String = cell.content.into(); + fn format_text<'a>(&self, item: &'a T, data: &'a D) -> Cow<'a, str> { + let text: String = self.format(item, data).content.into(); text.into() } } From 5eb348d8c0da9932d674a0aa6dab1a0f864bb793 Mon Sep 17 00:00:00 2001 From: Peter Retzlaff Date: Thu, 10 Jul 2025 10:35:47 +0200 Subject: [PATCH 3/3] Make path truncation less aggressive We are now trying to truncate the path so it fits a fixed character limit, while preserving as much of the base path components as possible. Fixes #13608 by preserving more of the path to filter by. --- helix-stdx/src/path.rs | 64 ++++++++++++++++++++++++++++++------------ 1 file changed, 46 insertions(+), 18 deletions(-) diff --git a/helix-stdx/src/path.rs b/helix-stdx/src/path.rs index 53081b0f0..2087f0e04 100644 --- a/helix-stdx/src/path.rs +++ b/helix-stdx/src/path.rs @@ -157,8 +157,10 @@ where path } -/// Returns a truncated filepath where the basepart of the path is reduced to the first -/// char of the folder and the whole filename appended. +/// Returns a truncated filepath where the basepart of the path is reduced to +/// try and fit the entire path into the character limit. The filename is never +/// truncated and the basepart components will each keep at least their first character, +/// so it is still possible for the resulting path to exceed the character limit. /// /// Also strip the current working directory from the beginning of the path. /// Note that this function does not check if the truncated path is unambiguous. @@ -167,9 +169,17 @@ where /// use helix_stdx::path::get_truncated_path; /// use std::path::Path; /// -/// assert_eq!( -/// get_truncated_path("/home/cnorris/documents/jokes.txt").as_path(), -/// Path::new("/h/c/d/jokes.txt") +/// assert_eq!( +/// get_truncated_path("/home/cnorris/documents/jokes_and_more.txt").as_path(), +/// Path::new("/home/cnorr/docum/jokes_and_more.txt") +/// ); +/// assert_eq!( +/// get_truncated_path("/home/cnorris/documents/jokes_with_a_very_long_name.txt").as_path(), +/// Path::new("/h/c/d/jokes_with_a_very_long_name.txt") +/// ); +/// assert_eq!( +/// get_truncated_path("crates/args_internal/src/format_argument.rs").as_path(), +/// Path::new("crates/args_i/src/format_argument.rs") /// ); /// assert_eq!( /// get_truncated_path("jokes.txt").as_path(), @@ -190,21 +200,39 @@ pub fn get_truncated_path(path: impl AsRef) -> PathBuf { let cwd = current_working_dir(); let path = path.as_ref(); let path = path.strip_prefix(cwd).unwrap_or(path); - let file = path.file_name().unwrap_or_default(); - let base = path.parent().unwrap_or_else(|| Path::new("")); - let mut ret = PathBuf::with_capacity(file.len()); - // A char can't be directly pushed to a PathBuf - let mut first_char_buffer = String::new(); - for d in base { - let Some(first_char) = d.to_string_lossy().chars().next() else { + + let char_limit = 36; + let mut result = path.to_path_buf(); + + while result.to_string_lossy().chars().count() > char_limit { + // find the longest path component + let longest = result + .components() + .rev() + .skip(1) // never truncate the last component, i.e. filename + .filter_map(|component| match component { + Component::Normal(c) => Some(c.to_string_lossy()), + _ => None, + }) + .max_by(|c1, c2| c1.len().cmp(&c2.len())) + .unwrap_or_else(|| Cow::from("")); + + if longest.chars().count() <= 1 { break; - }; - first_char_buffer.push(first_char); - ret.push(&first_char_buffer); - first_char_buffer.clear(); + } + + // truncate longest component by one character + let mut new_path = PathBuf::with_capacity(result.capacity()); + for c in result.components() { + if c.as_os_str().to_string_lossy() == longest { + new_path.push(&longest[..longest.len() - 1]); + } else { + new_path.push(c); + } + } + result = new_path; } - ret.push(file); - ret + result } fn path_component_regex(windows: bool) -> String {