Skip to content
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

Request : TSX Lexical classifier #5545

Closed
basarat opened this issue Nov 6, 2015 · 5 comments
Closed

Request : TSX Lexical classifier #5545

basarat opened this issue Nov 6, 2015 · 5 comments
Labels
Suggestion An idea for TypeScript Too Complex An issue which adding support for may be too complex for the value it adds

Comments

@basarat
Copy link
Contributor

basarat commented Nov 6, 2015

A classifier that supports TSX would be nice. Doubt that we will be able to have it though considering what we ended up doing template strings https://github.com/Microsoft/TypeScript/wiki/FAQs-for-API-Consumers#why-does-the-lexical-classifier-sometimes-give-inaccurate-results-for-nested-template-strings

PS

I looked into the TypeScript mode that ships with VSCode publicly, apparently it uses tm languages only. The advanced classifiers are only used by VSProper I think. I might need to boldly go where no OSS project has gone yet or does someone have something public that uses the SemanticClassifier 🌹?

Refs #5544

@DanielRosenwasser DanielRosenwasser added Suggestion An idea for TypeScript In Discussion Not yet reached consensus labels Nov 6, 2015
@DanielRosenwasser
Copy link
Member

TL;DR: Ultimately, we're planning to re-enable our syntactic classifier in Visual Studio, and the only reason I worked to get template strings supported in the lexical classifier was because the syntactic classifier had been disabled. So it's questionable whether we want to continue investing in the lexical classifier.

The syntactic classifier is always going to be the source of truth, and once you get that working, you get the semantic classifier for free since they're effectively the same API. If you do decide to go down that route, you should know that we on the team are more than happy to help you out with that. Also, my understanding is that @derekcicerone uses the syntactic classifier in Eclipse, but probably not through JS.

If you're not comfortable with that, or there are technical limitations that would make it way too hard to get working right, we always have the React TmLanguage file.


The thing about nested templates was that it was expected that nobody would be doing completely insane stuff, and by "doing completely insane stuff", I mean writing ridiculous/pointless code where you have multi-line templates, object literals, or function bodies, all within another template expression. And for the most part, that assumption has worked out. We haven't gotten a complaint about it in a while, and most complaints have just been out of curious experimenters and tool-writers who are stress-testing.

But having a nested JSX element span multiple lines is not just sane, it's extremely commonplace. I could pick an arbitrary level of nesting to support, like 6-8 levels deep, and that probably would do the trick to be honest (though I don't know enough about the JSX/React world). It could be pinned as a "feature" where your code highlighting breaks if your components need to get broken down into smaller elements[1]. This already sounds a little weird though, and self-closing tags would make this more awkward.

Another heuristic, which would fix your example in #5544 is to assume a regex never follows a < in a TSX file, but that still doesn't solve several issues regarding nesting.

[1] This is a joke

@basarat
Copy link
Contributor Author

basarat commented Nov 6, 2015

my understanding is that @derekcicerone uses the syntactic classifier in Eclipse

Not yet. I'm subscribed to : palantir/eclipse-typescript#228

The syntactic classifier is always going to be the source of truth, and once you get that working

Codemirror (the editor I am using) mode system needs arbitrary resume at any line whereas Syntactic Classifier is full file text only :-/

Thanks for the info though. I'll see what I can do 🌹

@CyrusNajmabadi
Copy link
Contributor

The syntactic classifier is not "full file text only". It definitely supports classifying just a range of your file:

getSyntacticClassifications(fileName: string, span: TextSpan): ClassifiedSpan[];

Why not just get the span for the line being queried and ask for the syntactic classifications for that?

@basarat
Copy link
Contributor Author

basarat commented Nov 7, 2015

Why not just get the span for the line being queried and ask for the syntactic classifications for that

Lovely. Will give that a go!

@basarat
Copy link
Contributor Author

basarat commented Nov 10, 2015

I have this working 🌹 It is a bit full file in the sense that its by fileName and the text needs to be stored with the langauge service.

I basically ported tsserver line management code into a lighter client side only language service host that gets updated before we request for tokenization. Seems to work pretty fast for now.

However there are issues around getSyntacticClassifications. I'll create new issues for that 🌹

@basarat basarat closed this as completed Nov 10, 2015
@DanielRosenwasser DanielRosenwasser added Too Complex An issue which adding support for may be too complex for the value it adds and removed In Discussion Not yet reached consensus labels Nov 10, 2015
@microsoft microsoft locked and limited conversation to collaborators Jun 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Suggestion An idea for TypeScript Too Complex An issue which adding support for may be too complex for the value it adds
Projects
None yet
Development

No branches or pull requests

3 participants