Enable ANSI support on Windows #186

[jaredtrog]

This commit enables support for correct ANSI output on Windows when the colorama library is installed. If the colorama library is not installed then ANSI characters in output are disabled with the ANSI_COLORS_DISABLED environment variable.

Additionally, native ANSI sequences are enabled on the Windows 10 platform when support is detected.

Tested on the following platforms:

• Windows Server 2012R2
• Windows 10
• Windows Server 2016
• Windows Server 2019

With the following consoles:

• conhost.exe (cmd.exe)
• powershell.exe
• openconsole.exe Windows Terminal

Fixes #186

[dbieber]

This looks good.

If we're on windows, we try to import colorama; if we cannot, we'll just disable coloring at line 61.
If we can import colorama, the logic from L37-L58 is entirely about determining SHOULD_WRAP.
I can't say I fully understand the criteria for setting SHOULD_WRAP, but you've tested it thoroughly so sgtm.

Do we actually want that print statement at 55?
It seems like even in that case (and I don't fully understand what that case is), Fire will continue operating normally.

[jaredtrog]

I agree, the flow here is a bit opaque. Here is a bit more detail around SHOULD_WRAP.

The SHOULD_WRAP decision refers to a boolean kwarg in colorama.init called wrap. By default wrap is True and it:

'''
Implements a 'write()' method which, on Windows, will strip ANSI character
sequences from the text, and if outputting to a tty, will convert them into
win32 function calls.
'''

We get clean output with the win32 function calls that includes bold and other formatting attributes but not pretty output, like underlines, since those characters are apparently not supported in the win32 function calls.

The goal was to prefer native ANSI where supported to get the most consistent output but fall back to win32 function calls if native ANSI is not supported. That is the purpose of the SHOULD_WRAP decision.

On Windows 10 platforms (including Server 2016 and 2019), ANSI support can be enabled in the console. L39 - L58 is an adaptation from the function enable_native_ansi in setup_color.py that attempts to do so. If enabling native ansi support fails, it falls back to win32 calls by setting SHOULD_WRAP = True. All other versions of Windows will use the win32 calls for clean, but not pretty, output.

Arriving at the print statements means that something has gone quite wrong when attempting to enable native ansi support in a console that should support it.

I agree that it isn't necessarily Fire's job to ensure consoles are configured correctly so it may be better to have this portion fail silently by disabling ANSI?

[dbieber]

Thanks for the explanation. 🙏

so it may be better to have this portion fail silently by disabling ANSI?

Yeah, I think so. No need to update the PR though. We'll handle that as part of merging it.

[dbieber]

Merged!
🙏