fix indentation of line breaks in long type hints by adding parens#3899
Merged
JelleZijlstra merged 2 commits intopsf:mainfrom Sep 22, 2023
Merged
fix indentation of line breaks in long type hints by adding parens#3899JelleZijlstra merged 2 commits intopsf:mainfrom
JelleZijlstra merged 2 commits intopsf:mainfrom
Conversation
…es, and remove unnecessary parentheses
Collaborator
|
Thanks. This looks generally good, but we'll want to make the change only in the preview style for now. Look for existing code like |
|
diff-shades results comparing this PR (391a413) to main (e974fc3). The full diff is available in the logs under the "Generate HTML diff report" step. |
Contributor
Author
Thanks, fixed! |
JelleZijlstra
approved these changes
Sep 22, 2023
Collaborator
|
Looking at the diff-shades output, all changes seem like improvements: https://github.com/psf/black/actions/runs/6275527706/job/17044773640#step:10:1 |
This was referenced Oct 7, 2023
Closed
28 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
The simple way of resolving #2316
I would personally prefer to resolve it by formatting it without added parentheses (i.e. option 3 in #2316) to save two unnecessary lines, but that will likely require a bunch of messing around in
_maybe_split_omitting_optional_parensand could also impact some other cases - so I think I'll hold off on that one for the moment.I had to extend
maybe_make_parens_invisible_in_atomto also allow expressions, I was quite afraid this would have side effects elsewhere in the code - but it seems to be fine? I could be defensive and add a parameter that enables checkingsyms.expr, and only set it when called fromvisit_tname.There's minor changes to two existing test cases, but they both seem fine to me.
Checklist - did you ...
CHANGES.mdif necessary? [TODO]