Skip to content
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 new DISABLE_SPLIT_LIST_WITH_COMMENT flag. #1177

Merged
merged 4 commits into from
Nov 8, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 64 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,70 @@
# All notable changes to this project will be documented in this file.
# This project adheres to [Semantic Versioning](http://semver.org/).

## (0.40.3) UNRELEASED
## (0.41.0) UNRELEASED
### Added
- New `DISABLE_SPLIT_LIST_WITH_COMMENT` flag.
`DISABLE_SPLIT_LIST_WITH_COMMENT` is a new knob that changes the
behavior of splitting a list when a comment is present inside the list.

Before, we split a list containing a comment just like we split a list
containing a trailing comma: Each element goes on its own line (unless
`DISABLE_ENDING_COMMA_HEURISTIC` is true).

This new flag allows you to control the behavior of a list with a comment
*separately* from the behavior when the list contains a trailing comma.

This mirrors the behavior of clang-format, and is useful for e.g. forming
"logical groups" of elements in a list.

Without this flag:

```
[
a,
b, #
c
]
```

With this flag:

```
[
a, b, #
c
]
```

Before we had one flag that controlled two behaviors.

- `DISABLE_ENDING_COMMA_HEURISTIC=false` (default):
- Split a list that has a trailing comma.
- Split a list that contains a comment.
- `DISABLE_ENDING_COMMA_HEURISTIC=true`:
- Don't split on trailing comma.
- Don't split on comment.

Now we have two flags.

- `DISABLE_ENDING_COMMA_HEURISTIC=false` and `DISABLE_SPLIT_LIST_WITH_COMMENT=false` (default):
- Split a list that has a trailing comma.
- Split a list that contains a comment.
Behavior is unchanged from the default before.
- `DISABLE_ENDING_COMMA_HEURISTIC=true` and `DISABLE_SPLIT_LIST_WITH_COMMENT=false` :
- Don't split on trailing comma.
- Do split on comment. **This is a change in behavior from before.**
- `DISABLE_ENDING_COMMA_HEURISTIC=false` and `DISABLE_SPLIT_LIST_WITH_COMMENT=true` :
- Split on trailing comma.
- Don't split on comment.
- `DISABLE_ENDING_COMMA_HEURISTIC=true` and `DISABLE_SPLIT_LIST_WITH_COMMENT=true` :
- Don't split on trailing comma.
- Don't split on comment.
**You used to get this behavior just by setting one flag, but now you have to set both.**

Note the behavioral change above; if you set
`DISABLE_ENDING_COMMA_HEURISTIC=true` and want to keep the old behavior, you
now also need to set `DISABLE_SPLIT_LIST_WITH_COMMENT=true`.
### Changes
- Remove dependency on importlib-metadata
- Remove dependency on tomli when using >= py311
Expand Down
35 changes: 35 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -512,6 +512,41 @@ optional arguments:

> Disable the heuristic which places each list element on a separate line if
> the list is comma-terminated.
>
> Note: The behavior of this flag changed in v0.40.3. Before, if this flag
> was true, we would split lists that contained a trailing comma or a
> comment. Now, we have a separate flag, `DISABLE_SPLIT_LIST_WITH_COMMENT`,
> that controls splitting when a list contains a comment. To get the old
> behavior, set both flags to true. More information in
> [CHANGELOG.md](CHANGELOG.md#new-disable_split_list_with_comment-flag).

#### `DISABLE_DISABLE_SPLIT_LIST_WITH_COMMENT` (new in 0.40.3)

> Don't put every element on a new line within a list that contains
> interstitial comments.
>
> Without this flag (default):
>
> ```
> [
> a,
> b, #
> c
> ]
> ```
>
> With this flag:
>
> ```
> [
> a, b, #
> c
> ]
> ```
>
> This mirrors the behavior of clang-format and is useful for forming
> "logical groups" of elements in a list. It also works in function
> declarations.

#### `EACH_DICT_ENTRY_ON_SEPARATE_LINE`

Expand Down
21 changes: 16 additions & 5 deletions yapf/pytree/pytree_unwrapper.py
Original file line number Diff line number Diff line change
Expand Up @@ -407,16 +407,27 @@ def _AdjustSplitPenalty(line):

def _DetermineMustSplitAnnotation(node):
"""Enforce a split in the list if the list ends with a comma."""
if style.Get('DISABLE_ENDING_COMMA_HEURISTIC'):
return
if not _ContainsComments(node):

def SplitBecauseTrailingComma():
if style.Get('DISABLE_ENDING_COMMA_HEURISTIC'):
return False
token = next(node.parent.leaves())
if token.value == '(':
if sum(1 for ch in node.children if ch.type == grammar_token.COMMA) < 2:
return
return False
if (not isinstance(node.children[-1], pytree.Leaf) or
node.children[-1].value != ','):
return
return False
return True

def SplitBecauseListContainsComment():
return (not style.Get('DISABLE_SPLIT_LIST_WITH_COMMENT') and
_ContainsComments(node))

if (not SplitBecauseTrailingComma() and
not SplitBecauseListContainsComment()):
return

num_children = len(node.children)
index = 0
_SetMustSplitOnFirstLeaf(node.children[0])
Expand Down
12 changes: 12 additions & 0 deletions yapf/yapflib/style.py
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,16 @@ def method():
DISABLE_ENDING_COMMA_HEURISTIC=textwrap.dedent("""\
Disable the heuristic which places each list element on a separate line
if the list is comma-terminated.

Note: The behavior of this flag changed in v0.40.3. Before, if this flag
was true, we would split lists that contained a trailing comma or a
comment. Now, we have a separate flag, `DISABLE_SPLIT_LIT_WITH_COMMENT`,
that controls splitting when a list contains a comment. To get the old
behavior, set both flags to true. More information in CHANGELOG.md.
"""),
DISABLE_SPLIT_LIST_WITH_COMMENT=textwrap.dedent("""
Don't put every element on a new line within a list that contains
interstitial comments.
"""),
EACH_DICT_ENTRY_ON_SEPARATE_LINE=textwrap.dedent("""\
Place each dictionary entry onto its own line.
Expand Down Expand Up @@ -483,6 +493,7 @@ def CreatePEP8Style():
CONTINUATION_INDENT_WIDTH=4,
DEDENT_CLOSING_BRACKETS=False,
DISABLE_ENDING_COMMA_HEURISTIC=False,
DISABLE_SPLIT_LIST_WITH_COMMENT=False,
EACH_DICT_ENTRY_ON_SEPARATE_LINE=True,
FORCE_MULTILINE_DICT=False,
I18N_COMMENT='',
Expand Down Expand Up @@ -671,6 +682,7 @@ def _IntOrIntListConverter(s):
CONTINUATION_INDENT_WIDTH=int,
DEDENT_CLOSING_BRACKETS=_BoolConverter,
DISABLE_ENDING_COMMA_HEURISTIC=_BoolConverter,
DISABLE_SPLIT_LIST_WITH_COMMENT=_BoolConverter,
EACH_DICT_ENTRY_ON_SEPARATE_LINE=_BoolConverter,
FORCE_MULTILINE_DICT=_BoolConverter,
I18N_COMMENT=str,
Expand Down
23 changes: 23 additions & 0 deletions yapftests/reformatter_basic_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -345,6 +345,29 @@ def f( # Intermediate comment
llines = yapf_test_helper.ParseAndUnwrap(unformatted_code)
self.assertCodeEqual(expected_formatted_code, reformatter.Reformat(llines))

def testParamListWithTrailingComments(self):
unformatted_code = textwrap.dedent("""\
def f(a,
b, #
c):
pass
""")
expected_formatted_code = textwrap.dedent("""\
def f(a, b, #
c):
pass
""")
try:
style.SetGlobalStyle(
style.CreateStyleFromConfig(
'{based_on_style: yapf,'
' disable_split_list_with_comment: True}'))
llines = yapf_test_helper.ParseAndUnwrap(unformatted_code)
self.assertCodeEqual(expected_formatted_code,
reformatter.Reformat(llines))
finally:
style.SetGlobalStyle(style.CreateYapfStyle())

def testBlankLinesBetweenTopLevelImportsAndVariables(self):
unformatted_code = textwrap.dedent("""\
import foo as bar
Expand Down