-
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
Fix lexing single-quoted f-string with multi-line format spec #7787
Conversation
Current dependencies on/for this PR: This comment was auto-generated by Graphite. |
CodSpeed Performance ReportMerging #7787 will improve performances by 2.5%Comparing Summary
Benchmarks breakdown
|
PR Check ResultsEcosystem✅ ecosystem check detected no changes. |
68f3d35
to
9c3edf5
Compare
9c3edf5
to
745a878
Compare
The CPython PR has been merged: python/cpython#110271 |
Friendly reminder to review the discussion in python/cpython#110259 so you end implementing the same as CPython 😉 |
745a878
to
8638838
Compare
I did, thanks for the heads up! :) |
...rser/src/snapshots/ruff_python_parser__lexer__tests__fstring_with_multiline_format_spec.snap
Show resolved
Hide resolved
Actually, there's more to it for single-quoted f-string that I missed (sorry!). The newline should basically end the lexing of format spec which we currently don't. |
That's why I suggested to double check :) |
Hehe, I must've missed that part 😅 I'm going to try a couple solutions and look at the benchmarks, please bear with me :) |
This reverts commit 637862c. Guess this doesn't work at all :)
FStringMiddle { | ||
value: "a", | ||
is_raw: false, | ||
}, | ||
99..100, | ||
), | ||
( | ||
NonLogicalNewline, | ||
100..101, | ||
), | ||
( | ||
Name { | ||
name: "b", | ||
}, | ||
109..110, | ||
), |
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's important that b
is a Name
token as we ended the format spec lexing at the newline. This will help the parser to raise a syntax error (unexpected token 'b') similar to the CPython implementation.
f'__{
x:a
b
}__'
Update after the first review is that the following case will now raise a syntax error as it should: f'__{
x:a
b
}__' The reason it didn't earlier is because we didn't end the format spec lexing after we reached the newline (after |
## Summary Reported at python/cpython#110259 ## Test Plan Add test cases for the fix and update the snapshots
Summary
Reported at python/cpython#110259
Test Plan
Add test cases for the fix and update the snapshots