Skip to content

Add completions from literal contextual types#24674

Merged
3 commits merged into
masterfrom
completionsLiterals
Jun 7, 2018
Merged

Add completions from literal contextual types#24674
3 commits merged into
masterfrom
completionsLiterals

Conversation

@ghost
Copy link
Copy Markdown

@ghost ghost commented Jun 4, 2018

Fixes #16863

@ghost ghost force-pushed the completionsLiterals branch 4 times, most recently from ed58a9d to 18f6953 Compare June 5, 2018 15:29
@ghost ghost force-pushed the completionsLiterals branch from 18f6953 to e46d214 Compare June 5, 2018 16:04
@ghost ghost requested a review from sheetalkamat June 5, 2018 16:49
Comment thread src/services/utilities.ts Outdated
node.getEnd() <= textSpanEnd(span);
}

export function getTypesOfUnion(type: Type): ReadonlyArray<Type> {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think we need better name as the name suggests that it handles only union.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Also this creates unnecessary array for the non union type. Depending on how frequently this would be called, that's too many arrays for no reason?

if (type.flags & TypeFlags.String) {
return createLiteral("");
}
else if (type.flags & TypeFlags.Number) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

not sure about string and number.. the possible values here are infinite, any thing we suggest will likely be wrong.

@ghost ghost force-pushed the completionsLiterals branch from baba716 to 581e350 Compare June 7, 2018 20:34
@ghost ghost merged commit 33d0893 into master Jun 7, 2018
@ghost ghost deleted the completionsLiterals branch June 7, 2018 22:03
@microsoft microsoft locked as resolved and limited conversation to collaborators Oct 21, 2025
This pull request was closed.
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.

2 participants