-
Notifications
You must be signed in to change notification settings - Fork 890
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
Add a new knob and a change for inline comment alignment #1022
base: main
Are you sure you want to change the base?
Add a new knob and a change for inline comment alignment #1022
Conversation
…values and added tests
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.
Thanks for the PR! Please run yapf over the changes. Also please update the CHANGELOG and README.rst files.
# only when the commet is not inside an object(list, dictionary, | ||
# function call)logical line that is in many output lines |
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.
Could you clarify this comment, please? I'm not sure what it's trying to suggest.
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.
Thanks for the replies. I will fix all the problems today. We know that a dictionary, a list and an argument list is one logical line no matter how long it is with its entries all output in separate lines. Here it means that only when the comment is not inside this kind of logical line, aka, the comment has parent level equal to 0, we set the spaces_required_before to 0. Because we don't want the comments inside a dictionary to align with its parent (the dictionary variable name). Examples are given in pull request README.
yapf/pytree/comment_splicer.py
Outdated
@@ -42,7 +42,6 @@ def SpliceComments(tree): | |||
# This is a list because Python 2.x doesn't have 'nonlocal' :) | |||
prev_leaf = [None] | |||
_AnnotateIndents(tree) | |||
|
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.
Please retain this newline.
yapf/yapflib/reformatter.py
Outdated
# All trailing comments | ||
# NOTE not including comments that appear on a line by themselves |
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.
Please reflow this comment. And place any "NOTE" comments at the end of the comment block.
yapf/yapflib/reformatter.py
Outdated
@@ -309,7 +311,16 @@ def _AlignTrailingComments(final_lines): | |||
line_content = '' | |||
pc_line_lengths = [] | |||
|
|||
#NOTE |
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.
Empty note.
yapf/yapflib/reformatter.py
Outdated
for line_tok in this_line.tokens: | ||
|
||
#NOTE if a line with inline comment is itself |
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.
I don't think you need a "NOTE" tag here (or in some other places).
yapf/yapflib/reformatter.py
Outdated
''' this is added when we don't want comments on newlines | ||
to align with comments inline | ||
''' |
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.
This should be a comment instead of multiline string.
yapftests/yapf_test.py
Outdated
@@ -26,7 +26,7 @@ | |||
|
|||
from lib2to3.pgen2 import tokenize | |||
|
|||
from yapf.yapflib import errors | |||
from yapf.yapflib import errors, reformatter |
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.
I don't think you need this.
Hello yapf founders!
I want to share one knob and one change for the AlignTrailingComments feature.
The knob: ALIGN_NEWLINE_COMMENTS_WITH_INLINE_COMMENTS
This is a style option I added for when we do not want to align the newline comments with the inline comments in the same alignment block.
This option is used together with the SPACE_BEFORE_COMMENT = a list of numbers.
The default setting is True. When set to False, newline comments won't align with inline comments.
For example, instead of
we want
A problem rises when using this new knob, instead of remaining the spaces before the standalone newline comment inside a logical line:
The comment is dedented to its parent level like:
This is also fixed by adding one line in _GetNewlineColumn(self), restricting to set the spaces before the comment to 0 only when it’s not inside a logical line(aka with parent_level==0).
One change
Another change I added is when there is an object(dictionary, argument list, list, function call) with its entries on separate lines with its closing bracket also on a separate line, it starts a new alignment block.
For example:
Finally