[PATCH 1 of 3 V2] color: stop mutating the default effects map

Matt Harbison mharbison72 at gmail.com
Mon Mar 27 22:50:26 EDT 2017

# HG changeset patch
# User Matt Harbison <matt_harbison at yahoo.com>
# Date 1490464217 14400
#      Sat Mar 25 13:50:17 2017 -0400
# Node ID 2566b7eac73c4851edc21b73a833f86bf878285e
# Parent  e86eb75e74ce1b0803c26d86a229b9b711f6d76a
color: stop mutating the default effects map

A future change will make color.setup() callable a second time when the pager is
spawned, in order to honor the 'color.pagermode' setting.  The problem was that
when 'color.mode=auto' was resolved to 'win32' in the first pass, the default
ANSI effects were overwritten, making it impossible to honor 'pagermode=ansi'.
Also, the two separate maps didn't have the same keys.  The symmetric difference
is 'dim' and 'italic' (from ANSI), and 'bold_background' (from win32).  Thus,
the update left entries that didn't belong for the current mode.  This bled
through `hg debugcolor`, where the unsupported ANSI keys were listed in 'win32'

As an added bonus, this now correctly enables color with MSYS `less` for a
command like this, where pager is forced on:

    $ hg log --config color.pagermode=ansi --pager=yes --color=auto

Previously, the output was corrupted.  The raw output, as seen through the ANSI
blind `more.com` was:

    <-[-1;6mchangeset:   34840:3580d1197af9<-[-1m

which MSYS `less -FRX` rendered as:

    1;6mchangeset:   34840:3580d1197af91m

(The two '<-' instances were actually an arrow character that TortoiseHg warned
couldn't be encoded, and notepad++ translated to a single '?'.)

Returning an empty map for 'ui._colormode == None' seems better that defaulting
to '_effects' (since some keys are mode dependent), and is better than None,
which blows up `hg debugcolor --color=never`.

diff --git a/mercurial/color.py b/mercurial/color.py
--- a/mercurial/color.py
+++ b/mercurial/color.py
@@ -238,7 +238,6 @@
         if not w32effects:
             return None
-        _effects.update(w32effects)
     elif realmode == 'ansi':
     elif realmode == 'terminfo':
@@ -271,9 +270,17 @@
                             % (e, status))
             ui._styles[status] = ' '.join(good)
+def _activeeffects(ui):
+    '''Return the effects map for the color mode set on the ui.'''
+    if ui._colormode == 'win32':
+        return w32effects
+    elif ui._colormode != None:
+        return _effects
+    return {}
 def valideffect(ui, effect):
     'Determine if the effect is valid or not.'
-    return ((not ui._terminfoparams and effect in _effects)
+    return ((not ui._terminfoparams and effect in _activeeffects(ui))
              or (effect in ui._terminfoparams
                  or effect[:-11] in ui._terminfoparams))
@@ -324,9 +331,10 @@
                         for effect in ['none'] + effects.split())
         stop = _effect_str(ui, 'none')
-        start = [str(_effects[e]) for e in ['none'] + effects.split()]
+        activeeffects = _activeeffects(ui)
+        start = [str(activeeffects[e]) for e in ['none'] + effects.split()]
         start = '\033[' + ';'.join(start) + 'm'
-        stop = '\033[' + str(_effects['none']) + 'm'
+        stop = '\033[' + str(activeeffects['none']) + 'm'
     return _mergeeffects(text, start, stop)
 _ansieffectre = re.compile(br'\x1b\[[0-9;]*m')
diff --git a/mercurial/debugcommands.py b/mercurial/debugcommands.py
--- a/mercurial/debugcommands.py
+++ b/mercurial/debugcommands.py
@@ -360,7 +360,7 @@
 def _debugdisplaycolor(ui):
     ui = ui.copy()
-    for effect in color._effects.keys():
+    for effect in color._activeeffects(ui).keys():
         ui._styles[effect] = effect
     if ui._terminfoparams:
         for k, v in ui.configitems('color'):

More information about the Mercurial-devel mailing list