Patchwork color: replace "purple_background" by "magenta_background"

login
register
mail settings
Submitter Denis Laxalde
Date Oct. 11, 2016, 9:39 a.m.
Message ID <1cf20c1c54cb81728166.1476178766@sh77.tls.logilab.fr>
Download mbox | patch
Permalink /patch/17029/
State Changes Requested
Headers show

Comments

Denis Laxalde - Oct. 11, 2016, 9:39 a.m.
# HG changeset patch
# User Denis Laxalde <denis.laxalde@logilab.fr>
# Date 1476176128 -7200
#      Tue Oct 11 10:55:28 2016 +0200
# Node ID 1cf20c1c54cb8172816604de5e1d98d3cd62d711
# Parent  b85fa6bf298be07804a74d8fdec0d19fdbc6d740
# EXP-Topic color-purple-crash
color: replace "purple_background" by "magenta_background"

`hg debugcolor` crashes on my terminal:

      File "/usr/lib/python2.7/dist-packages/hgext/color.py", line 530, in debugcolor
        ui.write(('%s\n') % colors, label=label)
      File "/usr/lib/python2.7/dist-packages/hgext/color.py", line 446, in write
        *[self.label(a, label) for a in args], **opts)
      File "/usr/lib/python2.7/dist-packages/hgext/color.py", line 488, in label
        for s in msg.split('\n')])
      File "/usr/lib/python2.7/dist-packages/hgext/color.py", line 396, in render_effects
        for effect in ['none'] + effects.split())
      File "/usr/lib/python2.7/dist-packages/hgext/color.py", line 396, in <genexpr>
        for effect in ['none'] + effects.split())
      File "/usr/lib/python2.7/dist-packages/hgext/color.py", line 378, in _effect_str
        attr, val = _terminfo_params[effect]
    KeyError: 'purple'

Not sure where this "purple" comes (been there from initial file check in)
from but "magenta" seems more appropriate.
Pierre-Yves David - Oct. 13, 2016, 5:26 p.m.
On 10/11/2016 11:39 AM, Denis Laxalde wrote:
> # HG changeset patch
> # User Denis Laxalde <denis.laxalde@logilab.fr>
> # Date 1476176128 -7200
> #      Tue Oct 11 10:55:28 2016 +0200
> # Node ID 1cf20c1c54cb8172816604de5e1d98d3cd62d711
> # Parent  b85fa6bf298be07804a74d8fdec0d19fdbc6d740
> # EXP-Topic color-purple-crash
> color: replace "purple_background" by "magenta_background"

We probably want to keep both value around for backward compatibility 
reason.

I'm also a bit curious about what all this is about. Can I convinced you 
to dig it a bit further?

> `hg debugcolor` crashes on my terminal:
>
>       File "/usr/lib/python2.7/dist-packages/hgext/color.py", line 530, in debugcolor
>         ui.write(('%s\n') % colors, label=label)
>       File "/usr/lib/python2.7/dist-packages/hgext/color.py", line 446, in write
>         *[self.label(a, label) for a in args], **opts)
>       File "/usr/lib/python2.7/dist-packages/hgext/color.py", line 488, in label
>         for s in msg.split('\n')])
>       File "/usr/lib/python2.7/dist-packages/hgext/color.py", line 396, in render_effects
>         for effect in ['none'] + effects.split())
>       File "/usr/lib/python2.7/dist-packages/hgext/color.py", line 396, in <genexpr>
>         for effect in ['none'] + effects.split())
>       File "/usr/lib/python2.7/dist-packages/hgext/color.py", line 378, in _effect_str
>         attr, val = _terminfo_params[effect]
>     KeyError: 'purple'
>
> Not sure where this "purple" comes (been there from initial file check in)
> from but "magenta" seems more appropriate.
>
> diff --git a/hgext/color.py b/hgext/color.py
> --- a/hgext/color.py
> +++ b/hgext/color.py
> @@ -182,7 +182,7 @@ testedwith = 'ships-with-hg-core'
>              'italic': 3, 'underline': 4, 'inverse': 7, 'dim': 2,
>              'black_background': 40, 'red_background': 41,
>              'green_background': 42, 'yellow_background': 43,
> -            'blue_background': 44, 'purple_background': 45,
> +            'blue_background': 44, 'magenta_background': 45,
>              'cyan_background': 46, 'white_background': 47}
>
>  def _terminfosetup(ui, mode):
> @@ -591,7 +591,7 @@ else:
>          'green_background': _BACKGROUND_GREEN,
>          'yellow_background': _BACKGROUND_RED | _BACKGROUND_GREEN,
>          'blue_background': _BACKGROUND_BLUE,
> -        'purple_background': _BACKGROUND_BLUE | _BACKGROUND_RED,
> +        'magenta_background': _BACKGROUND_BLUE | _BACKGROUND_RED,
>          'cyan_background': _BACKGROUND_BLUE | _BACKGROUND_GREEN,
>          'white_background': (_BACKGROUND_RED | _BACKGROUND_GREEN |
>                               _BACKGROUND_BLUE),
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
>
Danek Duvall - Oct. 13, 2016, 7:07 p.m.
Pierre-Yves David wrote:

> 
> 
> On 10/11/2016 11:39 AM, Denis Laxalde wrote:
> ># HG changeset patch
> ># User Denis Laxalde <denis.laxalde@logilab.fr>
> ># Date 1476176128 -7200
> >#      Tue Oct 11 10:55:28 2016 +0200
> ># Node ID 1cf20c1c54cb8172816604de5e1d98d3cd62d711
> ># Parent  b85fa6bf298be07804a74d8fdec0d19fdbc6d740
> ># EXP-Topic color-purple-crash
> >color: replace "purple_background" by "magenta_background"
> 
> We probably want to keep both value around for backward compatibility
> reason.
> 
> I'm also a bit curious about what all this is about. Can I convinced you to
> dig it a bit further?

_effect_str() strips "_background" off of an effect name to grab the color
name so it can figure out what color code to use.  Since we define
"purple_background", but "magenta" instead of "purple" (and
"magenta_background" doesn't exist, either), then this blows up.

I'll be sending out a patch shortly that just covers up this KeyError, but
either defining purple or magenta_background (or, probably, both) probably
ought to be done, anyway.

Danek

> >`hg debugcolor` crashes on my terminal:
> >
> >      File "/usr/lib/python2.7/dist-packages/hgext/color.py", line 530, in debugcolor
> >        ui.write(('%s\n') % colors, label=label)
> >      File "/usr/lib/python2.7/dist-packages/hgext/color.py", line 446, in write
> >        *[self.label(a, label) for a in args], **opts)
> >      File "/usr/lib/python2.7/dist-packages/hgext/color.py", line 488, in label
> >        for s in msg.split('\n')])
> >      File "/usr/lib/python2.7/dist-packages/hgext/color.py", line 396, in render_effects
> >        for effect in ['none'] + effects.split())
> >      File "/usr/lib/python2.7/dist-packages/hgext/color.py", line 396, in <genexpr>
> >        for effect in ['none'] + effects.split())
> >      File "/usr/lib/python2.7/dist-packages/hgext/color.py", line 378, in _effect_str
> >        attr, val = _terminfo_params[effect]
> >    KeyError: 'purple'
> >
> >Not sure where this "purple" comes (been there from initial file check in)
> >from but "magenta" seems more appropriate.
> >
> >diff --git a/hgext/color.py b/hgext/color.py
> >--- a/hgext/color.py
> >+++ b/hgext/color.py
> >@@ -182,7 +182,7 @@ testedwith = 'ships-with-hg-core'
> >             'italic': 3, 'underline': 4, 'inverse': 7, 'dim': 2,
> >             'black_background': 40, 'red_background': 41,
> >             'green_background': 42, 'yellow_background': 43,
> >-            'blue_background': 44, 'purple_background': 45,
> >+            'blue_background': 44, 'magenta_background': 45,
> >             'cyan_background': 46, 'white_background': 47}
> >
> > def _terminfosetup(ui, mode):
> >@@ -591,7 +591,7 @@ else:
> >         'green_background': _BACKGROUND_GREEN,
> >         'yellow_background': _BACKGROUND_RED | _BACKGROUND_GREEN,
> >         'blue_background': _BACKGROUND_BLUE,
> >-        'purple_background': _BACKGROUND_BLUE | _BACKGROUND_RED,
> >+        'magenta_background': _BACKGROUND_BLUE | _BACKGROUND_RED,
> >         'cyan_background': _BACKGROUND_BLUE | _BACKGROUND_GREEN,
> >         'white_background': (_BACKGROUND_RED | _BACKGROUND_GREEN |
> >                              _BACKGROUND_BLUE),
> >_______________________________________________
> >Mercurial-devel mailing list
> >Mercurial-devel@mercurial-scm.org
> >https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
> >
> 
> -- 
> Pierre-Yves David
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Denis Laxalde - Oct. 14, 2016, 7:26 a.m.
Pierre-Yves David a écrit :
> On 10/11/2016 11:39 AM, Denis Laxalde wrote:
>> # HG changeset patch
>> # User Denis Laxalde <denis.laxalde@logilab.fr>
>> # Date 1476176128 -7200
>> #      Tue Oct 11 10:55:28 2016 +0200
>> # Node ID 1cf20c1c54cb8172816604de5e1d98d3cd62d711
>> # Parent  b85fa6bf298be07804a74d8fdec0d19fdbc6d740
>> # EXP-Topic color-purple-crash
>> color: replace "purple_background" by "magenta_background"
>
> We probably want to keep both value around for backward compatibility
> reason.

Aggreed.

> I'm also a bit curious about what all this is about. Can I convinced you
> to dig it a bit further?

Not sure how deep you'd like me to dig. This occurs with "terminfo" 
color mode. So, to reproduce:

     HGRCPATH= hg --config extensions.color= --config 
color.mode=terminfo debugcolor

Patch

diff --git a/hgext/color.py b/hgext/color.py
--- a/hgext/color.py
+++ b/hgext/color.py
@@ -182,7 +182,7 @@  testedwith = 'ships-with-hg-core'
             'italic': 3, 'underline': 4, 'inverse': 7, 'dim': 2,
             'black_background': 40, 'red_background': 41,
             'green_background': 42, 'yellow_background': 43,
-            'blue_background': 44, 'purple_background': 45,
+            'blue_background': 44, 'magenta_background': 45,
             'cyan_background': 46, 'white_background': 47}
 
 def _terminfosetup(ui, mode):
@@ -591,7 +591,7 @@  else:
         'green_background': _BACKGROUND_GREEN,
         'yellow_background': _BACKGROUND_RED | _BACKGROUND_GREEN,
         'blue_background': _BACKGROUND_BLUE,
-        'purple_background': _BACKGROUND_BLUE | _BACKGROUND_RED,
+        'magenta_background': _BACKGROUND_BLUE | _BACKGROUND_RED,
         'cyan_background': _BACKGROUND_BLUE | _BACKGROUND_GREEN,
         'white_background': (_BACKGROUND_RED | _BACKGROUND_GREEN |
                              _BACKGROUND_BLUE),