Conversation
|
Current dependencies on/for this PR:
This comment was auto-generated by Graphite. |
| ) | ||
|
|
||
|
|
||
| # Black breaks the right side first for the following expressions: |
There was a problem hiding this comment.
Not all of these are handled correctly yet because the format implementation of the right hand side is missing
| write!( | ||
| f, | ||
| [ | ||
| left.format(), |
There was a problem hiding this comment.
This implementation heavily relies on recursion. We may want to explore to manually unroll left-balanced binary expressions because this could stack-overflow for very deep binary expressions (but so could any of our analyzer infrastructure).
|
|
||
| fn is_expression_parenthesized(expr: &Expr, contents: &str) -> bool { | ||
| // Search backwards to avoid ambiguity with `(a, )` and because it's faster | ||
| matches!( |
There was a problem hiding this comment.
This is unfortunate. We now have to inspect the source code for every expression that we format but I haven't been able to come up with a better approach. Let's see what the benchmark says to this.
There was a problem hiding this comment.
What are the alternatives? In the initial prototype, I think I handled these similarly to comments -- so, for every parenthesis, I associated it with the child it contains.
There was a problem hiding this comment.
I considered extracting the tokenized ranges from the token stream and either:
- Store them in a vec and perform a binary search: I don't think this is faster because it trades the few character iterations with a binary search for every expression (except maybe if there are None).
- Storing the
parenthesizedin aTriviastruct and look it up by node. Again, I don't think this would be faster because we trade the iterating over a few characters vs performing a hash map lookup (while fast, iterating over 1- characters is super fast too)
| # Has many lines. Many, many lines. | ||
| # Many, many, many lines. | ||
| -"""Module docstring. | ||
| +( |
There was a problem hiding this comment.
We'll need to handle docstrings manually in the Suite formatting but that's not part of this PR
f66044f to
8070eef
Compare
0b45066 to
364f656
Compare
PR Check ResultsEcosystem✅ ecosystem check detected no changes. BenchmarkLinuxWindows |
364f656 to
5ee366e
Compare
| impl FormatNodeRule<ExprList> for FormatExprList { | ||
| fn fmt_fields(&self, item: &ExprList, f: &mut PyFormatter) -> FormatResult<()> { | ||
| write!(f, [verbatim_text(item.range)]) | ||
| let ExprList { |
There was a problem hiding this comment.
This is not a full implementation, but it is good enough to test the right side breaking of binary expressions
| aaaaaaaaaaaaaa | ||
| + {a for x in bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb} | ||
| ) | ||
| # Wraps it in parentheses if it needs to break both left and right |
There was a problem hiding this comment.
The new lines before the comment get removed. I'll look into this tomorrow as a separate PR
5ee366e to
5899fa9
Compare
28014f5 to
5dde064
Compare
5dde064 to
5645338
Compare
Pull request was closed
* Format Binary Expressions * Extract NeedsParentheses trait
Summary
This PR implements basic formatting for binary expressions. It does not yet handle the special case where black breaks the right side first for call expressions, tuples, lists, ...
This PR also adds the infrastructure to preserve parentheses around expressions (depending on the context)
Test Plan
cargo test