Skip to content

Conversation

ghost
Copy link

@ghost ghost commented Jun 23, 2017

Some uses of this function were overly complicated or unnecessary.

@ghost ghost requested a review from jramsay June 23, 2017 17:19
scriptKind = host.getScriptKind(fileName);
}
if (!scriptKind) {
scriptKind = getScriptKindFromFileName(fileName);
Copy link
Member

Choose a reason for hiding this comment

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

is host.getScriptKind the same as getScriptKindFromFileName here? If not, where is host.getScriptKind defined? (I'm trying to figure out why you removed the fallback to getScriptKindFromFileName.)

Copy link
Author

@ghost ghost Jun 28, 2017

Choose a reason for hiding this comment

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

There's some complex logic here, but I think what I did is equivalent. In both cases:

  • If host.getScriptKind(fileName) is defined, uses that.
  • Else, uses getScriptKindFromFileName(fileName).
  • If that fails, uses ScriptKind.TS.

ensureScriptKind itself handles falling back to getScriptKindFromFileName, so there's no need to do that here.

Copy link
Member

Choose a reason for hiding this comment

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

Ah, point 3 is what I missed. Thanks.

@ghost ghost merged commit c4319e3 into master Jun 28, 2017
@ghost ghost deleted the ensureScriptKind branch June 28, 2017 21:29
@microsoft microsoft locked and limited conversation to collaborators Jun 14, 2018
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