Skip to content

Add support for syntactic classification. #672

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

Merged
merged 5 commits into from
Sep 16, 2014

Conversation

CyrusNajmabadi
Copy link
Contributor

Tests pending.

else if (flags & SymbolFlags.TypeParameter) {
return ClassificationTypeNames.typeParameterName;
}
}
Copy link
Member

Choose a reason for hiding this comment

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

I'd put classifySymol after processNode just because it throws me off after not seeing classifySymbol being used

@DanielRosenwasser
Copy link
Member

👍

var sourceFile = getSourceFile(fileName);

var result: ClassifiedSpan[] = [];
processNode(sourceFile.getSourceFile());
Copy link
Contributor

Choose a reason for hiding this comment

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

why not just processNode(sourceFile); ?

@yuit yuit closed this Sep 15, 2014
@yuit yuit reopened this Sep 15, 2014
@JsonFreeman
Copy link
Contributor

Do you need to add fourslash hooks for these?

}

function getSyntacticClassifications(fileName: string, span: TypeScript.TextSpan): ClassifiedSpan[] {
// doesn't use compiler - no need to synchronize with host
Copy link
Contributor

Choose a reason for hiding this comment

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

Why don't we use compiler in this case?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because it's a purely syntactic feature. The comment isn't great (it was copied from elsewhere). What we should be saying is "doesn't use semantics."

CyrusNajmabadi added a commit that referenced this pull request Sep 16, 2014
Add support for syntactic classification.
@CyrusNajmabadi CyrusNajmabadi merged commit f89804e into master Sep 16, 2014
@CyrusNajmabadi CyrusNajmabadi deleted the getSyntacticClassifications branch September 16, 2014 01:56
@microsoft microsoft locked and limited conversation to collaborators Jun 18, 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