-
Notifications
You must be signed in to change notification settings - Fork 30
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
fix: _comment_action
rule
#27
Conversation
make_grammar.js
Outdated
@@ -107,7 +107,16 @@ module.exports = function make_grammar(dialect) { | |||
), | |||
|
|||
_comment_action: ($) => | |||
seq($._left_delimiter, $.comment, $._right_delimiter), | |||
choice( | |||
seq(token('{{'), $.comment, token.immediate('}}')), |
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.
The mixed case is missing:
{{- /* comment */}}
{{/* comment */ -}}
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.
Ups 😅 Pushed fix.
7684032
to
d015e71
Compare
I noticed this repo has a |
Sure, you will have to rebase the generated files anyway. |
comment: ($) => | ||
token( | ||
choice( | ||
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.
I wonder why comments like
{{// a comment }}
where supported when gotemplate clearly does not support them.
But since there is no test for them I doubt it was intentional.
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.
Yeah, I think this was probably taken from tree-sitter-go https://github.com/tree-sitter/tree-sitter-go/blob/12fe553fdaaa7449f764bc876fd777704d4fb752/grammar.js#L915-L924
218fff6
to
0b0ae62
Compare
Applied prettier formatting and removed unused functions/variables. tree-sitter-go-template/make_grammar.js Line 85 in 4fc529d
due to being a single token in a choice . Is this intentional?
|
I think you can just remove the |
0b0ae62
to
43207df
Compare
This triggered some changes in the test case, so I updated the tests with |
The diff is getting very difficult to read, could you remove the cosmetic changes (maybe we can later open a dedicated PR for them)? |
- Remove unused variables/functions - Remove single token choice node - Apply prettier formatting
43207df
to
85e3ec5
Compare
Sorry about that. It should be better now.
Maybe that could be done on your tree-sitter bump PR? Running |
Thanks for the contributions 😄 Also feel free to review my open PRs. |
Thank you. Will do! |
Problem:
Comments are defined in https://pkg.go.dev/text/template like so:
Solution
Change the
_comment_action
rule so a comment always has either the delimiter{{
or{{-
(note the space at the end). Also, every element in the rule should be immediate to each other. Meaning that delimiters can't be alone on a line.