Description

12rambau

opened on Sep 7, 2021

Issue body actions

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):

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 ?

Activity

1. 12rambau

   added

   type: bug

   on Sep 7, 2021
2. Lee-W commented on Sep 11, 2021

   

   Lee-W

   on Sep 11, 2021

   Member

   More actions

   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
3. Lee-W

   added

   development-sprint

   on Nov 26, 2021
4. 12rambau commented on Jan 15, 2023

   

   12rambau

   on Jan 15, 2023

   ContributorAuthor

   More actions

   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 ?
5. Lee-W commented on Jan 18, 2023

   

   Lee-W

   on Jan 18, 2023

   Member

   More actions

   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.
6. Lee-W

   removed

   development-sprint

   on Jan 18, 2023
7. 12rambau commented on Jan 19, 2023

   

   12rambau

   on Jan 19, 2023

   ContributorAuthor

   More actions

   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
8. Lee-W commented on Jan 19, 2023

   

   Lee-W

   on Jan 19, 2023

   Member

   More actions

   @12rambau Great! Thanks for your help!
9. Lee-W

   closed this as completedin #657on Jan 22, 2023
10. Lee-W

    added

    issue-status: wont-fix

    on Jan 22, 2023

Issue actions