Simplify a few things

pre_commit/commands/install_uninstall.py

@@ -9,6 +9,7 @@ import stat
 import sys
 
 from pre_commit.logging_handler import LoggingHandler
+from pre_commit.util import resource_filename
 
 
 logger = logging.getLogger('pre_commit')
@@ -41,19 +42,10 @@ def make_executable(filename):
     )
 
 
-def get_hook_path(runner, hook_type):
-    if hook_type == 'pre-commit':
-        hook_path = runner.pre_commit_path
-        legacy_path = runner.pre_commit_legacy_path
-    else:
-        hook_path = runner.pre_push_path
-        legacy_path = runner.pre_push_legacy_path
-    return hook_path, legacy_path
-
-
 def install(runner, overwrite=False, hooks=False, hook_type='pre-commit'):
     """Install the pre-commit hooks."""
-    hook_path, legacy_path = get_hook_path(runner, hook_type)
+    hook_path = runner.get_hook_path(hook_type)
+    legacy_path = hook_path + '.legacy'
 
     # If we have an existing hook, move it to pre-commit.legacy
     if (
@@ -76,12 +68,12 @@ def install(runner, overwrite=False, hooks=False, hook_type='pre-commit'):
 
     with io.open(hook_path, 'w') as pre_commit_file_obj:
         if hook_type == 'pre-push':
-            with io.open(runner.pre_push_template) as fp:
+            with io.open(resource_filename('pre-push-tmpl')) as fp:
                 pre_push_contents = fp.read()
         else:
             pre_push_contents = ''
 
-        contents = io.open(runner.pre_template).read().format(
+        contents = io.open(resource_filename('hook-tmpl')).read().format(
             sys_executable=sys.executable,
             hook_type=hook_type,
             pre_push=pre_push_contents,
@@ -104,7 +96,8 @@ def install(runner, overwrite=False, hooks=False, hook_type='pre-commit'):
 
 def uninstall(runner, hook_type='pre-commit'):
     """Uninstall the pre-commit hooks."""
-    hook_path, legacy_path = get_hook_path(runner, hook_type)
+    hook_path = runner.get_hook_path(hook_type)
+    legacy_path = hook_path + '.legacy'
     # If our file doesn't exist or it isn't ours, gtfo.
     if (
         not os.path.exists(hook_path) or (

pre_commit/commands/run.py

@@ -50,18 +50,16 @@ def _print_user_skipped(hook, write, args):
 
 
 def get_changed_files(new, old):
-    changed_files = cmd_output(
+    return cmd_output(
         'git', 'diff', '--name-only', '{0}..{1}'.format(old, new),
     )[1].splitlines()
-    for f in changed_files:
-        if f:
-            yield f
 
 
 def _run_single_hook(runner, repository, hook, args, write, skips=set()):
     if args.origin and args.source:
         get_filenames = git.get_files_matching(
-            lambda: get_changed_files(args.origin, args.source))
+            lambda: get_changed_files(args.origin, args.source),
+        )
     elif args.files:
         get_filenames = git.get_files_matching(lambda: args.files)
     elif args.all_files:
@@ -150,9 +148,8 @@ def run(runner, args, write=sys_stdout_write_wrapper, environ=os.environ):
     if _has_unmerged_paths(runner):
         logger.error('Unmerged files.  Resolve before committing.')
         return 1
-    if (args.source and not args.origin) or \
+    if bool(args.source) != bool(args.origin):
-       (args.origin and not args.source):
+        logger.error('Specify both --origin and --source.')
-        logger.error('--origin and --source depend on each other.')
         return 1
 
     # Don't stash if specified or files are specified

pre_commit/main.py

@@ -79,11 +79,11 @@ def main(argv=None):
     )
 
     run_parser.add_argument(
-        '--origin', '-o', default='',
+        '--origin', '-o',
         help='The origin branch"s commit_id when using `git push`',
     )
     run_parser.add_argument(
-        '--source', '-s', default='',
+        '--source', '-s',
         help='The remote branch"s commit_id when using `git push`',
     )
     run_mutex_group = run_parser.add_mutually_exclusive_group(required=False)

pre_commit/runner.py

@@ -10,7 +10,6 @@ from pre_commit import git
 from pre_commit.clientlib.validate_config import load_config
 from pre_commit.repository import Repository
 from pre_commit.store import Store
-from pre_commit.util import resource_filename
 
 
 class Runner(object):
@@ -55,28 +54,6 @@ class Runner(object):
     def pre_push_path(self):
         return self.get_hook_path('pre-push')
 
-    @cached_property
-    def pre_template(self):
-        return resource_filename('hook-tmpl')
-
-    @cached_property
-    def pre_push_template(self):
-        return resource_filename('pre-push-tmpl')
-
-    @property
-    def pre_commit_legacy_path(self):
-        """The path in the 'hooks' directory representing the temporary
-        storage for existing pre-commit hooks.
-        """
-        return self.pre_commit_path + '.legacy'
-
-    @property
-    def pre_push_legacy_path(self):
-        """The path in the 'hooks' directory representing the temporary
-        storage for existing pre-push hooks.
-        """
-        return self.pre_push_path + '.legacy'
-
     @cached_property
     def cmd_runner(self):
         # TODO: remove this and inline runner.store.cmd_runner

tests/commands/install_uninstall_test.py

@@ -10,7 +10,7 @@ import subprocess
 import sys
 
 import mock
-from pre_commit.commands.install_uninstall import get_hook_path
+
 from pre_commit.commands.install_uninstall import IDENTIFYING_HASH
 from pre_commit.commands.install_uninstall import install
 from pre_commit.commands.install_uninstall import is_our_pre_commit
@@ -22,7 +22,6 @@ from pre_commit.runner import Runner
 from pre_commit.util import cmd_output
 from pre_commit.util import cwd
 from pre_commit.util import resource_filename
-
 from testing.fixtures import git_dir
 from testing.fixtures import make_consuming_repo
 
@@ -71,11 +70,12 @@ def test_install_pre_commit(tmpdir_factory):
     assert ret == 0
     assert os.path.exists(runner.pre_push_path)
     pre_push_contents = io.open(runner.pre_push_path).read()
-    pre_push_template_contents = io.open(runner.pre_push_template).read()
+    pre_push_tmpl = resource_filename('pre-push-tmpl')
+    pre_push_template_contents = io.open(pre_push_tmpl).read()
     expected_contents = io.open(pre_commit_script).read().format(
         sys_executable=sys.executable,
         hook_type='pre-push',
-        pre_push=pre_push_template_contents
+        pre_push=pre_push_template_contents,
     )
     assert pre_push_contents == expected_contents
 
@@ -406,17 +406,3 @@ def test_installed_from_venv(tmpdir_factory):
         )
         assert ret == 0
         assert NORMAL_PRE_COMMIT_RUN.match(output)
-
-
-def test_get_hook_path(tmpdir_factory):
-    path = git_dir(tmpdir_factory)
-    with cwd(path):
-        runner = Runner(path)
-        expected_paths = (os.path.join(path, '.git/hooks/pre-commit'),
-                          os.path.join(path, '.git/hooks/pre-commit.legacy')
-                          )
-        assert expected_paths == get_hook_path(runner, 'pre-commit')
-        expected_paths = (os.path.join(path, '.git/hooks/pre-push'),
-                          os.path.join(path, '.git/hooks/pre-push.legacy')
-                          )
-        assert expected_paths == get_hook_path(runner, 'pre-push')

tests/commands/run_test.py

@@ -132,20 +132,21 @@ def test_run(
 
 
 @pytest.mark.parametrize(
-    ('origin', 'source', 'expect_stash'),
+    ('origin', 'source', 'expect_failure'),
     (
         ('master', 'master', False),
         ('master', '', True),
         ('', 'master', True),
     )
 )
-def test_origin_source_define(
+def test_origin_source_error_msg(
-        repo_with_passing_hook, origin, source, expect_stash,
+        repo_with_passing_hook, origin, source, expect_failure,
-        mock_out_store_directory):
+        mock_out_store_directory,
+):
     args = _get_opts(origin=origin, source=source)
     ret, printed = _do_run(repo_with_passing_hook, args)
-    warning_msg = '--origin and --source depend on each other.'
+    warning_msg = 'Specify both --origin and --source.'
-    if expect_stash:
+    if expect_failure:
         assert ret == 1
         assert warning_msg in printed
     else:
@@ -297,7 +298,8 @@ def test_stdout_write_bug_py26(
 
 
 def test_get_changed_files():
-    files = list(get_changed_files('78c682a1d13ba20e7cb735313b9314a74365cd3a',
+    files = get_changed_files(
-                                   '3387edbb1288a580b37fe25225aa0b856b18ad1a'
+        '78c682a1d13ba20e7cb735313b9314a74365cd3a',
-                                   ))
+        '3387edbb1288a580b37fe25225aa0b856b18ad1a',
+    )
     assert files == ['CHANGELOG.md', 'setup.py']

tests/runner_test.py

@@ -7,7 +7,6 @@ import os.path
 import pre_commit.constants as C
 from pre_commit.runner import Runner
 from pre_commit.util import cwd
-from pre_commit.util import resource_filename
 from testing.fixtures import git_dir
 from testing.fixtures import make_consuming_repo
 
@@ -65,28 +64,6 @@ def test_pre_push_path():
     assert runner.pre_push_path == expected_path
 
 
-def test_pre_commit_legacy_path():
-    runner = Runner('foo/bar')
-    expected_path = os.path.join('foo/bar', '.git/hooks/pre-commit.legacy')
-    assert runner.pre_commit_legacy_path == expected_path
-
-
-def test_pre_push_legacy_path():
-    runner = Runner('foo/bar')
-    expected_path = os.path.join('foo/bar', '.git/hooks/pre-push.legacy')
-    assert runner.pre_push_legacy_path == expected_path
-
-
-def test_pre_template():
-    runner = Runner('foo/bar')
-    assert runner.pre_template == resource_filename('hook-tmpl')
-
-
-def test_pre_push_template():
-    runner = Runner('foo/bar')
-    assert runner.pre_push_template == resource_filename('pre-push-tmpl')
-
-
 def test_cmd_runner(mock_out_store_directory):
     runner = Runner('foo/bar')
     ret = runner.cmd_runner