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

Properly multiline user-multilined call chains #382

Merged
merged 3 commits into from
Dec 3, 2022

Conversation

reese
Copy link
Collaborator

@reese reese commented Dec 3, 2022

Related to #359

In general, rubyfmt respects (within limits) user choices about multilining expressions, but we don't currently do this for multiline expressions. This has a few poor side effects: (1) users often want to multiline to make e.g. adding comments for specific sections of a call chain easier, and (2) there end up being some (IMO) awkward formatting of chains like the following:

MyClass.call(
  [],
  ""
).do_some_other_stuff!

which is inconsistent with rubyfmt's normal call chain styling.

This PR implements multilining for user-multilined expressions. It does this by checking the line location of the DotTypeOrOp items in the call chain and multilining if they're on multiple lines. This might be a slightly imperfect if the user writes something like this

# both format as `foo.bar.baz`
foo
.bar.baz

foo.bar.
baz

but IMO this is a reasonable implementation tradeoff. We could alternatively check every expression's start/end, but this ends up requiring a lot more trickery and is more prone to inconsistency.

@reese reese force-pushed the reese-user-multilined-method-chains branch from a6a49c1 to 82cc86c Compare December 3, 2022 20:41
Comment on lines +30 to +32
foo::bar
&.nil?::klass
.true?
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is blursed but technically legal -- I handled it by just multilining where possible, but open to other thoughts on how we should handle :: calls.

Copy link
Owner

Choose a reason for hiding this comment

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

I hate you

Copy link
Owner

Choose a reason for hiding this comment

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

(j/k obvi)

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 pretty much every fixture I write should just have # im so sorry at the top of them

@reese reese marked this pull request as ready for review December 3, 2022 20:55
@reese reese requested a review from fables-tales December 3, 2022 20:55
@reese reese merged commit 9ed05fc into trunk Dec 3, 2022
@reese reese deleted the reese-user-multilined-method-chains branch December 3, 2022 21:47
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