tartley / colorama

Simple cross-platform colored terminal text in Python
BSD 3-Clause "New" or "Revised" License
3.55k stars 252 forks source link

Windows: Style.RESET_ALL does not work if stdout is redirected. #200

Open ArkBrj opened 5 years ago

ArkBrj commented 5 years ago

The following code demonstrates the problem with stdout redirection on Windows. Note that coloring is used on stderr only. Try to run this code with and without stdout redirection and note the difference: python foo.py python foo.py >nul

In the first case the first line will be printed in green and the second - using the default cmd color, which is the expected behavior. In the second case both lines will be printed in green. This issue pretty much disallows using colors in logging because normally log messages go to stderr while stdout is used for program output, which sometimes may be redirected to a file.

import sys
import colorama

colorama.init()

print(colorama.Fore.GREEN + 'Should be in green color' + colorama.Style.RESET_ALL, file = sys.stderr)
print('Should be in default color', file = sys.stderr)
cjerdonek commented 5 years ago

Maybe this is in part caused by the fact that colorama's reset_all() which it calls on exit only resets orig_stdout and not orig_stderr, which is where the color is happening: https://github.com/tartley/colorama/blob/d7a538212c3b8d00dc0051a45f99dc9b201dd3b2/colorama/initialise.py#L18-L20

wiggin15 commented 5 years ago

@cjerdonek I don't think this is it (although this could be a problem too, maybe). The code you mentioned is only called at program exit.

This problem is caused by WinTerm not passing on_stderr to set_console in its reset_all method (different file): https://github.com/tartley/colorama/blob/d7a538212c3b8d00dc0051a45f99dc9b201dd3b2/colorama/winterm.py#L44-L46 Unfortunately the fix is not trivial, because there is an additional problem. The "_default" attribute (which we reset to) is taken from stdout explicitly: https://github.com/tartley/colorama/blob/d7a538212c3b8d00dc0051a45f99dc9b201dd3b2/colorama/winterm.py#L24-L25 when stdout redirection is on, the attributes are 0. Resetting to this value will cause stderr to become black. The problem here is that we use a single instance of WinTerm that holds the color and default attributes for both stdout and stderr, without correctly separating the two.

cjerdonek commented 5 years ago

Thanks, @wiggin15.

The code you mentioned is only called at program exit.

Yes, this is why I hedged and said "in part caused by the fact..." My thinking was that if the "during program" fix would take a lot more work (which it sounds like it might), maybe there's a simpler fix that can at least restore things to the initial state on exit -- even if the state during the execution isn't fixed. That way people's terminal at least won't be corrupted "permanently," which is a more serious problem IMO than just the color being off during the running of the program.

cjerdonek commented 5 years ago

Note: when I said "permanently," I had in mind this issue-- https://github.com/pypa/pip/issues/6354 rather than the issue as reported in the original comment of the current issue.

wiggin15 commented 5 years ago

I pushed a fix to my personal forked repository that separates the WinTerm instances - https://github.com/wiggin15/colorama/commit/eccb87c747d8ac712aeab671ad7e1dba0f83b1bf This should fix the original issue. Unfortunately I don't have time to properly test this so I'm holding off merging this.

This change also causes a strange side-effect. Consider the following code:

print(colorama.Fore.GREEN + 'stderr should be with green foreground and regular background', file=sys.stderr)
print(colorama.Back.GREEN + 'stdout should be with green background and regular foreground')

In this case we don't use "reset" but print to two separate streams (stderr and stdout). My fix will separate the attributes of the two streams so each will get the correct colors - but from what I can see, Unix terminals will not separate the colors, and stdout will get the green foreground from stderr... I didn't expect this and we may need to look into this a bit. Maybe the streams shouldn't be separated like this.

@cjerdonek Thanks for reporting the problem with resetting stderr on program exit. I think it would be a good idea to open a separate issue for that.

cjerdonek commented 5 years ago

I didn't expect this and we may need to look into this a bit. Maybe the streams shouldn't be separated like this.

My instinct is that people should be invoking colorama in a way that doesn't depend on stdout and stderr going to the same (or a different) place. The reason is that, for example, if someone redirects stdout or stderr to a different location, I don't think the program has any way to tell.

In particular, if someone wants to reset stdout and stderr, it won't necessarily suffice only to reset stdout. A proper usage should reset both.

Thanks for reporting the problem with resetting stderr on program exit. I think it would be a good idea to open a separate issue for that.

I created issue #218 for this.

SeaHOH commented 5 years ago

https://github.com/SeaHOH/colorama/commit/966474a38fe91f1a78605f018167a24bfbfbec80 How about this patch?

It works fine, except redirect to nul.

abarger-bss commented 4 years ago

Coming to this issue from the Azure CLI. My team runs into this issue daily - seems like a lot of productivity lost for console colors. Is there a mitigation for this issue? I would be happy if I could disable colorama via environment variable or some function call at the start of my terminal session. Since az cli owns the dependency I'm not sure how much control I can take.

jhhcs commented 2 years ago

Is there any hope of having this fixed in a release in the near future? It has been open for 4 years; it would be helpful to know if it is going to be fixed or not; either is fine, but depending on the answer I would either implement a workaround or switch to a different library.

jhhcs commented 1 year ago

I guess that's a "no".