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

Do not suggest rare autocompletion values #29921

Closed
mikemaccana opened this issue Feb 14, 2019 · 5 comments
Closed

Do not suggest rare autocompletion values #29921

mikemaccana opened this issue Feb 14, 2019 · 5 comments
Labels
Needs Investigation This issue needs a team member to investigate its status.

Comments

@mikemaccana
Copy link

This not a question.

See typing assert.anything inside js files will result in MSFIDOCredentialAssertion.anything

Instead of disabling editor.acceptSuggestionOnCommitCharacter, vscode should present better suggestions.

The only references to MSFIDOCredentialAssertion on the internet are JS users using vscode getting weird suggestions.

This not a question. @vscodebot please don't say it is. I know about Stack Overflow. I am not asking there because this is not a question. This not a question. You are a silly bot.

@mjbvz mjbvz transferred this issue from microsoft/vscode Feb 14, 2019
@mjbvz mjbvz removed their assignment Feb 14, 2019
@RyanCavanaugh
Copy link
Member

Removing "rare" things doesn't fix anything, even setting aside the question of who gets to define "rare". If MSFIDOCredentialAssertion isn't in the suggestion list, you're going to complete to some other partial-match of assert instead.

I don't really see what we're supposed to do from the semantic side here. The completion is a valid partial match, and you have turned on the setting that controls whether partial matches get completed to full matches on ..

We can't even add a shim .d.ts file for "common" JS identifiers, because when a real .d.ts gets loaded, it'll conflict with that symbol.

Probably the only thing to do is insert truly fake entries in the global completion list for things like assert - it's probably more on VS Code's side to do that, though; we don't want TS Server sending back made-up entries for this scenario.

@RyanCavanaugh RyanCavanaugh added the Needs Investigation This issue needs a team member to investigate its status. label Feb 14, 2019
@mikemaccana
Copy link
Author

mikemaccana commented Feb 16, 2019

I don't really see what we're supposed to do from the semantic side here.

Being that 'assert' is a common thing to require / import - and assuming MSFIDOCredentialAssertion is currently at the top because the user has forgotten to require / import it, the top suggestion would be to add the missing / require / import.

you have turned on the setting that controls whether partial matches get completed to full matches on .

As a side note, that setting is on by default, the user has not turned it on.

@mjbvz
Copy link
Contributor

mjbvz commented Aug 28, 2019

For the original problem, the fix is to make sure that TypeScript IntelliSense knows that a function called assert exists. This may require adding an import to the relevant package or installing some typings. However this won't prevent these other suggestions from showing up (which microsoft/vscode#80016 also brings up).

It is subjective which suggestions are useful or not, and failing to show a less common suggestion that a user really is looking for is a worse UX than showing this suggestion in cases where the user doesn't need it. Letting users configure a blacklist of completion names in VS Code is also not a good idea as these lists would be quite long and need to updated constantly as new apis are added

Instead, a better solution in my opinion would be to split lib.dom.d.ts into to a small set of core apis and then a set of additional supporting files (although what exactly constitutes core dom API is difficult to say). This would let users configure which specific subset of the dom api they wish to see. This idea was brought up in #25567

@mjbvz mjbvz removed their assignment Nov 6, 2019
@mjbvz
Copy link
Contributor

mjbvz commented Nov 6, 2019

Closing this since it has only seen two upvotes since February

@mjbvz mjbvz closed this as completed Nov 6, 2019
@madiodio
Copy link

madiodio commented Nov 7, 2019

Hey @mjbvz, I understand that maintaining these issues is kind of challenging but closing issues based solely on upvotes may be shortsighted imho. Instead I suggest looking at the number of duplicates for example (#10565, #54971, #22372, #15750, #27899, #37982, #26624, #26340, #45039, #80016, ...) factored in with the number of upvotes, reactions and comments. Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Investigation This issue needs a team member to investigate its status.
Projects
None yet
Development

No branches or pull requests

4 participants