Re: [PATCH 3 of 3] color: debugcolor should emit the user-defined colors

2016-10-16 Thread Yuya Nishihara 
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.duv...@oracle.com wrote:
> > > # HG changeset patch
> > > # User Danek Duvall 
> > > # 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.
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Re: [PATCH 3 of 3] color: debugcolor should emit the user-defined colors

2016-10-15 Thread Danek Duvall 
Yuya Nishihara wrote:

> On Thu, 13 Oct 2016 13:27:35 -0700, danek.duv...@oracle.com wrote:
> > # HG changeset patch
> > # User Danek Duvall 
> > # 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
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Re: [PATCH 3 of 3] color: debugcolor should emit the user-defined colors

2016-10-15 Thread Yuya Nishihara 
On Thu, 13 Oct 2016 13:27:35 -0700, danek.duv...@oracle.com wrote:
> # HG changeset patch
> # User Danek Duvall 
> # 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.
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel