Fix: incorrect to_s for Attribute AST node #4098 #4099
Conversation
olbat
force-pushed
the
fix_compiler_to_s_attribute
branch
from
626313b
to
bda3efd
Compare
|
Oops, you're right that's a better way to fix it, done ! |
Commas are missing in the result of the `to_s` method of Attribute nodes so the output is invalid (i.e. `@[Foo(a: 1b: 2)]`).
src/compiler/crystal/syntax/to_s.cr
Outdated
| @str << ", " if printed_arg | ||
| named_args.each do |named_arg| | ||
| named_args.each_with_index do |named_arg, i| | ||
| @str << ", " if printed_arg || i > 0 |
There was a problem hiding this comment.
The variable i is not needed here. And printed_arg = true below can remain. That's the only change needed (you only needed to move exactly one line one position down ^_^)
There was a problem hiding this comment.
Again you're right ... So much changes for such a small modification, sorry for that and thank for your time :)
|
@olbat I added another request for change. In addition to that, I don't understand why the other specs for |
Commas are missing in the result of the `to_s` method of Attribute nodes so the output is invalid (i.e. `@[Foo(a: 1b: 2)]`).
It's related to a discussion in #4098, should I create another pull request for this change ? |
olbat
force-pushed
the
fix_compiler_to_s_attribute
branch
from
bda3efd
to
a4c5aab
Compare
|
@olbat Ooh... I see. Yes, as another PR is good, thank you! |
Commas are missing in the result of the
to_smethod of Attribute nodes so the output is invalid (i.e.@[Foo(a: 1b: 2)]).$ cat test.cr require "compiler/crystal/syntax" ast = Crystal::Parser.parse(%{@[Test(1, 2, n1: 1, n2: 2)]}) puts code = ast.to_s Crystal::Parser.parse(code) $ crystal run test.cr @[Test(1, 2, n1: 1, n2: 2)]