Format PatternMatchClass#6860
Conversation
|
I don't know how to solve the conflict for the |
...python_formatter/tests/snapshots/black_compatibility@py_310__pattern_matching_extras.py.snap
Outdated
Show resolved
Hide resolved
| join.finish() | ||
| }); | ||
|
|
||
| write!(f, [cls.format(), parenthesized("(", &items, ")")]) |
There was a problem hiding this comment.
Try adding an extra group around items
There was a problem hiding this comment.
Would the function group work here?
There was a problem hiding this comment.
Yes, group(&items) should do the trick if I'm not mistaken
There was a problem hiding this comment.
Nothing changed for this crates/ruff_python_formatter/tests/snapshots/black_compatibility@py_310__pattern_matching_extras.py.snap file when using group
There was a problem hiding this comment.
See my comment below. This is fine, assuming we're talking about the same mismatch.
MichaReiser
left a comment
MichaReiser
left a comment
There was a problem hiding this comment.
This looks good to me. Thank you.
Could you add one test case to
that tests for the following weird comment placement
match x:
case (
Point2D
# awkward
(x = 2)
: ...| - ): | ||
| + case NOT_YET_IMPLEMENTED_PatternMatchClass(0, 0): | ||
| + normal=x, | ||
| + perhaps=[list, {"NOT_YET_IMPLEMENTED_PatternMatchMapping": _, 2: _}] as y, |
There was a problem hiding this comment.
Okay, the problem here is that PatternMatchMapping isn't yet implemented and the text emitted by the "fake" implementation is much longer than the expected string.
We can ignore this.
| join.finish() | ||
| }); | ||
|
|
||
| write!(f, [cls.format(), parenthesized("(", &items, ")")]) |
There was a problem hiding this comment.
See my comment below. This is fine, assuming we're talking about the same mismatch.
| join.nodes(patterns.iter()); | ||
| } | ||
|
|
||
| if !kwd_attrs.is_empty() { |
There was a problem hiding this comment.
Do we need the if !kwd_attrs.is_empty()?
| # trailing | ||
| # trailing | ||
| ): | ||
| ... |
There was a problem hiding this comment.
Could you add something like
match a:
case A(
b # b
= # c
2 # d
# e
):
passwith the left hand side and the right hand side split of the assignment split by comment? I wanted to add an additional comment before the b but that's blocked by #6866
Just pick anything (it's an autogenerated file) and recreate file, e.g. with |
|
Will review and merge today. I can also follow-up with new tests once I fix #6866. |
|
Okay, I had to make some changes to the PR to adapt to our parenthesis handling for patterns that changed a bit upstream (mostly, to ensure we handle the single-argument case correctly, like we do for call expressions). We also now handle comments before the arguments, like we do for call expressions. There are several comment cases that we're still getting wrong (valid syntax, but slightly incorrect placement), but that are really hard to fix without changing the AST. (Namely, we need an I'm going to update the AST, then do another pass on this node. |
| ... | ||
|
|
||
| case A( | ||
| b=# b |
There was a problem hiding this comment.
This is not quite right, but very tedious to fix without a node for the whole argument.
| ... | ||
|
|
||
| case Point2D( | ||
| # end of line |
There was a problem hiding this comment.
This is not quite right (it should be immediately after the (), but very hard to fix without a node for the arguments.
|
Filed here: #6880. I'm on it. |
PR Check ResultsBenchmarkLinuxWindows |
4 participants
Summary
Closes #6642
Test Plan
cargo test