Enable color support based on tty and config

Use the git config command and provided options to have git detect
whether color support should be enabled or disabled. Additionally ensure
that git-review color support can be controlled separately via the
option 'color.review', while still falling back to 'color.ui' if the app
specific option is not defined.

This will ensure that when piping or redirecting the output that colour
output will be disabled unless color.review, or it's fallback color.ui,
is set to always.

Also by utilizing 'git config', the any config options specified on the
command line that change behaviour will be adhered to. e.g.

  git -c color.review=never review -l

Will always output without color enabled.

Change-Id: I8f83ed8623da88e87972109af956331704e15d38
Closes-Bug: #1097961

git-review.1

@@ -221,6 +221,20 @@ in the
 .Pa .gitreview
 file.
 .El
+.Bl -tag
+.It color.review
+Whether to use ANSI escape sequences to add color to the output displayed by
+this command. Default value is determined by color.ui.
+.Bl -tag
+.It auto or true
+If you want output to use color when written to the terminal (default with Git
+1.8.4 and newer).
+.It always
+If you want all output to use color
+.It never or false
+If you wish not to use color for any output. (default with Git older than 1.8.4)
+.El
+.El
 .Sh FILES
 To use
 .Nm

git_review/cmd.py

@@ -60,6 +60,7 @@ DEFAULTS = dict(scheme='ssh', hostname=False, port=None, project=False,
 
 _branch_name = None
 _has_color = None
+_use_color = None
 _orig_head = None
 
 
@@ -529,6 +530,29 @@ def query_reviews_over_ssh(remote_url, change=None, current_patch_set=True,
     return changes
 
 
+def set_color_output(color="auto"):
+    global _use_color
+    if check_color_support():
+        if color == "auto":
+            check_use_color_output()
+        else:
+            _use_color = color == "always"
+
+
+def check_use_color_output():
+    global _use_color
+    if _use_color is None:
+        if check_color_support():
+            # we can support color, now check if we should use it
+            stdout = "true" if sys.stdout.isatty() else "false"
+            test_command = "git config --get-colorbool color.review " + stdout
+            color = run_command(test_command)
+            _use_color = color == "true"
+        else:
+            _use_color = False
+    return _use_color
+
+
 def check_color_support():
     global _has_color
     if _has_color is None:
@@ -688,23 +712,10 @@ def get_branch_name(target_branch):
 
 
 def assert_one_change(remote, branch, yes, have_hook):
-    has_color = check_color_support()
+    if check_use_color_output():
-    if has_color:
+        use_color = "--color=always"
-        color = git_config_get_value("color", "ui")
-        if color is None:
-            color = "auto"
-        else:
-            color = color.lower()
-        if (color == "" or color == "true"):
-            color = "auto"
-        elif color == "false":
-            color = "never"
-        elif color == "auto":
-            # Python is not a tty, we have to force colors
-            color = "always"
-        use_color = "--color=%s" % color
     else:
-        use_color = ""
+        use_color = "--color=never"
     cmd = ("git log %s --decorate --oneline HEAD --not --remotes=%s" % (
            use_color, remote))
     (status, output) = run_command_status(cmd)
@@ -809,7 +820,7 @@ def list_reviews(remote):
 
     REVIEW_FIELDS = ('number', 'branch', 'subject')
     FIELDS = range(len(REVIEW_FIELDS))
-    if check_color_support():
+    if check_use_color_output():
         review_field_color = (colors.yellow, colors.green, "")
         color_reset = colors.reset
     else:
@@ -1153,6 +1164,18 @@ def _main():
     parser.add_argument("--no-custom-script", dest="custom_script",
                         action="store_false", default=True,
                         help="Do not run custom scripts.")
+    parser.add_argument("--color", dest="color", metavar="<when>",
+                        nargs="?", choices=["always", "never", "auto"],
+                        help="Show color output. --color (without [<when>]) "
+                             "is the same as --color=always. <when> can be "
+                             "one of %(choices)s. Behaviour can also be "
+                             "controlled by the color.ui and color.review "
+                             "configuration settings.")
+    parser.add_argument("--no-color", dest="color", action="store_const",
+                        const="never",
+                        help="Turn off colored output. Can be used to "
+                             "override configuration options. Same as "
+                             "setting --color=never.")
     parser.add_argument("--license", dest="license", action="store_true",
                         help="Print the license and exit")
     parser.add_argument("--version", action="version",
@@ -1197,6 +1220,9 @@ def _main():
     check_remote(branch, remote, config['scheme'],
                  config['hostname'], config['port'], config['project'])
 
+    if options.color:
+        set_color_output(options.color)
+
     if options.changeidentifier:
         if options.compare:
             compare_review(options.changeidentifier,

git_review/tests/test_unit.py

@@ -15,18 +15,23 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+import functools
+import os
+import textwrap
+
+import fixtures
+import mock
+import testtools
+
+from git_review import cmd
+from git_review.tests import utils
+
 # Use of io.StringIO in python =< 2.7 requires all strings handled to be
 # unicode. See if StringIO.StringIO is available first
 try:
     import StringIO as io
 except ImportError:
     import io
-import textwrap
-
-import mock
-import testtools
-
-from git_review import cmd
 
 
 class ConfigTestCase(testtools.TestCase):
@@ -50,7 +55,7 @@ class ConfigTestCase(testtools.TestCase):
         self.assertFalse(exists_mock.called)
 
 
-class GitReviewConsole(testtools.TestCase):
+class GitReviewConsole(testtools.TestCase, fixtures.TestWithFixtures):
     """Class for testing the console output of git-review."""
 
     reviews = [
@@ -70,6 +75,29 @@ class GitReviewConsole(testtools.TestCase):
                        'max width is short enough'
         }]
 
+    def setUp(self):
+        super(GitReviewConsole, self).setUp()
+        # ensure all tests get a separate git dir to work in to avoid
+        # local git config from interfering
+        self.tempdir = self.useFixture(fixtures.TempDir())
+        self._run_git = functools.partial(utils.run_git,
+                                          chdir=self.tempdir.path)
+
+        self.run_cmd_patcher = mock.patch('git_review.cmd.run_command_status')
+        run_cmd_partial = functools.partial(
+            cmd.run_command_status, GIT_WORK_TREE=self.tempdir.path,
+            GIT_DIR=os.path.join(self.tempdir.path, '.git'))
+        self.run_cmd_mock = self.run_cmd_patcher.start()
+        self.run_cmd_mock.side_effect = run_cmd_partial
+
+        self._run_git('init')
+        self._run_git('commit', '--allow-empty', '-m "initial commit"')
+        self._run_git('commit', '--allow-empty', '-m "2nd commit"')
+
+    def tearDown(self):
+        self.run_cmd_patcher.stop()
+        super(GitReviewConsole, self).tearDown()
+
     @mock.patch('git_review.cmd.query_reviews')
     @mock.patch('git_review.cmd.get_remote_url', mock.MagicMock)
     @mock.patch('git_review.cmd._has_color', False)
@@ -87,3 +115,40 @@ class GitReviewConsole(testtools.TestCase):
                 self.assertEqual(line.isspace(), False,
                                  "Extra blank lines appearing between reviews"
                                  "in console output")
+
+    @mock.patch('git_review.cmd._use_color', None)
+    def test_color_output_disabled(self):
+        """Test disabling of colour output color.ui defaults to enabled
+        """
+
+        # git versions < 1.8.4 default to 'color.ui' being false
+        # so must be set to auto to correctly test
+        self._run_git("config", "color.ui", "auto")
+
+        self._run_git("config", "color.review", "never")
+        self.assertFalse(cmd.check_use_color_output(),
+                         "Failed to detect color output disabled")
+
+    @mock.patch('git_review.cmd._use_color', None)
+    def test_color_output_forced(self):
+        """Test force enable of colour output when color.ui
+        is defaulted to false
+        """
+
+        self._run_git("config", "color.ui", "never")
+
+        self._run_git("config", "color.review", "always")
+        self.assertTrue(cmd.check_use_color_output(),
+                        "Failed to detect color output forcefully "
+                        "enabled")
+
+    @mock.patch('git_review.cmd._use_color', None)
+    def test_color_output_fallback(self):
+        """Test fallback to using color.ui when color.review is not
+        set
+        """
+
+        self._run_git("config", "color.ui", "always")
+        self.assertTrue(cmd.check_use_color_output(),
+                        "Failed to use fallback to color.ui when "
+                        "color.review not present")

git_review/tests/utils.py

@@ -54,9 +54,9 @@ def run_cmd(*args, **kwargs):
     return out.strip()
 
 
-def run_git(command, *args):
+def run_git(command, *args, **kwargs):
     """Run git command with the specified args."""
-    return run_cmd("git", command, *args)
+    return run_cmd("git", command, *args, **kwargs)
 
 
 def write_to_file(path, content):