Skip to content

Fix multilining of call writes#756

Merged
reese merged 3 commits intotrunkfrom
reese-multiline-csend
Jan 5, 2026
Merged

Fix multilining of call writes#756
reese merged 3 commits intotrunkfrom
reese-multiline-csend

Conversation

@reese
Copy link
Collaborator

@reese reese commented Jan 4, 2026

Closes #752

This is a two-for-one PR that fixes two different bugs for calls where the left-hand side is a call chain that should break to multiple lines.

For Prism, attr_write call chains were always single-lined, I believe largely as a workaround for the left and right sides being wrapped in the same breakable. This separately formats the left- and right-hand side in their own breakables so that they can multiline independently of each other.

For Ripper, the issue was that Field nodes were not handled as call chains and had their own formatting logic that doesn't understand multilining. This now converts them into CallChainElements so that they can use the regular call chain machinery.

Each change is done in its own commit.

Copy link
Collaborator

@froydnj froydnj left a comment

Choose a reason for hiding this comment

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

FWIW, I think there are a few heredoc formatting issues that I need to reduce down, and a couple other miscellaneous things, but the bulk of the remaining reformatting on Stripe's codebase is #713, and we've decided we're going to live with that.

Comment on lines +2127 to +2129
// Render the right-hand side of the write in its own breakable, so that each side can
// break independently of each other
let trailing_attr_write_value = chain_elements.last().and_then(|last| {
Copy link
Collaborator

Choose a reason for hiding this comment

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

It would be nice if we could only do this calculation once, rather than for every node in the call chain...or are there cases where we could have multiple attribute writes in the call chain? I guess the same thing goes for computing trailing arefs, too?

Those changes could be handled separately, if at all.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think in practice this almost always does happen only once for the chain unless you have arefs in the middle of the chain, which is not the most common construct. This runs once per "segment" where a segment is more-or-less call chains separated by arefs. That is, something foo.bar[1].baz would be two segments, foo.bar[1] and then <previous-segment>.baz. And for attr_writes specifically, I believe they'll always be exactly one segment so this only runs once.

(I grant that this segment stuff is slightly awkward, but it's because Prism handles arefs as calls instead of as their own separate construct, so we had to manually split up the chains into these segments to mirror the Ripper behavior.)

@reese reese merged commit 3cc697c into trunk Jan 5, 2026
8 checks passed
@reese reese deleted the reese-multiline-csend branch January 5, 2026 15:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[prism] refusing to line-break extremely long csend call chains

2 participants