From d35b00352fcdb33a51facbaf0bfe08da0fd3aca5 Mon Sep 17 00:00:00 2001 From: Anthony Sottile Date: Sun, 23 Feb 2020 11:07:57 -0800 Subject: [PATCH] Make more readable --from-ref / --to-ref aliases for --source / --origin --- pre_commit/commands/hook_impl.py | 16 +++++----- pre_commit/commands/run.py | 20 +++++++----- pre_commit/git.py | 2 +- pre_commit/main.py | 53 ++++++++++++++++++-------------- testing/util.py | 8 ++--- tests/commands/hook_impl_test.py | 12 ++++---- tests/commands/run_test.py | 16 +++++----- tests/git_test.py | 6 ++-- 8 files changed, 72 insertions(+), 61 deletions(-) diff --git a/pre_commit/commands/hook_impl.py b/pre_commit/commands/hook_impl.py index 890cedb5..5ff4555e 100644 --- a/pre_commit/commands/hook_impl.py +++ b/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}') diff --git a/pre_commit/commands/run.py b/pre_commit/commands/run.py index 4f2ead78..43bcabad 100644 --- a/pre_commit/commands/run.py +++ b/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 diff --git a/pre_commit/git.py b/pre_commit/git.py index edde4b08..7e757f24 100644 --- a/pre_commit/git.py +++ b/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', diff --git a/pre_commit/main.py b/pre_commit/main.py index 47dd73a5..77833447 100644 --- a/pre_commit/main.py +++ b/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( - '--origin', '-o', + '--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( + '--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=( diff --git a/testing/util.py b/testing/util.py index 2875993c..439bee79 100644 --- a/testing/util.py +++ b/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, diff --git a/tests/commands/hook_impl_test.py b/tests/commands/hook_impl_test.py index 556ea363..032fa8fa 100644 --- a/tests/commands/hook_impl_test.py +++ b/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): diff --git a/tests/commands/run_test.py b/tests/commands/run_test.py index f48f71b3..63129ff5 100644 --- a/tests/commands/run_test.py +++ b/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, diff --git a/tests/git_test.py b/tests/git_test.py index 4a5bfb9b..e73a6f24 100644 --- a/tests/git_test.py +++ b/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 == ['интервью']