-
Notifications
You must be signed in to change notification settings - Fork 31
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
Tree-sitter grammar for expressions #222
Conversation
Going to merge this so I can move forward with using it. If there's any feedback on these changes, we can address them in a separate PR. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for the late review today. Left a few comments, but looks good overall.
"src" | ||
], | ||
"sources": [ | ||
"bindings/node/binding.cc", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you intentionally include the node bindings and this file? They are not used when using tree-sitter with Rust.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will remove this
$.expression, | ||
$.stringLiteral, | ||
), | ||
escapedStringLiteral: $ => token(seq('[[', /.*/)), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What if the string is not closed with ]]
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As far as I can tell from ARM docs, it's only that start to indicate it's not an expression. There isn't a corresponding closing ]]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
However, it looks like I did miss a test case [test] string
should just be a string
number: $ => /\d+/, | ||
boolean: $ => choice('true', 'false'), | ||
_members: $ => repeat1($._member), | ||
_member: $ => seq('.', $.memberName), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A member should be in the format <target>.<member-name>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
<target>
is only ever the return value of a function
function: $ => seq($.functionName, '(', optional($._arguments), ')'), | ||
functionName: $ => /[a-zA-Z]+/, | ||
_arguments: $ => seq($._argument, repeat(seq(',', $._argument))), | ||
_argument: $ => choice($.function, $.string, $.number, $.boolean), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can argument be a member expression? Like [funcA(funcB().member)]
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, good catch
_arguments: $ => seq($._argument, repeat(seq(',', $._argument))), | ||
_argument: $ => choice($.function, $.string, $.number, $.boolean), | ||
string: $ => seq("'", /[^']*/, "'"), | ||
number: $ => /\d+/, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe need to take into account decimal too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Best I can tell, decimals are not supported
PR Summary
ARM expressions are used as values of JSON properties:
They are identified by the square brackets. To have a literal square bracket as a string instead of an expression, you need to double it:
[[this is just a string]
which would be used as[this is just a string]
. Anything else would be treated as a string literal.Function args can take other functions. Strings passed to functions need to be surrounded by single-quotes.
Functions can return JSON objects which can be accessed by dot-notation:
[functionReturnObject().prop1.prop2]
.Tree-sitter grammar for expressions in DSC (as defined by ARM)
Includes the generated source code as it only needs to be generated if the grammar changes. Since this is generated code, having it as
unclean
as clippy complains.The only files worth reviewing (as they aren't generated) is
grammar.js
and the two test.txt
files