Format Function definitions #4951
Merged
Conversation
This file contains 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
This was referenced Jun 8, 2023
Merged
|
Current dependencies on/for this PR:
This comment was auto-generated by Graphite. |
PR Check ResultsEcosystem✅ ecosystem check detected no changes. BenchmarkLinuxWindows |
MichaReiser
force-pushed
the
format-function-definition
branch
from
01a9a40 to
c0c645e
Compare
MichaReiser
added
internal
MichaReiser
force-pushed
the
format-function-definition
branch
from
7ba434b to
875cd8b
Compare
MichaReiser
commented
Jun 8, 2023
| @@ -77,6 +77,11 @@ def to_camel_case(node: str) -> str: | |||
| # ) | |||
| # src.joinpath(groups[group]).joinpath("mod.rs").write_text(rustfmt(mod_section)) | |||
| 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(): | |||
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); |
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 ;)
| @@ -39,19 +39,17 @@ where | |||
| N: AstNode, | |||
| { | |||
| fn fmt(&self, node: &N, f: &mut PyFormatter) -> FormatResult<()> { | |||
| write!(f, [source_position(node.start())])?; | |||
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.
| @@ -234,65 +234,69 @@ def stable_quote_normalization_with_immediate_inner_single_quote(self): | |||
| ```diff | |||
| --- Black | |||
| +++ Ruff | |||
| @@ -1,219 +1,105 @@ | |||
| @@ -1,219 +1,149 @@ | |||
| -class MyClass: | |||
There was a problem hiding this comment.
This PR does not implement any special handling for docstrings yet.
MichaReiser
force-pushed
the
format-function-definition
branch
from
875cd8b to
3c45cf3
Compare
konstin
approved these changes
Jun 8, 2023
|
|
||
| write!(f, [if_group_breaks(&text(","))])?; | ||
|
|
||
| if let Some(last_node) = last_node { |
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 { |
There was a problem hiding this comment.
Does this also need to read the magic trailing comma setting?
There was a problem hiding this comment.
If we introduce a setting for this, then yes
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
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
MichaReiser
force-pushed
the
format-function-definition
branch
from
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)
MichaReiser
force-pushed
the
format-function-definition
branch
from
60dd48a to
26a4a83
Compare
MichaReiser
force-pushed
the
format-function-definition
branch
from
26a4a83 to
3be5049
Compare
MichaReiser
force-pushed
the
format-function-definition
branch
from
3be5049 to
dcb2aaa
Compare
auto-merge was automatically disabled
June 8, 2023 15:54
Pull request was closed
MichaReiser
force-pushed
the
format-function-definition
branch
from
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.