Merged
Conversation
This was referenced Jun 8, 2023
Merged
Member
Author
|
Current dependencies on/for this PR:
This comment was auto-generated by Graphite. |
Contributor
PR Check ResultsEcosystem✅ ecosystem check detected no changes. BenchmarkLinuxWindows |
01a9a40 to
c0c645e
Compare
7ba434b to
875cd8b
Compare
MichaReiser
commented
Jun 8, 2023
| for node in group_nodes: | ||
| node_path = src.joinpath(groups[group]).joinpath(f"{to_camel_case(node)}.rs") | ||
| # Don't override existing manual implementations | ||
| if node_path.exists(): |
Member
Author
There was a problem hiding this comment.
This was a bit unexpected haha...
| .unwrap_or_default() | ||
| .len(); | ||
| let match_stmt_indentation = | ||
| whitespace::indentation(locator, match_stmt).map_or(usize::MAX, str::len); |
Member
Author
There was a problem hiding this comment.
using default here yields an incorrect result if the body is on the same line as the case header. Using usize::MAX should be fairly safe ;)
| N: AstNode, | ||
| { | ||
| fn fmt(&self, node: &N, f: &mut PyFormatter) -> FormatResult<()> { | ||
| write!(f, [source_position(node.start())])?; |
Member
Author
There was a problem hiding this comment.
The leading and trailing comments normally appear before the node and have lower source positions. That's why we should move the node's source position to just cover the node itself.
| +++ Ruff | ||
| @@ -1,219 +1,105 @@ | ||
| @@ -1,219 +1,149 @@ | ||
| -class MyClass: |
Member
Author
There was a problem hiding this comment.
This PR does not implement any special handling for docstrings yet.
875cd8b to
3c45cf3
Compare
konstin
approved these changes
Jun 8, 2023
|
|
||
| write!(f, [if_group_breaks(&text(","))])?; | ||
|
|
||
| if let Some(last_node) = last_node { |
Member
There was a problem hiding this comment.
Suggested change
| if let Some(last_node) = last_node { | |
| // Break group if there is a trailing comma | |
| if let Some(last_node) = last_node { |
Member
There was a problem hiding this comment.
Does this also need to read the magic trailing comma setting?
Member
Author
There was a problem hiding this comment.
If we introduce a setting for this, then yes
Member
There was a problem hiding this comment.
i forgot this isn't merged: https://github.com/astral-sh/ruff/pull/4878/files#diff-c1f1071e384c9f14936cad67780f301345744791e34afd93cada2cf43af8dcb5R18
I haven't looked into how to pass settings, but i think it makes sense to introduce the boolean for that now
Member
Author
There was a problem hiding this comment.
The intended way for settings is that Ruff defines a PyFormatOption struct that implements FormatOptions and is passed to the PyFormatContext (instead of using the generic SimpleFormatOptions). You can then access it with f.context().options()....
konstin
added a commit
that referenced
this pull request
Jun 8, 2023
This implements StmtPass as `pass`. The snapshot diff is small because pass mainly occurs in bodies and function (#4951) and if/for bodies.
Merged
konstin
added a commit
that referenced
this pull request
Jun 8, 2023
This implements StmtReturn as `return` or `return {value}`.
The snapshot diff is small because return occurs in functions (#4951)
Merged
3c45cf3 to
60dd48a
Compare
Merged
konstin
added a commit
that referenced
this pull request
Jun 8, 2023
This implements StmtPass as `pass`. The snapshot diff is small because pass mainly occurs in bodies and function (#4951) and if/for bodies.
konstin
added a commit
that referenced
this pull request
Jun 8, 2023
* Implement StmtPass This implements StmtPass as `pass`. The snapshot diff is small because pass mainly occurs in bodies and function (#4951) and if/for bodies. * Implement StmtReturn This implements StmtReturn as `return` or `return {value}`. The snapshot diff is small because return occurs in functions (#4951)
60dd48a to
26a4a83
Compare
26a4a83 to
3be5049
Compare
3be5049 to
dcb2aaa
Compare
auto-merge was automatically disabled
June 8, 2023 15:54
Pull request was closed
dcb2aaa to
3b8b3a9
Compare
This was referenced Jun 9, 2023
konstin
added a commit
that referenced
this pull request
Jun 13, 2023
This implements StmtPass as `pass`. The snapshot diff is small because pass mainly occurs in bodies and function (#4951) and if/for bodies.
konstin
added a commit
that referenced
this pull request
Jun 13, 2023
* Implement StmtPass This implements StmtPass as `pass`. The snapshot diff is small because pass mainly occurs in bodies and function (#4951) and if/for bodies. * Implement StmtReturn This implements StmtReturn as `return` or `return {value}`. The snapshot diff is small because return occurs in functions (#4951)
konstin
pushed a commit
that referenced
this pull request
Jun 13, 2023
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
2 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 PR implements formatting for function definitions and async function definitions. The formatting should be fairly complete.
Test Plan
I added a few of my own test cases and reviewed the black differences.