From 7f69ada39684690028d2e1c1e0fd58825c5c879d Mon Sep 17 00:00:00 2001 From: Darragh Bailey Date: Sun, 13 Jul 2014 16:52:48 +0100 Subject: [PATCH] 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 | 14 +++++++ git_review/cmd.py | 60 ++++++++++++++++++-------- git_review/tests/test_unit.py | 79 +++++++++++++++++++++++++++++++---- git_review/tests/utils.py | 4 +- 4 files changed, 131 insertions(+), 26 deletions(-) diff --git a/git-review.1 b/git-review.1 index 2ef03ca..9941694 100644 --- a/git-review.1 +++ b/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 diff --git a/git_review/cmd.py b/git_review/cmd.py index eeb7b0e..12b0a3c 100755 --- a/git_review/cmd.py +++ b/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 has_color: - 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 + if check_use_color_output(): + use_color = "--color=always" 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="", + nargs="?", choices=["always", "never", "auto"], + help="Show color output. --color (without []) " + "is the same as --color=always. 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, diff --git a/git_review/tests/test_unit.py b/git_review/tests/test_unit.py index 5d6b612..98eaef6 100644 --- a/git_review/tests/test_unit.py +++ b/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") diff --git a/git_review/tests/utils.py b/git_review/tests/utils.py index b6b51a0..337f48a 100644 --- a/git_review/tests/utils.py +++ b/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):