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

Don’t offer import statement completions at from position #44125

Conversation

andrewbranch
Copy link
Member

@andrewbranch andrewbranch commented May 17, 2021

Hoping to get this fix into for 4.3. The current state is pretty disruptive if your muscle memory expects a completion for from but you get an import completion instead.

Fixes this:

import statement completions showing auto imports starting with 'f' at 'import {} f'

By making it this:

completions at 'f' now shows 'from' and no auto imports

@@ -183,6 +185,20 @@ namespace ts.Completions {
return { isGlobalCompletion: false, isMemberCompletion: false, isNewIdentifierLocation: false, entries };
}

function specificKeywordCompletionInfo(keywords: readonly SyntaxKind[]): CompletionInfo {
Copy link
Member Author

@andrewbranch andrewbranch May 17, 2021

Choose a reason for hiding this comment

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

Making a way to return specific keywords is extra credit beyond just fixing the obvious regression; the behavior in 4.2 was not great:

image

@@ -183,6 +185,20 @@ namespace ts.Completions {
return { isGlobalCompletion: false, isMemberCompletion: false, isNewIdentifierLocation: false, entries };
}

function specificKeywordCompletionInfo(keywords: readonly SyntaxKind[]): CompletionInfo {
return {
isGlobalCompletion: true,
Copy link
Member Author

@andrewbranch andrewbranch May 17, 2021

Choose a reason for hiding this comment

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

I tried making this false but the snippets for for loops and stuff still showed up 🤔

Copy link
Member

@DanielRosenwasser DanielRosenwasser May 17, 2021

Choose a reason for hiding this comment

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

Should it be false then?

@mjbvz any idea why that might be happening?

Copy link
Member Author

@andrewbranch andrewbranch May 17, 2021

Choose a reason for hiding this comment

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

Set to false now despite seeing no difference locally

Copy link
Member

@DanielRosenwasser DanielRosenwasser left a comment

Seems right, but I feel like you should specify isGlobalCompletions as false (or at least get clarity on behavior), and you could also just switch to a cheaper lookup for name.

Once that's done, we can cherry-pick into release-4.3.

src/services/completions.ts Outdated Show resolved Hide resolved
@andrewbranch
Copy link
Member Author

andrewbranch commented May 17, 2021

@typescript-bot cherry-pick this to release-4.3

@typescript-bot
Copy link
Collaborator

typescript-bot commented May 17, 2021

Heya @andrewbranch, I've started to run the task to cherry-pick this into release-4.3 on this PR at afa4d05. You can monitor the build here.

@typescript-bot
Copy link
Collaborator

typescript-bot commented May 17, 2021

Hey @andrewbranch, I've opened #44136 for you.

typescript-bot pushed a commit to typescript-bot/TypeScript that referenced this pull request May 17, 2021
Component commits:
34b80a5 Don’t offer import statement completions at `from` position

afa4d05 Set isGlobalCompletion to false, use indexOf lookup
@andrewbranch andrewbranch merged commit 271e069 into microsoft:master May 18, 2021
9 checks passed
@andrewbranch andrewbranch deleted the bug/import-statement-completions-from branch May 18, 2021
andrewbranch added a commit that referenced this pull request May 18, 2021
Component commits:
34b80a5 Don’t offer import statement completions at `from` position

afa4d05 Set isGlobalCompletion to false, use indexOf lookup

Co-authored-by: Andrew Branch <andrew@wheream.io>
@DanielRosenwasser DanielRosenwasser added this to the TypeScript 4.3.2 milestone May 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Author: Team For Uncommitted Bug PR for untriaged, rejected, closed or missing bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants