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

Add Win 10 native ANSI cmd support #105

Closed
wants to merge 4 commits into from
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
24 changes: 22 additions & 2 deletions colorama/ansitowin32.py
Expand Up @@ -21,6 +21,23 @@ def is_a_tty(stream):
return hasattr(stream, 'isatty') and stream.isatty()


def check_windows_ansi_support():
try:
import winreg
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a problem here because winreg only exists in Python 3 but we support Python 2 too. Also, this function fails on older versions of Windows because the ReleaseID value doesn't exist in the registry. I am reluctant to pull this function - too much platform specific code has a high chance of breaking things.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I worked with Python 2 when I encountered the error and fixed it with this check function. Though, I agree it contains a lot of platform specific code, but I don't know what else would work.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure why I can't import it anymore. I am sure it worked in Python 2.7.x

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The module is named _winreg on Python 2.7 (notice the leading underscore). https://docs.python.org/2/library/_winreg.html

except ImportError:
return False

# Check the registry for release ID
key = r"SOFTWARE\Microsoft\Windows NT\CurrentVersion"
val = r"ReleaseID"
key = winreg.OpenKey(winreg.HKEY_LOCAL_MACHINE, key)
releaseId = int(winreg.QueryValueEx(key,val)[0])
winreg.CloseKey(key)

# Windows 10 supports ANSI cmd since release 1511
return releaseId >= 1511


class StreamWrapper(object):
'''
Wraps a stream (such as stdout), acting as a transparent proxy for all
Expand Down Expand Up @@ -69,14 +86,17 @@ def __init__(self, wrapped, convert=None, strip=None, autoreset=False):
# to support the ANSI codes.
conversion_supported = on_windows and winapi_test()

# Does this version of Win 10 support ANSI?
windows_ansi_support = on_windows and check_windows_ansi_support()

# should we strip ANSI sequences from our output?
if strip is None:
strip = conversion_supported or (not is_stream_closed(wrapped) and not is_a_tty(wrapped))
strip = conversion_supported or not windows_ansi_support or (not is_stream_closed(wrapped) and not is_a_tty(wrapped))
self.strip = strip

# should we should convert ANSI sequences into win32 calls?
if convert is None:
convert = conversion_supported and not is_stream_closed(wrapped) and is_a_tty(wrapped)
convert = not windows_ansi_support or (conversion_supported and not is_stream_closed(wrapped) and is_a_tty(wrapped))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you sure this should be "or"-ed and not "and"-ed? This way if the old condition with conversion_supported is True, then convert will be True even if windows_ansi_support is True, which (I think) is not the desired outcome. I think that there is the same problem with strip (but I may be mistaken). This is a difficult part.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not really sure. That's why I put the check on the if line at first. I tried several combinations, but couldn't find one that fits for all cases.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should have separate ifs, one for on_windows and one for other platforms.

self.convert = convert

# dict of ansi codes to win32 functions and parameters
Expand Down