Skip to content

Fuzzy autocompletion of files bugs #12680

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
umnikos opened this issue Apr 27, 2024 · 5 comments · May be fixed by nushell/reedline#887
Open

Fuzzy autocompletion of files bugs #12680

umnikos opened this issue Apr 27, 2024 · 5 comments · May be fixed by nushell/reedline#887
Labels
🐛 bug Something isn't working completions Issues related to tab completion polish this problem makes nu feel unpolished

Comments

@umnikos
Copy link

umnikos commented Apr 27, 2024

Describe the bug

Sorry for the vague title. This issue is several issues all related to the autocompletion of file names for commands:

  1. The way case insensitive matching is currently implemented is to turn all filenames and the query to lowercase before calling .matches_str(). The problem with this is that the matcher used (SkimMatcherV2 from the fuzzy_matcher crate) already does case insensitive matching even if you do not turn everything lower case. So as it currently stands "case sensitive" mode is case insensitive, and "case insensitive" mode is again case insensitive but offers worse matches. The latter is currently the default.
  2. The fuzzy matcher returns a number based on how well the given string matches, but that number is not used anywhere to order the results. This often means even verbatim substrings not found in any other filename do not appear at the top of the suggestions.
  3. Nushell panics when it tries to complete a filename with unicode symbols in it due to trying to split the string mid-character. Steps to reproduce listed below.

How to reproduce

~ $ mkdir nutest
~ $ cd nutest
~/nutest $ touch aba
~/nutest $ touch aæææa
~/nutest $ [Hit Ctrl+L to clear the terminal]
~/nutest $ open 'a[Hit tab]
[Scroll up to see the panic:]

~/nutest $ open 'aError:   × Main thread panicked.
  ├─▶ at /rustc/25ef9e3d85d934b27d9dada2f9dd52b1dc63bb04/library/core/src/str/mod.rs:666:21
  ╰─▶ byte index 2 is not a char boundary; it is inside 'æ' (bytes 1..3) of `aæææa`
  help: set the `RUST_BACKTRACE=1` environment variable to display a backtrace.

Expected behavior

ls | get name | input list --fuzzy seems to do a good job on all account raised above. It orders its matches from most to least relevant, is correctly case-insensitive while still using the case for better matches, and does not panic on unicode, even with unicode inside the prompt string.

It might be relevant that input list --fuzzy uses the fuzzy-select crate instead of the fuzzy-matcher crate, which use different matching engines under the hood and may be the cause of some of the differences.

Screenshots

No response

Configuration

key value
version 0.92.2
branch
commit_hash
build_os linux-x86_64
build_target x86_64-unknown-linux-gnu
rust_version rustc 1.77.2 (25ef9e3d8 2024-04-09)
rust_channel stable-x86_64-unknown-linux-gnu
cargo_version cargo 1.77.2 (e52e36006 2024-03-26)
build_time 2024-04-20 14:57:31 +03:00
build_rust_channel release
allocator mimalloc
features default, sqlite, trash, which
installed_plugins

Additional context

No response

@umnikos umnikos added the needs-triage An issue that hasn't had any proper look label Apr 27, 2024
@fdncred
Copy link
Contributor

fdncred commented Apr 27, 2024

There is no need for to be rude and disrespectful. Please change the title of this issue to something that is more respectful to the nushell contributors.

@umnikos umnikos changed the title Fuzzy autocompletion of files sucks Fuzzy autocompletion of files bugs Apr 27, 2024
@IanManske IanManske added 🐛 bug Something isn't working good first issue Good for newcomers polish this problem makes nu feel unpolished completions Issues related to tab completion and removed needs-triage An issue that hasn't had any proper look good first issue Good for newcomers labels Apr 27, 2024
@cpg314
Copy link

cpg314 commented May 19, 2024

The fuzzy matcher returns a number based on how well the given string matches, but that number is not used anywhere to order the results. This often means even verbatim substrings not found in any other filename do not appear at the top of the suggestions.

I concur on this: the fuzzy finder matches without taking into account the score often give very surprising results.

As a temporary fix, I have replaced the fuzzy matching by a substring search akin to what I had in zsh. It stands between the prefix algorithm and the fuzzy one in terms of flexibility.

diff --git a/crates/nu-cli/src/completions/completion_options.rs b/crates/nu-cli/src/completions/completion_options.rs
index a414aafed..c1eaf540c 100644
--- a/crates/nu-cli/src/completions/completion_options.rs
+++ b/crates/nu-cli/src/completions/completion_options.rs
@@ -1,6 +1,7 @@
 use fuzzy_matcher::{skim::SkimMatcherV2, FuzzyMatcher};
 use nu_parser::trim_quotes_str;
 use nu_protocol::CompletionAlgorithm;
+use std::borrow::Borrow;
 use std::fmt::Display;

 #[derive(Copy, Clone)]
@@ -33,10 +34,7 @@ impl MatchAlgorithm {
         let needle = trim_quotes_str(needle);
         match *self {
             MatchAlgorithm::Prefix => haystack.starts_with(needle),
-            MatchAlgorithm::Fuzzy => {
-                let matcher = SkimMatcherV2::default();
-                matcher.fuzzy_match(haystack, needle).is_some()
-            }
+            MatchAlgorithm::Fuzzy => haystack.contains(needle),
         }
     }

@@ -46,10 +44,10 @@ impl MatchAlgorithm {
             MatchAlgorithm::Prefix => haystack.starts_with(needle),
             MatchAlgorithm::Fuzzy => {
                 let haystack_str = String::from_utf8_lossy(haystack);
+                let haystack_str: &str = haystack_str.borrow();
                 let needle_str = String::from_utf8_lossy(needle);

-                let matcher = SkimMatcherV2::default();
-                matcher.fuzzy_match(&haystack_str, &needle_str).is_some()
+                haystack_str.contains(&*needle_str)
             }
         }
     }

To properly fix the fuzzy matching, one needs to:

I think there would be also value in adding a completions.algorithm = "substring" option, corresponding to the above. I'm happy to send a PR if you wish.

@ysthakur
Copy link
Member

ysthakur commented May 26, 2024

@cpg314 I believe you can already do completions based on substrings by using the prefix algorithm and then setting positional to false. I agree, though, an explicit substring option would be nicer than having a separate positional option (since it doesn't seem all that useful for fuzzy matching)

Edit: positional: true is only an option when custom completions are provided, not for all completions.

@umnikos
Copy link
Author

umnikos commented Dec 30, 2024

From playing around with version 0.101.0, issues 1) and 2) appear to have been fixed, only 3) remains.

@sholderbach
Copy link
Member

sholderbach commented Mar 13, 2025

The reproducible example panic is resolved by nushell/reedline#886 as shipped with #15310

Image

But there remains a case for non-prefix matches that @ysthakur is trying to address with nushell/reedline#887

This is triggered if the file doesn't start with the searched for character

e.g.

touch æææa
open 'a<TAB>

Image

@sholderbach sholderbach linked a pull request Mar 13, 2025 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐛 bug Something isn't working completions Issues related to tab completion polish this problem makes nu feel unpolished
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants