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

Adds color to ERROR and SUCCESS in check mode. #1349

Merged

Conversation

sztamas
Copy link
Collaborator

@sztamas sztamas commented Jul 26, 2020

Fixes #1177.

Did this with colorama as I think it doesn't warrant introducing something like termcolor or blessings. colorama is needed anyways with both of those to support Windows.

Some further considerations:

  • we might want a --no-color option to turned the colors explicitly off. Colorama turns them off by default if you're piping the results into another command
  • the diffs aren't colored when using --diff

Sidenote for @timothycrosley : we're running out of issues marked with help_wanted, so you might want to add a few more if you have the time 😄

@codecov-commenter
Copy link

codecov-commenter commented Jul 26, 2020

Codecov Report

Merging #1349 into develop will not change coverage.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##           develop     #1349   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           34        34           
  Lines         2288      2315   +27     
=========================================
+ Hits          2288      2315   +27     

@timothycrosley
Copy link
Member

This looks great! Unfortunately, this is complicated a bit by isorts policy to never include dependencies. isort is depended on by some large projects such as pylint, so in the past when it did include or rely on dependencies the odds of conflicts were very high. See: https://timothycrosley.github.io/isort/docs/major_releases/release_policy/#packaging-guarantees. I do think that the policy could be clarified to say that will change isort file output. However, it would still be necessary I believe to make this the non default, and require a flag such as --color for those that want it.

@sztamas
Copy link
Collaborator Author

sztamas commented Jul 26, 2020

OK, thanks for sharing the background info!

How about adding colorama to extras (https://python-poetry.org/docs/pyproject/#extras) then?

We can keep the existing functionality as is, add a new --color option which errors out with a descriptive message to install the colors extra if colorama is missing.

What do you think?

@timothycrosley
Copy link
Member

@sztamas Exactly! That sounds like a perfect solution

@timothycrosley
Copy link
Member

This looks awesome! Thank you!

@timothycrosley timothycrosley merged commit 2115ed8 into PyCQA:develop Jul 28, 2020
@sztamas sztamas deleted the issue/1177/colored-output-in-check-mode branch August 13, 2020 05:07
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.

Verbose -- colored result
3 participants