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

remove coloring from pre-commit prints #417

Closed
12rambau opened this issue Sep 7, 2021 · 5 comments · Fixed by #657
Closed

remove coloring from pre-commit prints #417

12rambau opened this issue Sep 7, 2021 · 5 comments · Fixed by #657

Comments

@12rambau
Copy link
Contributor

12rambau commented Sep 7, 2021

Description

I use the pre-commit framework to apply some sanity check before committing to my repositories. I recently discover this lib and wanted to give it a try. Fantastic work !

I use te following command to create a new commit:

cz commit

And I get the following results (sorry for the image but everything is about coloring):
Capture d’écran 2021-09-07 à 16 41 00

after #258which is the commit message, the pre-commit checks start and normally at the end of their lines skippedshould be blue, passed: green and failed red. when creating a commit with czI loose these colors. I tested back using simply git commitand it worked.

Question then: why is the coloring of pre-commit disabled by cz ?

@Lee-W
Copy link
Member

Lee-W commented Sep 11, 2021

Thanks for reporting! if my memory serves me right, we did not do anything to disable the color 🤔 might need to take a look at the pre-commit side as well

@12rambau
Copy link
Contributor Author

Finally took the time to dive into this one:

That's actually quite simple and it's definitely coming from here. Going upstream from cli.py I think commit are created using : cmd.run(f"git commit {args} -F {f.name}") where cmd is a Command object. the run method execute a subprocess call and redirect the resulting lines to out.

If git is called from a subprocess it doesn't know if it's outputted to a terminal and switch off the ANSI coloring (ref: https://stackoverflow.com/questions/42589584/ansi-color-lost-when-using-python-subprocess, https://stackoverflow.com/questions/38084815/can-you-have-subprocesss-popen-retain-color-in-stdout-stderr).

Now the question is: should the ANSI colorisation be authorized in the Command object or should we keep it that way to avoid compatibility issues whith platforms ?

@Lee-W
Copy link
Member

Lee-W commented Jan 18, 2023

Hi @12rambau Thanks for your help! Does that mean if we keep the ANSI coloring, we might break platform compatibility? If that's the case, I'd say platform compatibility is way more important.

@12rambau
Copy link
Contributor Author

If that's the case, I'd say platform compatibility is way more important.

Agreed. Then I'll check if it can be set somewhere in the documentation in a note and then I'll close this issue

@Lee-W
Copy link
Member

Lee-W commented Jan 19, 2023

@12rambau Great! Thanks for your help!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants