[PATCH 1 of 3] windows: add a method to enable ANSI color code processing on Windows 10

[Matt Harbison]

On Tue, 23 May 2017 10:35:24 -0400, FUJIWARA Katsunori  
<foozy at lares.dti.ne.jp> wrote:

>
> At Tue, 23 May 2017 08:34:13 -0400,
> Matt Harbison wrote:
>>
>> # HG changeset patch
>> # User Matt Harbison <matt_harbison at yahoo.com>
>> # Date 1495504856 14400
>> #      Mon May 22 22:00:56 2017 -0400
>> # Node ID 58dbbf67bd726092374403b2805d9ef9f5118ebe
>> # Parent  8db2feb04cebc1878c6232dd2650f2c5468d350e
>> windows: add a method to enable ANSI color code processing on Windows 10
>>
>> SetConsoleMode() fails with an invalid parameter error if given this  
>> option
>> prior to Windows 10, so indicate that to the caller instead of doing  
>> explicit
>> version checks.
>>
>> +def enablevtmode():
>> +    '''Enable virtual terminal mode for the associated console.   
>> Return True if
>> +    enabled, else False.'''
>> +
>> +    ENABLE_VIRTUAL_TERMINAL_PROCESSING = 0x4
>> +
>> +    # Query stderr, like termsize().  Either stdout or stderr will  
>> work.
>> +    handle = _kernel32.GetStdHandle(_STD_ERROR_HANDLE) # don't close  
>> the handle
>
> Current color.py implementation uses only STD_OUTPUT_HANDLE in order
> to initialize for colorization, and redirection of stdout disables
> colorization itself.

By disable color, do you mean color.setup(), but then not use?  I noticed  
that GetConsoleMode() would complain on some systems about an invalid  
handle when output is redirected to a file.  I noticed today that the  
reason for "some systems" is that it happens when STD_OUTPUT_HANDLE is  
used here, which was how I originally coded it.  It's not a problem in  
practice (we just return False), but seems like more code than necessary  
is running when we don't want to color (though that's unrelated to this  
series).

I was unable to force ANSI gibberish to a file, so the visible behavior is  
correct.

> Therefore, GetConsoleMode() on STD_OUTPUT_HANDLE seems consistent with
> color.py, at least now, even though win32.termsize() uses
> STD_ERROR_HANDLE for safety at redirection of stdout.

Seems reasonable.

> BTW, I'm planning to examine both stdout/stderr for console
> information on Windows (termsize, colorization, and so on) for strict
> detection, because only stderr might be redirected at runtime. Of
> course, it should be very rare case, though :-)
>
>
>> +    if handle == _INVALID_HANDLE_VALUE:
>> +        return False
>> +
>> +    mode = _DWORD(0)
>> +
>> +    if not _kernel32.GetConsoleMode(handle, ctypes.byref(mode)):
>> +        return False
>> +
>> +    if (mode.value & ENABLE_VIRTUAL_TERMINAL_PROCESSING) == 0:
>> +        mode.value |= ENABLE_VIRTUAL_TERMINAL_PROCESSING
>> +
>> +        if not _kernel32.SetConsoleMode(handle, mode):
>> +            return False
>> +
>> +    return True
>> +
>
> Is there any reason to place this implementation in win32.py, and to
> use it via util.py proxy ?
>
> color.py already has win32 specific code path, and it seems better to
> place this implementation in color.py, IMHO. This can centralize
> colorization specific logic into color.py. Introducing empty
> enablevtmode() into posix.py seems redundant, too.

Good call.  I didn't like the empty proxying either, and I forgot that  
color already imported ctypes.

>
> BTW, enabling VTP is needed only once. Therefore, if this
> implementation is placed in color.py, its result can be cached to
> reuse :-)

This might get tricky.  We could of course just blindly call it when the  
module loads, and store the
result to figure out how to resolve 'auto'.  But if MS backed off the  
automatic enable of it, should we be automatically enabling it?  The way  
this code is written, we won't enable it if 'xterm' is in $TERM, because  
apparently those tools handled ANSI already.  Not sure if there's any harm  
enabling VTP on them.

In theory, _modesetup() should only get called once (or maybe twice to  
pick up 'color.pagermode'), but it looks like it is getting called in  
groups of 3 when I added prints in there.

>
> Implementation itself LGTM!
>
>>  def _1stchild(pid):
>>      '''return the 1st found child of the given pid
>>
>> diff --git a/mercurial/windows.py b/mercurial/windows.py
>> --- a/mercurial/windows.py
>> +++ b/mercurial/windows.py
>> @@ -30,6 +30,7 @@
>>
>>  osutil = policy.importmod(r'osutil')
>>
>> +enablevtmode = win32.enablevtmode
>>  executablepath = win32.executablepath
>>  getuser = win32.getuser
>>  hidewindow = win32.hidewindow
>> _______________________________________________
>> Mercurial-devel mailing list
>> Mercurial-devel at mercurial-scm.org
>> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel