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

remove obviously unnecessary parentheses #850

Merged
merged 2 commits into from
May 16, 2019

Conversation

JelleZijlstra
Copy link
Collaborator

Fixes #548

This removes all parentheses around very simple nodes (e.g., (1), (x)). Perhaps we can do something more aggressive, like removing them around function call or attribute access expressions.

Copy link
Collaborator

@zsol zsol left a comment

Choose a reason for hiding this comment

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

I was wondering if we could do parens normalization as a post-processing step once we already have decided on the lines, or during splitting (like we currently do), or pre-split (like this PR is doing). I doing it in one place makes Black more easy to understand.

Until then, this looks good enough :) 👍

@JelleZijlstra JelleZijlstra merged commit 957ba24 into psf:master May 16, 2019
@JelleZijlstra JelleZijlstra deleted the removeparens branch May 16, 2019 04:11
@nickpresta
Copy link

@JelleZijlstra does this also remove parens in except blocks that don't need it?

# in
try:
    raise ValueError("hi")
except (ValueError):
    pass

# out
try:
    raise ValueError("hi")
except ValueError:
    pass

Thanks!

@JelleZijlstra JelleZijlstra mentioned this pull request Oct 20, 2019
JelleZijlstra added a commit to JelleZijlstra/black that referenced this pull request Oct 20, 2019
@JelleZijlstra JelleZijlstra mentioned this pull request Oct 20, 2019
JelleZijlstra added a commit that referenced this pull request Oct 20, 2019
Fixes #1042 (and probably #1044 which looks like the same thing).

The issue with the "obviously unnecessary" parentheses that #850 removed is that sometimes they're necessary to help Black fit something in one line. I didn't see an obvious solution that still removes the parens #850 was intended to remove, so let's back out this change for now in the interest of unblocking a release.

This PR also adds a test adapted from the failing example in #1042, so that if we try to reapply the #850 change we don't break the same case again.
amyreese pushed a commit to amyreese/black that referenced this pull request May 7, 2020
Fixes psf#1042 (and probably psf#1044 which looks like the same thing).

The issue with the "obviously unnecessary" parentheses that psf#850 removed is that sometimes they're necessary to help Black fit something in one line. I didn't see an obvious solution that still removes the parens psf#850 was intended to remove, so let's back out this change for now in the interest of unblocking a release.

This PR also adds a test adapted from the failing example in psf#1042, so that if we try to reapply the psf#850 change we don't break the same case again.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Black should remove unnecessary outer parentheses around function arguments
3 participants