Patchwork [2,of,3] color: turn on by default (but for windows)

login
register
mail settings
Submitter Pierre-Yves David
Date April 16, 2017, 12:55 a.m.
Message ID <7ec1415a4de91a5c56e5.1492304126@nodosa.octopoid.net>
Download mbox | patch
Permalink /patch/20235/
State Accepted
Headers show

Comments

Pierre-Yves David - April 16, 2017, 12:55 a.m.
# HG changeset patch
# User Pierre-Yves David <pierre-yves.david@ens-lyon.org>
# Date 1492302771 -7200
#      Sun Apr 16 02:32:51 2017 +0200
# Node ID 7ec1415a4de91a5c56e549cd4a40cb8e9411f8d9
# Parent  276dd8ba6084f1463faca2b57914b8fc3c96c776
# EXP-Topic color
# Available At https://www.mercurial-scm.org/repo/users/marmoute/mercurial/
#              hg pull https://www.mercurial-scm.org/repo/users/marmoute/mercurial/ -r 7ec1415a4de9
color: turn on by default (but for windows)

Color support is all in core for a couple of months. I've browsed the bug tracker
without finding any blocker bug. So I'm moving forward and enable color on by
default before '4.2-rc'. In the worse case, having it on in the release
candidate will help us to find blocker bug and we can turn it off for the final
release.

I remember people talking about issue with Windows during the freeze so I'm
keeping it off by default on that OS.

We could do various cleaning of the color used and the label issued.  However
the label are probably already in our backward compatibility envelope since the
color extensions has been around since for ever and I do not think the color
choice themself should be considered BC. So I think we should rather gives color
to all user sooner than later.

A couple of test needs to be updated to avoid having color related control code
spoil the tested output.
Yuya Nishihara - April 17, 2017, 2:24 p.m.
On Sun, 16 Apr 2017 02:55:26 +0200, Pierre-Yves David wrote:
> # HG changeset patch
> # User Pierre-Yves David <pierre-yves.david@ens-lyon.org>
> # Date 1492302771 -7200
> #      Sun Apr 16 02:32:51 2017 +0200
> # Node ID 7ec1415a4de91a5c56e549cd4a40cb8e9411f8d9
> # Parent  276dd8ba6084f1463faca2b57914b8fc3c96c776
> # EXP-Topic color
> # Available At https://www.mercurial-scm.org/repo/users/marmoute/mercurial/
> #              hg pull https://www.mercurial-scm.org/repo/users/marmoute/mercurial/ -r 7ec1415a4de9
> color: turn on by default (but for windows)
> 
> Color support is all in core for a couple of months. I've browsed the bug tracker
> without finding any blocker bug. So I'm moving forward and enable color on by
> default before '4.2-rc'. In the worse case, having it on in the release
> candidate will help us to find blocker bug and we can turn it off for the final
> release.

Seems fine. Queued the series, thanks.

Can you update the help as a followup? hgext/color.py can also be removed
and marked as a builtin, but that isn't important.
Pierre-Yves David - April 17, 2017, 6:44 p.m.
On 04/17/2017 04:24 PM, Yuya Nishihara wrote:
> On Sun, 16 Apr 2017 02:55:26 +0200, Pierre-Yves David wrote:
>> # HG changeset patch
>> # User Pierre-Yves David <pierre-yves.david@ens-lyon.org>
>> # Date 1492302771 -7200
>> #      Sun Apr 16 02:32:51 2017 +0200
>> # Node ID 7ec1415a4de91a5c56e549cd4a40cb8e9411f8d9
>> # Parent  276dd8ba6084f1463faca2b57914b8fc3c96c776
>> # EXP-Topic color
>> # Available At https://www.mercurial-scm.org/repo/users/marmoute/mercurial/
>> #              hg pull https://www.mercurial-scm.org/repo/users/marmoute/mercurial/ -r 7ec1415a4de9
>> color: turn on by default (but for windows)
>>
>> Color support is all in core for a couple of months. I've browsed the bug tracker
>> without finding any blocker bug. So I'm moving forward and enable color on by
>> default before '4.2-rc'. In the worse case, having it on in the release
>> candidate will help us to find blocker bug and we can turn it off for the final
>> release.
>
> Seems fine. Queued the series, thanks.
>
> Can you update the help as a followup?

ah, yes. patch coming soon.

> hgext/color.py can also be removed
> and marked as a builtin, but that isn't important.

I wanted to mimize the possible churn on stable if we have to backout 
the change in default during the freeze. My plan is to clan this up 
early in the 4.3 cycle if the changes survive in the 4.2 release.

Cheers,
Yuya Nishihara - April 18, 2017, 11:13 a.m.
On Mon, 17 Apr 2017 20:44:16 +0200, Pierre-Yves David wrote:
> > hgext/color.py can also be removed
> > and marked as a builtin, but that isn't important.
> 
> I wanted to mimize the possible churn on stable if we have to backout 
> the change in default during the freeze. My plan is to clan this up 
> early in the 4.3 cycle if the changes survive in the 4.2 release.

Makes sense.

Patch

diff --git a/mercurial/color.py b/mercurial/color.py
--- a/mercurial/color.py
+++ b/mercurial/color.py
@@ -45,8 +45,7 @@  except ImportError:
     curses = None
     _baseterminfoparams = {}
 
-# allow the extensions to change the default
-_enabledbydefault = False
+_enabledbydefault = pycompat.osname != 'nt'
 
 # start and stop parameters for effects
 _effects = {
diff --git a/tests/test-mq.t b/tests/test-mq.t
--- a/tests/test-mq.t
+++ b/tests/test-mq.t
@@ -365,10 +365,10 @@  pop/push -a in subdir
 
 setting columns & formatted tests truncating (issue1912)
 
-  $ COLUMNS=4 hg qseries --config ui.formatted=true
+  $ COLUMNS=4 hg qseries --config ui.formatted=true --color=no
   test.patch
   test2.patch
-  $ COLUMNS=20 hg qseries --config ui.formatted=true -vs
+  $ COLUMNS=20 hg qseries --config ui.formatted=true -vs --color=no
   0 A test.patch: f...
   1 A test2.patch: 
   $ hg qpop
diff --git a/tests/test-pager-legacy.t b/tests/test-pager-legacy.t
--- a/tests/test-pager-legacy.t
+++ b/tests/test-pager-legacy.t
@@ -10,6 +10,7 @@  pager was running.
   $ cat >> $HGRCPATH <<EOF
   > [ui]
   > formatted = yes
+  > color = no
   > [extensions]
   > pager=
   > [pager]
diff --git a/tests/test-pager.t b/tests/test-pager.t
--- a/tests/test-pager.t
+++ b/tests/test-pager.t
@@ -10,6 +10,7 @@  pager was running.
   $ cat >> $HGRCPATH <<EOF
   > [ui]
   > formatted = yes
+  > color = no
   > [pager]
   > pager = python $TESTTMP/fakepager.py
   > EOF