GTS Dialect #308
GTS Dialect #308
Conversation
| } else { | ||
| throw new Error(`Unknown dialect ${dialect}`); | ||
| } | ||
|
|
||
| return choice(...choices); | ||
| }, | ||
|
|
||
| // This rule is only referenced by expression when the dialect is 'gts' |
There was a problem hiding this comment.
this would be provided by tree-sitter-javascript, yea?
| @@ -419,6 +449,7 @@ module.exports = function defineGrammar(dialect) { | |||
| class_body: $ => seq( | |||
| '{', | |||
| repeat(choice( | |||
| field('template', $.glimmer_template), | |||
There was a problem hiding this comment.
only one of these may be present within the class_body
|
|
||
| ---- | ||
|
|
||
| (program |
There was a problem hiding this comment.
this is wrong, but I haven't gotten to the point where I can run the tests with the gts language yet, because I haven't been able to build them yet
|
I appreciate the PR, but this should really be its own grammar that extends typescript, and not built into this one. (See how C++ extends C) |
|
Should that then also be true of tsx? |
|
No, since TSX isn't an extension (in the same manner that GTS is) of Typescript, and users of one dialect will oftentimes want to be using both. |
|
Theyre Both implemented upstream in tree-sitter-glimmer. The glimmer_* nodes were implemented here: https:/tree-sitter/tree-sitter-javascript/pull/208/files#diff-919ac210accac9ecc55a76d10a7590e3d85ca3f0e165b52d30f08faee486d0cbr547 And right now, it seems those nodes are deprioritized / covered up by generic-type tokens. If the conflict were resolved, we wouldn't even need a dialect - no extra grammar either. Maybe this is the path i take in tree-sitter-typescript? Find the conflict and fix it? |
|
FWIW, I totally disagree with glimmer being added in JavaScript, but that PR predates my involvement (and I'm not even sure why that was merged, but it wasn't by Max). I would rather remove glimmer support from JS/TS since IMO, it should be built on top of the languages in a GJS/GTS grammar - adding support just sets the precedence that any framework can be added, which doesn't make sense |
|
As does jsx. In any case, is the strategy to do as tree-sitter-typescript does, and extend tree-sitter-javascript? Originally, Adding glimmer to JavaScript makes more sense to me, because it's just one ast node. As injection query takes it from there in tree-sitter-glimmer, and the precedence is jsx -- albeit, waaaaay less invasive than jsx (as jsx has to interveve it's syntax in and out of regular JavaScript recursively -- and this is not the case for the one glimmer_template node). |
Yeah, this is how you should do it (for glimmer JS and TS)
But JSX is valid in Javascript (.js) files, and is very tightly coupled to the ecosystem, unlike Glimmer. "Just one ast node" is not a compelling argument to support any third party framework |
This is surprising to me, and it feels like someone made a mistake 🙈
Thank you! |
|
No problem 🙂 do you mind letting me know when you have those grammars ready, and then we can remove it from the JavaScript grammar itself? It'd be a breaking change technically, but as long as there's a viable alternative that's fine for us then. |
|
Will do! |
Resolves #307
wip -- not ready for review