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

Actually add trailing commas to collection literals even if there are terminating comments #3393

Merged
merged 6 commits into from Feb 5, 2023
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGES.md
Expand Up @@ -16,6 +16,8 @@

<!-- Changes that affect Black's preview style -->

- Correctly handle trailing commas inside dictionaries where a comment is after the
final entry (#3393)
- Improve the performance on large expressions that contain many strings (#3467)
- Fix a crash in preview style with assert + parenthesized string (#3415)
- Fix crashes in preview style with walrus operators used in function return annotations
Expand Down
38 changes: 29 additions & 9 deletions src/black/linegen.py
Expand Up @@ -938,6 +938,25 @@ def split_wrapper(line: Line, features: Collection[Feature] = ()) -> Iterator[Li
return split_wrapper


def _get_last_non_comment_leaf(line: Line) -> Optional[int]:
for leaf_idx in range(len(line.leaves) - 1, 0, -1):
if line.leaves[leaf_idx].type != STANDALONE_COMMENT:
return leaf_idx
return None


def _safe_add_trailing_comma(safe: bool, delimiter_priority: int, line: Line) -> Line:
if (
safe
and delimiter_priority == COMMA_PRIORITY
and line.leaves[-1].type != token.COMMA
and line.leaves[-1].type != STANDALONE_COMMENT
):
new_comma = Leaf(token.COMMA, ",")
line.append(new_comma)
return line


@dont_increase_indentation
def delimiter_split(line: Line, features: Collection[Feature] = ()) -> Iterator[Line]:
"""Split according to delimiters of the highest priority.
Expand Down Expand Up @@ -979,7 +998,8 @@ def append_to_line(leaf: Leaf) -> Iterator[Line]:
)
current_line.append(leaf)

for leaf in line.leaves:
last_non_comment_leaf = _get_last_non_comment_leaf(line)
for leaf_idx, leaf in enumerate(line.leaves):
yield from append_to_line(leaf)

for comment_after in line.comments_after(leaf):
Expand All @@ -996,6 +1016,11 @@ def append_to_line(leaf: Leaf) -> Iterator[Line]:
trailing_comma_safe and Feature.TRAILING_COMMA_IN_CALL in features
)

if last_leaf.type == STANDALONE_COMMENT and leaf_idx == last_non_comment_leaf:
current_line = _safe_add_trailing_comma(
trailing_comma_safe, delimiter_priority, current_line
)

leaf_priority = bt.delimiters.get(id(leaf))
if leaf_priority == delimiter_priority:
yield current_line
Expand All @@ -1004,14 +1029,9 @@ def append_to_line(leaf: Leaf) -> Iterator[Line]:
mode=line.mode, depth=line.depth, inside_brackets=line.inside_brackets
)
if current_line:
if (
trailing_comma_safe
and delimiter_priority == COMMA_PRIORITY
and current_line.leaves[-1].type != token.COMMA
and current_line.leaves[-1].type != STANDALONE_COMMENT
):
new_comma = Leaf(token.COMMA, ",")
current_line.append(new_comma)
current_line = _safe_add_trailing_comma(
trailing_comma_safe, delimiter_priority, current_line
)
yield current_line


Expand Down
52 changes: 52 additions & 0 deletions tests/data/simple_cases/function_trailing_comma.py
@@ -1,5 +1,29 @@
def f(a,):
d = {'key': 'value',}
e = {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add similar test cases using set, tuple, and list literals?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the comment. Yes, I've just added those.

"a": fun(msg, "ts"),
"longggggggggggggggid": ...,
"longgggggggggggggggggggkey": ..., "created": ...
# "longkey": ...
}
f = [
arg1,
arg2,
arg3, arg4
# comment
]
g = (
arg1,
arg2,
arg3, arg4
# comment
)
h = {
arg1,
arg2,
arg3, arg4
# comment
}
tup = (1,)

def f2(a,b,):
Expand Down Expand Up @@ -68,6 +92,34 @@ def f(
d = {
"key": "value",
}
e = {
"a": fun(msg, "ts"),
"longggggggggggggggid": ...,
"longgggggggggggggggggggkey": ...,
"created": ...,
# "longkey": ...
}
f = [
arg1,
arg2,
arg3,
arg4,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems short enough to fit in one line, so it's a little weird to me that we don't. However, current behavior is already to put everything on its own line and that seems fine, so let's leave it as is.

# comment
]
g = (
arg1,
arg2,
arg3,
arg4,
# comment
)
h = {
arg1,
arg2,
arg3,
arg4,
# comment
}
tup = (1,)


Expand Down