Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[no_commit_to_branch] Add user friendly error message #979

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 32 additions & 11 deletions pre_commit_hooks/no_commit_to_branch.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,29 +9,37 @@
from pre_commit_hooks.util import cmd_output


def is_on_branch(
protected: AbstractSet[str],
patterns: AbstractSet[str] = frozenset(),
) -> bool:
def get_current_branch() -> str | None:
try:
ref_name = cmd_output('git', 'symbolic-ref', 'HEAD')
except CalledProcessError:
return False
return None
chunks = ref_name.strip().split('/')
branch_name = '/'.join(chunks[2:])
return branch_name in protected or any(
re.match(p, branch_name) for p in patterns
return '/'.join(chunks[2:])


def is_on_branch(
current_branch: str,
protected: AbstractSet[str],
patterns: AbstractSet[str] = frozenset(),
) -> bool:
return current_branch in protected or any(
re.match(p, current_branch) for p in patterns
)


def main(argv: Sequence[str] | None = None) -> int:
parser = argparse.ArgumentParser()
parser.add_argument(
'-b', '--branch', action='append',
'-b',
'--branch',
action='append',
help='branch to disallow commits to, may be specified multiple times',
)
parser.add_argument(
'-p', '--pattern', action='append',
'-p',
'--pattern',
action='append',
help=(
'regex pattern for branch name to disallow commits to, '
'may be specified multiple times'
Expand All @@ -41,7 +49,20 @@ def main(argv: Sequence[str] | None = None) -> int:

protected = frozenset(args.branch or ('master', 'main'))
patterns = frozenset(args.pattern or ())
return int(is_on_branch(protected, patterns))
current_branch = get_current_branch()

if current_branch is None:
return 0

on_branch = is_on_branch(current_branch, protected, patterns)

if on_branch:
print(
f'You are currently on {current_branch} branch, for which '
f'pre-commit script does not permit this action.',
)

return int(on_branch)


if __name__ == '__main__':
Expand Down
29 changes: 19 additions & 10 deletions tests/no_commit_to_branch_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import pytest

from pre_commit_hooks.no_commit_to_branch import get_current_branch
from pre_commit_hooks.no_commit_to_branch import is_on_branch
from pre_commit_hooks.no_commit_to_branch import main
from pre_commit_hooks.util import cmd_output
Expand All @@ -11,24 +12,38 @@
def test_other_branch(temp_git_dir):
with temp_git_dir.as_cwd():
cmd_output('git', 'checkout', '-b', 'anotherbranch')
assert is_on_branch({'master'}) is False
current_branch = get_current_branch()
assert current_branch == 'anotherbranch'
assert is_on_branch(current_branch, {'master'}) is False


def test_multi_branch(temp_git_dir):
with temp_git_dir.as_cwd():
cmd_output('git', 'checkout', '-b', 'another/branch')
assert is_on_branch({'master'}) is False
current_branch = get_current_branch()
assert current_branch == 'another/branch'
assert is_on_branch(current_branch, {'master'}) is False


def test_multi_branch_fail(temp_git_dir):
with temp_git_dir.as_cwd():
cmd_output('git', 'checkout', '-b', 'another/branch')
assert is_on_branch({'another/branch'}) is True
current_branch = get_current_branch()
assert current_branch == 'another/branch'
assert is_on_branch(current_branch, {'another/branch'}) is True


def test_master_branch(temp_git_dir):
with temp_git_dir.as_cwd():
assert is_on_branch({'master'}) is True
current_branch = get_current_branch()
assert current_branch == 'master'
assert is_on_branch(current_branch, {'master'}) is True


def test_branch_pattern_fail(temp_git_dir):
with temp_git_dir.as_cwd():
cmd_output('git', 'checkout', '-b', 'another/branch')
assert is_on_branch('another/branch', set(), {'another/.*'}) is True


def test_main_branch_call(temp_git_dir):
Expand All @@ -44,12 +59,6 @@ def test_forbid_multiple_branches(temp_git_dir, branch_name):
assert main(('--branch', 'b1', '--branch', 'b2'))


def test_branch_pattern_fail(temp_git_dir):
with temp_git_dir.as_cwd():
cmd_output('git', 'checkout', '-b', 'another/branch')
assert is_on_branch(set(), {'another/.*'}) is True


@pytest.mark.parametrize('branch_name', ('master', 'another/branch'))
def test_branch_pattern_multiple_branches_fail(temp_git_dir, branch_name):
with temp_git_dir.as_cwd():
Expand Down