chore: add Grit target node bindings #2746
Conversation
github-actions
bot
added
A-Project
| fn language_name(&self) -> &'static str; | ||
|
|
||
| fn snippet_context_strings(&self) -> &[(&'static str, &'static str)]; | ||
|
|
||
| fn is_comment(&self, node: &GritTargetNode) -> bool; | ||
|
|
||
| fn metavariable_kind() -> Self::Kind; |
There was a problem hiding this comment.
Can we document the functions? For example, I checked snippet_context_strings and really can't understand what it is.
| // Grit metavariable | ||
| "JS_GRIT_METAVARIABLE", |
There was a problem hiding this comment.
I have mixed feelings about this
There was a problem hiding this comment.
I have some thoughts I don't know if they fit here. If we are to support embedded language formatting in the future, we may also need a "metavaraiable" kind node. For example, in JavaScript (there may be similar concepts in other languages), the embedded language are usually embedded as a template string literal that can optionally hold expression placeholders. When we want to parse and format the template literal, I think we can see those placeholders as metavariables in our internal structure.
const foo = JSON.stringify("bar");
const snippet = js`console.log(${ foo })`; // the "${ foo }" is a placeholderSo I think this kind of node is necessary but I just don't know if we should have GRIT in the name.
There was a problem hiding this comment.
That’s a very insightful comment, @Sec-ant , thanks! There is definitely a parallel between metavariables as used in Grit snippets and embedded expressions inside template literals, even though the syntax is different.
I would be happy to revise this (in a separate PR) if we can reuse these node kinds for that. Definitely something to keep in mind!
CodSpeed Performance ReportMerging #2746 will not alter performanceComparing Summary
|
Summary
This adds a few things in relation to Grit node bindings:
GritTargetNodenode in addition to the existingGritNode. The main difference is thatGritNodewrapsGritSyntaxNode, whileGritTargetNodeis an enum that can wrap nodes for any language we support in the Grit runtime (for now JS only, but other languages will likely follow).GritTargetTokenandGritTargetSyntaxKindwere added as well.GritTargetLanguagehas been further implemented along the same lines as well.JsTargetLanguagewas added so there's a concrete implementation.JS_GRIT_METAVARIABLEkind has been added to the JavaScript grammar specification. We don't have a parser/lexer for it yet, but this will be the kind to match against for Grit metavariables.Test Plan
None yet.