Do not consider f-strings with escaped newlines as multiline#14624
Do not consider f-strings with escaped newlines as multiline#14624dhruvmanila merged 1 commit intomainfrom
Conversation
dhruvmanila
added
bug
|
I'm not sure I understand this part. Isn't the reason why no preview gating is required because this new logic is only used in preview mode? |
The |
There was a problem hiding this comment.
Looking at #14454 is_multiline again, I think the change we made there has a high potential for unwanted stable style changes. We should change the is_multiline to keep using the "old" memchr2(b'\n', b'\r', source[fstring.range].as_bytes()).is_some() if f-string formatting is disabled and only use the new logic if f-string formatting is enabled. We otherwise risk that bugs like the one you found here impact stable style users.
|
|
||
| # This is not a multiline f-string, but the expression is too long so it should be | ||
| # wrapped in parentheses. | ||
| f"hellooooooooooooooooooooooo \ |
There was a problem hiding this comment.
Could we add some tests in assignment positions and with implicit concatenated strings (especially with inline comments) just to make sure strings with line continuations are handled correctly in those positions too
There was a problem hiding this comment.
Adding these test case reveals one more change in implicitly concatenated strings. Given:
f"hellooooooooooooooooooooooo \
worldddddddddddddddddd" "another string"On main, we don't concatenate it because the f-string is considered as multiline but here we would. I'm going to make the is_multiline change gated behind preview first, rebase this PR and add the test cases.
There was a problem hiding this comment.
I think concatenating the string makes sense as long as it fits into the line length
I see, I think that makes sense. I'll do that as a follow-up. |
dhruvmanila
force-pushed
the
dhruv/f-string-multiline-triple-quoted
branch
from
3b8a1cc to
2573374
Compare
dhruvmanila
changed the base branch from
main
to
dhruv/f-string-multiline-preview
## Summary Ref: #14624 (review) ## Test Plan The test case in the follow-up PR showcases the difference between preview and non-preview formatting: https://github.com/astral-sh/ruff/pull/14624/files#diff-dc25bd4df280d9a9180598075b5bc2d0bac30af956767b373561029309c8f024
dhruvmanila
force-pushed
the
dhruv/f-string-multiline-triple-quoted
branch
from
2573374 to
d720c51
Compare
2 participants
Summary
This PR fixes a bug in the f-string formatting to not consider the escaped newlines for
is_multiline. This is done by checking if the f-string is triple-quoted or not similar to normal string literals.This is not required to be gated behind preview because the logic change for
is_multilinewas added in #14454.Test Plan
Add a test case which formats differently on
main: https://play.ruff.rs/ea3c55c2-f0fe-474e-b6b8-e3365e0ede5e