Skip to content

Commit d35b003

Browse files
asottileasottile
committed
Make more readable --from-ref / --to-ref aliases for --source / --origin
1 parent 566f1af commit d35b003

File tree

8 files changed

+72
-61
lines changed

8 files changed

+72
-61
lines changed

‎pre_commit/commands/hook_impl.py‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -69,8 +69,8 @@ def _ns(
6969
color: bool,
7070
*,
7171
all_files: bool = False,
72-
origin: Optional[str] = None,
73-
source: Optional[str] = None,
72+
from_ref: Optional[str] = None,
73+
to_ref: Optional[str] = None,
7474
remote_name: Optional[str] = None,
7575
remote_url: Optional[str] = None,
7676
commit_msg_filename: Optional[str] = None,
@@ -79,8 +79,8 @@ def _ns(
7979
return argparse.Namespace(
8080
color=color,
8181
hook_stage=hook_type.replace('pre-', ''),
82-
origin=origin,
83-
source=source,
82+
from_ref=from_ref,
83+
to_ref=to_ref,
8484
remote_name=remote_name,
8585
remote_url=remote_url,
8686
commit_msg_filename=commit_msg_filename,
@@ -112,7 +112,7 @@ def _pre_push_ns(
112112
elif remote_sha != Z40 and _rev_exists(remote_sha):
113113
return _ns(
114114
'pre-push', color,
115-
origin=local_sha, source=remote_sha,
115+
from_ref=remote_sha, to_ref=local_sha,
116116
remote_name=remote_name, remote_url=remote_url,
117117
)
118118
else:
@@ -139,7 +139,7 @@ def _pre_push_ns(
139139
source = subprocess.check_output(rev_cmd).decode().strip()
140140
return _ns(
141141
'pre-push', color,
142-
origin=local_sha, source=source,
142+
from_ref=source, to_ref=local_sha,
143143
remote_name=remote_name, remote_url=remote_url,
144144
)
145145

@@ -161,8 +161,8 @@ def _run_ns(
161161
return _ns(hook_type, color)
162162
elif hook_type == 'post-checkout':
163163
return _ns(
164-
hook_type, color, source=args[0], origin=args[1],
165-
checkout_type=args[2],
164+
hook_type, color,
165+
from_ref=args[0], to_ref=args[1], checkout_type=args[2],
166166
)
167167
else:
168168
raise AssertionError(f'unexpected hook type: {hook_type}')

‎pre_commit/commands/run.py‎

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -215,8 +215,8 @@ def _compute_cols(hooks: Sequence[Hook]) -> int:
215215

216216

217217
def _all_filenames(args: argparse.Namespace) -> Collection[str]:
218-
if args.origin and args.source:
219-
return git.get_changed_files(args.origin, args.source)
218+
if args.from_ref and args.to_ref:
219+
return git.get_changed_files(args.from_ref, args.to_ref)
220220
elif args.hook_stage in {'prepare-commit-msg', 'commit-msg'}:
221221
return (args.commit_msg_filename,)
222222
elif args.files:
@@ -297,8 +297,8 @@ def run(
297297
if _has_unmerged_paths():
298298
logger.error('Unmerged files. Resolve before committing.')
299299
return 1
300-
if bool(args.source) != bool(args.origin):
301-
logger.error('Specify both --origin and --source.')
300+
if bool(args.from_ref) != bool(args.to_ref):
301+
logger.error('Specify both --from-ref and --to-ref.')
302302
return 1
303303
if stash and _has_unstaged_config(config_file):
304304
logger.error(
@@ -316,10 +316,14 @@ def run(
316316
)
317317
return 1
318318

319-
# Expose origin / source as environment variables for hooks to consume
320-
if args.origin and args.source:
321-
environ['PRE_COMMIT_ORIGIN'] = args.origin
322-
environ['PRE_COMMIT_SOURCE'] = args.source
319+
# Expose from-ref / to-ref as environment variables for hooks to consume
320+
if args.from_ref and args.to_ref:
321+
# legacy names
322+
environ['PRE_COMMIT_ORIGIN'] = args.from_ref
323+
environ['PRE_COMMIT_SOURCE'] = args.to_ref
324+
# new names
325+
environ['PRE_COMMIT_FROM_REF'] = args.from_ref
326+
environ['PRE_COMMIT_TO_REF'] = args.to_ref
323327

324328
if args.remote_name and args.remote_url:
325329
environ['PRE_COMMIT_REMOTE_NAME'] = args.remote_name

‎pre_commit/git.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,7 @@ def get_all_files() -> List[str]:
129129
return zsplit(cmd_output('git', 'ls-files', '-z')[1])
130130

131131

132-
def get_changed_files(new: str, old: str) -> List[str]:
132+
def get_changed_files(old: str, new: str) -> List[str]:
133133
return zsplit(
134134
cmd_output(
135135
'git', 'diff', '--name-only', '--no-ext-diff', '-z',

‎pre_commit/main.py‎

Lines changed: 30 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -90,18 +90,42 @@ def _add_hook_type_option(parser: argparse.ArgumentParser) -> None:
9090
def _add_run_options(parser: argparse.ArgumentParser) -> None:
9191
parser.add_argument('hook', nargs='?', help='A single hook-id to run')
9292
parser.add_argument('--verbose', '-v', action='store_true', default=False)
93+
mutex_group = parser.add_mutually_exclusive_group(required=False)
94+
mutex_group.add_argument(
95+
'--all-files', '-a', action='store_true', default=False,
96+
help='Run on all the files in the repo.',
97+
)
98+
mutex_group.add_argument(
99+
'--files', nargs='*', default=[],
100+
help='Specific filenames to run hooks on.',
101+
)
102+
parser.add_argument(
103+
'--show-diff-on-failure', action='store_true',
104+
help='When hooks fail, run `git diff` directly afterward.',
105+
)
106+
parser.add_argument(
107+
'--hook-stage', choices=C.STAGES, default='commit',
108+
help='The stage during which the hook is fired. One of %(choices)s',
109+
)
93110
parser.add_argument(
94-
'--origin', '-o',
111+
'--from-ref', '--source', '-s',
95112
help=(
96-
"The origin branch's commit_id when using `git push`. "
97-
'The ref of the previous HEAD when using `git checkout`.'
113+
'(for usage with `--from-ref`) -- this option represents the '
114+
'destination ref in a `from_ref...to_ref` diff expression. '
115+
'For `pre-push` hooks, this represents the branch being pushed. '
116+
'For `post-checkout` hooks, this represents the branch that is '
117+
'now checked out.'
98118
),
99119
)
100120
parser.add_argument(
101-
'--source', '-s',
121+
'--to-ref', '--origin', '-o',
102122
help=(
103-
"The remote branch's commit_id when using `git push`. "
104-
'The ref of the new HEAD when using `git checkout`.'
123+
'(for usage with `--to-ref`) -- this option represents the '
124+
'original ref in a `from_ref...to_ref` diff expression. '
125+
'For `pre-push` hooks, this represents the branch you are pushing '
126+
'to. '
127+
'For `post-checkout` hooks, this represents the branch which was '
128+
'previously checked out.'
105129
),
106130
)
107131
parser.add_argument(
@@ -112,23 +136,6 @@ def _add_run_options(parser: argparse.ArgumentParser) -> None:
112136
'--remote-name', help='Remote name used by `git push`.',
113137
)
114138
parser.add_argument('--remote-url', help='Remote url used by `git push`.')
115-
parser.add_argument(
116-
'--hook-stage', choices=C.STAGES, default='commit',
117-
help='The stage during which the hook is fired. One of %(choices)s',
118-
)
119-
parser.add_argument(
120-
'--show-diff-on-failure', action='store_true',
121-
help='When hooks fail, run `git diff` directly afterward.',
122-
)
123-
mutex_group = parser.add_mutually_exclusive_group(required=False)
124-
mutex_group.add_argument(
125-
'--all-files', '-a', action='store_true', default=False,
126-
help='Run on all the files in the repo.',
127-
)
128-
mutex_group.add_argument(
129-
'--files', nargs='*', default=[],
130-
help='Specific filenames to run hooks on.',
131-
)
132139
parser.add_argument(
133140
'--checkout-type',
134141
help=(

‎testing/util.py‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -65,8 +65,8 @@ def run_opts(
6565
color=False,
6666
verbose=False,
6767
hook=None,
68-
origin='',
69-
source='',
68+
from_ref='',
69+
to_ref='',
7070
remote_name='',
7171
remote_url='',
7272
hook_stage='commit',
@@ -82,8 +82,8 @@ def run_opts(
8282
color=color,
8383
verbose=verbose,
8484
hook=hook,
85-
origin=origin,
86-
source=source,
85+
from_ref=from_ref,
86+
to_ref=to_ref,
8787
remote_name=remote_name,
8888
remote_url=remote_url,
8989
hook_stage=hook_stage,

‎tests/commands/hook_impl_test.py‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -109,8 +109,8 @@ def test_run_ns_post_checkout():
109109
assert ns is not None
110110
assert ns.hook_stage == 'post-checkout'
111111
assert ns.color is True
112-
assert ns.source == 'a'
113-
assert ns.origin == 'b'
112+
assert ns.from_ref == 'a'
113+
assert ns.to_ref == 'b'
114114
assert ns.checkout_type == 'c'
115115

116116

@@ -140,8 +140,8 @@ def test_run_ns_pre_push_updating_branch(push_example):
140140
assert ns.color is False
141141
assert ns.remote_name == 'origin'
142142
assert ns.remote_url == src
143-
assert ns.source == src_head
144-
assert ns.origin == clone_head
143+
assert ns.from_ref == src_head
144+
assert ns.to_ref == clone_head
145145
assert ns.all_files is False
146146

147147

@@ -154,8 +154,8 @@ def test_run_ns_pre_push_new_branch(push_example):
154154
ns = hook_impl._run_ns('pre-push', False, args, stdin)
155155

156156
assert ns is not None
157-
assert ns.source == src_head
158-
assert ns.origin == clone_head
157+
assert ns.from_ref == src_head
158+
assert ns.to_ref == clone_head
159159

160160

161161
def test_run_ns_pre_push_new_branch_existing_rev(push_example):

‎tests/commands/run_test.py‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -446,29 +446,29 @@ def test_hook_verbose_enabled(cap_out, store, repo_with_passing_hook):
446446

447447

448448
@pytest.mark.parametrize(
449-
('origin', 'source'), (('master', ''), ('', 'master')),
449+
('from_ref', 'to_ref'), (('master', ''), ('', 'master')),
450450
)
451-
def test_origin_source_error_msg_error(
452-
cap_out, store, repo_with_passing_hook, origin, source,
451+
def test_from_ref_to_ref_error_msg_error(
452+
cap_out, store, repo_with_passing_hook, from_ref, to_ref,
453453
):
454-
args = run_opts(origin=origin, source=source)
454+
args = run_opts(from_ref=from_ref, to_ref=to_ref)
455455
ret, printed = _do_run(cap_out, store, repo_with_passing_hook, args)
456456
assert ret == 1
457-
assert b'Specify both --origin and --source.' in printed
457+
assert b'Specify both --from-ref and --to-ref.' in printed
458458

459459

460460
def test_all_push_options_ok(cap_out, store, repo_with_passing_hook):
461461
args = run_opts(
462-
origin='master', source='master',
462+
from_ref='master', to_ref='master',
463463
remote_name='origin', remote_url='https://example.com/repo',
464464
)
465465
ret, printed = _do_run(cap_out, store, repo_with_passing_hook, args)
466466
assert ret == 0
467-
assert b'Specify both --origin and --source.' not in printed
467+
assert b'Specify both --from-ref and --to-ref.' not in printed
468468

469469

470470
def test_checkout_type(cap_out, store, repo_with_passing_hook):
471-
args = run_opts(origin='', source='', checkout_type='1')
471+
args = run_opts(from_ref='', to_ref='', checkout_type='1')
472472
environ: EnvironT = {}
473473
ret, printed = _do_run(
474474
cap_out, store, repo_with_passing_hook, args, environ,

‎tests/git_test.py‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -100,11 +100,11 @@ def test_get_changed_files(in_git_dir):
100100
in_git_dir.join('b.txt').ensure()
101101
cmd_output('git', 'add', '.')
102102
git_commit()
103-
files = git.get_changed_files('HEAD', 'HEAD^')
103+
files = git.get_changed_files('HEAD^', 'HEAD')
104104
assert files == ['a.txt', 'b.txt']
105105

106106
# files changed in source but not in origin should not be returned
107-
files = git.get_changed_files('HEAD^', 'HEAD')
107+
files = git.get_changed_files('HEAD', 'HEAD^')
108108
assert files == []
109109

110110

@@ -142,7 +142,7 @@ def test_staged_files_non_ascii(non_ascii_repo):
142142

143143

144144
def test_changed_files_non_ascii(non_ascii_repo):
145-
ret = git.get_changed_files('HEAD', 'HEAD^')
145+
ret = git.get_changed_files('HEAD^', 'HEAD')
146146
assert ret == ['интервью']
147147

148148

0 commit comments

Comments
 (0)