Skip to content

remove obviously unnecessary parentheses#850

Merged
JelleZijlstra merged 2 commits intopsf:masterfrom
JelleZijlstra:removeparens
May 16, 2019
Merged

remove obviously unnecessary parentheses#850
JelleZijlstra merged 2 commits intopsf:masterfrom
JelleZijlstra:removeparens

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