| 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
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.
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: