From 838f48a8619e9772324d6b38fc263e2e9bbdf357 Mon Sep 17 00:00:00 2001 From: Johannes Altmanninger Date: Mon, 24 Nov 2025 21:58:14 +0100 Subject: [PATCH] Show completions from separator only if there are no whole-token ones Completions for "foo bar=baz" are sourced from 1. file paths (and custom completions) where "bar=baz" is a subsequence. 2. file paths where "baz" is a subsequence. I'm not sure if there is ever a case where 1 is non-empty and we still want to show 2. Bravely include this property in our completion ranking, and only show the best ones. This enables us to get rid of the hack added by b6c249be0c4 (Back out "Escape : and = in file completions", 2025-01-10). Closes #5363 --- CHANGELOG.rst | 1 + src/complete.rs | 73 +++++++++++++++++++++++++++++---------------- src/wcstringutil.rs | 13 ++++++-- 3 files changed, 58 insertions(+), 29 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 89cb970a5..20a0b6a12 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -10,6 +10,7 @@ Deprecations and removed features Interactive improvements ------------------------ - When typing immediately after starting fish, the first prompt is now rendered correctly. +- Completion accuracy was improved for file paths containing ``=`` or ``:`` (:issue:`5363`). Improved terminal support ------------------------- diff --git a/src/complete.rs b/src/complete.rs index a8ed6e011..d3ec0fec6 100644 --- a/src/complete.rs +++ b/src/complete.rs @@ -1601,18 +1601,6 @@ fn complete_param_expand( return; }; - // We generally expand both, the whole token ("foo=bar") and also just the "bar" - // suffix. If the whole token is a valid path prefix, completions of just the suffix - // are probably false positives, and are confusing when I'm using completions to list - // directory contents. Apply a wonky heuristic to work around the most visible case -- - // the empty suffix -- where all files in $PWD are completed/autosuggested. - if self.completions[first_from_start..] - .iter() - .any(|c| !c.replaces_token()) - && sep_index + 1 == s.len() - { - return; - } let sep_string = s.slice_from(sep_index + 1); let mut local_completions = Vec::new(); if matches!( @@ -1635,6 +1623,7 @@ fn complete_param_expand( let prefix_with_sep = s.as_char_slice()[..sep_index + 1].into(); for comp in &mut local_completions { comp.prepend_token_prefix(prefix_with_sep); + comp.r#match.from_separator = true; } if !self.completions.extend(local_completions) { return; @@ -2680,6 +2669,7 @@ fn test_complete() { assert_eq!(completions[1].completion, L!("$Foo1")); assert_eq!(completions[2].completion, L!("$gamma1")); + let _ = std::fs::remove_dir_all("test/complete_test"); std::fs::create_dir_all("test/complete_test").unwrap(); std::fs::write("test/complete_test/has space", []).unwrap(); std::fs::write("test/complete_test/bracket[abc]", []).unwrap(); @@ -2687,7 +2677,7 @@ fn test_complete() { // Backslashes and colons are not legal filename characters on WIN32/CYGWIN { std::fs::write(r"test/complete_test/gnarlybracket\[abc]", []).unwrap(); - std::fs::write(r"test/complete_test/colon:abc", []).unwrap(); + std::fs::write(r"test/complete_test/colon:TTestWithColon", []).unwrap(); } std::fs::write(r"test/complete_test/equal=abc", []).unwrap(); // On MSYS, the executable bit cannot be set manually, is set automatically @@ -2731,12 +2721,47 @@ fn test_complete() { #[cfg(not(cygwin))] // Backslashes and colons are not legal filename characters on WIN32/CYGWIN { - completions = do_complete( - L!(": test/complete_test/colon:"), + macro_rules! whole_token_completion_dominates { + ( + $cmd:literal, + $options:expr, + $completion_from_token_start:literal, + $completion_from_separator:literal, + ) => { + completions = do_complete(L!($cmd), $options); + assert_eq!(completions.len(), 2); + let c0 = &completions[0]; + let c1 = &completions[1]; + // Might be replacing. + assert_eq!(c0.completion, L!($completion_from_token_start)); + assert!(!c0.r#match.from_separator); + assert_eq!(c1.completion, L!($completion_from_separator)); + assert!(c1.r#match.from_separator); + // Completion pager will only show better (lower) rank. + assert!(c0.r#match.rank() < c1.r#match.rank()); + }; + } + + whole_token_completion_dominates!( + ": test/complete_test/colon:", CompletionRequestOptions::default(), + "TTestWithColon", + "test/", + ); + // Even when it has a case mismatch. + whole_token_completion_dominates!( + ": test/complete_test/colon:t", + CompletionRequestOptions::default(), + "test/complete_test/colon:TTestWithColon", + "est/", + ); + // Even when it is not a prefix. + whole_token_completion_dominates!( + ": test/complete_test/colon:Tes", + fuzzy_options, + "test/complete_test/colon:TTestWithColon", + "test/complete_test/colon:test/", ); - assert_eq!(completions.len(), 1); - assert_eq!(completions[0].completion, L!("abc")); } macro_rules! unique_completion_applies_as { @@ -2800,18 +2825,14 @@ macro_rules! unique_completion_applies_as { { unique_completion_applies_as!( r"touch test/complete_test/colon", - r":abc", - r"touch test/complete_test/colon:abc ", - ); - unique_completion_applies_as!( - r"touch test/complete_test/colon:", - r"abc", - r"touch test/complete_test/colon:abc ", + r":TTestWithColon", + r"touch test/complete_test/colon:TTestWithColon ", ); + unique_completion_applies_as!( r#"touch "test/complete_test/colon:"#, - r"abc", - r#"touch "test/complete_test/colon:abc" "#, + r"TTestWithColon", + r#"touch "test/complete_test/colon:TTestWithColon" "#, ); } diff --git a/src/wcstringutil.rs b/src/wcstringutil.rs index 8ec05fefb..06de0c9fa 100644 --- a/src/wcstringutil.rs +++ b/src/wcstringutil.rs @@ -140,13 +140,18 @@ pub enum CaseSensitivity { /// A lightweight value-type describing how closely a string fuzzy-matches another string. #[derive(Copy, Clone, Debug, Eq, PartialEq)] pub struct StringFuzzyMatch { + pub from_separator: bool, pub typ: ContainType, pub case_fold: CaseSensitivity, } impl StringFuzzyMatch { pub fn new(typ: ContainType, case_fold: CaseSensitivity) -> Self { - Self { typ, case_fold } + Self { + from_separator: false, + typ, + case_fold, + } } // Helper to return an exact match. #[inline(always)] @@ -282,8 +287,10 @@ pub fn rank(&self) -> u32 { self.case_fold }; - // Type dominates fold. - effective_type as u32 * 8 + effective_case as u32 + // Separator dominates type dominates fold. + ((self.from_separator as u32) << 4) + + ((effective_type as u32) << 2) + + (effective_case as u32) } }