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

Represent binary operators as breakables #452

Merged
merged 3 commits into from
Jan 8, 2024
Merged

Conversation

reese
Copy link
Collaborator

@reese reese commented Dec 20, 2023

Closes #379

This PR yanks out the hacked-together multilining strategy for binary operators and makes them breakables instead. This was largely easier than I though -- IIRC the original reason for not doing this was that call chains weren't also breakables, so that resulted in weird interactions, but since we did that in #443, this is a pretty straightforward change.

The way this works is that instead of our old hacked-together system, we can wrap the "top-level" binary in a breakable, and then for nested breakables, we just recurse into the formatting function instead of making a new breakable, which keeps the old behavior of keeping chained binary operations all at the same level.

This also fixed a number of accidental interactions that I wasn't aware of but that I think are actually bugs -- in the fixtures that we changed, this PR makes it so that binary operators now respect user-multiling in all cases, which they didn't used to do but which is consistent with the approach taken for other breakable items.

Comment on lines +3 to +4
foo ||
bar
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

For basically all of these fixture updates, in the original versions, these were multilined, so keeping them multiline seems correct (and consistent with how we handle every other breakable)

@reese reese merged commit 91dc87d into trunk Jan 8, 2024
7 checks passed
@reese reese deleted the reese-breakable-binaries branch January 8, 2024 15:48
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.

Max line length isn't consistently respected
2 participants