[PATCH stable] color: tweak behavior of ui.color config knob

[Augie Fackler]

# HG changeset patch
# User Augie Fackler <augie at google.com>
# Date 1493678508 14400
#      Mon May 01 18:41:48 2017 -0400
# Branch stable
# Node ID 9524b0b5eedb877c4dc2d1afefa423ea9149e874
# Parent  7c76f3923b6a4fd6a35a9b498be15b9c549955eb
color: tweak behavior of ui.color config knob

marmoute sent some other patches today that tried to bring
pager.enabled into conformity with ui.color, and while reviewing those
patches I tripped on some unfortunate usability warts on the behavior
of the (new in 4.2) ui.color config knob. Specifically, ui.color=yes
actually means "always show color", not "use color automatically when
it appears sensible". This feels problematic because if an
administrator has disabled color, and a user wants to turn it on, the
obvious setting (color=on) is wrong (because it breaks their output
when redirected to a file.) This patch changes ui.color to only move
the default value of --color from "never" to "auto", which matches the
relationship of pager.enabed and --pager.

That is, before this patch:

+--------------------+--------------------+--------------------+
|                    | not a tty          | a tty              |
|                    | --color not set    | --color not set    |
|                    |                    |                    |
+--------------------+--------------------+--------------------+
| [ui]               |                    |                    |
| color (not set)    | no color           | no color           |
|                    |                    |                    |
+--------------------+--------------------+--------------------+
| [ui]               |                    |                    |
| color = auto       | no color           | color              |
|                    |                    |                    |
+--------------------+--------------------+--------------------+
| [ui]               |                    |                    |
| color = yes        | *color*            | color              |
|                    |                    |                    |
+--------------------+--------------------+--------------------+
| [ui]               |                    |                    |
| color = no         | no color           | no color           |
|                    |                    |                    |
+--------------------+--------------------+--------------------+
(if --color is specified, it always clobbers the setting in [ui])

and after this patch:

+--------------------+--------------------+--------------------+
|                    | not a tty          | a tty              |
|                    | --color not set    | --color not set    |
|                    |                    |                    |
+--------------------+--------------------+--------------------+
| [ui]               |                    |                    |
| color (not set)    | no color           | no color           |
|                    |                    |                    |
+--------------------+--------------------+--------------------+
| [ui]               |                    |                    |
| color = auto       | no color           | color              |
|                    |                    |                    |
+--------------------+--------------------+--------------------+
| [ui]               |                    |                    |
| color = yes        | *no color*         | color              |
|                    |                    |                    |
+--------------------+--------------------+--------------------+
| [ui]               |                    |                    |
| color = no         | no color           | no color           |
|                    |                    |                    |
+--------------------+--------------------+--------------------+
(if --color is specified, it always clobbers the setting in [ui])

Users that want to force colors without specifying --color on the
command line can use the ui.formatted config knob, which had to be
enabled in a handful of tests for this patch.

In the name of consistency we've also discussed renaming pager.enabled
to ui.pager. This feels like the wrong move, and I think Jun Wu's
observation on IRC feels like the right justification: "ui.pager as a
bool is confusing because we have pager.pager and ui.editor" - that
is, if we have ui.editor and that's an executable to use as the
editor, it feels like ui.pager should be the executable to use as the
pager, not the knob to enable/disable pager.

Since this is a behavior change in a flag that I'm proposing for
stable unacceptably late in a cycle, I went ahead and made this change
in the simplest possible way. We should probably plan to do some
further cleanups in the future to make this a little less cluttered.

diff --git a/mercurial/color.py b/mercurial/color.py
--- a/mercurial/color.py
+++ b/mercurial/color.py
@@ -185,10 +185,17 @@ def setup(ui):
 def _modesetup(ui):
     if ui.plain():
         return None
+    # ui.color only controls the default of the --color flag: true
+    # means to automatically use color when sensible, and false means
+    # to never use color.
+    wantcolor = ui.configbool('ui', 'color', _enabledbydefault)
     default = 'never'
-    if _enabledbydefault:
+    if wantcolor:
         default = 'auto'
-    config = ui.config('ui', 'color', default)
+    # internal config: ui.color-flag
+    # Used for carrying --color state on ui objects.
+    config = ui.config('ui', 'color-flag', default)
+
     if config == 'debug':
         return 'debug'
 
diff --git a/mercurial/dispatch.py b/mercurial/dispatch.py
--- a/mercurial/dispatch.py
+++ b/mercurial/dispatch.py
@@ -846,7 +846,7 @@ def _dispatch(req):
         coloropt = options['color']
         for ui_ in uis:
             if coloropt:
-                ui_.setconfig('ui', 'color', coloropt, '--color')
+                ui_.setconfig('ui', 'color-flag', coloropt, '--color')
             color.setup(ui_)
 
         if options['version']:
diff --git a/tests/test-diff-color.t b/tests/test-diff-color.t
--- a/tests/test-diff-color.t
+++ b/tests/test-diff-color.t
@@ -3,6 +3,7 @@ Setup
   $ cat <<EOF >> $HGRCPATH
   > [ui]
   > color = always
+  > formatted = yes
   > [color]
   > mode = ansi
   > EOF
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
@@ -161,6 +161,7 @@ even though stdout is no longer a tty.
   $ cat >> $HGRCPATH <<EOF
   > [ui]
   > color = yes
+  > formatted = yes
   > [color]
   > mode = ansi
   > EOF
diff --git a/tests/test-pager.t b/tests/test-pager.t
--- a/tests/test-pager.t
+++ b/tests/test-pager.t
@@ -142,6 +142,7 @@ even though stdout is no longer a tty.
   $ cat >> $HGRCPATH <<EOF
   > [ui]
   > color = yes
+  > formatted = yes
   > [color]
   > mode = ansi
   > EOF
diff --git a/tests/test-status-color.t b/tests/test-status-color.t
--- a/tests/test-status-color.t
+++ b/tests/test-status-color.t
@@ -1,6 +1,7 @@
   $ cat <<EOF >> $HGRCPATH
   > [ui]
   > color = always
+  > formatted = yes
   > [color]
   > mode = ansi
   > EOF