Patchwork crecord: honor "ui.color = no" config option

login
register
mail settings
Submitter Elmar Bartel
Date Jan. 4, 2018, 11:40 a.m.
Message ID <04fe74a8269a68c78a57.1515066039@nuc15.leo.org>
Download mbox | patch
Permalink /patch/26539/
State Superseded
Headers show

Comments

Elmar Bartel - Jan. 4, 2018, 11:40 a.m.
mercurial/crecord.py |  21 ++++++++++++++++-----
 1 files changed, 16 insertions(+), 5 deletions(-)


# HG changeset patch
# User Elmar Bartel <elb_hg@leo.org>
# Date 1515064327 -3600
#      Thu Jan 04 12:12:07 2018 +0100
# Node ID 04fe74a8269a68c78a57a9f3698f1c87eff09e74
# Parent  31fe397f2bdab5fea4752947e70549e7a7d04f75
crecord: honor "ui.color = no" config option

The current implementation of crecord ignores the ui.color setting.
This patch checks the ui.color config option and does the curses setup
without colors when the option was set to "no". With other (or missing)
setting of ui.color, curses setup is done as before and uses colors.
Yuya Nishihara - Jan. 5, 2018, 6:15 a.m.
On Thu, 04 Jan 2018 12:40:39 +0100, Elmar Bartel wrote:
>  mercurial/crecord.py |  21 ++++++++++++++++-----
>  1 files changed, 16 insertions(+), 5 deletions(-)
> 
> 
> # HG changeset patch
> # User Elmar Bartel <elb_hg@leo.org>
> # Date 1515064327 -3600
> #      Thu Jan 04 12:12:07 2018 +0100
> # Node ID 04fe74a8269a68c78a57a9f3698f1c87eff09e74
> # Parent  31fe397f2bdab5fea4752947e70549e7a7d04f75
> crecord: honor "ui.color = no" config option

Looks good, but I'm not a curses user so I'm not sure if there's anyone wanting
to disable text color, but not curses'.

> The current implementation of crecord ignores the ui.color setting.
> This patch checks the ui.color config option and does the curses setup
> without colors when the option was set to "no". With other (or missing)
> setting of ui.color, curses setup is done as before and uses colors.
> 
> diff -r 31fe397f2bda -r 04fe74a8269a mercurial/crecord.py
> --- a/mercurial/crecord.py	Wed Dec 27 00:24:53 2017 +0530
> +++ b/mercurial/crecord.py	Thu Jan 04 12:12:07 2018 +0100
> @@ -581,6 +581,8 @@ class curseschunkselector(object):
>          # maps custom nicknames of color-pairs to curses color-pair values
>          self.colorpairnames = {}
>  
> +        self.usecolor = self.ui.config('ui', 'color') != 'no'

Nit: "util.parsebool() is not False" can be used to support more falsy values.
Elmar Bartel - Jan. 5, 2018, 10:23 a.m.
On Fri, Jan 05, 2018 at 03:15:40PM +0900, Yuya Nishihara wrote:
> On Thu, 04 Jan 2018 12:40:39 +0100, Elmar Bartel wrote:
> >  mercurial/crecord.py |  21 ++++++++++++++++-----
> >  1 files changed, 16 insertions(+), 5 deletions(-)
> > 
> > 
> > # HG changeset patch
> > # User Elmar Bartel <elb_hg@leo.org>
> > # Date 1515064327 -3600
> > #      Thu Jan 04 12:12:07 2018 +0100
> > # Node ID 04fe74a8269a68c78a57a9f3698f1c87eff09e74
> > # Parent  31fe397f2bdab5fea4752947e70549e7a7d04f75
> > crecord: honor "ui.color = no" config option
> 
> Looks good, but I'm not a curses user so I'm not sure if there's anyone wanting
> to disable text color, but not curses'.
At least me: I do not like color everywhere but like the curses interface.
But the more serious reason for this patch was to have the ground layed
for my second patch to crecord: when curses is not able to setup
colors, it fails badly (try "TERM=vt100 hg commit -i"). The user is 
then (whether he likes color or not) left with a Python stacktrace and
cannot continue. I think it's better to let the user continue his work
without color instead of throwing a stack trace at him.

> > The current implementation of crecord ignores the ui.color setting.
> > This patch checks the ui.color config option and does the curses setup
> > without colors when the option was set to "no". With other (or missing)
> > setting of ui.color, curses setup is done as before and uses colors.
> > 
> > diff -r 31fe397f2bda -r 04fe74a8269a mercurial/crecord.py
> > --- a/mercurial/crecord.py	Wed Dec 27 00:24:53 2017 +0530
> > +++ b/mercurial/crecord.py	Thu Jan 04 12:12:07 2018 +0100
> > @@ -581,6 +581,8 @@ class curseschunkselector(object):
> >          # maps custom nicknames of color-pairs to curses color-pair values
> >          self.colorpairnames = {}
> >  
> > +        self.usecolor = self.ui.config('ui', 'color') != 'no'
> 
> Nit: "util.parsebool() is not False" can be used to support more falsy values.
Definitily better!
(sorry I'am not yet fully familiar with mercurial internals).
I'll make a second version of the patch.

Patch

diff -r 31fe397f2bda -r 04fe74a8269a mercurial/crecord.py
--- a/mercurial/crecord.py	Wed Dec 27 00:24:53 2017 +0530
+++ b/mercurial/crecord.py	Thu Jan 04 12:12:07 2018 +0100
@@ -581,6 +581,8 @@  class curseschunkselector(object):
         # maps custom nicknames of color-pairs to curses color-pair values
         self.colorpairnames = {}
 
+        self.usecolor = self.ui.config('ui', 'color') != 'no'
+
         # the currently selected header, hunk, or hunk-line
         self.currentselecteditem = self.headerlist[0]
 
@@ -1371,11 +1373,20 @@  class curseschunkselector(object):
                 colorpair = self.colorpairs[(fgcolor, bgcolor)]
             else:
                 pairindex = len(self.colorpairs) + 1
-                curses.init_pair(pairindex, fgcolor, bgcolor)
-                colorpair = self.colorpairs[(fgcolor, bgcolor)] = (
-                    curses.color_pair(pairindex))
-                if name is not None:
-                    self.colorpairnames[name] = curses.color_pair(pairindex)
+                if self.usecolor:
+                    curses.init_pair(pairindex, fgcolor, bgcolor)
+                    colorpair = self.colorpairs[(fgcolor, bgcolor)] = (
+                        curses.color_pair(pairindex))
+                    if name is not None:
+                        self.colorpairnames[name] = curses.color_pair(
+                                                                  pairindex)
+                else:
+                    cval = 0
+                    if name is not None:
+                        if name == 'selected':
+                            cval = curses.A_REVERSE
+                        self.colorpairnames[name] = cval
+                    colorpair = self.colorpairs[(fgcolor, bgcolor)] = cval
 
         # add attributes if possible
         if attrlist is None: