Patchwork [4,of,7] color: move the dict with terminfo parameters on the ui object

login
register
mail settings
Submitter Pierre-Yves David
Date Feb. 27, 2017, 3 p.m.
Message ID <bdebc6e9429942e8dec6.1488207600@nodosa.octopoid.net>
Download mbox | patch
Permalink /patch/18803/
State Superseded
Headers show

Comments

Pierre-Yves David - Feb. 27, 2017, 3 p.m.
# HG changeset patch
# User Pierre-Yves David <pierre-yves.david@ens-lyon.org>
# Date 1488031251 -3600
#      Sat Feb 25 15:00:51 2017 +0100
# Node ID bdebc6e9429942e8dec6b9b6fe38be8c11bb06d9
# Parent  c6d19f0ac5ae80f933602fe217ff12ace828d02f
# EXP-Topic color
color: move the dict with terminfo parameters on the ui object

This dictionnary is affected by the content of the config, so we should have
one for each ui config.

We rename the global dict to '_baseterminfoparams' to make the situation
clearer.
via Mercurial-devel - Feb. 28, 2017, 7:16 a.m.
On Mon, Feb 27, 2017 at 7:00 AM, Pierre-Yves David
<pierre-yves.david@ens-lyon.org> wrote:
> # HG changeset patch
> # User Pierre-Yves David <pierre-yves.david@ens-lyon.org>
> # Date 1488031251 -3600
> #      Sat Feb 25 15:00:51 2017 +0100
> # Node ID bdebc6e9429942e8dec6b9b6fe38be8c11bb06d9
> # Parent  c6d19f0ac5ae80f933602fe217ff12ace828d02f
> # EXP-Topic color
> color: move the dict with terminfo parameters on the ui object
>
> This dictionnary is affected by the content of the config, so we should have
> one for each ui config.
>
> We rename the global dict to '_baseterminfoparams' to make the situation
> clearer.
>
> diff --git a/hgext/color.py b/hgext/color.py
> --- a/hgext/color.py
> +++ b/hgext/color.py
> @@ -212,7 +212,7 @@ def _debugdisplaycolor(ui):
>          color._styles.clear()
>          for effect in color._effects.keys():
>              color._styles[effect] = effect
> -        if color._terminfo_params:
> +        if ui._terminfoparams:
>              for k, v in ui.configitems('color'):
>                  if k.startswith('color.'):
>                      color._styles[k] = k[6:]
> diff --git a/mercurial/color.py b/mercurial/color.py
> --- a/mercurial/color.py
> +++ b/mercurial/color.py
> @@ -19,7 +19,7 @@ try:
>      import curses
>      # Mapping from effect name to terminfo attribute name (or raw code) or
>      # color number.  This will also force-load the curses module.
> -    _terminfo_params = {'none': (True, 'sgr0', ''),
> +    _baseterminfoparams = {'none': (True, 'sgr0', ''),

You forgot to re-indent the following lines :-) (This is why I prefer
putting the braces on separate lines as I mentioned in an earlier
email.)

>                          'standout': (True, 'smso', ''),
>                          'underline': (True, 'smul', ''),
>                          'reverse': (True, 'rev', ''),
> @@ -39,7 +39,7 @@ try:
>                          'white': (False, curses.COLOR_WHITE, '')}
>  except ImportError:
>      curses = None
> -    _terminfo_params = {}
> +    _baseterminfoparams = {}
>
>  # allow the extensions to change the default
>  _enabledbydefault = False
> @@ -134,35 +134,36 @@ def _terminfosetup(ui, mode):
>      # Otherwise, see what the config file says.
>      if mode not in ('auto', 'terminfo'):
>          return
> +    ui._terminfoparams.update(_baseterminfoparams)
>
>      for key, val in ui.configitems('color'):
>          if key.startswith('color.'):
>              newval = (False, int(val), '')
> -            _terminfo_params[key[6:]] = newval
> +            ui._terminfoparams[key[6:]] = newval
>          elif key.startswith('terminfo.'):
>              newval = (True, '', val.replace('\\E', '\x1b'))
> -            _terminfo_params[key[9:]] = newval
> +            ui._terminfoparams[key[9:]] = newval
>      try:
>          curses.setupterm()
>      except curses.error as e:
> -        _terminfo_params.clear()
> +        ui._terminfoparams.clear()
>          return
>
> -    for key, (b, e, c) in _terminfo_params.items():
> +    for key, (b, e, c) in ui._terminfoparams.items():
>          if not b:
>              continue
>          if not c and not curses.tigetstr(e):
>              # Most terminals don't support dim, invis, etc, so don't be
>              # noisy and use ui.debug().
>              ui.debug("no terminfo entry for %s\n" % e)
> -            del _terminfo_params[key]
> +            del ui._terminfoparams[key]
>      if not curses.tigetstr('setaf') or not curses.tigetstr('setab'):
>          # Only warn about missing terminfo entries if we explicitly asked for
>          # terminfo mode.
>          if mode == "terminfo":
>              ui.warn(_("no terminfo entry for setab/setaf: reverting to "
>                "ECMA-48 color\n"))
> -        _terminfo_params.clear()
> +        ui._terminfoparams.clear()
>
>  def setup(ui):
>      """configure color on a ui
> @@ -226,16 +227,16 @@ def _modesetup(ui):
>              ui.warn(_('warning: failed to set color mode to %s\n') % mode)
>
>      if realmode == 'win32':
> -        _terminfo_params.clear()
> +        ui._terminfoparams.clear()
>          if not w32effects:
>              modewarn()
>              return None
>          _effects.update(w32effects)
>      elif realmode == 'ansi':
> -        _terminfo_params.clear()
> +        ui._terminfoparams.clear()
>      elif realmode == 'terminfo':
>          _terminfosetup(ui, mode)
> -        if not _terminfo_params:
> +        if not ui._terminfoparams:
>              ## FIXME Shouldn't we return None in this case too?
>              modewarn()
>              realmode = 'ansi'
> @@ -264,9 +265,9 @@ def configstyles(ui):
>
>  def valideffect(ui, effect):
>      'Determine if the effect is valid or not.'
> -    return ((not _terminfo_params and effect in _effects)
> -             or (effect in _terminfo_params
> -                 or effect[:-11] in _terminfo_params))
> +    return ((not ui._terminfoparams and effect in _effects)
> +             or (effect in ui._terminfoparams
> +                 or effect[:-11] in ui._terminfoparams))
>
>  def _effect_str(ui, effect):
>      '''Helper function for render_effects().'''
> @@ -276,7 +277,7 @@ def _effect_str(ui, effect):
>          bg = True
>          effect = effect[:-11]
>      try:
> -        attr, val, termcode = _terminfo_params[effect]
> +        attr, val, termcode = ui._terminfoparams[effect]
>      except KeyError:
>          return ''
>      if attr:
> @@ -293,7 +294,7 @@ def _render_effects(ui, text, effects):
>      'Wrap text in commands to turn on each effect.'
>      if not text:
>          return text
> -    if _terminfo_params:
> +    if ui._terminfoparams:
>          start = ''.join(_effect_str(ui, effect)
>                          for effect in ['none'] + effects.split())
>          stop = _effect_str(ui, 'none')
> diff --git a/mercurial/ui.py b/mercurial/ui.py
> --- a/mercurial/ui.py
> +++ b/mercurial/ui.py
> @@ -157,6 +157,7 @@ class ui(object):
>          self.logblockedtimes = False
>          # color mode: see mercurial/color.py for possible value
>          self._colormode = None
> +        self._terminfoparams = {}
>
>          if src:
>              self.fout = src.fout
> @@ -174,6 +175,7 @@ class ui(object):
>              self.callhooks = src.callhooks
>              self.insecureconnections = src.insecureconnections
>              self._colormode = src._colormode
> +            self._terminfoparams = src._terminfoparams.copy()
>
>              self.fixconfig()
>
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Sean Farley - March 9, 2017, 4:35 a.m.
Martin von Zweigbergk via Mercurial-devel
<mercurial-devel@mercurial-scm.org> writes:

> On Mon, Feb 27, 2017 at 7:00 AM, Pierre-Yves David
> <pierre-yves.david@ens-lyon.org> wrote:
>> # HG changeset patch
>> # User Pierre-Yves David <pierre-yves.david@ens-lyon.org>
>> # Date 1488031251 -3600
>> #      Sat Feb 25 15:00:51 2017 +0100
>> # Node ID bdebc6e9429942e8dec6b9b6fe38be8c11bb06d9
>> # Parent  c6d19f0ac5ae80f933602fe217ff12ace828d02f
>> # EXP-Topic color
>> color: move the dict with terminfo parameters on the ui object
>>
>> This dictionnary is affected by the content of the config, so we should have
>> one for each ui config.
>>
>> We rename the global dict to '_baseterminfoparams' to make the situation
>> clearer.
>>
>> diff --git a/hgext/color.py b/hgext/color.py
>> --- a/hgext/color.py
>> +++ b/hgext/color.py
>> @@ -212,7 +212,7 @@ def _debugdisplaycolor(ui):
>>          color._styles.clear()
>>          for effect in color._effects.keys():
>>              color._styles[effect] = effect
>> -        if color._terminfo_params:
>> +        if ui._terminfoparams:
>>              for k, v in ui.configitems('color'):
>>                  if k.startswith('color.'):
>>                      color._styles[k] = k[6:]
>> diff --git a/mercurial/color.py b/mercurial/color.py
>> --- a/mercurial/color.py
>> +++ b/mercurial/color.py
>> @@ -19,7 +19,7 @@ try:
>>      import curses
>>      # Mapping from effect name to terminfo attribute name (or raw code) or
>>      # color number.  This will also force-load the curses module.
>> -    _terminfo_params = {'none': (True, 'sgr0', ''),
>> +    _baseterminfoparams = {'none': (True, 'sgr0', ''),
>
> You forgot to re-indent the following lines :-) (This is why I prefer
> putting the braces on separate lines as I mentioned in an earlier
> email.)

Yeah, I _strongly_ prefer braces on separate lines as Martin does.

Patch

diff --git a/hgext/color.py b/hgext/color.py
--- a/hgext/color.py
+++ b/hgext/color.py
@@ -212,7 +212,7 @@  def _debugdisplaycolor(ui):
         color._styles.clear()
         for effect in color._effects.keys():
             color._styles[effect] = effect
-        if color._terminfo_params:
+        if ui._terminfoparams:
             for k, v in ui.configitems('color'):
                 if k.startswith('color.'):
                     color._styles[k] = k[6:]
diff --git a/mercurial/color.py b/mercurial/color.py
--- a/mercurial/color.py
+++ b/mercurial/color.py
@@ -19,7 +19,7 @@  try:
     import curses
     # Mapping from effect name to terminfo attribute name (or raw code) or
     # color number.  This will also force-load the curses module.
-    _terminfo_params = {'none': (True, 'sgr0', ''),
+    _baseterminfoparams = {'none': (True, 'sgr0', ''),
                         'standout': (True, 'smso', ''),
                         'underline': (True, 'smul', ''),
                         'reverse': (True, 'rev', ''),
@@ -39,7 +39,7 @@  try:
                         'white': (False, curses.COLOR_WHITE, '')}
 except ImportError:
     curses = None
-    _terminfo_params = {}
+    _baseterminfoparams = {}
 
 # allow the extensions to change the default
 _enabledbydefault = False
@@ -134,35 +134,36 @@  def _terminfosetup(ui, mode):
     # Otherwise, see what the config file says.
     if mode not in ('auto', 'terminfo'):
         return
+    ui._terminfoparams.update(_baseterminfoparams)
 
     for key, val in ui.configitems('color'):
         if key.startswith('color.'):
             newval = (False, int(val), '')
-            _terminfo_params[key[6:]] = newval
+            ui._terminfoparams[key[6:]] = newval
         elif key.startswith('terminfo.'):
             newval = (True, '', val.replace('\\E', '\x1b'))
-            _terminfo_params[key[9:]] = newval
+            ui._terminfoparams[key[9:]] = newval
     try:
         curses.setupterm()
     except curses.error as e:
-        _terminfo_params.clear()
+        ui._terminfoparams.clear()
         return
 
-    for key, (b, e, c) in _terminfo_params.items():
+    for key, (b, e, c) in ui._terminfoparams.items():
         if not b:
             continue
         if not c and not curses.tigetstr(e):
             # Most terminals don't support dim, invis, etc, so don't be
             # noisy and use ui.debug().
             ui.debug("no terminfo entry for %s\n" % e)
-            del _terminfo_params[key]
+            del ui._terminfoparams[key]
     if not curses.tigetstr('setaf') or not curses.tigetstr('setab'):
         # Only warn about missing terminfo entries if we explicitly asked for
         # terminfo mode.
         if mode == "terminfo":
             ui.warn(_("no terminfo entry for setab/setaf: reverting to "
               "ECMA-48 color\n"))
-        _terminfo_params.clear()
+        ui._terminfoparams.clear()
 
 def setup(ui):
     """configure color on a ui
@@ -226,16 +227,16 @@  def _modesetup(ui):
             ui.warn(_('warning: failed to set color mode to %s\n') % mode)
 
     if realmode == 'win32':
-        _terminfo_params.clear()
+        ui._terminfoparams.clear()
         if not w32effects:
             modewarn()
             return None
         _effects.update(w32effects)
     elif realmode == 'ansi':
-        _terminfo_params.clear()
+        ui._terminfoparams.clear()
     elif realmode == 'terminfo':
         _terminfosetup(ui, mode)
-        if not _terminfo_params:
+        if not ui._terminfoparams:
             ## FIXME Shouldn't we return None in this case too?
             modewarn()
             realmode = 'ansi'
@@ -264,9 +265,9 @@  def configstyles(ui):
 
 def valideffect(ui, effect):
     'Determine if the effect is valid or not.'
-    return ((not _terminfo_params and effect in _effects)
-             or (effect in _terminfo_params
-                 or effect[:-11] in _terminfo_params))
+    return ((not ui._terminfoparams and effect in _effects)
+             or (effect in ui._terminfoparams
+                 or effect[:-11] in ui._terminfoparams))
 
 def _effect_str(ui, effect):
     '''Helper function for render_effects().'''
@@ -276,7 +277,7 @@  def _effect_str(ui, effect):
         bg = True
         effect = effect[:-11]
     try:
-        attr, val, termcode = _terminfo_params[effect]
+        attr, val, termcode = ui._terminfoparams[effect]
     except KeyError:
         return ''
     if attr:
@@ -293,7 +294,7 @@  def _render_effects(ui, text, effects):
     'Wrap text in commands to turn on each effect.'
     if not text:
         return text
-    if _terminfo_params:
+    if ui._terminfoparams:
         start = ''.join(_effect_str(ui, effect)
                         for effect in ['none'] + effects.split())
         stop = _effect_str(ui, 'none')
diff --git a/mercurial/ui.py b/mercurial/ui.py
--- a/mercurial/ui.py
+++ b/mercurial/ui.py
@@ -157,6 +157,7 @@  class ui(object):
         self.logblockedtimes = False
         # color mode: see mercurial/color.py for possible value
         self._colormode = None
+        self._terminfoparams = {}
 
         if src:
             self.fout = src.fout
@@ -174,6 +175,7 @@  class ui(object):
             self.callhooks = src.callhooks
             self.insecureconnections = src.insecureconnections
             self._colormode = src._colormode
+            self._terminfoparams = src._terminfoparams.copy()
 
             self.fixconfig()