-
Notifications
You must be signed in to change notification settings - Fork 12.6k
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
Fix import statement completions followed by interface declaration #50350
Conversation
// includeCompletionsWithSnippetText: true, | ||
// } | ||
// }); | ||
// }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess I had previously commented this out while debugging so I could skip straight to the later tests, then forgot to uncomment. Luckily they were all still passing 😅
// line was parsed as the module specifier of a partially-typed import, e.g. | ||
// import Foo| | ||
// interface Blah {} | ||
// This appears to be a multiline-import, and editors can't replace multiple lines. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
editors can't replace multiple lines
Is this detail actually relevant to the change? It sounds like even if editors could do that, you would still want to assume that the next line is a separate statement and recover appropriately, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, fair; but it explains why the failure mode is “no completions” rather than “overwriting later declarations.”
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Basically looks good, but I can't tell if an unclosed import list would behave the same as the two existing test cases.
Fixes #48904