Patchwork [4,of,4] ui: rerun color.setup() once the pager has spawned to honor 'color.pagermode'

login
register
mail settings
Submitter Matt Harbison
Date March 26, 2017, 4:41 a.m.
Message ID <4713a38672f2d0790477.1490503267@Envy>
Download mbox | patch
Permalink /patch/19677/
State Accepted
Headers show

Comments

Matt Harbison - March 26, 2017, 4:41 a.m.
# HG changeset patch
# User Matt Harbison <matt_harbison@yahoo.com>
# Date 1490483831 14400
#      Sat Mar 25 19:17:11 2017 -0400
# Node ID 4713a38672f2d0790477b6c22180bd453f61851d
# Parent  84bda5db69dbe3d550f45ccd6d6eda2aad760ee4
ui: rerun color.setup() once the pager has spawned to honor 'color.pagermode'

Otherwise, ui.pageractive is False when color is setup in dispatch.py (without
--pager=on), and this config option is ignored.
Yuya Nishihara - March 26, 2017, 2:44 p.m.
On Sun, 26 Mar 2017 00:41:07 -0400, Matt Harbison wrote:
> # HG changeset patch
> # User Matt Harbison <matt_harbison@yahoo.com>
> # Date 1490483831 14400
> #      Sat Mar 25 19:17:11 2017 -0400
> # Node ID 4713a38672f2d0790477b6c22180bd453f61851d
> # Parent  84bda5db69dbe3d550f45ccd6d6eda2aad760ee4
> ui: rerun color.setup() once the pager has spawned to honor 'color.pagermode'
> 
> Otherwise, ui.pageractive is False when color is setup in dispatch.py (without
> --pager=on), and this config option is ignored.
> 
> diff --git a/mercurial/ui.py b/mercurial/ui.py
> --- a/mercurial/ui.py
> +++ b/mercurial/ui.py
> @@ -861,6 +861,12 @@
>              # auto-detection of things being formatted.
>              self.setconfig('ui', 'formatted', wasformatted, 'pager')
>              self.setconfig('ui', 'interactive', False, 'pager')
> +
> +            # If pagermode differs from color.mode, reconfigure color now that
> +            # pageractive is set.
> +            cm = self._colormode
> +            if cm != self.config('color', 'pagermode', cm):
> +                color.setup(self)

This also looks good. Maybe we can refactor color._modesetup() further so that
we can do something like

  newmode = color.mode(self)
  if self._colormode != newmode:
      color.setup(self, newmode)
Matt Harbison - March 26, 2017, 6:05 p.m.
On Sun, 26 Mar 2017 10:44:01 -0400, Yuya Nishihara <yuya@tcha.org> wrote:

> On Sun, 26 Mar 2017 00:41:07 -0400, Matt Harbison wrote:
>> # HG changeset patch
>> # User Matt Harbison <matt_harbison@yahoo.com>
>> # Date 1490483831 14400
>> #      Sat Mar 25 19:17:11 2017 -0400
>> # Node ID 4713a38672f2d0790477b6c22180bd453f61851d
>> # Parent  84bda5db69dbe3d550f45ccd6d6eda2aad760ee4
>> ui: rerun color.setup() once the pager has spawned to honor  
>> 'color.pagermode'
>>
>> Otherwise, ui.pageractive is False when color is setup in dispatch.py  
>> (without
>> --pager=on), and this config option is ignored.
>>
>> diff --git a/mercurial/ui.py b/mercurial/ui.py
>> --- a/mercurial/ui.py
>> +++ b/mercurial/ui.py
>> @@ -861,6 +861,12 @@
>>              # auto-detection of things being formatted.
>>              self.setconfig('ui', 'formatted', wasformatted, 'pager')
>>              self.setconfig('ui', 'interactive', False, 'pager')
>> +
>> +            # If pagermode differs from color.mode, reconfigure color  
>> now that
>> +            # pageractive is set.
>> +            cm = self._colormode
>> +            if cm != self.config('color', 'pagermode', cm):
>> +                color.setup(self)
>
> This also looks good. Maybe we can refactor color._modesetup() further  
> so that
> we can do something like
>
>   newmode = color.mode(self)
>   if self._colormode != newmode:
>       color.setup(self, newmode)

Is a follow up OK?  There are some warnings being printed in _modesetup()  
that I don't think we want firing more than once, so it may not be a  
trivial refactoring.
Yuya Nishihara - March 27, 2017, 12:18 p.m.
On Sun, 26 Mar 2017 14:05:35 -0400, Matt Harbison wrote:
> On Sun, 26 Mar 2017 10:44:01 -0400, Yuya Nishihara <yuya@tcha.org> wrote:
> 
> > On Sun, 26 Mar 2017 00:41:07 -0400, Matt Harbison wrote:
> >> # HG changeset patch
> >> # User Matt Harbison <matt_harbison@yahoo.com>
> >> # Date 1490483831 14400
> >> #      Sat Mar 25 19:17:11 2017 -0400
> >> # Node ID 4713a38672f2d0790477b6c22180bd453f61851d
> >> # Parent  84bda5db69dbe3d550f45ccd6d6eda2aad760ee4
> >> ui: rerun color.setup() once the pager has spawned to honor  
> >> 'color.pagermode'
> >>
> >> Otherwise, ui.pageractive is False when color is setup in dispatch.py  
> >> (without
> >> --pager=on), and this config option is ignored.
> >>
> >> diff --git a/mercurial/ui.py b/mercurial/ui.py
> >> --- a/mercurial/ui.py
> >> +++ b/mercurial/ui.py
> >> @@ -861,6 +861,12 @@
> >>              # auto-detection of things being formatted.
> >>              self.setconfig('ui', 'formatted', wasformatted, 'pager')
> >>              self.setconfig('ui', 'interactive', False, 'pager')
> >> +
> >> +            # If pagermode differs from color.mode, reconfigure color  
> >> now that
> >> +            # pageractive is set.
> >> +            cm = self._colormode
> >> +            if cm != self.config('color', 'pagermode', cm):
> >> +                color.setup(self)
> >
> > This also looks good. Maybe we can refactor color._modesetup() further  
> > so that
> > we can do something like
> >
> >   newmode = color.mode(self)
> >   if self._colormode != newmode:
> >       color.setup(self, newmode)
> 
> Is a follow up OK?

Sure. It will be far from trivial.

Patch

diff --git a/mercurial/ui.py b/mercurial/ui.py
--- a/mercurial/ui.py
+++ b/mercurial/ui.py
@@ -861,6 +861,12 @@ 
             # auto-detection of things being formatted.
             self.setconfig('ui', 'formatted', wasformatted, 'pager')
             self.setconfig('ui', 'interactive', False, 'pager')
+
+            # If pagermode differs from color.mode, reconfigure color now that
+            # pageractive is set.
+            cm = self._colormode
+            if cm != self.config('color', 'pagermode', cm):
+                color.setup(self)
         else:
             # If the pager can't be spawned in dispatch when --pager=on is
             # given, don't try again when the command runs, to avoid a duplicate