Merge pull request #1343 from pre-commit/from_ref_to_ref

Make more readable --from-ref / --to-ref aliases for --source / --origin

pre_commit/commands/hook_impl.py

@@ -69,8 +69,8 @@ def _ns(
         color: bool,
         *,
         all_files: bool = False,
-        origin: Optional[str] = None,
-        source: Optional[str] = None,
+        from_ref: Optional[str] = None,
+        to_ref: Optional[str] = None,
         remote_name: Optional[str] = None,
         remote_url: Optional[str] = None,
         commit_msg_filename: Optional[str] = None,
@@ -79,8 +79,8 @@ def _ns(
     return argparse.Namespace(
         color=color,
         hook_stage=hook_type.replace('pre-', ''),
-        origin=origin,
-        source=source,
+        from_ref=from_ref,
+        to_ref=to_ref,
         remote_name=remote_name,
         remote_url=remote_url,
         commit_msg_filename=commit_msg_filename,
@@ -112,7 +112,7 @@ def _pre_push_ns(
         elif remote_sha != Z40 and _rev_exists(remote_sha):
             return _ns(
                 'pre-push', color,
-                origin=local_sha, source=remote_sha,
+                from_ref=remote_sha, to_ref=local_sha,
                 remote_name=remote_name, remote_url=remote_url,
             )
         else:
@@ -139,7 +139,7 @@ def _pre_push_ns(
                     source = subprocess.check_output(rev_cmd).decode().strip()
                     return _ns(
                         'pre-push', color,
-                        origin=local_sha, source=source,
+                        from_ref=source, to_ref=local_sha,
                         remote_name=remote_name, remote_url=remote_url,
                     )
 
@@ -161,8 +161,8 @@ def _run_ns(
         return _ns(hook_type, color)
     elif hook_type == 'post-checkout':
         return _ns(
-            hook_type, color, source=args[0], origin=args[1],
-            checkout_type=args[2],
+            hook_type, color,
+            from_ref=args[0], to_ref=args[1], checkout_type=args[2],
         )
     else:
         raise AssertionError(f'unexpected hook type: {hook_type}')

pre_commit/commands/run.py

@@ -215,8 +215,8 @@ def _compute_cols(hooks: Sequence[Hook]) -> int:
 
 
 def _all_filenames(args: argparse.Namespace) -> Collection[str]:
-    if args.origin and args.source:
-        return git.get_changed_files(args.origin, args.source)
+    if args.from_ref and args.to_ref:
+        return git.get_changed_files(args.from_ref, args.to_ref)
     elif args.hook_stage in {'prepare-commit-msg', 'commit-msg'}:
         return (args.commit_msg_filename,)
     elif args.files:
@@ -297,8 +297,8 @@ def run(
     if _has_unmerged_paths():
         logger.error('Unmerged files.  Resolve before committing.')
         return 1
-    if bool(args.source) != bool(args.origin):
-        logger.error('Specify both --origin and --source.')
+    if bool(args.from_ref) != bool(args.to_ref):
+        logger.error('Specify both --from-ref and --to-ref.')
         return 1
     if stash and _has_unstaged_config(config_file):
         logger.error(
@@ -316,10 +316,14 @@ def run(
         )
         return 1
 
-    # Expose origin / source as environment variables for hooks to consume
-    if args.origin and args.source:
-        environ['PRE_COMMIT_ORIGIN'] = args.origin
-        environ['PRE_COMMIT_SOURCE'] = args.source
+    # Expose from-ref / to-ref as environment variables for hooks to consume
+    if args.from_ref and args.to_ref:
+        # legacy names
+        environ['PRE_COMMIT_ORIGIN'] = args.from_ref
+        environ['PRE_COMMIT_SOURCE'] = args.to_ref
+        # new names
+        environ['PRE_COMMIT_FROM_REF'] = args.from_ref
+        environ['PRE_COMMIT_TO_REF'] = args.to_ref
 
     if args.remote_name and args.remote_url:
         environ['PRE_COMMIT_REMOTE_NAME'] = args.remote_name

pre_commit/git.py

@@ -129,7 +129,7 @@ def get_all_files() -> List[str]:
     return zsplit(cmd_output('git', 'ls-files', '-z')[1])
 
 
-def get_changed_files(new: str, old: str) -> List[str]:
+def get_changed_files(old: str, new: str) -> List[str]:
     return zsplit(
         cmd_output(
             'git', 'diff', '--name-only', '--no-ext-diff', '-z',

pre_commit/main.py

@@ -90,18 +90,42 @@ def _add_hook_type_option(parser: argparse.ArgumentParser) -> None:
 def _add_run_options(parser: argparse.ArgumentParser) -> None:
     parser.add_argument('hook', nargs='?', help='A single hook-id to run')
     parser.add_argument('--verbose', '-v', action='store_true', default=False)
+    mutex_group = parser.add_mutually_exclusive_group(required=False)
+    mutex_group.add_argument(
+        '--all-files', '-a', action='store_true', default=False,
+        help='Run on all the files in the repo.',
+    )
+    mutex_group.add_argument(
+        '--files', nargs='*', default=[],
+        help='Specific filenames to run hooks on.',
+    )
+    parser.add_argument(
+        '--show-diff-on-failure', action='store_true',
+        help='When hooks fail, run `git diff` directly afterward.',
+    )
+    parser.add_argument(
+        '--hook-stage', choices=C.STAGES, default='commit',
+        help='The stage during which the hook is fired.  One of %(choices)s',
+    )
     parser.add_argument(
-        '--origin', '-o',
+        '--from-ref', '--source', '-s',
         help=(
-            "The origin branch's commit_id when using `git push`.  "
-            'The ref of the previous HEAD when using `git checkout`.'
+            '(for usage with `--from-ref`) -- this option represents the '
+            'destination ref in a `from_ref...to_ref` diff expression.  '
+            'For `pre-push` hooks, this represents the branch being pushed.  '
+            'For `post-checkout` hooks, this represents the branch that is '
+            'now checked out.'
         ),
     )
     parser.add_argument(
-        '--source', '-s',
+        '--to-ref', '--origin', '-o',
         help=(
-            "The remote branch's commit_id when using `git push`.  "
-            'The ref of the new HEAD when using `git checkout`.'
+            '(for usage with `--to-ref`) -- this option represents the '
+            'original ref in a `from_ref...to_ref` diff expression.  '
+            'For `pre-push` hooks, this represents the branch you are pushing '
+            'to.  '
+            'For `post-checkout` hooks, this represents the branch which was '
+            'previously checked out.'
         ),
     )
     parser.add_argument(
@@ -112,23 +136,6 @@ def _add_run_options(parser: argparse.ArgumentParser) -> None:
         '--remote-name', help='Remote name used by `git push`.',
     )
     parser.add_argument('--remote-url', help='Remote url used by `git push`.')
-    parser.add_argument(
-        '--hook-stage', choices=C.STAGES, default='commit',
-        help='The stage during which the hook is fired.  One of %(choices)s',
-    )
-    parser.add_argument(
-        '--show-diff-on-failure', action='store_true',
-        help='When hooks fail, run `git diff` directly afterward.',
-    )
-    mutex_group = parser.add_mutually_exclusive_group(required=False)
-    mutex_group.add_argument(
-        '--all-files', '-a', action='store_true', default=False,
-        help='Run on all the files in the repo.',
-    )
-    mutex_group.add_argument(
-        '--files', nargs='*', default=[],
-        help='Specific filenames to run hooks on.',
-    )
     parser.add_argument(
         '--checkout-type',
         help=(

testing/util.py

@@ -65,8 +65,8 @@ def run_opts(
         color=False,
         verbose=False,
         hook=None,
-        origin='',
-        source='',
+        from_ref='',
+        to_ref='',
         remote_name='',
         remote_url='',
         hook_stage='commit',
@@ -82,8 +82,8 @@ def run_opts(
         color=color,
         verbose=verbose,
         hook=hook,
-        origin=origin,
-        source=source,
+        from_ref=from_ref,
+        to_ref=to_ref,
         remote_name=remote_name,
         remote_url=remote_url,
         hook_stage=hook_stage,

tests/commands/hook_impl_test.py

@@ -109,8 +109,8 @@ def test_run_ns_post_checkout():
     assert ns is not None
     assert ns.hook_stage == 'post-checkout'
     assert ns.color is True
-    assert ns.source == 'a'
-    assert ns.origin == 'b'
+    assert ns.from_ref == 'a'
+    assert ns.to_ref == 'b'
     assert ns.checkout_type == 'c'
 
 
@@ -140,8 +140,8 @@ def test_run_ns_pre_push_updating_branch(push_example):
     assert ns.color is False
     assert ns.remote_name == 'origin'
     assert ns.remote_url == src
-    assert ns.source == src_head
-    assert ns.origin == clone_head
+    assert ns.from_ref == src_head
+    assert ns.to_ref == clone_head
     assert ns.all_files is False
 
 
@@ -154,8 +154,8 @@ def test_run_ns_pre_push_new_branch(push_example):
         ns = hook_impl._run_ns('pre-push', False, args, stdin)
 
     assert ns is not None
-    assert ns.source == src_head
-    assert ns.origin == clone_head
+    assert ns.from_ref == src_head
+    assert ns.to_ref == clone_head
 
 
 def test_run_ns_pre_push_new_branch_existing_rev(push_example):

tests/commands/run_test.py

@@ -446,29 +446,29 @@ def test_hook_verbose_enabled(cap_out, store, repo_with_passing_hook):
 
 
 @pytest.mark.parametrize(
-    ('origin', 'source'), (('master', ''), ('', 'master')),
+    ('from_ref', 'to_ref'), (('master', ''), ('', 'master')),
 )
-def test_origin_source_error_msg_error(
-        cap_out, store, repo_with_passing_hook, origin, source,
+def test_from_ref_to_ref_error_msg_error(
+        cap_out, store, repo_with_passing_hook, from_ref, to_ref,
 ):
-    args = run_opts(origin=origin, source=source)
+    args = run_opts(from_ref=from_ref, to_ref=to_ref)
     ret, printed = _do_run(cap_out, store, repo_with_passing_hook, args)
     assert ret == 1
-    assert b'Specify both --origin and --source.' in printed
+    assert b'Specify both --from-ref and --to-ref.' in printed
 
 
 def test_all_push_options_ok(cap_out, store, repo_with_passing_hook):
     args = run_opts(
-        origin='master', source='master',
+        from_ref='master', to_ref='master',
         remote_name='origin', remote_url='https://example.com/repo',
     )
     ret, printed = _do_run(cap_out, store, repo_with_passing_hook, args)
     assert ret == 0
-    assert b'Specify both --origin and --source.' not in printed
+    assert b'Specify both --from-ref and --to-ref.' not in printed
 
 
 def test_checkout_type(cap_out, store, repo_with_passing_hook):
-    args = run_opts(origin='', source='', checkout_type='1')
+    args = run_opts(from_ref='', to_ref='', checkout_type='1')
     environ: EnvironT = {}
     ret, printed = _do_run(
         cap_out, store, repo_with_passing_hook, args, environ,

tests/git_test.py

@@ -100,11 +100,11 @@ def test_get_changed_files(in_git_dir):
     in_git_dir.join('b.txt').ensure()
     cmd_output('git', 'add', '.')
     git_commit()
-    files = git.get_changed_files('HEAD', 'HEAD^')
+    files = git.get_changed_files('HEAD^', 'HEAD')
     assert files == ['a.txt', 'b.txt']
 
     # files changed in source but not in origin should not be returned
-    files = git.get_changed_files('HEAD^', 'HEAD')
+    files = git.get_changed_files('HEAD', 'HEAD^')
     assert files == []
 
 
@@ -142,7 +142,7 @@ def test_staged_files_non_ascii(non_ascii_repo):
 
 
 def test_changed_files_non_ascii(non_ascii_repo):
-    ret = git.get_changed_files('HEAD', 'HEAD^')
+    ret = git.get_changed_files('HEAD^', 'HEAD')
     assert ret == ['интервью']