Leading, Dangling, and Trailing comments formatting #4785
Conversation
|
Current dependencies on/for this PR:
This comment was auto-generated by Graphite. |
MichaReiser
added
internal
|
|
||
| /// What's the enclosing level of the outer node. | ||
| #[derive(Copy, Clone, Debug, Eq, PartialEq, Default)] | ||
| pub(crate) enum NodeLevel { |
There was a problem hiding this comment.
I'm not super happy with this because it will require that any code that formats an indented block needs to set the level to NodeLevel::Statement.
The reason why I introduced it is because the formatting logic needs to know whether two empty lines are acceptable or not (or any, in the case of parenthesized expressions)
There was a problem hiding this comment.
Will we need even more granular states here? E.g., Black has different rules for docstrings within classes vs. docstrings within functions (requires one newline for the former, allows one newline for the latter):
def f():
"""foo"""
x = 1
class F:
"""foo"""
x = 1There was a problem hiding this comment.
Possibly, but I would prefer adding global state when possible (it comes at a cost that we need to snapshot the state and restore it if we ever add error handling support to the formatter).
Away we could do docstring formatting without adding any additional global state is to:
- Get the first statement
- Test if it is a docstring
- If so, format it, and insert the appropriate number of newlines after
- Otherwise format it as usual
MichaReiser
force-pushed
the
format-leading-comments
branch
from
60f2b16 to
66615bb
Compare
PR Check ResultsEcosystem✅ ecosystem check detected no changes. BenchmarkLinuxWindows |
|
|
||
| /// What's the enclosing level of the outer node. | ||
| #[derive(Copy, Clone, Debug, Eq, PartialEq, Default)] | ||
| pub(crate) enum NodeLevel { |
There was a problem hiding this comment.
Will we need even more granular states here? E.g., Black has different rules for docstrings within classes vs. docstrings within functions (requires one newline for the former, allows one newline for the latter):
def f():
"""foo"""
x = 1
class F:
"""foo"""
x = 1
MichaReiser
force-pushed
the
format-leading-comments
branch
from
66615bb to
5794efd
Compare
Summary
This PR implements the basic leading, dangling, and trailing comments formatting and adds it to the
FormatNodeRule.This PR does not yet formatt the content of the comment. This will be implemented in the next PR.
Test Plan
The comments are now included in the formatted output, which is good. I wasn't able to really verify if they are all placed correctly because we only format top-level statements.
I recommend that we fine tune the formatting when we have better coverage and know which comments are formatted incorrectly.