-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Avoid multiline expression if format specifier is present #11123
Conversation
|
f266faf
to
ba88497
Compare
bracket_spacing.fmt(f) | ||
if conversion.is_none() && format_spec.is_none() { | ||
bracket_spacing.fmt(f)?; | ||
} | ||
|
||
Ok(()) |
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 is an additional change which isn't relevant to this PR but should be a follow-up. I've included it in here for simplicity. For example,
x = f"aaaaaaaaaaaaaaa {
{'aaaaaaaaaaaaaaaaaa', 'dddddddddddddddddd'}!s
}"
This should be formatted as:
x = f"aaaaaaaaaaaaaaa { {'aaaaaaaaaaaaaaaaaa', 'dddddddddddddddddd'}!s}"
Instead of:
x = f"aaaaaaaaaaaaaaa { {'aaaaaaaaaaaaaaaaaa', 'dddddddddddddddddd'}!s }"
# ^ this isn't required if there's format spec or conversion spec
I'm confused, you say:
f"""aaaaaaaaaaaaaaaa bbbbbbbbbbbbbbbbbb ccccccccccc {variable:.3f
} ddddddddddddddd eeeeeeee""" But the original example had no newline after f"""aaaaaaaaaaaaaaaa bbbbbbbbbbbbbbbbbb ccccccccccc {
variable:.3f} ddddddddddddddd eeeeeeee""" |
crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/fstring.py
Outdated
Show resolved
Hide resolved
} cccccccccc""" | ||
aaaaaaaaaaa = f"""asaaaaaaaaaaaaaaaa { | ||
aaaaaaaaaaaa + bbbbbbbbbbbb + ccccccccccccccc + dddddddd:.3f | ||
# comment} cccccccccc""" |
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.
Lol what so # comment
here is not a comment 🤣
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.
Yeah, it isn't but only if it's a triple-quoted f-string
The note is for the two snippets above it. The two code snippets you've provided aren't connected. I meant that the following code f"""aaaaaaaaaaaaaaaa bbbbbbbbbbbbbbbbbb ccccccccccc {
variable:.3f
} ddddddddddddddd eeeeeeee""" will be formatted as -f"""aaaaaaaaaaaaaaaa bbbbbbbbbbbbbbbbbb ccccccccccc {
- variable:.3f
+f"""aaaaaaaaaaaaaaaa bbbbbbbbbbbbbbbbbb ccccccccccc {variable:.3f
} ddddddddddddddd eeeeeeee""" The formatter collapsed the f-string expression element but there's still a newline in the format spec which is why you see that the f-string is still multiline. |
6a2d693
to
427e3e2
Compare
Summary
This PR fixes the bug where the formatter would format an f-string and could potentially change the AST.
For a triple-quoted f-string, the element can't be formatted into multiline if it has a format specifier because otherwise the newline would be treated as part of the format specifier.
Given the following f-string:
The formatter sees that the f-string is already multiline so it assumes that it can contain line breaks i.e., broken into multiple lines. But, in this specific case we can't format it as:
Because the format specifier string would become ".3f\n", which is not the original string (
.3f
).If the original source code already contained a newline, they'll be preserved. For example:
The above will be formatted as:
Note that the newline after
.3f
is part of the format specifier which needs to be preserved.The Python version is irrelevant in this case.
fixes: #10040
Test Plan
Add some test cases to verify this behavior.