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

Handle .COLOR_DIFF in the same way as .DIFF #1673

Merged
merged 6 commits into from Sep 5, 2020

Conversation

tom-saunders
Copy link
Contributor

Whilst poking around checking how diff output was generated I noticed that the behaviour for these two values seemed to not be handled consistently.

Added a couple of tests to demonstrate and resolved the points where that seems to occur.

This isn't _quite_ true - this demonstrates that we never get
the manager used to provide the lock, but without that we're
clearly not getting the lock.
It seems that when .COLOR_DIFF is used we don't use the locks in a manner
consistent with how .DIFF is using them.
@tom-saunders
Copy link
Contributor Author

well that's interesting - these pass locally. I imagine it's something I am doing locally which breaks them elsewhere.

@ichard26
Copy link
Collaborator

ichard26 commented Sep 4, 2020

Don't worry, it's not as bad as you think. Please ignore the primer failures, those are unrelated and I am currently working on a fix.

As modified they fail local to me, but I suspect that's a me problem :)
Copy link
Collaborator

@ichard26 ichard26 left a comment

Choose a reason for hiding this comment

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

The PR itself looks good to me, but can you please add an entry in the changelog (CHANGES.md)? Perhaps something like "Coloured diff output is no longer mangled when formatting multiple files at once (#1673)"?

I was wondering why the coloured diff output was all mixed up while I doing some debugging. I saw that there was a lock being used so I just thought that my environment was broken. I forgot that there is also a WriteBack.COLOR_DIFF enum value 😅

Also if you'd like, feel free to add youself to the author list in the README. Congrats on your first PR in the psf repo!

@JelleZijlstra JelleZijlstra merged commit 6b5753a into psf:master Sep 5, 2020
@tom-saunders tom-saunders deleted the lock-colourdiff branch September 5, 2020 19:24
noxan pushed a commit to noxan/black that referenced this pull request Jun 6, 2021
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.

None yet

3 participants