GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — Support completions for enums in switch cases · Pull Request #16864 · microsoft/TypeScript

Conversation

@ghost

@ghost Deleted user (ghost) commented Jun 30, 2017 •
edited by rbuckton
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown

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).

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Andy Hanson
Support completions for enums in switch cases
c60ed78
@msftclas Microsoft Contribution License Agreements (msftclas) added the cla-not-required label Jun 30, 2017
@weswigham

Wesley Wigham (weswigham) commented Jun 30, 2017

Copy link
Copy Markdown
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.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@ghost

Deleted user (ghost) commented Jun 30, 2017

Copy link
Copy Markdown
Author

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.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@weswigham

Wesley Wigham (weswigham) commented Jul 3, 2017

Copy link
Copy Markdown
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.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
rbuckton
Ron Buckton (rbuckton) suggested changes Aug 7, 2017
View reviewed changes

@rbuckton Ron Buckton (rbuckton) left a comment

Copy link
Copy Markdown
Contributor

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.

This seems too restrictive, and I concur with Wesley Wigham (@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?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread src/services/completions.ts
if (isRightOfDot) {
getTypeScriptMemberSymbols();
}
else if (isRightOfCase) {

@rbuckton Ron Buckton (rbuckton) Aug 7, 2017

Copy link
Copy Markdown
Contributor

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.

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@ghost ghost mentioned this pull request Aug 30, 2017
Allow quoted names in completions #18162
Merged
@msftgits Microsoft GitHub User (msftgits) removed the cla-not-required label Sep 26, 2017
mhegazy
Mohamed Hegazy (mhegazy) reviewed Nov 7, 2017
View reviewed changes
Comment thread src/compiler/checker.ts
return links.switchTypes;
}

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

@mhegazy Mohamed Hegazy (mhegazy) Nov 7, 2017

Copy link
Copy Markdown
Contributor

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 would move this to services, and use getTypeAtLocation for switch labels.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
mhegazy
Mohamed Hegazy (mhegazy) suggested changes Nov 8, 2017
View reviewed changes

@mhegazy Mohamed Hegazy (mhegazy) left a comment

Copy link
Copy Markdown
Contributor

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 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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@mhegazy Mohamed Hegazy (mhegazy) mentioned this pull request Nov 9, 2017
[typescript] Offer IntelliSense suggestions based on method parameter or local type #19855
Closed
@mhegazy

Mohamed Hegazy (mhegazy) commented Jan 8, 2018

Copy link
Copy Markdown
Contributor

I do not think this is needed anylonger.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@mhegazy Mohamed Hegazy (mhegazy) closed this Jan 8, 2018
@ghost
ghost deleted the completionsSwitchCase branch January 9, 2018 21:20
@ghost

Deleted user (ghost) commented Jan 9, 2018

Copy link
Copy Markdown
Author

Ref: #20020
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@microsoft 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.

Reviewers

No reviews

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

5 participants

@weswigham @mhegazy @rbuckton @msftclas @msftgits
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.