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

Typescript: sort auto-complete results by priority #47727

Closed
mindplay-dk opened this issue Apr 12, 2018 · 11 comments
Closed

Typescript: sort auto-complete results by priority #47727

mindplay-dk opened this issue Apr 12, 2018 · 11 comments
Assignees
Labels
feature-request Request for new features or functionality javascript JavaScript support issues typescript Typescript support issues

Comments

@mindplay-dk
Copy link

The auto-completion feature for Typescript in VS Code usually makes the wrong guess - and it looks like the results are merely ordered alphabetically?

Example:

image

In this case (as most of the time) it is far more likely I want the nearest matching variable connection rather than the global (window) method confirm or connect.

In PHP Storm or WebStorm, this works much better, and I've gotten used to typing extremely fast by relying heavily on it's ability to more accurately predict what I want after just two or three characters - I've learned to not even look at the auto-complete results before pressing TAB, and 95% of the time, it guesses exactly the thing I'm trying to type.

I think the Storm IDEs order the auto-complete results (in part) by vicinity?

That is, out of the results where the start of what I've typed matches, it orders those according to where I'm typing - variables in the closest scope to where I'm typing appear first, then variables in the parent scope, and so on, with results in the global scope being the least likely.

As a result, I'm learning not to trust auto-completion results, which is making me slower is Storm as well.

Could this be improved in VS Code, please?

@mjbvz mjbvz self-assigned this Apr 12, 2018
@mjbvz
Copy link
Collaborator

mjbvz commented Jul 19, 2018

@jrieken Could typescript fix this by returning proper sortText?

@mjbvz mjbvz added typescript Typescript support issues javascript JavaScript support issues labels Jul 19, 2018
@jrieken
Copy link
Member

jrieken commented Jul 19, 2018

Yeah, set sortText and/or preselect. The sortText will be used when no prefix has been typed yet or when in the presence of a prefix multiple equal ranks exists (e.g sort by rank then sort by sortText-order)

@mjbvz mjbvz added the feature-request Request for new features or functionality label Sep 11, 2018
@NimaZahedi
Copy link

@mjbvz @jrieken I'd like to work on this, I just play a bit with the code and try to get the overall understanding of that part of the code. I've made some changes to wordDistance (I'm not sure what the responsibility of wordDistance is! I just want to get it work and see how the result would be).
But I'm wondering to ask if this issue should be fixed on typescript or vscode!
Based on Matt's comment it should be fixed on typescript repo by changing getCompletionsAtPosition (I'm not sure actually just investigated a bit) or all places which is using CompletionInfo.
Also we could set sortText up based upon tsEntry.kind within MyCompletionItem on vscode repo.
Could you please explain a bit more on your comments?
Please Let me know if I'm thinking too crazy or I'm on track.

@jrieken
Copy link
Member

jrieken commented Oct 1, 2018

The wordDistance logic is new for the 1.28, see https://github.com/Microsoft/vscode-docs/blob/vnext/release-notes/v1_28.md#intellisense-locality-bonus. What changes did you have to make there, what is still missing?

Generally, "yes" this is something TypeScript should get right but the locality bonus feature is now here to compensate for that.

@NimaZahedi
Copy link

@jrieken Based on _compareCompletionItems conditions I just changed the distance of the local variables/Constants based on their kind to get them prioritised (definitely it's not a real solution).
So in that case TypeScript should fix the sortText and the we need to bring the sortText into the consideration in here.
Then we need to wait for TypeScript to make that change first and then adopt it here, right?

@NimaZahedi
Copy link

Btw, I'm wondering to ask if there is any issue regarding to sortText (^^) on TypeScript's repo.

@jrieken
Copy link
Member

jrieken commented Oct 1, 2018

we need to bring the sortText into the consideration in here.

We already do that. The initial sort order is defined by the sortText-property and later carried on as idx. There is potential for conflict with the word distance but that's implemented to be so that it doesn't conflict too much, e.g. the distance is the containing block not line/character distance

@NimaZahedi
Copy link

NimaZahedi commented Oct 1, 2018

Ah, Ok. Now I got :) Thanks.
So here we need to get locality bonus feature to work.

@NimaZahedi
Copy link

@jrieken Do we need to do anything here at all? I'm confused!
It seems locality bonus feature has been implemented already!

@jrieken
Copy link
Member

jrieken commented Oct 2, 2018

:-) yeah, I think with the locality bonus we are in a pretty good spot. having said that, it's pretty language agnostics by design and more or less works by looking at brackets only. TypeScript, having all the knowledge, will likely be better at computing those orders but lets see how far we get here

@mjbvz
Copy link
Collaborator

mjbvz commented Mar 22, 2019

I believe that microsoft/TypeScript#15024 is tracking this

We're going to look into improving suggestion sort order with typescript 3.5. I'll link back to this issue so that we can take this feedback into account too

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
feature-request Request for new features or functionality javascript JavaScript support issues typescript Typescript support issues
Projects
None yet
Development

No branches or pull requests

4 participants