Skip to content
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
4 changes: 3 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,9 @@ verifies that test files are named correctly.
#### `no-commit-to-branch`
Protect specific branches from direct checkins.
- Use `args: [--branch, staging, --branch, main]` to set the branch.
Both `main` and `master` are protected by default if no branch argument is set.
If no branch argument is set, the hook auto-detects the repository's default
branch from `origin/HEAD`. Falls back to protecting both `main` and `master`
if `origin/HEAD` is not configured.
Comment on lines -155 to +157
Copy link
Copy Markdown
Member

@asottile asottile Apr 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as written this is a breaking change

please in the future discuss feature ideas before wasting time on implementation such that we can find the problems during the design phase rather than at the last mile of the coding phase

I suspect if this is going to be accepted at all this would need to be a separate option rather than an implicit behaviour -- especially because as written it is an inconsistent implicit behaviour (based on the repository state)

and with a required option to trigger this behaviour -- I kinda see little point to the option at all (since you could just specify the option that's already there with the proper value instead and accomplish the same thing)

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's theoretically a breaking change, but not practically. I suspect anyone relying on the default is doing so because of an intent to protect their trunk branch.

As for the process quibble: the greater waste would have been a discussion -- I needed this behavior, now I have a fork with it. An unexpected 9-line PR isn't dropping a huge amount of work on you and other maintainers without any notice, so I decided to offer it to the community for a tool I've found generally delightful to use rather than keeping it in an isolated fork.

You're welcome to take the change, reject it, or modify it to your liking. Consider this PR a notice of interest from a community member and an offer to save you some work in the case your philosophy as a maintainer aligns with mine as a user.

- `-b` / `--branch` may be specified multiple times to protect multiple
branches.
- `-p` / `--pattern` can be used to protect branches that match a supplied regex
Expand Down
13 changes: 12 additions & 1 deletion pre_commit_hooks/no_commit_to_branch.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,17 @@
from pre_commit_hooks.util import cmd_output


def _default_branch() -> frozenset[str]:
try:
ref = cmd_output('git', 'rev-parse', '--abbrev-ref', 'origin/HEAD')
branch = ref.strip().removeprefix('origin/')
if branch:
return frozenset((branch,))
except CalledProcessError:
pass
return frozenset(('master', 'main'))


def is_on_branch(
protected: AbstractSet[str],
patterns: AbstractSet[str] = frozenset(),
Expand Down Expand Up @@ -39,7 +50,7 @@ def main(argv: Sequence[str] | None = None) -> int:
)
args = parser.parse_args(argv)

protected = frozenset(args.branch or ('master', 'main'))
protected = frozenset(args.branch) if args.branch else _default_branch()
patterns = frozenset(args.pattern or ())
return int(is_on_branch(protected, patterns))

Expand Down
43 changes: 43 additions & 0 deletions tests/no_commit_to_branch_test.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
from __future__ import annotations

from unittest.mock import patch

import pytest

from pre_commit_hooks.no_commit_to_branch import _default_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 Down Expand Up @@ -78,3 +81,43 @@ def test_default_branch_names(temp_git_dir, branch_name):
with temp_git_dir.as_cwd():
cmd_output('git', 'checkout', '-b', branch_name)
assert main(()) == 1


def test_default_branch_falls_back_when_empty_branch(tmpdir):
with patch(
'pre_commit_hooks.no_commit_to_branch.cmd_output',
return_value='origin/',
):
assert _default_branch() == frozenset({'master', 'main'})


def test_default_branch_detects_from_origin(tmpdir):
remote = tmpdir.join('remote')
cmd_output('git', 'init', '--', str(remote))
with remote.as_cwd():
cmd_output('git', 'checkout', '-b', 'develop')
git_commit('--allow-empty', '-m', 'init')

local = tmpdir.join('local')
cmd_output('git', 'clone', str(remote), str(local))

with local.as_cwd():
assert _default_branch() == frozenset({'develop'})


def test_main_blocks_detected_default_branch(tmpdir):
remote = tmpdir.join('remote')
cmd_output('git', 'init', '--', str(remote))
with remote.as_cwd():
cmd_output('git', 'checkout', '-b', 'develop')
git_commit('--allow-empty', '-m', 'init')

local = tmpdir.join('local')
cmd_output('git', 'clone', str(remote), str(local))

with local.as_cwd():
# On detected default branch — should be blocked
assert main(()) == 1
# On a feature branch — should pass
cmd_output('git', 'checkout', '-b', 'feature')
assert main(()) == 0
Loading