Tweak completion thresholds for quick completions #6918
Conversation
* don't even start auto-complete if fewer than 3 chars * look for 100 fuzzy matched symbols, but *stop* at the first ten actually importable.
There was a problem hiding this comment.
Might be worth to update the feature description about the 3 symbol trigger now.
I'm not sure about the limit(n) usage here: in my experiments, the rest of the completion item rendering was not that heavy after the import insert was moved away.
It also spoils the completion results a bit: I feel like precision might be more important in such case.
I wonder if we can come up with some heuristical order of importance we could put those completions in before calling take
| @@ -144,6 +150,7 @@ fn fuzzy_completion(acc: &mut Completions, ctx: &CompletionContext) -> Option<() | |||
| }) | |||
| }) | |||
| .filter(|(mod_path, _)| mod_path.len() > 1) | |||
| .take(10) | |||
There was a problem hiding this comment.
That will, most probably, will cause #6594 to reappear: in my experiments, this one had appeared somewhere after 20th result.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Hm, it definitelly shows for me:
There was a problem hiding this comment.
Aha!
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
There was a problem hiding this comment.
And we need to support #![doc(hidden)] (inner) -- thtat's why flt2dec there. rust-analyzer discloses all of the compiler's easer eggs :D
|
The placement of 20 is tricky: it does reduce the number of times we invoke
find_path, although it happens afterwards. The iterators are lazy
…On Thursday, 17 December 2020, Kirill Bulatov ***@***.***> wrote:
***@***.**** commented on this pull request.
Might be worth to update the feature description about the 3 symbol
trigger now.
I'm not sure about the limit(n) usage here: in my experiments, the rest
of the completion item rendering was not that heavy after the import insert
was mover away.
It also spoils the completion results a bit: I feel like precision might
be more important in such case.
I wonder if we can come up with some heuristical order of importance we
could put those completions in before calling take
------------------------------
In crates/completion/src/completions/unqualified_path.rs
<#6918 (comment)>
:
> @@ -144,6 +150,7 @@ fn fuzzy_completion(acc: &mut Completions, ctx: &CompletionContext) -> Option<()
})
})
.filter(|(mod_path, _)| mod_path.len() > 1)
+ .take(10)
That will, most probably, will cause #6594
<#6594> to reappear:
in my experiments, this one had appeared somewhere after 20th result.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#6918 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AANB3M677VE5YAGLS74QFATSVJGRPANCNFSM4U754A4Q>
.
|
|
My main issue is that we start to loose some common results from stdlib with this threshold set. What if we use something as simple as #6922 with this threshold? |
|
Yeah, #6922 is a great idea regardless of whether we go forward with this or not. But, looking carefully at the second screenshot, seems like we can prune quite a bit of things at the text maching level. |
|
Found a surprising reason for high (persieved) completion latency: for some reason, my dispaly switched to 30Hz refresh rate =/ |
|
So the main issue is that we suggest substring completions across path segments, right? |
|
yup
…On Sat, 19 Dec 2020 at 21:52, Laurențiu Nicola ***@***.***> wrote:
So the main issue is that we suggest substring completions across path
segments, right?
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#6918 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AANB3M2YIMAPMDVV5NX75F3SVTY6RANCNFSM4U754A4Q>
.
|
|
I think we should still improve filtering/ordering here, but I don't think it makes sense to merge this particular PR without that work first. |
7064: Ignore qualifiers when doing autoimport completions lookup r=lnicola a=SomeoneToIgnore A follow-up of #6918 (comment) and the PR itself. Tweaks the `import_map` query api to be more flexible with the ways to match against the import path and now fuzzy imports search in names only. This had improved the completion speed for me locally in ~5 times for `fuzzy_completion` span time, but please recheck me here. IMO we're fast and presice enough now, so I've added the modules back to the fuzzy search output. Also tweaks the the expect tests to display functions explicitly, to avoid confusing "duplicate" results. Co-authored-by: Kirill Bulatov <mail4score@gmail.com>
No description provided.