diff --git a/git-review.1 b/git-review.1 index 2ef03ca7..99416940 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 eeb7b0ee..12b0a3c0 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 5d6b612c..98eaef61 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 b6b51a06..337f48aa 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):