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

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

Open
ArkBrj opened this issue Oct 22, 2018 · 10 comments
Open

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

ArkBrj opened this issue Oct 22, 2018 · 10 comments

Comments

@ArkBrj
Copy link

ArkBrj commented Oct 22, 2018

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
Copy link

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:

def reset_all():
if AnsiToWin32 is not None: # Issue #74: objects might become None at exit
AnsiToWin32(orig_stdout).reset_all()

@wiggin15
Copy link
Collaborator

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

def reset_all(self, on_stderr=None):
self.set_attrs(self._default)
self.set_console(attrs=self._default)

Unfortunately the fix is not trivial, because there is an additional problem. The "_default" attribute (which we reset to) is taken from stdout explicitly:
def __init__(self):
self._default = win32.GetConsoleScreenBufferInfo(win32.STDOUT).wAttributes

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
Copy link

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.

wiggin15 pushed a commit to wiggin15/colorama that referenced this issue Mar 25, 2019
@cjerdonek
Copy link

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

@wiggin15
Copy link
Collaborator

I pushed a fix to my personal forked repository that separates the WinTerm instances - wiggin15@eccb87c
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
Copy link

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 added a commit to SeaHOH/colorama that referenced this issue Mar 30, 2019
@SeaHOH
Copy link

SeaHOH commented Mar 30, 2019

SeaHOH@966474a
How about this patch?

It works fine, except redirect to nul.

@abarger-bss
Copy link

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
Copy link

jhhcs commented May 20, 2022

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
Copy link

jhhcs commented May 31, 2023

I guess that's a "no".

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

No branches or pull requests

6 participants