commit_check/branch.py (1)

17-17: Good fix: guard on None instead of truthiness.

This addresses the prior AttributeError risk noted earlier.

tests/main_test.py (1)

165-177: Fix: invalid keyword passed to mocker.patch (will raise TypeError)

stdin_text is a kwarg to the target function, not to patch(). This breaks test collection/execution. Remove stdin_text=None from patch calls and, if needed, assert on called kwargs instead.

Apply:

-        mocker.patch(
-            "commit_check.commit.check_commit_msg", return_value=message_result, stdin_text=None
-        )
+        mocker.patch("commit_check.commit.check_commit_msg", return_value=message_result)
@@
-        mocker.patch(
-            "commit_check.commit.check_commit_signoff",
-            return_value=commit_signoff_result, stdin_text=None
-        )
+        mocker.patch("commit_check.commit.check_commit_signoff", return_value=commit_signoff_result)
@@
-        mocker.patch("commit_check.branch.check_branch", return_value=branch_result, stdin_text=None)
+        mocker.patch("commit_check.branch.check_branch", return_value=branch_result)
@@
-        mocker.patch(
-            "commit_check.branch.check_merge_base", return_value=merge_base_result, stdin_text=None
-        )
+        mocker.patch("commit_check.branch.check_merge_base", return_value=merge_base_result)
-        mocker.patch("commit_check.commit.check_imperative", return_value=PASS, stdin_text=None)
+        mocker.patch("commit_check.commit.check_imperative", return_value=PASS)

Optional: after main(), verify kwargs for the calls that should have occurred:

for m, expected_calls in [
    (mocker.get_mock("commit_check.commit.check_commit_msg"), check_commit_call_count),
    (mocker.get_mock("commit_check.commit.check_commit_signoff"), check_commit_signoff_call_count),
    (mocker.get_mock("commit_check.branch.check_branch"), check_branch_call_count),
    (mocker.get_mock("commit_check.commit.check_imperative"), check_imperative_call_count),
]:
    if expected_calls:
        assert "stdin_text" in m.call_args.kwargs

commit_check/branch.py (1)

16-18: Clarify behavior for blank/whitespace-only stdin.

Current logic treats any provided stdin (even " \n") as the branch name after strip(), yielding "" and likely failing regex. If prior behavior expected empty stdin to fall back to git branch, consider:

-            branch_name = stdin_text.strip() if stdin_text is not None else get_branch_name()
+            if stdin_text is not None:
+                branch_name = stdin_text.strip()
+                if branch_name == "":
+                    branch_name = get_branch_name()
+            else:
+                branch_name = get_branch_name()

commit_check/author.py (1)

20-27: Harden check_type handling and normalize stdin.

• Use .strip() on stdin to avoid newline/space mismatches.
• Make the author name/email selection mutually exclusive and fail loudly on unknown types to avoid UnboundLocalError.

-            if stdin_text is not None:
-                config_value = stdin_text
+            if stdin_text is not None:
+                config_value = stdin_text.strip()
             else:
-                if check_type == "author_name":
-                    format_str = "an"
-                if check_type == 'author_email':
-                    format_str = "ae"
+                if check_type == "author_name":
+                    format_str = "an"
+                elif check_type == "author_email":
+                    format_str = "ae"
+                else:
+                    raise ValueError(f"Unsupported check_type: {check_type}")
                 config_value = str(get_commit_info(format_str))

commit_check/commit.py (3)

31-46: Skip work when no message checks are configured; minor truthiness nit.

• Add a fast path to avoid reading messages when no 'message' check exists.
• Prefer a simple truthiness check for commit_msg_file.

 def check_commit_msg(checks: list, commit_msg_file: str = "", stdin_text: Optional[str] = None) -> int:
@@
-    if stdin_text is None and has_commits() is False:
+    if stdin_text is None and has_commits() is False:
         return PASS # pragma: no cover
 
+    # Fast path: if no 'message' check configured, skip reading
+    if not any(c.get('check') == 'message' for c in checks):
+        return PASS
+
     if stdin_text is not None:
         commit_msg = stdin_text
     else:
-        if commit_msg_file is None or commit_msg_file == "":
+        if not commit_msg_file:
             commit_msg_file = get_default_commit_msg_file()
         commit_msg = read_commit_msg(commit_msg_file)

---

70-81: Add fast path for signoff like imperative.

Avoid reading commit message when no 'commit_signoff' check is present.

 def check_commit_signoff(checks: list, commit_msg_file: str = "", stdin_text: Optional[str] = None) -> int:
@@
-    if stdin_text is not None:
+    # Fast path: if no 'commit_signoff' check configured, skip
+    if not any(c.get('check') == 'commit_signoff' for c in checks):
+        return PASS
+
+    if stdin_text is not None:
         commit_msg = stdin_text
     else:
-        if commit_msg_file is None or commit_msg_file == "":
+        if not commit_msg_file:
             commit_msg_file = get_default_commit_msg_file()
         commit_msg = read_commit_msg(commit_msg_file)

---

127-129: Consistency nit: use if not commit_msg_file in all three places.

Unify the None/empty checks across functions for readability.

-        if commit_msg_file is None or commit_msg_file == "":
+        if not commit_msg_file:
             commit_msg_file = get_default_commit_msg_file()

Also applies to: 79-80, 43-45

tests/commit_test.py (1)

360-363: LGTM: stdin overrides file read path is not exercised here

This test asserts early-exit on different check type and ensures read_commit_msg isn’t called. Consider adding a separate test that actually exercises the imperative path using stdin_text (e.g., “feat: Add …”) and asserts read_commit_msg is not used.
Example new test (outside this hunk):

def test_check_imperative_uses_stdin_when_provided(mocker):
    checks = [{"check": "imperative", "regex": "", "error": "", "suggest": ""}]
    m_read = mocker.patch("commit_check.commit.read_commit_msg")
    assert check_imperative(checks, None, stdin_text="feat: Add feature") == PASS
    m_read.assert_not_called()

commit_check/main.py (1)

128-130: Deduplicate validate_config call

validate_config is invoked twice; cache the result.

-        config = validate_config(args.config) if validate_config(
-            args.config,
-        ) else DEFAULT_CONFIG
+        _cfg = validate_config(args.config)
+        config = _cfg if _cfg else DEFAULT_CONFIG

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

commit_check/branch.py (1)

16-16: Good fix: guard on None rather than truthiness for stdin_text

stdin_text.strip() if stdin_text is not None correctly handles empty strings and resolves the prior review concern.

commit_check/commit.py (1)

118-125: Optional: fast-exit and truly lazy message loading in imperative check

You always read the commit message even when no imperative rules are configured. Early-return to skip I/O, then keep the existing lazy read. Note: this changes current test expectations (one test asserts a read on empty checks).

 def check_imperative(checks: list, commit_msg_file: str = "", stdin_text: Optional[str] = None) -> int:
     """Check if commit message uses imperative mood."""
     if stdin_text is None and has_commits() is False:
         return PASS # pragma: no cover
+
+    # Fast-exit if no imperative rules configured
+    if not any(c.get('check') == 'imperative' for c in checks):
+        return PASS
@@
-    # Lazily obtain commit message only when needed
-    if stdin_text is not None:
-        commit_msg = stdin_text
-    else:
-        if commit_msg_file is None or commit_msg_file == "":
-            commit_msg_file = get_default_commit_msg_file()
-        commit_msg = read_commit_msg(commit_msg_file)
+    # Lazily obtain commit message only when needed
+    if stdin_text is not None:
+        commit_msg = stdin_text
+    else:
+        if not commit_msg_file:
+            commit_msg_file = get_default_commit_msg_file()
+        commit_msg = read_commit_msg(commit_msg_file)

Also applies to: 126-153

commit_check/main.py (2)

123-124: Remove redundant second dry-run check

There’s already an early return at Line 112.

-    if args.dry_run:
-        return PASS

---

8-8: Narrow the broad except when capturing stdin

Catching Exception is too wide. Limit to expected I/O errors and add the import.

@@
-import sys
+import sys
+import io
@@
-    except Exception:
+    except (OSError, io.UnsupportedOperation):
         stdin_text = None

Optionally, only read stdin when a stdin-aware check is requested (message, author_*, branch, commit_signoff, imperative) to avoid surprising reads for --merge-base-only runs. Happy to draft a follow-up diff if you want it.

Also applies to: 121-122

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

commit_check/author.py (3)

26-28: Treat blank stdin as “not provided” and trim inputs

Whitespace-only stdin currently forces validation of an empty string. Prefer trimming and falling back to git-derived value when stdin is blank; also strip git value to avoid newline artifacts.

-    if stdin_text is None and has_commits() is False:
+    if (stdin_text is None || (isinstance(stdin_text, str) and not stdin_text.strip())) and has_commits() is False:
         return PASS # pragma: no cover
@@
-    if stdin_text is not None:
-        value = stdin_text
-    else:
-        value = _get_author_value(check_type)
+    if stdin_text is not None and stdin_text.strip():
+        value = stdin_text.strip()
+    else:
+        value = _get_author_value(check_type).strip()

Also applies to: 40-43

---

35-38: Polish user-facing message

Small grammar fix for clarity.

-        print(f"{YELLOW}Not found regex for {check_type}. skip checking.{RESET_COLOR}")
+        print(f"{YELLOW}No regex found for {check_type}; skipping.{RESET_COLOR}")

---

45-45: Use fullmatch to prevent partial matches

re.match allows prefix matches; fullmatch enforces complete validation (useful for strict author name/email formats).

-    if re.match(regex, value):
+    if re.fullmatch(regex, value):

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Learnt from: CR
PR: commit-check/commit-check#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-07T15:16:47.288Z
Learning: Ensure author information in commits is valid: proper author name and email formats