Implement FormatPatternMatchValue#6799
Merged
charliermarsh merged 3 commits intomainfrom Aug 23, 2023
Merged
Conversation
851c962 to
62857fc
Compare
cbdbb72 to
a304c80
Compare
62857fc to
c9e664d
Compare
Contributor
PR Check ResultsBenchmarkLinuxWindows |
dhruvmanila
approved these changes
Aug 23, 2023
| match foo: | ||
| case 1: | ||
| y = 0 | ||
| case ((1)): |
Member
There was a problem hiding this comment.
What would the formatting be like after the fix for double parentheses? Would it still keep the parentheses from the original code i.e., format is as (1)?
Member
There was a problem hiding this comment.
Ah nevermind, it's answered in your other PR.
c9e664d to
382e30f
Compare
MichaReiser
approved these changes
Aug 23, 2023
Member
MichaReiser
left a comment
There was a problem hiding this comment.
Nice, this will make testing of other match patterns much easier.
I'm not sure if you saw this, but @evanrittenhouse was working on this #6608
Comment on lines
+1
to
+4
| use ruff_formatter::{write, Buffer, FormatResult}; | ||
| use ruff_python_ast::PatternMatchValue; | ||
|
|
||
| use crate::{not_yet_implemented_custom_text, FormatNodeRule, PyFormatter}; | ||
| use crate::{AsFormat, FormatNodeRule, PyFormatter}; |
Member
There was a problem hiding this comment.
Nit: use crate::prelude::*. It should allow removing Buffer, FormatResult, PyFormatter, and AsFormat (and it makes it easier to change the code in the future without having to add a ton of imports)
crates/ruff_python_formatter/src/pattern/pattern_match_value.rs
Outdated
Show resolved
Hide resolved
konstin
approved these changes
Aug 23, 2023
Member
konstin
left a comment
There was a problem hiding this comment.
I added Closes #6555 to the description
crates/ruff_python_formatter/resources/test/fixtures/ruff/statement/match.py
Show resolved
Hide resolved
382e30f to
fb314e6
Compare
Member
Author
|
Thanks! I did see the other PR but I wanted to push these along so that I could start working on some of the other problems around comments and parentheses without so many |
charliermarsh
added a commit
that referenced
this pull request
Aug 25, 2023
## Summary This PR fixes the duplicate-parenthesis problem that's visible in the tests from #6799. The issue is that we might have parentheses around the entire match-case pattern, like in `(1)` here: ```python match foo: case (1): y = 0 ``` In this case, the inner expression (`1`) will _think_ it's parenthesized, but we'll _also_ detect the parentheses at the case level -- so they get rendered by the case, then again by the expression. Instead, if we detect parentheses at the case level, we can force-off the parentheses for the pattern using a design similar to the way we handle parentheses on expressions. Closes #6753. ## Test Plan `cargo test`
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This is effectively #6608, but with additional tests.
We aren't properly handling parenthesized patterns, but that needs to be dealt with separately as it's somewhat involved.
Closes #6555