Skip to content
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

Tweak completion thresholds for quick completions #6918

Closed
wants to merge 1 commit into from
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion crates/completion/src/completions/unqualified_path.rs
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,9 @@ fn complete_enum_variants(acc: &mut Completions, ctx: &CompletionContext, ty: &T
// .Fuzzy search details
//
// To avoid an excessive amount of the results returned, completion input is checked for inclusion in the identifiers only
// (i.e. in `HashMap` in the `std::collections::HashMap` path), also not in the module indentifiers.
// (i.e. in `HashMap` in the `std::collections::HashMap` path), also not in the module identifiers.
//
// It also avoids searching for any imports for inputs with their length less that 3 symbols.
//
// .Merge Behavior
//
Expand All @@ -122,6 +124,10 @@ fn fuzzy_completion(acc: &mut Completions, ctx: &CompletionContext) -> Option<()
let _p = profile::span("fuzzy_completion");
let potential_import_name = ctx.token.to_string();

if potential_import_name.len() < 3 {
return None;
}

let current_module = ctx.scope.module()?;
let anchor = ctx.name_ref_syntax.as_ref()?;
let import_scope = ImportScope::find_insert_use_container(anchor.syntax(), &ctx.sema)?;
Expand All @@ -144,6 +150,7 @@ fn fuzzy_completion(acc: &mut Completions, ctx: &CompletionContext) -> Option<()
})
})
.filter(|(mod_path, _)| mod_path.len() > 1)
.take(10)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That will, most probably, will cause #6594 to reappear: in my experiments, this one had appeared somewhere after 20th result.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've installed your version and fiddling with it, so far we cannot propose PhantomData for phant input, we need more than 10 here for sure.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, it definitelly shows for me:

image

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aha!

image

So looks like VS Code rejects some of the completions we are showing. And I've noticed something very interesting as well!

Note how the untaken list contains std::sync::mpsc::channel. I think thisshoudn't be a suggestion for phan, because phan spanns several path components. cc @jonas-schievink

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And we need to support #![doc(hidden)] (inner) -- thtat's why flt2dec there. rust-analyzer discloses all of the compiler's easer eggs :D

.filter_map(|(import_path, definition)| {
render_resolution_with_import(
RenderContext::new(ctx),
Expand Down