Skip to content

Support completions for enums in switch cases #16864

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

Closed
wants to merge 1 commit into from

Conversation

ghost
Copy link

@ghost ghost commented Jun 30, 2017

Support getting completions to the right of the case keyword.
Doesn't include already-covered cases.
Does not address #13711 as that scenario is if you just typed E.. That will require a lot more refactoring (which should be done anyway).

@weswigham
Copy link
Member

Is there prior art where we return an entire qualified name as a completion? I was expecting we'd return the valid enum type names as completions, you pick one, you type ".", then member completion takes over.

@ghost
Copy link
Author

ghost commented Jun 30, 2017

99% of the time there will only be one valid enum. I think this is the first time we would have a place where we return a qualified name though; it's also the first place where we return something semantically meaningful that isn't a property name or the name of a local/global variable.

@weswigham
Copy link
Member

We don't populate the completion list with all the members on this when you type thi. This just feels really inconsistent with how we handle completions elsewhere.

Copy link
Member

@rbuckton rbuckton left a comment

Choose a reason for hiding this comment

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

This seems too restrictive, and I concur with @weswigham that we currently don't use qualified names anywhere else. C# does not do this in VS, are there any other languages/editors where this is a common practice?

let symbols: Symbol[] = [];

if (isRightOfDot) {
getTypeScriptMemberSymbols();
}
else if (isRightOfCase) {
Copy link
Member

Choose a reason for hiding this comment

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

Wouldn't this restrict completions to only enum completions and prevent completions for locals? That is too restrictive.

@@ -11077,6 +11078,15 @@ namespace ts {
return links.switchTypes;
}

function getRemainingSwitchCaseTypes(node: SwitchStatement): Type[] | undefined {
Copy link
Contributor

Choose a reason for hiding this comment

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

i would move this to services, and use getTypeAtLocation for switch labels.

Copy link
Contributor

@mhegazy mhegazy left a comment

Choose a reason for hiding this comment

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

I do not think this is the right approach here..

A better approach is to mark completion list item as "suggested ", I think Roslyn refers to this as suggestedCompletion https://github.com/dotnet/roslyn/blob/878ffad23b8b06cb229c9ab31eada7634a473508/src/Features/Core/Portable/Completion/CompletionList.cs#L48

For instance in switch (node.kind) { case | you would get SyntaxKind highlighted as a suggested completion automatically..

We should also do the same for operators of ===/==/!=/!== and whenever there is a contextual type.

@mhegazy
Copy link
Contributor

mhegazy commented Jan 8, 2018

I do not think this is needed anylonger.

@mhegazy mhegazy closed this Jan 8, 2018
@ghost ghost deleted the completionsSwitchCase branch January 9, 2018 21:20
@ghost
Copy link
Author

ghost commented Jan 9, 2018

Ref: #20020

@microsoft microsoft locked and limited conversation to collaborators Jul 3, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants