Provide pass-through ANSI on Windows 10 powershell #133
Conversation
|
Pictures! CMD (so only 16 color mode, but that works):
Powershell (full 256 color glory!):
|
|
@MinchinWeb Are you sure this only works in powershell? I think you simply need to enable
0x4 is Reference: https://msdn.microsoft.com/en-us/library/windows/desktop/ms686033(v=vs.85).aspx |
| winreg.CloseKey(key) | ||
|
|
||
| # Windows 10 supports ANSI cmd since release 1511 | ||
| if releaseId >= 1511: |
There was a problem hiding this comment.
You probably need to check that this is Windows 10 too, I'm not sure ReleaseID isn't used in previous versions of Windows. l Windows since 8.1 has a nasty habit of lying about it's version though. I think platform.win32_ver() does work around that but be sure to check (sys.getwindowsversion doesn't workaround this at least on Python 2.7.13 so you get the version of Windows 8).
| # Windows 10 supports ANSI cmd since release 1511 | ||
| if releaseId >= 1511: | ||
| # but only on powershell | ||
| import psutil |
There was a problem hiding this comment.
I think this isn't really related to powershell but to the ENABLE_VIRTUAL_TERMINAL_PROCESSING flag. colorama should enable this flag when supported and restore it's previous value on exit.
I think we can simply try to SetConsoleMode(h, GetConsoleMode(h) | ENABLE_VIRTUAL_TERMINAL_PROCESSING) to check for support of this (Need to be sure the stream is a tty/console before calling {Get,Set}ConsoleMode). I think it fails with INVALID_ARGUMENT when unsupported. But this needs some testing.
See: #133 (comment) & https://msdn.microsoft.com/en-us/library/windows/desktop/ms686033(v=vs.85).aspx
| @@ -64,6 +64,9 @@ def get_version(path): | |||
| 'Programming Language :: Python :: 3.4', | |||
| 'Programming Language :: Python :: 3.5', | |||
| 'Topic :: Terminals', | |||
| ] | |||
| ], | |||
| extras_require={ | |||
There was a problem hiding this comment.
Indentation is messed up.
| # 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)) | ||
| if windows_ansi_support: | ||
| strip = False |
There was a problem hiding this comment.
You still need to check that the stream isn't closed and is a tty otherwise we will skip stripping ansi codes with redirected output. Be careful, all those boolean conditions are quite confusing.
| # Check the registry for release ID | ||
| key = r"SOFTWARE\Microsoft\Windows NT\CurrentVersion" | ||
| val = r"ReleaseID" | ||
| key = winreg.OpenKey(winreg.HKEY_LOCAL_MACHINE, key) |
There was a problem hiding this comment.
Use try finally (It is a context manager since 2.6 but I think colorama still declares compatibility with Python 2.5).
|
Hey. FYI, yesterday I created a PR to test releases before we push them to PyPI. When that is merged, I'll be more confident about resuming merges and releases. I'll try to look at this PR soon. Thank you for creating it! |
|
This seems like it fixes a genuine problem, and if this is the best way to detect powershell then I'd like to merge, but I am hazy on whether this change is still needed if we merge #139. I'll experiment with them both later this week. If anyone has any insight (I do not know what I'm doing) I'd be grateful. Apologies for ignoring this (and all other PRs) for years. |
|
Also: We can't merge this until someone provides corresponding changes to the tests. |
|
I think that if #139 is robust, that would be preferred just because it doesn't add another dependency to the project. But I'd be happy to see either merged! |
|
#139 uses a more direct/reliable strategy to handle the ANSI stuff, so closing this in favor of it |
This provides pass-thru to ANSI codes on new enough Windows 10 versions, and to Powershell only. It does this by using
psutilto check the process name that is running python, and if that matchespowershell.exe, ANSI pass-thru is enabled.This does add
psutilas a dependency on Windows.Closes #105
Basic local testing on both Python 2.7 and Python 3.6