Skip to content

fix: add support for commit message file argument for pre-commit #366

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.

Sign up for GitHub

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

Merged
shenxianpeng merged 6 commits into from
Feb 3, 2026
Merged

fix: add support for commit message file argument for pre-commit #366

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
936b2b7
feat: add support for commit message file argument for pre-commit com…
shenxianpeng Feb 3, 2026
9eb7877
fix: refactor commit message handling
shenxianpeng Feb 3, 2026
732bbda
fix: accept both hyphen and underscore formats for CLI args
shenxianpeng Feb 3, 2026
b312f54
Revert "fix: accept both hyphen and underscore formats for CLI args"
shenxianpeng Feb 3, 2026
e593590
fix: Update commit_check/main.py per review
shenxianpeng Feb 3, 2026
8adb4db
test: add test coverage for positional commit_msg_file argument (#367)
Copilot Feb 3, 2026
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
1 change: 1 addition & 0 deletions commit_check/imperatives.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,7 @@
"read",
"record",
"redesign",
"refactor",
"refer",
"refresh",
"register",
Expand Down
38 changes: 24 additions & 14 deletions commit_check/main.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,13 @@ def _get_parser() -> argparse.ArgumentParser:
help="path to config file (cchk.toml or commit-check.toml). If not specified, searches for config in: cchk.toml, commit-check.toml, .github/cchk.toml, .github/commit-check.toml",
)

parser.add_argument(
"commit_msg_file",
nargs="?",
default=None,
help="path to commit message file (positional argument for pre-commit compatibility)",
)

# Main check type arguments
check_group = parser.add_argument_group(
"check types", "Specify which validation checks to run"
Expand All @@ -55,9 +62,8 @@ def _get_parser() -> argparse.ArgumentParser:
check_group.add_argument(
"-m",
"--message",
nargs="?",
const="",
help="validate commit message. Optionally specify file path, otherwise reads from stdin if available",
action="store_true",
help="validate commit message (file path can be provided as positional argument for pre-commit compatibility)",
Comment on lines +65 to +66
Copy link
Contributor

@coderabbitai coderabbitai bot Feb 3, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# First, let's understand the current structure of the repository
git ls-files | head -20

Repository: commit-check/commit-check

Length of output: 561

🏁 Script executed:

# Check the git history of main.py to see if --message behavior changed
git log --oneline commit_check/main.py | head -10

Repository: commit-check/commit-check

Length of output: 120

🏁 Script executed:

# Look at the current main.py to understand the full context of lines 65-66 and the positional argument handling
wc -l commit_check/main.py

Repository: commit-check/commit-check

Length of output: 93

🏁 Script executed:

# Read the relevant section of main.py around lines 65-66 and surrounding code
sed -n '50,100p' commit_check/main.py

Repository: commit-check/commit-check

Length of output: 1332

🏁 Script executed:

# Check the recent commit that modified main.py
git show e593590 --stat

Repository: commit-check/commit-check

Length of output: 3037

🏁 Script executed:

# View the actual changes in that commit for main.py
git show e593590 -- commit_check/main.py | head -150

Repository: commit-check/commit-check

Length of output: 4457

🏁 Script executed:

# Check if there's an even earlier version to understand the full history
git log -p --follow -S "--message" -- commit_check/main.py | head -200

Repository: commit-check/commit-check

Length of output: 51

🏁 Script executed:

# Double-check if there are any earlier commits or branches with main.py
git log --all --follow --source -- commit_check/main.py

Repository: commit-check/commit-check

Length of output: 14151

🏁 Script executed:

# Verify the tests to see what the intended behavior is
grep -n "\-\-message\|commit_msg_file" tests/main_test.py | head -30

Repository: commit-check/commit-check

Length of output: 1331

🏁 Script executed:

# Check the previous version of main.py from the commit before e593590
git show bd1abc3:commit_check/main.py | head -100

Repository: commit-check/commit-check

Length of output: 2914

🏁 Script executed:

# Check if the old version had --message accepting a file path
git show bd1abc3:commit_check/main.py | grep -A 5 "\-\-message"

Repository: commit-check/commit-check

Length of output: 427

Document breaking change in --message flag behavior.

The -m/--message flag changed from accepting an optional file path argument to a boolean flag. Previous usage like -m /path/to/file must now use the positional argument: commit-check -m /path/to/file. Update release notes or migration guide to inform users of this change.

🤖 Prompt for AI Agents 
In `@commit_check/main.py` around lines 65 - 66, Document that the semantics of
the -m/--message flag changed from accepting an optional file path to a boolean
store_true flag: update release notes and the migration guide to show the new
usage (use the positional argument for a message file, e.g., commit-check -m
/path/to/file) and add a clear note in the CLI help text for the --message/-m
option in main.py so users know to move prior invocations like `-m
/path/to/file` to the positional parameter.

)

check_group.add_argument(
Expand Down Expand Up @@ -310,11 +316,17 @@ def main() -> int:
rule_builder = RuleBuilder(config_data)
all_rules = rule_builder.build_all_rules()

# Handle positional commit_msg_file argument for pre-commit compatibility
# Store the file path separately from the boolean flag
commit_msg_file_path = None
if args.commit_msg_file:
commit_msg_file_path = args.commit_msg_file
# If a file was provided positionally, always enable message checking
args.message = True

# Filter rules based on CLI arguments
requested_checks = []
if (
args.message is not None
): # Check for None explicitly since empty string is valid
if args.message: # args.message is now a boolean flag
# Add commit message related checks
requested_checks.extend(
[
Expand Down Expand Up @@ -354,18 +366,16 @@ def main() -> int:
stdin_content = None
commit_file_path = None

if (
args.message is not None
): # Check explicitly for None since empty string is valid
if args.message == "":
# Only set stdin_content if there's actual piped input
if args.message: # args.message is a boolean flag
# Check if we have a file path from positional argument
if commit_msg_file_path:
commit_file_path = commit_msg_file_path
else:
# No file path provided, try reading from stdin
stdin_content = stdin_reader.read_piped_input()
if not stdin_content:
# No stdin and no file - let validators get data from git themselves
stdin_content = None
else:
# Message is a file path
commit_file_path = args.message
elif not any([args.branch, args.author_name, args.author_email]):
# If no specific validation type is requested, don't read stdin
pass
Expand Down
87 changes: 87 additions & 0 deletions tests/main_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -474,3 +474,90 @@ def test_env_overrides_default(self, mocker, monkeypatch):
sys.argv = ["commit-check", "--message"]
result = main()
assert result == 1 # Env var wins, should fail


class TestPositionalArgumentFeature:
"""Test positional commit_msg_file argument for pre-commit compatibility."""

@pytest.mark.benchmark
def test_positional_arg_without_message_flag(self):
"""Test using just the positional argument without --message flag."""
with tempfile.NamedTemporaryFile(mode="w", delete=False) as f:
f.write("feat: add positional argument support")
f.flush()

try:
# Use positional argument only (no --message flag)
sys.argv = ["commit-check", f.name]
result = main()
assert result == 0 # Should pass validation
finally:
os.unlink(f.name)

@pytest.mark.benchmark
def test_positional_arg_with_message_flag(self):
"""Test using positional argument with --message flag."""
with tempfile.NamedTemporaryFile(mode="w", delete=False) as f:
f.write("fix: resolve bug in validation")
f.flush()

try:
# Use both positional argument and --message flag
sys.argv = ["commit-check", "--message", f.name]
result = main()
assert result == 0 # Should pass validation
finally:
os.unlink(f.name)

@pytest.mark.benchmark
def test_positional_arg_with_branch_flag(self, mocker):
"""Test positional argument with other check flags (edge case)."""
# Mock git command to return a valid branch name
mocker.patch(
"subprocess.run",
return_value=type(
"MockResult", (), {"stdout": "feature/test-branch", "returncode": 0}
)(),
)

with tempfile.NamedTemporaryFile(mode="w", delete=False) as f:
f.write("chore: update documentation")
f.flush()

try:
# Use positional argument with --branch flag
sys.argv = ["commit-check", "--branch", f.name]
result = main()
# Should validate both commit message and branch name
assert result == 0 # Should pass both validations
finally:
os.unlink(f.name)

@pytest.mark.benchmark
def test_positional_arg_invalid_commit(self):
"""Test that positional argument correctly rejects invalid commits."""
with tempfile.NamedTemporaryFile(mode="w", delete=False) as f:
f.write("invalid commit message without type")
f.flush()

try:
# Use positional argument with invalid message
sys.argv = ["commit-check", f.name]
result = main()
assert result == 1 # Should fail validation
finally:
os.unlink(f.name)
Comment on lines +536 to +549
Copy link
Contributor

@coderabbitai coderabbitai bot Feb 3, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Add mock for get_commit_info to ensure consistent test behavior.

This test is missing a mock that similar tests include. Looking at test_message_validation_with_invalid_commit (line 50), it mocks commit_check.engine.get_commit_info to return a known author. Without this mock, the test result could vary depending on the git state of the test environment or if the author happens to match an ignore list entry.

Proposed fix 
     `@pytest.mark.benchmark`
     def test_positional_arg_invalid_commit(self):
         """Test that positional argument correctly rejects invalid commits."""
+        # Note: Consider adding mocker fixture and mocking get_commit_info
+        # similar to test_message_validation_with_invalid_commit
         with tempfile.NamedTemporaryFile(mode="w", delete=False) as f:

Or convert to use mocker:

     `@pytest.mark.benchmark`
-    def test_positional_arg_invalid_commit(self):
+    def test_positional_arg_invalid_commit(self, mocker):
         """Test that positional argument correctly rejects invalid commits."""
+        mocker.patch("commit_check.engine.get_commit_info", return_value="test-author")
         with tempfile.NamedTemporaryFile(mode="w", delete=False) as f:
🤖 Prompt for AI Agents 
In `@tests/main_test.py` around lines 536 - 549, The
test_positional_arg_invalid_commit test can be flaky because it does not mock
commit_check.engine.get_commit_info; update the test to patch
commit_check.engine.get_commit_info (the same way
test_message_validation_with_invalid_commit does) to return a deterministic
commit payload (e.g., a dict with a known "author") before calling main(),
ensuring the validation outcome is independent of the repo state; use the
existing mocking style in other tests (mocker or monkeypatch) to set the return
value and then call main() and assert result == 1.


@pytest.mark.benchmark
def test_positional_arg_nonexistent_file(self, mocker):
"""Test that positional argument with non-existent file falls back to git."""
# Mock git to return a valid commit message
mocker.patch(
"commit_check.engine.get_commit_info",
return_value="feat: add fallback commit from git",
)

sys.argv = ["commit-check", "/nonexistent/commit_msg.txt"]
result = main()
# Should fall back to git and pass
assert result == 0
Loading