-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Format PatternMatchAs
#6652
Format PatternMatchAs
#6652
Conversation
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.
I'm not sure how well we can test this just now with most of the pattern formatting missing, but could we add a few test cases that have comments between the pattern and the name?
PR Check ResultsBenchmarkLinux
Windows
|
2b6a0ec
to
9aace89
Compare
match pattern_comments: | ||
case ( | ||
# 1 | ||
pattern as name # 2 | ||
# 3 | ||
# 4 | ||
# 5 # 6 | ||
# 7 | ||
): | ||
pass |
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.
This might need to be handled in a different way as there seems to be 2 entries created for PatternMatchAs
node comments where the first entry contains 1, 6, 7 and the second contains 2, 3, 4, 5.
{
Node {
kind: PatternMatchAs,
range: 55..120,
source: `pattern # 2⏎`,
}: {
"leading": [
SourceComment {
text: "# 1",
position: OwnLine,
formatted: true,
},
],
"dangling": [],
"trailing": [
SourceComment {
text: "# 6",
position: EndOfLine,
formatted: true,
},
SourceComment {
text: "# 7",
position: OwnLine,
formatted: true,
},
],
},
Node {
kind: PatternMatchAs,
range: 55..62,
source: `pattern`,
}: {
"leading": [],
"dangling": [],
"trailing": [
SourceComment {
text: "# 2",
position: EndOfLine,
formatted: true,
},
SourceComment {
text: "# 3",
position: OwnLine,
formatted: true,
},
SourceComment {
text: "# 4",
position: EndOfLine,
formatted: true,
},
SourceComment {
text: "# 5",
position: OwnLine,
formatted: true,
},
],
},
}
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.
We'll need a custom handler in placement.rs for MatchAs
that assigns all comments that come after the start of the name to the MatchAs
clause.
Can you add a test case with a comment between the as
and the name
match pattern_comments:
case (
pattern
# 1
as # 2
# 3
name #4
# 5
):
pass
.. | ||
}) | ||
) { | ||
parenthesized("(", &pattern.format(), ")").fmt(f)?; |
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.
I expect that Black's logic is to preserve parentheses around nested patterns. But we can track this in a separate issue
5970b9a
to
74d776e
Compare
## Summary Follows up on #6652 (comment) with some modifications to the `PatternMatchAs` comment handling. Specifically, any comments between the `as` and the end are now formatted as dangling, and we now insert some newlines in the appropriate places. ## Test Plan `cargo test`
Summary
Add formatting for
PatternMatchAs
.This closes #6641.
Test Plan
Add tests for comments.