-
Notifications
You must be signed in to change notification settings - Fork 1
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
Allow identifiers with trailing ? or ! in more places #88
base: main
Are you sure you want to change the base?
Conversation
08ec53f
to
a9621dd
Compare
not equals call without spaces | ||
============================== | ||
|
||
# should be `(foo) != (true)` but is `(foo!) = (true)` |
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 need some scanner stuff to get this working
While this syntax wasn't intentional (see crystal-lang/crystal#10917), it may be used by some programs so we should support it. |
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'm hesitant about this change, considering:
- Hopefully Check that def, macro, and block parameters don't end with
?
or!
crystal-lang/crystal#12197 is moving forward to add a warning - A lot of rules need to be updated, so it would be a bit of trouble to roll this back if the syntax is removed
- tree-sitter's error recovery already deals with trailing
?
and!
pretty well
[ | ||
$.identifier_method_call, | ||
$.identifier, | ||
], |
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.
Why is this necessary?
That's valid. I'll leave this as a draft for now in case that doesn't go forward and the current behavior stays as-is |
@@ -1227,7 +1243,19 @@ module.exports = grammar({ | |||
), | |||
|
|||
c_struct_fields: $ => { | |||
const names = seq($.identifier, repeat(seq(',', $.identifier))) | |||
const names = 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.
Outside of params, this and type declaration I don't expect to change. What are your thoughts on just adding these?
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 don't think I understand, which ones specifically?
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.
type declaration names and c struct / union fields
No description provided.