Skip to content

Comments

Correctly handle trailing commas that are inside a line's leading non-nested parens#3370

Merged
JelleZijlstra merged 18 commits intopsf:mainfrom
yilei:trailing_comma
Nov 9, 2022
Merged

Correctly handle trailing commas that are inside a line's leading non-nested parens#3370
JelleZijlstra merged 18 commits intopsf:mainfrom
yilei:trailing_comma

Conversation

@yilei
Copy link
Contributor

@yilei yilei commented Nov 1, 2022 •
edited
Loading

Description

Correctly handle trailing commas that are inside a line's leading non-nested parens.

Checklist - did you ...

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

@yilei yilei marked this pull request as draft November 1, 2022 07:39
yilei added 4 commits November 1, 2022 10:49
@yilei
Fix two bugs and add more tests.
6a14f81 
1. The change wasn't guarded by preview mode.
2. Checking trailing commas in inner parens should only be done on the `head` component after bracket split.
@yilei
Switch to importing members.
c38bf9a
@yilei
Merge branch 'main' into trailing_comma
5253b0a
@yilei
WIP: fix the bracket tracker usage.
0ae0c5c
@github-actions
Copy link

github-actions bot commented Nov 2, 2022 •
edited
Loading

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

╭─────────────────────── Summary ────────────────────────╮
│ 2 projects & 7 files changed / 39 changes [+30/-9]     │
│                                                        │
│ ... out of 2 328 882 lines, 10 723 files & 23 projects │
╰────────────────────────────────────────────────────────╯

Differences found.

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

@yilei yilei marked this pull request as ready for review November 2, 2022 14:47
@yilei
Copy link
Contributor Author

yilei commented Nov 2, 2022

Alright, this PR is ready for review. It should unblock #3368.

My initial implementation accidentally fixed the trailing comma issue in #2052, but it had other issues and was wrong so I removed that reference. I'll take another look once this PR is merged.

JelleZijlstra
JelleZijlstra reviewed Nov 8, 2022
View reviewed changes
Copy link
Collaborator

@JelleZijlstra JelleZijlstra left a comment

Choose a reason for hiding this comment

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

Looks good!

@JelleZijlstra JelleZijlstra merged commit 8091b25 into psf:main Nov 9, 2022
@JelleZijlstra JelleZijlstra mentioned this pull request Nov 9, 2022
Closed
3 tasks
This was referenced Dec 9, 2022
Closed
Closed
@yilei yilei deleted the trailing_comma branch December 11, 2022 05:16
@MeGaGiGaGon MeGaGiGaGon mentioned this pull request Jul 23, 2025
Closed
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

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Trailing commas not removed when dict/function call collapsed into single line Only the last trailing comma in a statement is magic

2 participants

@yilei @JelleZijlstra