-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Simplify formatting of strings by using flags from the AST nodes #10489
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
Simplify formatting of strings by using flags from the AST nodes #10489
Conversation
|
c51c6d5
to
d10c4b8
Compare
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.
Nice. It allows us to delete quiet a lot of code.
There are two concerns:
- Using
impl StringPart
leads to monomorphization of not entirely trivial functions -> More generated code. I recommend extracting a struct that holds just the information we need to avoid this - We should move
StringKind
out of the parser crate because I want to makeruff_python_formatter
independent of the parser crate eventually. It should only depend on theruff_python_ast
and not be concerned about how the source file was parsed (that will happen in a caller crate).
d10c4b8
to
f0a47a7
Compare
@@ -2108,6 +2106,439 @@ impl From<BytesLiteral> for Expr { | |||
} | |||
} | |||
|
|||
bitflags! { |
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.
Not sure if all this belongs in nodes.rs
. ruff_python_ast::str.rs
didn't feel right, though, since nodes.rs
depends on str.rs
(it would have created a circular dependency between the two files.
Thanks! I added a commit to move the relevant abstractions from |
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.
Thank you
Summary
This PR simplifies the formatter by using the flags available on AST nodes, which were added in #10298 and refined in #10314. These can be used to query the quoting style and prefix used by a string. (Currently, we compute these by reparsing them from the source code.)
I was hoping this might lead to a speedup -- locally for me, it seems to be performance-neutral. It's (in my opinion) a nice simplification, anyway.
Each commit is atomic; the full test suite passes on each commit.
Test Plan
cargo test