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

Completion sorting ignores sortText from completion items when typing #79516

Closed
DanTup opened this issue Aug 20, 2019 · 26 comments
Closed

Completion sorting ignores sortText from completion items when typing #79516

DanTup opened this issue Aug 20, 2019 · 26 comments
Labels
*as-designed Described behavior is as designed *question Issue represents a question, should be posted to StackOverflow (VS Code)

Comments

@DanTup
Copy link
Contributor

DanTup commented Aug 20, 2019

It seems like there have been some issues around this before, but I can't find any conclusive description that this is by design (though there's a comment from @jrieken at #71660 (comment) suggesting there may be internal ranking going on that can't currently be disabled, but I can't find the related case to see if it's been addressed yet). I've tried all of the "suggest" config options I can find, but I can't find a way to disable this.

Here's what my completion item provider is returning:

const allResults: CompletionItem[] = [
	{ sortText: "998893", label: "MainAxisSize.min" },
	{ sortText: "999992", label: "CustomInactiveIcon" },
	{ sortText: "999993", label: "AnimationMin" },
	{ sortText: "999993", label: "BlendMode.luminosity" },
	{ sortText: "999993", label: "kMinInteractiveDimension" },
	{ sortText: "999993", label: "MinColumnWidth" },
	{ sortText: "999996", label: "$minus" },
	{ sortText: "999996", label: "kDoubleTapMinTime" },
	{ sortText: "999996", label: "kMinFlingVelocity" },
	{ sortText: "999996", label: "minBy(…)" },
	{ sortText: "999997", label: "Float32x4.fromInt32x4Bits(…)" },
	{ sortText: "999997", label: "FrameTiming" },
	{ sortText: "999997", label: "FrameTiming(…)" },
	{ sortText: "999997", label: "min(…)" },
	{ sortText: "999997", label: "RawSocketOption.fromInt(…)" },
	{ sortText: "999997", label: "TimingsCallback(…)" },
];

And when I invoke completion, the list sorted as expected:

Screenshot 2019-08-20 at 3 34 59 pm

However, when the user types min, VS Code does its own sorting, and the values the user is interested in (which we've ranked highest according to sort order) move right down the list:

Screenshot 2019-08-20 at 3 35 58 pm

I appreciate VS Code is trying to be helpful, but here it's magic is making things significantly worse. Is there some way to influence this (either as a user, or an extension author) that I've overlooked, or is this currently the expected behaviour?

(Raised at Dart-Code/Dart-Code#1934 by @dancojocaru2000)

@vscodebot
Copy link

vscodebot bot commented Aug 20, 2019

(Experimental duplicate detection)
Thanks for submitting this issue. Please also check if it is already covered by an existing one, like:

@DanTup
Copy link
Contributor Author

DanTup commented Aug 20, 2019

(I found #64367, but that seems more about filtering than sorting?)

@DanTup DanTup changed the title Completion sorting ignores sortText from completion items Completion sorting ignores sortText from completion items when typing Aug 20, 2019
@jrieken jrieken added *as-designed Described behavior is as designed *question Issue represents a question, should be posted to StackOverflow (VS Code) labels Aug 20, 2019
@vscodebot
Copy link

vscodebot bot commented Aug 20, 2019

Please ask your question on StackOverflow. We have a great community over there. They have already answered thousands of questions and are happy to answer yours as well. See also our issue reporting guidelines.

Happy Coding!

@vscodebot
Copy link

vscodebot bot commented Aug 20, 2019

The described behavior is how it is expected to work. If you disagree, please explain what is expected and what is not in more detail. See also our issue reporting guidelines.

Happy Coding!

@vscodebot vscodebot bot closed this as completed Aug 20, 2019
@jrieken
Copy link
Member

jrieken commented Aug 20, 2019

An extension can define the initial order with sortText but as soon as the user starts typing a prefix that initial order is overruled by the scores computed by comparing completions against the prefix.

@DanTup
Copy link
Contributor Author

DanTup commented Aug 20, 2019

@jrieken does this mean there's no way to override it, and it won't be considered? The example given above is a bad user experience (and leads to bugs being raised) - it seems silly not to consider an option to control this.

Also - out of interest - the IntelliCode extension claims to do intelligent sorting of completions - does that also only apply when there's no prefix?

@dancojocaru2000
Copy link

This behaviour is very limiting as the overruled results are completely irrelevant. When passing the value of an enum, I would much rather have the enum values completed than the min function.

@jrieken
Copy link
Member

jrieken commented Aug 20, 2019

@jrieken does this mean there's no way to override it, and it won't be considered?

Yes. However, if find cases in which the scorer selects the wrong item or ranks badly then feel free to file issues.

In above sample suggestions like min and minBy exist and when a user types min that's a very strong signal she/he doesn't want to insert MainAxisSize.min but something that starts with min, otherwise one should type Main, MAS, etc.

@DanTup
Copy link
Contributor Author

DanTup commented Aug 20, 2019

In above sample suggestions like min and minBy exist and when a user types min that's a very strong signal she/he doesn't want to insert MainAxisSize.min but something that starts with min

That's not true if you have the context that the language server has. The language server knows that's n argument and its type is MainAxisSize and that MainAxisSize.min is absolutely the best bet if they've typed min.

VS Code doesn't have anywhere near as much context as the language server so having it override the results the server came up with with simple text-based heuristics that don't understand the language will always be sub-optimal to a good language server. What's the reason this couldn't be controlled by an option (either for the user, or the completion provider)?

@jrieken
Copy link
Member

jrieken commented Aug 21, 2019

That's not true if you have the context that the language server has.

I agree that the language server has most context but this is about enabling users to pick any suggestion. Assuming I do want to insert min, what should I do? Type min and then press arrow down until I am there? Would that be good UX?

Resorting items based on user input isn't a critic to the language server but helping the user to insert the suggestion she/he wants to select. I understand that from your point of view it should be the absolute best suggestion but you have to accept users choosing different (or not make that suggestion to begin with).

The selection/resorting is based on an algorithm which, amongst other things, favours matches at the beginning of words over matches at the end. I think that's a fair assumption because words are usually typed from 'start to end' (and not 'end to start').

@dancojocaru2000
Copy link

If the parameter is an enum and the min function returns any kind of number type, I would argue that the user doesn't want the min function, but rather the Enum.min value.
In Swift, this problem is tackled by being able to type .min, without the enum type prefix. It is very tiring to be forced to type the enum type non stop.
Also, technically, min is at the beginning of the possible enum values, after the dot. Even if VS does the sorting, it seems weird to put kMinSomething before Enum.min.

@jrieken
Copy link
Member

jrieken commented Aug 21, 2019

In Swift, this problem is tackled by being able to type .min, without the enum type prefix. It is very tiring to be forced to type the enum type non stop.

Did you try that? You should be able to type .min and MainAxisSize.min should be the top match. If not then that's a bug.

Even if VS does the sorting, it seems weird to put kMinSomething before Enum.min.

Matches on upper-case characters and after separate characters (., _, -, etc) are boosted. So a draw between matching M and .m. In that case matching at a lower index (1 vs 5) makes the difference.

Last, if you think no-one ever types MainAxisSize than set .min or min as CompletionItem#filterText so that typing min makes that the top match (tho it disables typing MainAxisSize letters, so tweak the label and have a full type label)

@DanTup
Copy link
Contributor Author

DanTup commented Aug 21, 2019

I agree that the language server has most context but this is about enabling users to pick any suggestion. Assuming I do want to insert min, what should I do? Type min and then press arrow down until I am there? Would that be good UX?

In this specific case, yes - it would definitely be the best UX. Because the language server knows that the chance of that case is almost 0 and the case of the other is almost 100% because it has context. The language server has done hard work to provide contextual-relevant results but VS code is overriding them with naive text-based heuristics.

Resorting items based on user input isn't a critic to the language server but helping the user to insert the suggestion she/he wants to select.

But VS Code doesn't have all the information to make the best call, as is shown here. If VS Code wants to do the sorting, then it should allow servers to provide it more information to make a better decision.

If I change filterText on MainAxisSize.min to just min then the sorting remains good (because it "starts with min"), however if the user starts typing MainAxisSize to see what all the options are, it disappears. What if we could supply multiple filterTexts (for ex MainAxisSize.min and min) or the heuristic could consider foo.bar to be equally matched with things starting with bar?

Out of interest - how does IntelliCode work? Does it suffer from the same issue, or does it have some ability to mark preferred completions that we don't have?

@DanTup
Copy link
Contributor Author

DanTup commented Aug 21, 2019

You should be able to type .min and MainAxisSize.min should be the top match. If not then that's a bug.

Not only does it not work, it puts minBy above min :/

Screenshot 2019-08-21 at 9 40 37 am

Last, if you think no-one ever types MainAxisSize than set .min or min as CompletionItem#filterText

I mentioned that above, but that removes the ability for a user to type the enum name to see all the values if they don't know what they are (and that's even worse UX).

@dancojocaru2000
Copy link

Typing .min when the programming language doesn't support this feature is unintuitive, I wouldn't have thought of doing that. Also, as shown above, it doesn't even work.

@jrieken
Copy link
Member

jrieken commented Aug 21, 2019

The language server has done hard work to provide contextual-relevant results but VS code is overriding them with naive text-based heuristics.

Sorry, you don't understand what I am saying. This is not about VS Code but about users having the freedom to select the suggestions you are making. Don't make bad suggestions if you don't want them to be inserted.

Out of interest - how does IntelliCode work? Does it suffer from the same issue, or does it have some ability to mark preferred completions that we don't have?

There is no difference. They use CompletionItem#preselect in addition to sorting but they can also not overrule user intend.

Typing .min when the programming language doesn't support this feature is unintuitive, I wouldn't have thought of doing that. Also, as shown above, it doesn't even work.

I have filed #79558 for that.

@dancojocaru2000
Copy link

Freedom? What freedom? A list is shown anyway. This is about the language server saying "here's what they 99% meant" and VS Code saying "nope, they actually meant this because firstMatch.startIndex is lower", even if min(a, b) returns int but the function expects MainAxisSize.

@DanTup
Copy link
Contributor Author

DanTup commented Aug 21, 2019

Sorry, you don't understand what I am saying. This is not about VS Code but about users having the freedom to select the suggestions you are making. Don't make bad suggestions if you don't want them to be inserted.

This isn't about good and bad suggestions though, it's about "very relevant" and "not so relevant". We shouldn't exclude entries from completion just because they're very unlikely if they could still be valid (although min returns a number, some entries return classes, which could have fields that are the correct type).

There is no difference. They use CompletionItem#preselect

In their examples, the ones with the stars all appear at the top (it's possible the example gif isn't showing the whole picture though). Are they all set as preselect? If not, how do they all stay at the top?

I have filed #79558 for that.

Thanks - based on the text there it sounds like you already have a concept of a separator. Given that, couldn't you weight foo.bar against bar equally with the first matches? It seems likely if the language has gone to the effort of putting something with a separator in the list it's because they want the user to have the convenience of being able to pick it directly, so if the second part starts with, it seems also like a strong signal?

@gjsjohnmurray
Copy link
Contributor

To give my provideCompletionItems method more control over this kind of thing I have resorted to returning its results as a CompletionList with isIncomplete: true. This stops VSCode from resequencing what I gave it. When the user types another character I get called again and can supply another list. I also use this to achieve prefix-only matching.

@DanTup
Copy link
Contributor Author

DanTup commented Aug 21, 2019

@gjsjohnmurray does that actually work here? I see VS Code doing the same magic if I force completion open with Ctrl+Space after typing the prefix (so it's hitting my provider), but it still seems to re-sort them based on the prefix it sees in the doc, rather than one that was typed since opening the completion list.

@gjsjohnmurray
Copy link
Contributor

@DanTup I've certainly found isIncomplete useful, but YMMV

@DanTup
Copy link
Contributor Author

DanTup commented Aug 22, 2019

It's definitely useful, but I don't think it helps here. I tried enabling it but it made no difference.

@jrieken I just noticed that just typing m causes MainAxisSize.min to move down too. Is it intended that the casing matters? I don't expect many people type capitals when they want to complete something that starts with a capital.

Screenshot 2019-08-22 at 8 48 49 am

@jrieken
Copy link
Member

jrieken commented Aug 22, 2019

@jrieken I just noticed that just typing m causes MainAxisSize.min to move down too. Is it intended that the casing matters?

Yeah, that because m and min are perfect prefix matches and m and Main... aren't. I see the issue. We do boost matches when users invest effect in typing upper-case letters but shouldn't do it the other way around.

@jrieken
Copy link
Member

jrieken commented Aug 22, 2019

extracted #79627 -> there is merit in that behaviour: see #79627 (comment)

@DanTup
Copy link
Contributor Author

DanTup commented Sep 2, 2019

@jrieken not sure this warrants its own issue if it's expected, but here's another place I think the ranking is bad:

Screenshot 2019-09-02 at 10 36 07 am

I think things that have "end" as concurrent characters (and in exact matched case) should come above things that have e, n, d just in them in that order with mismatched casing (eg. MainAxisAlignment.end is far more likely for end than Encoding).

@dancojocaru2000
Copy link

Any updates on this? Any change of mind? There seems to be a tradition for VS Code to think it's smarter than the language server and mess things up, creating a bad user experience.
Why do we even have the language server if VS Code doesn't listen to it?

Time to move to JetBrains.

@vscodebot vscodebot bot locked and limited conversation to collaborators Oct 4, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
*as-designed Described behavior is as designed *question Issue represents a question, should be posted to StackOverflow (VS Code)
Projects
None yet
Development

No branches or pull requests

4 participants