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

fix incorrect call to right_hand_split() #762

Merged
merged 1 commit into from
Mar 15, 2019
Merged

Conversation

JelleZijlstra
Copy link
Collaborator

The second argument to right_hand_split() is an int (line length), but we were passing supports_trailing_commas as a bool. Mypy didn't catch this because bool is a subclass of int. Found this while working on a fix for #759 because I changed the type of supports_trailing_commas.

This affects one test that is now formatted differently, and it also causes a fair number of changes in our codebase. Now that I look at it, in most cases I actually feel like the old formatting is better, so maybe we need to do something different than just fix the type mismatch.

@JelleZijlstra JelleZijlstra changed the title fix incorrect call fix incorrect call to right_hand_split() Mar 15, 2019
@coveralls
Copy link

coveralls commented Mar 15, 2019

Pull Request Test Coverage Report for Build 971

  • 1 of 1 (100.0%) changed or added relevant line in 1 file are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage increased (+0.03%) to 95.953%

Files with Coverage Reduction New Missed Lines %
black.py 1 94.79%
Totals Coverage Status
Change from base Build 966: 0.03%
Covered Lines: 3011
Relevant Lines: 3138

💛 - Coveralls

1 similar comment
@coveralls
Copy link

Pull Request Test Coverage Report for Build 971

  • 1 of 1 (100.0%) changed or added relevant line in 1 file are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage increased (+0.03%) to 95.953%

Files with Coverage Reduction New Missed Lines %
black.py 1 94.79%
Totals Coverage Status
Change from base Build 966: 0.03%
Covered Lines: 3011
Relevant Lines: 3138

💛 - Coveralls

JelleZijlstra added a commit to JelleZijlstra/black that referenced this pull request Mar 15, 2019
JelleZijlstra added a commit to JelleZijlstra/black that referenced this pull request Mar 15, 2019
@ambv
Copy link
Collaborator

ambv commented Mar 15, 2019

Oh my, this one's embarrassing! It dates all the way back to 3eea3aa when I was fixing trailers (content with brackets) being unnecessarily exploded into their own lines after a dedented closing bracket.

@ambv ambv merged commit b396f13 into psf:master Mar 15, 2019
@ambv
Copy link
Collaborator

ambv commented Mar 15, 2019

@JelleZijlstra, please add a tiny description of the change to the change log for 19.4c0, please.

@JelleZijlstra JelleZijlstra deleted the fixtype branch March 15, 2019 18:42
@rouge8
Copy link
Contributor

rouge8 commented Mar 26, 2019

This affects one test that is now formatted differently, and it also causes a fair number of changes in our codebase. Now that I look at it, in most cases I actually feel like the old formatting is better, so maybe we need to do something different than just fix the type mismatch.

I had the same experience in one of our codebases where the old formatting was generally better. Is there an issue or plans to address this?

@JelleZijlstra
Copy link
Collaborator Author

Filed #781 to follow up on this.

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.

4 participants