TypeScript highlighting chokes on some casts

[danquirk]

Reviewable.io uses highlight.js and I tried to create a code review using it here https://reviewable.io/reviews/Microsoft/TypeScript/3642 and as you can see my code is essentially not highlighted at all.

@pkaminski thinks the cause is a cast like:

return getModuleInstanceState((<ModuleDeclaration>node).body);

causing the highlighter to go into an XML mode and then never get back to TypeScript syntax.

TypeScript will be supporting JSX in the near future so the syntax coloring around <> will get even more complicated.

[pkaminski]

Yeah, I'm pretty sure this production triggers XML mode when it sees the cast, but it's not clear to me how to distinguish between a cast and a JSX fragment. @danquirk, do you know how the TypeScript parser deals with this ambiguity?

[danquirk]

Well we have a big parser for one ;)

The main thing is that JSX elements aren't allowed in .ts files. They'll require .tsx. So we can sidestep the issue for now and just fix .ts to handle casts properly and then figure out full JSX later. All we're really doing is implementing the grammar here: https://facebook.github.io/jsx/

I was going to take a stab at fixing it myself but I can't even get 'npm install' in highlight.js to complete successfully.

[pkaminski]

I assume old-style casts aren't allowed in .tsx files? If so, it sounds like there's really two separate language definitions, but I don't know how best to handle this in highlight.js since it tries to auto-detect the language used, which will be essentially impossible to do based solely on regexes and getting it wrong will screw up the highlighting.

In the meantime, I've hacked the TypeScript language definition used in Reviewable to ignore anything that looks like embedded XML. It's not a long-term solution though.

[danquirk]

Right, in .tsx you'll use the as operator rather than <> to cast. microsoft/TypeScript#3201

My reviews are now beautifully colorful so thanks for unblocking it there. We'll need to figure out what to do with highlight.js as you say.

[isagalaev]

Guys, I'll try to look into it coming Tuesday.

[isagalaev]

Status update.

Since there are apparently two incompatible variants of the syntax, we could probably make it work with two separate language definitions. I couldn't put together one quickly though, need to work out a nice way of reusing the TypeScript definition to add an XML-like mode on top in TypeScript + JSX (or whatever we end up calling it).

[pkaminski]

Since TypeScript+JSX is not yet out, perhaps it's worth modifying the 'ts'
definition to remove the E4X production right now to unbreak the parser and
figure out the 'tsx' language later?

On Wed, Jul 1, 2015 at 12:52 PM, Ivan Sagalaev notifications@github.com
wrote:

Status update.

Since there are apparently two incompatible variants of the syntax, we
could probably make it work with two separate language definitions. I
couldn't put together one quickly though, need to work out a nice way of
reusing the TypeScript definition to add an XML-like mode on top in
TypeScript + JSX (or whatever we end up calling it).

—
Reply to this email directly or view it on GitHub
#878 (comment)
.

Piotr Kaminski piotr@ideanest.com
"That bun is dirty. Don't eat that bun."

[isagalaev]

@pkaminski: Indeed! e9b6c64.

[pkaminski]

Thanks!

On Wed, Jul 1, 2015 at 3:13 PM, Ivan Sagalaev notifications@github.com
wrote:

@pkaminski /~https://github.com/pkaminski: Indeed! e9b6c64
e9b6c64
.

—
Reply to this email directly or view it on GitHub
#878 (comment)
.

Piotr Kaminski piotr@ideanest.com
"That bun is dirty. Don't eat that bun."