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

Add option for printing a colored diff #1266

Merged
merged 13 commits into from
May 8, 2020
Merged

Add option for printing a colored diff #1266

merged 13 commits into from
May 8, 2020

Conversation

dougthor42
Copy link
Contributor

Fixes #994.

This adds a --color option to the black command line which adds very basic color support when printing diffs.

  • The coloring mimics git diff because why not. Headers are Bold White, line markers Cyan, subtractions Red, and additions Green.
  • There is no change to the default behavior.
  • --color has no effect if --diff is not supplied.

Example:

image

Discussion

  • As is, this will not work with Windows CMD or PowerShell. In order to enable Windows support, the colorama package should be included. I'm not sure how you feel about adding packages all willy-nilly, so I figured the first version of this PR would not include it.
  • I opted to add COLOR_DIFF to WriteBack because that meant I didn't have to change any call signatures.
  • An initial version used an enum for --color which let the user choose between always, none, and auto. This is still doable, but will take more effort. Personally I don't much see the need for it - using a boolean is sufficient in my opinion. That said, if you'd like me to pursue it just let me know.

@dougthor42 dougthor42 changed the title Add option for printing a color diff. Color diff gh994 Add option for printing a colored diff Feb 7, 2020
@dougthor42
Copy link
Contributor Author

Whoops, wasn't working off the latest master. Fixed and rebased.

@zsol
Copy link
Collaborator

zsol commented Feb 7, 2020

FWIW I like this feature. How about dropping the command line flag and always outputting colored diff if we're writing to a terminal (sys.stdout.isatty())?

As for Windows support, I'd be fine with adding colorama as an extra (extras_require in setup.py)

@dougthor42
Copy link
Contributor Author

dropping the command line flag

I think it would be prudent to keep some CLI flag because I'm sure there are cases where a user wants to write to a terminal but not colorize it. Perhaps we just invert it: --no-color.

if we're writing to a terminal (sys.stdout.isatty())

From what I can tell, whenever WriteBack.DIFF is used, black is already writing to io.TextIOWrapper(sys.stdout.buffer, ...). Do I even need to check isatty()?


# wrap_stream returns a `colorama.AnsiToWin32.AnsiToWin32` object
# which does not have a `detach()` method. So we fake one.
f.detach = lambda *args, **kwargs: None # type: ignore
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't like this, but I don't fully understand what detach does or why it's needed, so this is the best I could come up with.

@cooperlees cooperlees requested a review from zsol May 8, 2020 13:06
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.

lgtm. I'll add a changelog entry and merge this soon. Thanks for contributing!

black.py Outdated Show resolved Hide resolved
@zsol zsol merged commit 8d6d92a into psf:master May 8, 2020
@jshin49
Copy link

jshin49 commented Jun 18, 2020

Seems this PR is closed and merged into master. Yet, when I try to use the --color option, I get an error saying there is no such option. The version I am using is 19.10b0 on python 3.7.7 and python 3.6.10

@hugovk
Copy link
Contributor

hugovk commented Jun 18, 2020

Yep, it's not yet been released:

This was merged into master in May 2020.

Black 19.10b0 was released in October 2019 (19.10 is yy.mm, see https://calver.org/).

@jshin49
Copy link

jshin49 commented Jun 18, 2020 via email

@hugovk
Copy link
Contributor

hugovk commented Jun 18, 2020

I think it's coming sooner rather than later!

#517 is the tracking issue, and see #517 (comment) and https://github.com/psf/black/labels/stable.

@cooperlees
Copy link
Collaborator

cooperlees commented Jun 18, 2020

GitHub master or even a branch is basically released these days. Introducing your new friend if you're in a hurry for color:

pip install git+https://github.com/psf/black.git@master

cooper-mbp1:~ cooper$ /tmp/tb/bin/black --help | grep color
  --color / --no-color            Show colored diff. Only applies when

@jshin49
Copy link

jshin49 commented Jun 18, 2020

Awesome. Thanks for the help @cooperlees. I'm trying to set up a workflow for a new team project and I was comparing between Pylint vs Black vs Yapf. Not sure which is the best, but the color coded diffs of Black would definitely make a lot of difference.

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.

Coloured --diff output
5 participants