Merge pull request #89 from pre-commit/display_hook_ids_88

Include hook id in output with --verbose.  Closes #88.

pre_commit/commands.py

@@ -5,7 +5,6 @@ import os
 import pkg_resources
 import shutil
 import stat
-import subprocess
 import sys
 from asottile.ordereddict import OrderedDict
 from asottile.yaml import ordered_dump
@@ -19,6 +18,7 @@ from pre_commit.clientlib.validate_config import CONFIG_JSON_SCHEMA
 from pre_commit.clientlib.validate_config import load_config
 from pre_commit.jsonschema_extensions import remove_defaults
 from pre_commit.logging_handler import LoggingHandler
+from pre_commit.output import get_hook_message
 from pre_commit.repository import Repository
 from pre_commit.staged_files_only import staged_files_only
 from pre_commit.util import noop_context
@@ -26,10 +26,6 @@ from pre_commit.util import noop_context
 
 logger = logging.getLogger('pre_commit')
 
-COLS = int(subprocess.Popen(['tput', 'cols'], stdout=subprocess.PIPE).communicate()[0])
-
-PASS_FAIL_LENGTH = 6
-
 
 def install(runner):
     """Install the pre-commit hooks."""
@@ -107,7 +103,8 @@ def autoupdate(runner):
     )
 
     for repo_config in input_configs:
-        print('Updating {0}...'.format(repo_config['repo']), end='')
+        sys.stdout.write('Updating {0}...'.format(repo_config['repo']))
+        sys.stdout.flush()
         try:
             new_repo_config = _update_repository(repo_config)
         except RepositoryCannotBeUpdatedError as error:
@@ -149,34 +146,30 @@ def _get_skips(environ):
     return set(skip.strip() for skip in skips.split(',') if skip.strip())
 
 
-def _print_no_files_skipped(hook, write, args):
+def _hook_msg_start(hook, verbose):
-    no_files_msg = '(no files to check) '
+    return '{0}{1}'.format(
-    skipped_msg = 'Skipped'
+        '[{0}] '.format(hook['id']) if verbose else '',
-    write(
-        '{0}{1}{2}{3}\n'.format(
         hook['name'],
-            '.' * (
-                COLS -
-                len(hook['name']) -
-                len(no_files_msg) -
-                len(skipped_msg) -
-                1
-            ),
-            no_files_msg,
-            color.format_color(skipped_msg, color.TURQUOISE, args.color),
-        )
     )
 
 
+def _print_no_files_skipped(hook, write, args):
+    write(get_hook_message(
+        _hook_msg_start(hook, args.verbose),
+        postfix='(no files to check) ',
+        end_msg='Skipped',
+        end_color=color.TURQUOISE,
+        use_color=args.color,
+    ))
+
+
 def _print_user_skipped(hook, write, args):
-    skipped_msg = 'Skipped'
+    write(get_hook_message(
-    write(
+        _hook_msg_start(hook, args.verbose),
-        '{0}{1}{2}\n'.format(
+        end_msg='Skipped',
-            hook['name'],
+        end_color=color.YELLOW,
-            '.' * (COLS - len(hook['name']) - len(skipped_msg) - 1),
+        use_color=args.color,
-            color.format_color(skipped_msg, color.YELLOW, args.color),
+    ))
-        ),
-    )
 
 
 def _run_single_hook(runner, repository, hook_id, args, write, skips=set()):
@@ -199,12 +192,7 @@ def _run_single_hook(runner, repository, hook_id, args, write, skips=set()):
 
     # Print the hook and the dots first in case the hook takes hella long to
     # run.
-    write(
+    write(get_hook_message(_hook_msg_start(hook, args.verbose), end_len=6))
-        '{0}{1}'.format(
-            hook['name'],
-            '.' * (COLS - len(hook['name']) - PASS_FAIL_LENGTH - 1),
-        ),
-    )
     sys.stdout.flush()
 
     retcode, stdout, stderr = repository.run_hook(

pre_commit/output.py

@@ -0,0 +1,66 @@
+import subprocess
+
+from pre_commit import color
+
+
+# TODO: smell: import side-effects
+COLS = int(
+    subprocess.Popen(['tput', 'cols'], stdout=subprocess.PIPE).communicate()[0]
+)
+
+
+def get_hook_message(
+    start,
+    postfix='',
+    end_msg=None,
+    end_len=0,
+    end_color=None,
+    use_color=None,
+    cols=COLS,
+):
+    """Prints a message for running a hook.
+
+    This currently supports three approaches:
+
+    # Print `start` followed by dots, leaving 6 characters at the end
+    >>> print_hook_message('start', end_len=6)
+    start...............................................................
+
+    # Print `start` followed by dots with the end message colored if coloring is
+    # specified and a newline afterwards
+    >>> print_hook_message(
+        'start',
+        end_msg='end',
+        end_color=color.RED,
+        use_color=True,
+    )
+    start...................................................................end
+
+    # Print `start` followed by dots, followed by the `postfix` message
+    # uncolored, followed by the `end_msg` colored if specified and a newline
+    # afterwards
+    >>> print_hook_message(
+        'start',
+        postfix='postfix ',
+        end_msg='end',
+        end_color=color.RED,
+        use_color=True,
+    )
+    start...........................................................postfix end
+    """
+    if bool(end_msg) == bool(end_len):
+        raise ValueError('Expected one of (`end_msg`, `end_len`)')
+    if end_msg is not None and (end_color is None or use_color is None):
+        raise ValueError(
+            '`end_color` and `use_color` are required with `end_msg`'
+        )
+
+    if end_len:
+        return start + '.' * (cols - len(start) - end_len - 1)
+    else:
+        return '{0}{1}{2}{3}\n'.format(
+            start,
+            '.' * (cols - len(start) - len(postfix) - len(end_msg) - 1),
+            postfix,
+            color.format_color(end_msg, end_color, use_color),
+        )

tests/commands_test.py

@@ -330,3 +330,13 @@ def test_skip_hook(repo_with_passing_hook):
     )
     for msg in ('Bash hook', 'Skipped'):
         assert msg in printed
+
+
+def test_hook_id_not_in_non_verbose_output(repo_with_passing_hook):
+    ret, printed = _do_run(repo_with_passing_hook, _get_opts(verbose=False))
+    assert '[bash_hook]' not in printed
+
+
+def test_hook_id_in_verbose_output(repo_with_passing_hook):
+    ret, printed = _do_run(repo_with_passing_hook, _get_opts(verbose=True))
+    assert '[bash_hook] Bash hook' in printed

tests/output_test.py

@@ -0,0 +1,77 @@
+import pytest
+
+from pre_commit import color
+from pre_commit.output import get_hook_message
+
+
+@pytest.mark.parametrize(
+    'kwargs',
+    (
+        # both end_msg and end_len
+        {'end_msg': 'end', 'end_len': 1, 'end_color': '', 'use_color': True},
+        # Neither end_msg nor end_len
+        {},
+        # Neither color option for end_msg
+        {'end_msg': 'end'},
+        # No use_color for end_msg
+        {'end_msg': 'end', 'end_color': ''},
+        # No end_color for end_msg
+        {'end_msg': 'end', 'use_color': ''},
+    ),
+)
+def test_get_hook_message_raises(kwargs):
+    with pytest.raises(ValueError):
+        get_hook_message('start', **kwargs)
+
+
+def test_case_with_end_len():
+    ret = get_hook_message('start', end_len=5, cols=15)
+    assert ret == 'start' + '.' * 4
+
+
+def test_case_with_end_msg():
+    ret = get_hook_message(
+        'start',
+        end_msg='end',
+        end_color='',
+        use_color=False,
+        cols=15,
+    )
+    assert ret == 'start' + '.' * 6 + 'end' + '\n'
+
+
+def test_case_with_end_msg_using_color():
+    ret = get_hook_message(
+        'start',
+        end_msg='end',
+        end_color=color.RED,
+        use_color=True,
+        cols=15,
+    )
+    assert ret == 'start' + '.' * 6 + color.RED + 'end' + color.NORMAL + '\n'
+
+
+def test_case_with_postfix_message():
+    ret = get_hook_message(
+        'start',
+        postfix='post ',
+        end_msg='end',
+        end_color='',
+        use_color=False,
+        cols=20,
+    )
+    assert ret == 'start' + '.' * 6 + 'post ' + 'end' + '\n'
+
+
+def test_make_sure_postfix_is_not_colored():
+    ret = get_hook_message(
+        'start',
+        postfix='post ',
+        end_msg='end',
+        end_color=color.RED,
+        use_color=True,
+        cols=20,
+    )
+    assert ret == (
+        'start' + '.' * 6 + 'post ' + color.RED + 'end' + color.NORMAL + '\n'
+    )