-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Conversation
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.
well that's interesting - these pass locally. I imagine it's something I am doing locally which breaks them elsewhere. |
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 :)
There was a problem hiding this 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!
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.