-
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
Indent lambda parameters if parameters wrap #8465
Conversation
Current dependencies on/for this PR:
This stack of pull requests is managed by Graphite. |
670fea2
to
01c6438
Compare
crates/ruff_python_formatter/tests/snapshots/format@expression__lambda.py.snap
Outdated
Show resolved
Hide resolved
|
01c6438
to
8342fc3
Compare
8342fc3
to
70968de
Compare
@@ -289,7 +368,9 @@ a = ( | |||
|
|||
# lambda arguments don't have parentheses, so we never add a magic trailing comma ... | |||
def f( | |||
aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa: bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb = lambda x: y, | |||
aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa: bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb = lambda | |||
x |
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.
Undecided if this looked better before.
# Leading | ||
lambda x: ( | ||
lambda y: ( | ||
lambda z: ( |
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.
This seems better to me
|
||
|
||
# Leading | ||
lambda x: ( |
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.
This adds parentheses, but I don't mind them
c, | ||
d, | ||
e, | ||
f=lambda |
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.
It looks okay.... Parentheses would make it look so much better
70968de
to
a3fa295
Compare
The main case that is not necessarily better in my view is index 9855c318be..fd7770eb2e 100644
--- a/django/test/testcases.py
+++ b/django/test/testcases.py
@@ -1470,8 +1470,10 @@ def skipIfDBFeature(*features):
def skipUnlessDBFeature(*features):
"""Skip a test unless a database has all the named features."""
return _deferredSkip(
- lambda: not all(
- getattr(connection.features, feature, False) for feature in features
+ lambda: (
+ not all(
+ getattr(connection.features, feature, False) for feature in features
+ )
), 🤷 |
e7fb0ac
to
3e218fa
Compare
a3fa295
to
2a2886d
Compare
2a2886d
to
1534c23
Compare
1534c23
to
d5a18a6
Compare
Parenthesizing the body is probably less controversial. It might be worth avoiding to parenthesize the body if it has parentheses itself. But parenthesizing the body otherwise fits with the formatting that copilot would choose. |
Summary
A non-black compatible approach to #8179
The black-compatible formatting would enforce spaces when formatting parameters with the
ParameterParentheses::Never
. However, this doesn't work when using comments (which black doesn't support playground?)This PR implements two changes (we may want to split it in two):
a) It indents the lambda parameters if they don't fit on a single line:
b) Preview style that parenthesizes the lambda body if it expands.
Alternatives
Favor black compatibility and enforce using
space
in the parameters formatting when theParameterParentheses::Never
is used. Requires finding a comment placement that produces stable results.Unclear how we want to support
Notes
It's probably worth to split out the body changes. They seem to generally improve readability
Test Plan
The non-preview changes don't seem to change the similarity index (at least after only indenting if there are 2 or more parameters).
The preview changes regress the similarity index across the board
Main
Preview
Here a few examples
Most of these seem clear improvements to me.