This is an archive of the discontinued Mercurial Phabricator instance.

Differential D12158

color: don't infer vt status from TERM on Windows
ClosedPublic

Authored by mitchhentgesmozilla on Feb 9 2022, 1:15 PM.

Details

Reviewers
spectral
Alphare
Group Reviewers
hg-reviewers
Commits
rHGfd2cf9e0c64e: color: don't infer vt status from TERM on Windows
Summary

Previously, it was assumed that Windows environments with
"xterm" in the TERM environment variable meant that either
"virtual terminal mode" was already enabled, or that
we are running in an environment that didn't need a "virtual
terminal mode" (such as mintty, that interprets ANSI sequences
itself).

However, modern Cygwin and MSYS2 set TERM=xterm when using the
Command Prompt as the terminal, which needs "virtual terminal
mode" to be manually enabled. However, due to (issue6640),
the vtmode wasn't being enabled.

This patch ensures that we always try to enable vtmode on
Windows regardless of the state of TERM, so that:

  • ANSI-based colors work in modern Cygwin/MSYS2 (with Command Prompt), and
  • The vtmode is unnecessarily set when running in a different terminal such as mintty, but it is simply redundant and doesn't appear to have ill effects.

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

spectral accepted this revision.Feb 9 2022, 2:55 PM
spectral added a subscriber: spectral.

This really feels like a bug that they're advertising support for xterm-compatible escape codes, and then not setting everything up to handle it properly, but for all I know it's not possible for them to do so (i.e. maybe Windows requires that each Process enable vtmode for every Console it cares about, it can't be done "for us" by these tools)

mitchhentgesmozilla added a comment.Feb 9 2022, 3:54 PM
In D12158#186352, @spectral wrote:

This really feels like a bug that they're advertising support for xterm-compatible escape codes, and then not setting everything up to handle it properly, but for all I know it's not possible for them to do so (i.e. maybe Windows requires that each Process enable vtmode for every Console it cares about, it can't be done "for us" by these tools)

Yeah, it looks like Cygwin sets the term to xterm-256color based on whether the console supports it, not based on whether it's currently active.
I suppose that this makes sense considering that it's possible for children processes in the same terminal to SetConsoleMode(...) and enable/disable virtual terminal support without changing the $TERM.

Going into more theoretical territory, my team is currently investigating an issue where it seems like each cygwin binary that runs is implicitly unsetting virtual terminal support, which could be related. However, even if that's changed, due to the malleable nature of "virtual terminal mode", I still think that aggresively enabling it in HG-land is a good idea :)

Alphare accepted this revision.Feb 14 2022, 5:07 AM
This revision is now accepted and ready to land.Feb 14 2022, 5:07 AM

Revision Contents
Changeset List

PathPackages
Mmercurial/color.py (13 lines)

Diff 32162

mercurial/color.py

# If pager is active, color.pagermode overrides color.mode. # If pager is active, color.pagermode overrides color.mode.
if getattr(ui, 'pageractive', False): if getattr(ui, 'pageractive', False):
mode = ui.config(b'color', b'pagermode', mode) mode = ui.config(b'color', b'pagermode', mode)
realmode = mode realmode = mode
if pycompat.iswindows: if pycompat.iswindows:
from . import win32 from . import win32
term = encoding.environ.get(b'TERM')
# TERM won't be defined in a vanilla cmd.exe environment.
# UNIX-like environments on Windows such as Cygwin and MSYS will
# set TERM. They appear to make a best effort attempt at setting it
# to something appropriate. However, not all environments with TERM
# defined support ANSI.
ansienviron = term and b'xterm' in term
if mode == b'auto': if mode == b'auto':
# Since "ansi" could result in terminal gibberish, we error on the # Since "ansi" could result in terminal gibberish, we error on the
# side of selecting "win32". However, if w32effects is not defined, # side of selecting "win32". However, if w32effects is not defined,
# we almost certainly don't support "win32", so don't even try. # we almost certainly don't support "win32", so don't even try.
# w32effects is not populated when stdout is redirected, so checking # w32effects is not populated when stdout is redirected, so checking
# it first avoids win32 calls in a state known to error out. # it first avoids win32 calls in a state known to error out.
if ansienviron or not w32effects or win32.enablevtmode(): if not w32effects or win32.enablevtmode():
realmode = b'ansi' realmode = b'ansi'
else: else:
realmode = b'win32' realmode = b'win32'
# An empty w32effects is a clue that stdout is redirected, and thus # An empty w32effects is a clue that stdout is redirected, and thus
# cannot enable VT mode. # cannot enable VT mode.
elif mode == b'ansi' and w32effects and not ansienviron: elif mode == b'ansi' and w32effects:
win32.enablevtmode() win32.enablevtmode()
elif mode == b'auto': elif mode == b'auto':
realmode = b'ansi' realmode = b'ansi'
def modewarn(): def modewarn():
# only warn if color.mode was explicitly set and we're in # only warn if color.mode was explicitly set and we're in
# a formatted terminal # a formatted terminal
if mode == realmode and formatted: if mode == realmode and formatted: