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

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

Merged
merged 18 commits into from
Nov 9, 2022

Conversation

yilei
Copy link
Contributor

@yilei yilei commented Nov 1, 2022

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
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.
@github-actions
Copy link

github-actions bot commented Nov 2, 2022

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.

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!

src/black/linegen.py Outdated Show resolved Hide resolved
src/black/linegen.py Outdated Show resolved Hide resolved
src/black/linegen.py Outdated Show resolved Hide resolved
src/black/linegen.py Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants