Merge "Enable color support based on tty and config"

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