[PATCH 2 of 3 V2] color: enable ANSI support on Windows 10

Matt Harbison mharbison72 at gmail.com
Sun May 28 14:11:15 EDT 2017


> On May 28, 2017, at 1:14 PM, FUJIWARA Katsunori <foozy at lares.dti.ne.jp> wrote:
> 
> At Sat, 27 May 2017 00:19:22 -0400,
> Matt Harbison wrote:
>> 
>> # HG changeset patch
>> # User Matt Harbison <matt_harbison at yahoo.com>
>> # Date 1495506038 14400
>> #      Mon May 22 22:20:38 2017 -0400
>> # Node ID d35feab18347e36cf3e55b6b797602088917de09
>> # Parent  01e230f1d93a0b0e54164fbce14b5a0c093a4fc4
>> color: enable ANSI support on Windows 10
>> 
>> This will display color if "color.mode=ansi", and default to 'ansi' if the mode
>> is set to 'auto'.  The 'debugcolor' command also reflects this policy.
>> Previously, "color.mode=ansi" on Windows printed jibberish around the normal
>> text.  Using ANSI color is better, as it avoids the normal loss of color when
>> the default pager is enabled on Windows.  See also issue5570.
>> 
>> When the underlying function fails (e.g. when run on older Windows), 'auto'
>> still falls back to 'win32'.  Apparently, Microsoft originally had this feature
>> turned on by default, and then made it opt-in[1].  Therefore, not enabling it
>> unconditionally seems safer.  Instead, only do it in the 'auto' case after
>> processing the existing check for support in a Unix-like environment.
>> 
>> [1] https://github.com/symfony/symfony/issues/17499#issuecomment-243481052
>> 
>> diff --git a/mercurial/color.py b/mercurial/color.py
>> --- a/mercurial/color.py
>> +++ b/mercurial/color.py
>> @@ -215,8 +215,10 @@
>>         mode = ui.config('color', 'pagermode', mode)
>> 
>>     realmode = mode
>> -    if mode == 'auto':
>> -        if pycompat.osname == 'nt':
>> +    if pycompat.osname == 'nt':
>> +        from . import win32
>> +
>> +        if mode == 'auto':
>>             term = encoding.environ.get('TERM')
>>             # TERM won't be defined in a vanilla cmd.exe environment.
>> 
>> @@ -227,12 +229,15 @@
>>             # gibberish, we error on the side of selecting "win32". However, if
>>             # w32effects is not defined, we almost certainly don't support
>>             # "win32", so don't even try.
>> -            if (term and 'xterm' in term) or not w32effects:
>> +            if ((term and 'xterm' in term) or win32.enablevtmode()
>> +                 or not w32effects):
> 
> Is there any reason to examine win32.enablevtmode() before w32effects?
> 
> Examining w32effects earlier can avoid meaningless SetConsoleMode(),
> if stdout is redirected.

The way this conditional is here:

1) 'xterm' in term -> the status quo, where we think the environment supports ANSI without help from Windows.  So use ANSI without asking Windows for help, because presumably it worked before
2) win32.enablevtmode() == True -> Windows supports it (and it's now enabled), so go with ANSI
3) not w32effects -> we know win32 doesn't work, so try ANSI (without Windows support) and hope for the best
4) Otherwise, we're pretty sure that ANSI is not supported, but win32 is.  So go with that.

I think ANSI (with Windows support) should take precedence over win32, because that works with the default pager.  That's a way more common case than redirecting stdout, so it seems like this is a micro optimization outweighed by the normal UX.  Do you have a concern with calling SetConsoleMode() extraneously?

> 
>>                 realmode = 'ansi'
>>             else:
>>                 realmode = 'win32'
>> -        else:
>> -            realmode = 'ansi'
>> +        elif mode == 'ansi':
>> +            win32.enablevtmode()
> 
> Similarly, "mode == 'ansi' and w32effects".

In this case, the user explicitly set ANSI, so w32effects doesn't matter.  We do still need to enable support in Windows for cmd.exe.  (And yes, this does contravene the choice above to be conservative about enabling this, but I didn't see a better way, or know if it really matters.  I don't imagine that there are a lot of people setting ANSI on Windows explicitly.)

>> +    elif mode =='auto':
>> +        realmode = 'ansi'
>> 
>>     def modewarn():
>>         # only warn if color.mode was explicitly set and we're in
>> 
> 
> 
> -- 
> ----------------------------------------------------------------------
> [FUJIWARA Katsunori]                             foozy at lares.dti.ne.jp


More information about the Mercurial-devel mailing list