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

Transform multiline brace blocks to do/end blocks #407

Open
wants to merge 3 commits into
base: trunk
Choose a base branch
from

Conversation

reese
Copy link
Collaborator

@reese reese commented Jan 4, 2023

It's a pretty standard convention to use { } for single-line blocks and do/end for multiline blocks, and for the sake of consistency, we should also enforce that in rubyfmt.

(Note that while there are edge cases in terms of precedence differences between the two block types, this only happens in the case of unparenthesized arguments that use brace blocks, and since transforming them to do/end makes them multiline, rubyfmt will always wrap them in parentheses, which avoids the precedence issues. Because of that, this should be a safe transformation.)

@reese reese force-pushed the reese-multiline-blocks-to-do-end branch from 150aa6d to d61959f Compare January 4, 2023 06:28
@reese reese marked this pull request as ready for review January 4, 2023 06:37
@fables-tales
Copy link
Owner

I like multiline brace blocks

@reese
Copy link
Collaborator Author

reese commented Feb 13, 2023

I personally don't mind them, but I think the opinion from users (and the opinion I thought we landed on when we discussed this a few months back) was that historically we've operated on the rationale that it was best to pick one thing that worked everywhere and always use that, and allowing both braces and do/end broke that consistency. The other context was (1) the ideal way would be brace blocks for blocks that return things (e.g. map blocks) and do/end for blocks used for side effects (e.g. each blocks), but (2) that wasn't possible in rubyfmt so we were planning on going this way.

I think last time we talked about this it was another one of those "make fable really sad but at least it's consistent" things.

@reese reese assigned fables-tales and unassigned reese Feb 13, 2023
@shkm
Copy link
Contributor

shkm commented Feb 13, 2023

Random Ruby person here sliding in your PR. For what it's worth, I'd rather like this. But...

I just wanted to point out that there are some DSLs which promote use of multi-line brace blocks. The only example I can think of, off the top of my head, is Arbre.

@reese
Copy link
Collaborator Author

reese commented Feb 14, 2023

There will always be some degree of "other people do it differently," but that's sort of the entire point of having an opinionated formatter: it doesn't really matter what other tools do. I think the question here is whether or not that's the style rubyfmt should go with, not a question of whether or not it satisfies every possible style used by other tools.

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.

3 participants