From c1efc9943f1fbe17fb95b278f75b8e2a78c1251a Mon Sep 17 00:00:00 2001
From: Przemek Pawlas <3606072+Destroy666x@users.noreply.github.com>
Date: Thu, 19 Oct 2023 21:29:33 +0200
Subject: [PATCH] feat: [no_commit_to_branch] Add user friendly error message

---
 pre_commit_hooks/no_commit_to_branch.py | 43 ++++++++++++++++++-------
 tests/no_commit_to_branch_test.py       | 29 +++++++++++------
 2 files changed, 51 insertions(+), 21 deletions(-)

diff --git a/pre_commit_hooks/no_commit_to_branch.py b/pre_commit_hooks/no_commit_to_branch.py
index 741f7267..4b2388b5 100644
--- a/pre_commit_hooks/no_commit_to_branch.py
+++ b/pre_commit_hooks/no_commit_to_branch.py
@@ -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'
@@ -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__':
diff --git a/tests/no_commit_to_branch_test.py b/tests/no_commit_to_branch_test.py
index eaae5e62..4ae5aecd 100644
--- a/tests/no_commit_to_branch_test.py
+++ b/tests/no_commit_to_branch_test.py
@@ -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
@@ -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):
@@ -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():