-
Notifications
You must be signed in to change notification settings - Fork 1k
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
prefer_splitting_right_hand_side_of_assignments
preview style
#8943
prefer_splitting_right_hand_side_of_assignments
preview style
#8943
Conversation
Current dependencies on/for this PR: This stack of pull requests is managed by Graphite. |
79b44e8
to
e6977a2
Compare
e6977a2
to
9c4e2b1
Compare
|
0550d40
to
1a0e9e8
Compare
e465bfc
to
deae320
Compare
deae320
to
7a3c504
Compare
prefer_splitting_right_hand_side_of_assignments
preview styleprefer_splitting_right_hand_side_of_assignments
preview style
2d125ca
to
1fe91c8
Compare
I like the changes that I see in the ecosystem check |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks much better!
@@ -17,3 +17,10 @@ pub(crate) const fn is_hug_parens_with_braces_and_square_brackets_enabled( | |||
) -> bool { | |||
context.is_preview() | |||
} | |||
|
|||
/// Returns `true` if the [`prefer_splitting_right_hand_side_of_assignments`](https://github.com/astral-sh/ruff/issues/6975) preview style is enabled. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I missed that previously, but do we also want to do an enum here, and then have is_enabled(PreviewStyles::PreferSplittingRhsOfAssignments)
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't have an opinion. We could use f.context().is_enabled(PreviewStyle::LongName)
, which may be easier to document and add. For me, any approach that allows identifying the call sites easily works for me.
1fe91c8
to
ed58d9b
Compare
ed58d9b
to
7c7f6b9
Compare
Summary
This PR implements Black's
prefer_splitting_right_hand_side_of_assignments
preview style.The gist of the new style is to prefer breaking the value before breaking the target or type annotation of an assignment:
Before
New
Closes #6975
Details
It turned out that there are some more rules involved than just splitting the value before the targets.I For example:
I added extensive documentation to
FormatStatementLastExpression
ruff/crates/ruff_python_formatter/src/statement/stmt_assign.rs
Lines 140 to 270 in 7a3c504
Differences to Black
Black doesn't seem to implement this behavior for type alias statements. We do this to ensure all assignment-like statements are formatted the same.
Performance
The new right-to-left layout is more expensive than our existing layout because it requires using
BestFitting
(allocates, needs to try multiple different variants). The good news is that this layout is only necessary when the assignment has:This is rare in comparison to most assignments that are of the form
a = b
ora: b = c
.I checked Codspeed and our micro benchmarks only regress by 1-2%
Test Plan
I added new tests, reviewed the Black related preview style tests (that we now match except for comment handling).
The poetry compatibility improves from 0.96208 to 0.96224.