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

#151: Remove colorama init, Only use colorama on windows #152

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

li-wjohnson
Copy link

@li-wjohnson li-wjohnson commented Sep 9, 2020

Description of new feature, or changes

Colorama.init escapes all ansi codes by default on stdout / stderr when not in a tty, regardless of OS, which breaks color output. Instead of globally affecting stdout and stderr at library import time, only wrap the stream that Halo uses.

Also, I turned colorama into a conditional dependency, seeing as its only needed on windows.

Something to note: there's a change in behavior here.

Previously:

Windows Other
stdout isatty() convert ansi
stdout not isatty() strip strip
stream isatty() ansi ansi
stream not isatty() ansi ansi

Now:

Windows Other
stdout isatty() convert ansi
stdout not isatty() strip ansi
stream isatty() convert ansi
stream not isatty() strip ansi

(convert: ansi converted to win32 calls, strip: ansi removed, ansi: ansi unchanged)

You may consider using colorama to strip the output when the stream is not a tty, regardless of OS, but you should likely have a way to override the behavior. (See also: tartley/colorama#230)

Checklist

  • Your branch is up-to-date with the base branch
  • You've included at least one test if this is a new feature
  • All tests are passing

Related Issues and Discussions

#151

People to notify

@lemassykoi
Copy link

Thanks

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.

3 participants