-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Format multi-line braces blocks using do/end #4517
Conversation
@@ -2480,11 +2480,13 @@ module Crystal | |||
next_token_skip_space_or_newline | |||
end | |||
|
|||
if @token.keyword?(:do) | |||
needs_do_block = node.location.try(&.line_number) != node.end_location.try(&.line_number) |
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 not entirely sure this is the correct way to detect this but it seems to work.
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 was going to do it using the same logic, so I think it's OK :-)
I'm merging this because this will effectively end discussions about when to use |
While working on rufo I realized this is wrong. For example: foo bar {
1 + 2
} Here the block is associated with |
@asterite I don't think that foo(bar) do
1 + 2
end but either of those suggestions can be implemented without the other. I think it's the fact this changes semantics which should be fixed, not this PR (but reverting it is good temporarily). |
I've looked into making a PR for my above suggestion but I'm finding it hard. The crystal parser is very complex and has a lot of accumulated, undocumented state and undocumented abstractions. It's very hard to refactor the parser to take advantage of the added simplicity of having 1 unambiguous parsing rule for blocks. |
Perhaps @asterite would have fun unifying the two block syntaxes :). I still think it's a bad idea that |
Fixes #4504.