-
Notifications
You must be signed in to change notification settings - Fork 12.8k
Less aggresive completion list #1767
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
Conversation
…ic type + fourslash tests
@@ -852,13 +852,13 @@ module ts { | |||
// | |||
export interface LanguageServiceHost extends Logger { | |||
getCompilationSettings(): CompilerOptions; | |||
getNewLine?(): string; | |||
getNewLine? (): string; |
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.
bunch of these extra spaces showing up in the interface declaration
@@ -1079,6 +1079,7 @@ module ts { | |||
|
|||
export interface CompletionInfo { | |||
isMemberCompletion: boolean; | |||
isBuilder: boolean; |
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.
The name of the property is not clear. i think we should name it on what it stands for instead of how it is going to be used. so something a long the lines of possibleIdentifierDefinition, possibleDeclarationLocation..
also a comment explaining what it does would be appreciated.
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, I still don't know what this is apart from a term that came up as a conceptual solution for the issue this addresses.
@@ -2512,9 +2565,9 @@ module ts { | |||
case "class": | |||
case "interface": | |||
case "enum": | |||
case "module": |
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.
Why not "module"
?
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.
We want to show completion when you re-open an existing module in a new file.
@@ -2384,6 +2386,7 @@ module ts { | |||
else { | |||
// Get scope members | |||
isMemberCompletion = false; | |||
isNewIdentifierLocation = isNewIdentifierDefinitionLocation(previousToken); |
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.
set it to true for preceding if block also.
I don't quite agree with this approach (ditto for isIdentifierDefinitionLocation). The correct approach is to check the kind of the parent, and check which child of the parent a node is. That way you can see if its precedence allows for a lambda, or any construct that you care about. |
@JsonFreeman I doubt there will a net gain using that approach, you'll just end up with a different switch statement. I also want to get this in, so we can validate the experience. We can always decide to change the implementation at a later time. |
👍 |
Signing off just so we can test the experience 👍 @mhegazy we should review whether this is appropriate as time goes on. |
This adds a builder property to the completion list; this allows us to show the completion list in more places and keep support for alternative completion keys, like space and dot. Note this required changes on the visual studio side too, so the complete experience won't be available until those are released. This fixes #1629, #1505, #1428 and negates #315.
This adds a builder property to the completion list; this allows us to show the completion list in more places and keep support for alternative completion keys, like space and dot.
Note this required changes on the visual studio side too, so the complete experience won't be available until those are released.
This fixes #1629, #1505, #1428 and negates #315.