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.

Sign up for GitHub

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

Jump to bottom

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

Merged
merged 6 commits into from Feb 5, 2023
Merged

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

merged 6 commits into from Feb 5, 2023

Conversation

mainj12
Copy link
Contributor

@mainj12 mainj12 commented Nov 22, 2022 •
edited by ichard26

Fixes #3072

Description

Checklist - did you ...

  • Add an entry in CHANGES.md if necessary?
  • Add / update tests if necessary?
  • Add new / update outdated documentation?

@mainj12 mainj12 mentioned this pull request Nov 22, 2022
Closed
@mainj12 mainj12 force-pushed the issue-3072 branch 5 times, most recently from d8a621e to e9c4f5d Compare November 22, 2022 18:44
@github-actions
Copy link

github-actions bot commented Nov 23, 2022 •
edited

diff-shades results comparing this PR (aa6f082) to main (dd0e912). The full diff is available in the logs under the "Generate HTML diff report" step.

╭─────────────────────── Summary ────────────────────────╮
│ 2 projects & 7 files changed / 14 changes [+7/-7]      │
│                                                        │
│ ... out of 2 397 394 lines, 11 491 files & 23 projects │
╰────────────────────────────────────────────────────────╯

Differences found.

What is this? | Workflow run | diff-shades documentation

JelleZijlstra
JelleZijlstra reviewed Dec 15, 2022
View reviewed changes
tests/data/simple_cases/function_trailing_comma.py Outdated
@@ -1,5 +1,11 @@
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.

JelleZijlstra
JelleZijlstra approved these changes Dec 29, 2022
View reviewed changes
tests/data/simple_cases/function_trailing_comma.py Outdated
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.

ichard26
ichard26 approved these changes Jan 31, 2023
View reviewed changes
Copy link
Collaborator

@ichard26 ichard26 left a comment

Choose a reason for hiding this comment

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

Thank you! This is great 🚢

@ichard26 ichard26 added this to the Release 23.2.0 milestone Feb 5, 2023
@ichard26 ichard26 self-assigned this Feb 5, 2023
@ichard26
Only enable under preview
379bbc5 
This was annoying to change because I had to first allow transformers
to also receive a Mode() argument.
@ichard26 ichard26 changed the title Adding trailing commas to dictionaries when comments are present Actually add trailing commas to collection literals even if there are terminating comments Feb 5, 2023
@JelleZijlstra JelleZijlstra merged commit ff53fc1 into psf:main Feb 5, 2023
@hauntsaninja hauntsaninja mentioned this pull request Nov 13, 2023
Closed
@konstin konstin mentioned this pull request Nov 14, 2023
Closed
28 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JelleZijlstra JelleZijlstra JelleZijlstra approved these changes

@ichard26 ichard26 ichard26 approved these changes

Assignees

@ichard26 ichard26

Labels
None yet
Projects
None yet
Milestone
Release 23.2.0
Development

Successfully merging this pull request may close these issues.

Trailing Comma in Dictionary not working if a comment line is at the end
3 participants
@mainj12 @JelleZijlstra @ichard26