Patchwork [3,of,3] color: debugcolor should emit the user-defined colors

login
register
mail settings
Submitter Danek Duvall
Date Oct. 13, 2016, 8:27 p.m.
Message ID <c59334b806a8f10c80c2.1476390455@smelly>
Download mbox | patch
Permalink /patch/17069/
State Accepted
Headers show

Comments

Danek Duvall - Oct. 13, 2016, 8:27 p.m.
# HG changeset patch
# User Danek Duvall <danek.duvall@oracle.com>
# Date 1476389401 25200
#      Thu Oct 13 13:10:01 2016 -0700
# Node ID c59334b806a8f10c80c2ea196eba5f4b1b86b229
# Parent  333e875ce30da3760c8655e303d9bea554efe7e4
color: debugcolor should emit the user-defined colors

This also fixes a long-standing bug that reversed the sense of the color
name and the label used to print it, which was never relevant before.
Yuya Nishihara - Oct. 15, 2016, 2 p.m.
On Thu, 13 Oct 2016 13:27:35 -0700, danek.duvall@oracle.com wrote:
> # HG changeset patch
> # User Danek Duvall <danek.duvall@oracle.com>
> # Date 1476389401 25200
> #      Thu Oct 13 13:10:01 2016 -0700
> # Node ID c59334b806a8f10c80c2ea196eba5f4b1b86b229
> # Parent  333e875ce30da3760c8655e303d9bea554efe7e4
> color: debugcolor should emit the user-defined colors
> 
> This also fixes a long-standing bug that reversed the sense of the color
> name and the label used to print it, which was never relevant before.
> 
> diff --git a/hgext/color.py b/hgext/color.py
> --- a/hgext/color.py
> +++ b/hgext/color.py
> @@ -524,10 +524,16 @@ def debugcolor(ui, repo, **opts):
>      _styles = {}
>      for effect in _effects.keys():
>          _styles[effect] = effect
> +    if _terminfo_params:
> +        for k, v in ui.configitems('color'):
> +            if k.startswith('color.'):
> +                _styles[k] = k[6:]
> +            elif k.startswith('terminfo.'):
> +                _styles[k] = k[9:]
>      ui.write(('color mode: %s\n') % ui._colormode)
>      ui.write(_('available colors:\n'))
> -    for label, colors in _styles.items():
> -        ui.write(('%s\n') % colors, label=label)
> +    for colorname, label in _styles.items():
> +        ui.write(('%s\n') % colorname, label=label)

I was surprised to know that a label can be a valid effect name.
Danek Duvall - Oct. 15, 2016, 10:07 p.m.
Yuya Nishihara wrote:

> On Thu, 13 Oct 2016 13:27:35 -0700, danek.duvall@oracle.com wrote:
> > # HG changeset patch
> > # User Danek Duvall <danek.duvall@oracle.com>
> > # Date 1476389401 25200
> > #      Thu Oct 13 13:10:01 2016 -0700
> > # Node ID c59334b806a8f10c80c2ea196eba5f4b1b86b229
> > # Parent  333e875ce30da3760c8655e303d9bea554efe7e4
> > color: debugcolor should emit the user-defined colors
> > 
> > This also fixes a long-standing bug that reversed the sense of the color
> > name and the label used to print it, which was never relevant before.
> > 
> > diff --git a/hgext/color.py b/hgext/color.py
> > --- a/hgext/color.py
> > +++ b/hgext/color.py
> > @@ -524,10 +524,16 @@ def debugcolor(ui, repo, **opts):
> >      _styles = {}
> >      for effect in _effects.keys():
> >          _styles[effect] = effect
> > +    if _terminfo_params:
> > +        for k, v in ui.configitems('color'):
> > +            if k.startswith('color.'):
> > +                _styles[k] = k[6:]
> > +            elif k.startswith('terminfo.'):
> > +                _styles[k] = k[9:]
> >      ui.write(('color mode: %s\n') % ui._colormode)
> >      ui.write(_('available colors:\n'))
> > -    for label, colors in _styles.items():
> > -        ui.write(('%s\n') % colors, label=label)
> > +    for colorname, label in _styles.items():
> > +        ui.write(('%s\n') % colorname, label=label)
> 
> I was surprised to know that a label can be a valid effect name.

This bit confused me for a while, and maybe I didn't make things better in
that regard.  There's "label" in the sense of the keyword argument to
ui.write(), and "label" in the sense of the word that we're printing out
here, labelling the color.  I used the latter sense in my comment, but the
code uses the former sense.

Danek
Yuya Nishihara - Oct. 16, 2016, 9:57 a.m.
On Sat, 15 Oct 2016 15:07:08 -0700, Danek Duvall wrote:
> Yuya Nishihara wrote:
> > On Thu, 13 Oct 2016 13:27:35 -0700, danek.duvall@oracle.com wrote:
> > > # HG changeset patch
> > > # User Danek Duvall <danek.duvall@oracle.com>
> > > # Date 1476389401 25200
> > > #      Thu Oct 13 13:10:01 2016 -0700
> > > # Node ID c59334b806a8f10c80c2ea196eba5f4b1b86b229
> > > # Parent  333e875ce30da3760c8655e303d9bea554efe7e4
> > > color: debugcolor should emit the user-defined colors
> > > 
> > > This also fixes a long-standing bug that reversed the sense of the color
> > > name and the label used to print it, which was never relevant before.
> > > 
> > > diff --git a/hgext/color.py b/hgext/color.py
> > > --- a/hgext/color.py
> > > +++ b/hgext/color.py
> > > @@ -524,10 +524,16 @@ def debugcolor(ui, repo, **opts):
> > >      _styles = {}
> > >      for effect in _effects.keys():
> > >          _styles[effect] = effect
> > > +    if _terminfo_params:
> > > +        for k, v in ui.configitems('color'):
> > > +            if k.startswith('color.'):
> > > +                _styles[k] = k[6:]
> > > +            elif k.startswith('terminfo.'):
> > > +                _styles[k] = k[9:]
> > >      ui.write(('color mode: %s\n') % ui._colormode)
> > >      ui.write(_('available colors:\n'))
> > > -    for label, colors in _styles.items():
> > > -        ui.write(('%s\n') % colors, label=label)
> > > +    for colorname, label in _styles.items():
> > > +        ui.write(('%s\n') % colorname, label=label)
> > 
> > I was surprised to know that a label can be a valid effect name.
> 
> This bit confused me for a while, and maybe I didn't make things better in
> that regard.  There's "label" in the sense of the keyword argument to
> ui.write(), and "label" in the sense of the word that we're printing out
> here, labelling the color.  I used the latter sense in my comment, but the
> code uses the former sense.

Maybe we can refactor it to not modify _styles table. _styles is a
label-to-effects map and we build new _styles map in which label=colorname,
so colorname == label (in ui.label() sense), label (variable) == effect,
and colorui.write() can take label=label|effect. That made me a bit confused
while reviewing this patch.

Patch

diff --git a/hgext/color.py b/hgext/color.py
--- a/hgext/color.py
+++ b/hgext/color.py
@@ -524,10 +524,16 @@  def debugcolor(ui, repo, **opts):
     _styles = {}
     for effect in _effects.keys():
         _styles[effect] = effect
+    if _terminfo_params:
+        for k, v in ui.configitems('color'):
+            if k.startswith('color.'):
+                _styles[k] = k[6:]
+            elif k.startswith('terminfo.'):
+                _styles[k] = k[9:]
     ui.write(('color mode: %s\n') % ui._colormode)
     ui.write(_('available colors:\n'))
-    for label, colors in _styles.items():
-        ui.write(('%s\n') % colors, label=label)
+    for colorname, label in _styles.items():
+        ui.write(('%s\n') % colorname, label=label)
 
 if os.name != 'nt':
     w32effects = None