| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
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
| 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 | ||
| 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 thread
Comment on lines
+65
to
+66
Copy link
Copy Markdown
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality⚠️ Potential issue | 🟠 Major 🧩 Analysis chain🏁 Script executed: # First, let's understand the current structure of the repository
git ls-files | head -20Repository: 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 -10Repository: 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.pyRepository: 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.pyRepository: commit-check/commit-check Length of output: 1332 🏁 Script executed: # Check the recent commit that modified main.py
git show e593590 --statRepository: 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 -150Repository: 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 -200Repository: 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.pyRepository: 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 -30Repository: 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 -100Repository: 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 AgentsIn `@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.
Sorry, something went wrong.
All reactions
|
||
| ) | ||
|
|
||
| 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 | ||
|
|
||
|
Comment thread
shenxianpeng marked this conversation as resolved.
|
||
| # 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 | ||
| 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 thread
Comment on lines
+536
to
+549
Copy link
Copy Markdown
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality⚠️ 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: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.
Sorry, something went wrong.
All reactions
|
||
|
|
||
| @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 | ||
| Back | FazBrowse Home | New Git URL |
Uh oh!
There was an error while loading. Please reload this page.