-
Notifications
You must be signed in to change notification settings - Fork 5
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
Normalize line breaks visitor #101
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.
Apart from that small comment and the merging from main to get the parser fix, I think this looks good.
@@ -2,6 +2,7 @@ | |||
|
|||
from .blank_lines import BlankLinesVisitor | |||
from .normalize_format import NormalizeFormatVisitor | |||
from .normalize_line_breaks_visitor import NormalizeLineBreaksVisitor |
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.
It looks like we still need to call the visitor.
…linebreaks-visitor
self._stop = False | ||
self._style = style | ||
|
||
def visit_space(self, space: Optional[Space], loc: Optional[Union[PySpace.Location, Space.Location]], |
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.
Also here we will need an override for visit_marker
, because the base visitor class doesn't know about specific markers and thus won't visit the Space
of TrailingComma
.
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.
Should be fixed
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.
Can we add a test case with a trailing comma for this visitor? I don't see the visit_marker override anywhere.
What's changed?
A draft version of Normalize Line Breaks Visitor has been added, The goal of this draft PR is to gain some further suggestions for its final implementation.
What's your motivation?
Anything in particular you'd like reviewers to focus on?
Anyone you would like to review specifically?
Have you considered any alternatives or workarounds?
Any additional context
Checklist