-
-
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
Parser: fix #4590, correct to parse empty parenthesis "()" #4592
Parser: fix #4590, correct to parse empty parenthesis "()" #4592
Conversation
Should it not just remove the empty brackets? Why would you use empty brackets anyway? |
@RX14 I believe I and we don't write empty parenthesis by hand, but it is useful for code generated by program. |
@makenowjust but do empty brackets change the meaning of the program? I can't see a way they do so shouldn't they just be removed? |
@RX14 It has the result value |
@makenowjust Ah I see, yes this makes sense now. |
Another point is that an editor integration that continously formats shouldn't remove them. |
I think I copied |
@asterite How do you think about expressions list inside parenthesis like |
For multi expression one needs multiple expressions :-) So |
Multiple expressions is needed, really? I think all multiple expressions can be converted to |
A somewhat usefully application would be joining some expressions in a macro, like: macro exprs(*exprs)
({{ exprs.join(";").id }}).to_s
end
exprs(1, 2 + 3)
exprs If an empty @makenowjust |
Please give me a real example. I thought it a day, but I couldn't find it. Important points are two:
Thank you. |
Every expressions have no side effect, this discussion can be thrown out, though we can't see |
Hi, could be this a final solution to #4590? |
It is intended as such see |
In fact, no multiple expressions with parenthesis is used in compiler source codes, standard library and all samples in this repository. Where is real world use case? |
Let's remove |
Why not merge this clearly useful fix and then decide the fate of the syntax |
Currently "()" is parsed as NilLiteral, but it loses parenthesis information and it produces a formatter bug (crystal-lang#4590). Correct to parse "()" as Expressions whose keyword property is :"(" and which contains `Nop` only. This looks like empty begin block. Also formatter works fine.
0dead34
to
a7f574d
Compare
@mverzilli ping |
@RX14 @ysbaddaden @bcardiff @matiasgarciaisaia this is ready to merge IMO. |
@mverzilli Thank you ❤️ |
Currently "()" is parsed as
NilLiteral
, but it loses parenthesis information and it produces a formatter bug (#4590).Correct to parse "()" as
Expressions
whose keyword property is:"("
and which containsNop
only. This looks like empty begin block. Also formatter works fine.