Skip to content

Handle properly stringifying multiline named tuple literals#15566

Merged
straight-shoota merged 1 commit intocrystal-lang:masterfrom
Blacksmoke16:namedtupleliteral-string-representation
Apr 18, 2025
Merged

Handle properly stringifying multiline named tuple literals#15566
straight-shoota merged 1 commit intocrystal-lang:masterfrom
Blacksmoke16:namedtupleliteral-string-representation

Conversation

@Blacksmoke16
Copy link
Member

Depends on, and is similar to, #15305 but for NamedTupleLiteral nodes.

Previously code like:

{%
  {
    id: 1, active: true,
    name: "foo".upcase,
    pie: 3.14,
  }
%}

Would always be stringified as:

{% {id: 1, active: true, name: "foo".upcase, pie: 3.14} %}

But now, in-conjunction with the other PR, results in:

{%
  {
    id: 1, active: true,
    name: "foo".upcase,
    pie: 3.14,
  }
%}

I threw a bunch of test cases at it with the assumption the code may not have had the formatter ran on it and all seems 👍.

Marking as draft and skipping CI. Will rebase/trigger CI once the other dependent PR is merged. But wanted to get this opened for feedback.

@Blacksmoke16 Blacksmoke16 force-pushed the namedtupleliteral-string-representation branch from 9381f18 to 05b1dc1 Compare March 19, 2025 12:49
@Blacksmoke16 Blacksmoke16 marked this pull request as ready for review March 19, 2025 12:49
Comment on lines +206 to +210
# short-circuit to handle empty named tuple context
if node.entries.empty?
@str << '}'
return false
end
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's at least one case somewhere where an empty named tuple literal is to be stringified. Having this early return prevents having to use .first? and .last? and dealing with nil.

@straight-shoota straight-shoota added this to the 1.17.0 milestone Apr 17, 2025
@straight-shoota straight-shoota merged commit a04a7a6 into crystal-lang:master Apr 18, 2025
40 of 44 checks passed
@Blacksmoke16 Blacksmoke16 deleted the namedtupleliteral-string-representation branch April 18, 2025 19:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants