Skip to content
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

Render multiline brace blocks as breakables #426

Merged
merged 2 commits into from
May 14, 2023

Conversation

reese
Copy link
Collaborator

@reese reese commented May 1, 2023

A side effect of #397 is that some multiline-able constructs (notably brace blocks) that aren't modeled with breakables could have the inner contents break but not the surrounding block. For example, something like

sig { params(foo: SomePrettyLongClassName, bar: AnEvenLongerClassName::ThatMakesThisGoPrettyFar, baz: Hasdfasdfasdfasdas) }

would incorrectly render as

sig { params(
  foo: SomePrettyLongClassName,
  bar: AnEvenLongerClassName::ThatMakesThisGoPrettyFar,
  baz: Hasdfasdfasdfasdas
) }

instead of breaking at the block level. This caused rubyfmt to take multiple passes to settle on an output, which is incorrect.

This PR changes the way we do brace blocks to model them as breakables, which means they'll be more accurate when rendering on long lines and they won't fall prey to the multilining issues mentioned above. The implementation is admittedly not the cleanest in the world, but this is largely due to the fact that brace blocks aren't "traditional" breakables in the sense that we need to be able to support blockvars and because there are some additional requirements around when exactly to break them that aren't only about line length. As such, there's some hacks sprinkled in here, but I did my best to heavily comment and test them.

@reese reese marked this pull request as ready for review May 4, 2023 05:26
@reese reese requested a review from fables-tales May 4, 2023 05:27
@reese reese merged commit a6afe44 into trunk May 14, 2023
@reese reese deleted the reese-fix-multiline-brace-blocks branch May 14, 2023 15:14
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.

2 participants