-
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
Basic string formatting #5297
Basic string formatting #5297
Conversation
Current dependencies on/for this PR:
This comment was auto-generated by Graphite. |
PR Check ResultsEcosystem✅ ecosystem check detected no changes. BenchmarkLinux
Windows
|
afad5cc
to
0a3ed21
Compare
0bea2d4
to
a78246b
Compare
@@ -342,29 +377,8 @@ if True: | |||
let printed = format_module(&content)?; | |||
let formatted_code = printed.as_code(); | |||
|
|||
let reformatted = |
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.
why is that gone?
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.
My understanding is that ensure_stability_when_formatting_twice
now already asserts that reformatting the file twice yields the same result. Thanks to your refactor :)
a78246b
to
ae9cadf
Compare
895d416
to
54b2b11
Compare
ae9cadf
to
d2df571
Compare
Graphite rebased this pull request after merging its parent, because this pull request is set to merge when ready. |
d2df571
to
20158e7
Compare
@MichaReiser merged this pull request with Graphite. |
Summary
This PR implements formatting for non-f-string Strings that do not use implicit concatenation.
Docstring formatting is out of the scope of this PR.
Test Plan
I added a few tests for simple string literals.
Performance
Ouch. This is hitting performance somewhat hard. This is probably because we now iterate each string a couple of times:
Edit: I integrated the detection of newlines into the preferred quote detection so that we only iterate the string three time.
We can probably do better by merging the implicit string continuation with the quote detection and new line detection by iterating till the end of the string part and returning the offset. We then use our simple tokenizer to skip over any comments or whitespace until we find the first non trivia token. From there we keep continue doing this in a loop until we reach the end o the string. I'll leave this improvement for later.