refactor: check the length in Commit instead of Commitizen

Author: kevin1kevin1k

commitizen/commands/commit.py

@@ -12,6 +12,7 @@
 from commitizen.cz.utils import get_backup_file_path
 from commitizen.exceptions import (
     CommitError,
+    CommitMessageLengthExceededError,
     CustomError,
     DryRunExit,
     NoAnswersError,
@@ -62,8 +63,15 @@ def prompt_commit_questions(self) -> str:
         if not answers:
             raise NoAnswersError()
 
+        message = cz.message(answers)
+        message_len = len(message.partition("\n")[0])
         message_length_limit: int = self.arguments.get("message_length_limit", 0)
-        return cz.message(answers, message_length_limit=message_length_limit)
+        if message_length_limit > 0 and message_len > message_length_limit:
+            raise CommitMessageLengthExceededError(
+                f"Length of commit message exceeds limit ({message_len}/{message_length_limit})"
+            )
+
+        return message
 
     def __call__(self):
         dry_run: bool = self.arguments.get("dry_run")

commitizen/cz/base.py

@@ -9,7 +9,6 @@
 from commitizen import git
 from commitizen.config.base_config import BaseConfig
 from commitizen.defaults import Questions
-from commitizen.exceptions import CommitMessageLengthExceededError
 
 
 class MessageBuilderHook(Protocol):
@@ -72,7 +71,7 @@ def questions(self) -> Questions:
         """Questions regarding the commit message."""
 
     @abstractmethod
-    def message(self, answers: dict, message_length_limit: int) -> str:
+    def message(self, answers: dict) -> str:
         """Format your git message."""
 
     @property
@@ -106,12 +105,3 @@ def process_commit(self, commit: str) -> str:
         If not overwritten, it returns the first line of commit.
         """
         return commit.split("\n")[0]
-
-    def _check_message_length_limit(
-        self, message: str, message_length_limit: int
-    ) -> None:
-        message_len = len(message)
-        if message_length_limit > 0 and message_len > message_length_limit:
-            raise CommitMessageLengthExceededError(
-                f"Length of commit message exceeds limit ({message_len}/{message_length_limit})"
-            )

commitizen/cz/conventional_commits/conventional_commits.py

@@ -150,7 +150,7 @@ def questions(self) -> Questions:
         ]
         return questions
 
-    def message(self, answers: dict, message_length_limit: int = 0) -> str:
+    def message(self, answers: dict) -> str:
         prefix = answers["prefix"]
         scope = answers["scope"]
         subject = answers["subject"]
@@ -167,9 +167,9 @@ def message(self, answers: dict, message_length_limit: int = 0) -> str:
         if footer:
             footer = f"\n\n{footer}"
 
-        message = f"{prefix}{scope}: {subject}"
-        self._check_message_length_limit(message, message_length_limit)
-        return f"{message}{body}{footer}"
+        message = f"{prefix}{scope}: {subject}{body}{footer}"
+
+        return message
 
     def example(self) -> str:
         return (

commitizen/cz/customize/customize.py

@@ -61,15 +61,12 @@ def __init__(self, config: BaseConfig):
     def questions(self) -> Questions:
         return self.custom_settings.get("questions", [{}])
 
-    def message(self, answers: dict, message_length_limit: int = 0) -> str:
+    def message(self, answers: dict) -> str:
         message_template = Template(self.custom_settings.get("message_template", ""))
-        message: str = (
-            message_template.substitute(**answers)  # type: ignore
-            if getattr(Template, "substitute", None)
-            else message_template.render(**answers)
-        )
-        self._check_message_length_limit(message, message_length_limit)
-        return message
+        if getattr(Template, "substitute", None):
+            return message_template.substitute(**answers)  # type: ignore
+        else:
+            return message_template.render(**answers)
 
     def example(self) -> str | None:
         return self.custom_settings.get("example")

commitizen/cz/jira/jira.py

@@ -44,8 +44,8 @@ def questions(self) -> Questions:
         ]
         return questions
 
-    def message(self, answers: dict, message_length_limit: int = 0) -> str:
-        message = " ".join(
+    def message(self, answers: dict) -> str:
+        return " ".join(
             filter(
                 bool,
                 [
@@ -57,8 +57,6 @@ def message(self, answers: dict, message_length_limit: int = 0) -> str:
                 ],
             )
         )
-        self._check_message_length_limit(message, message_length_limit)
-        return message
 
     def example(self) -> str:
         return (

commitizen/exceptions.py

@@ -206,3 +206,4 @@ class ConfigFileIsEmpty(CommitizenException):
 
 class CommitMessageLengthExceededError(CommitizenException):
     exit_code = ExitCode.COMMIT_MESSAGE_LENGTH_LIMIT_EXCEEDED
+    message = "Length of commit message exceeds the given limit."

docs/commit.md

@@ -44,5 +44,6 @@ An exception would be raised when the message length exceeds the limit.
 For example, `cz commit -l 72` will limit the length of commit messages to 72 characters.
 By default the limit is set to 0, which means no limit on the length.
 
-Note that for `ConventionalCommitsCz`, the limit applies only from the prefix to the subject.
-In other words, everything after the first line (the body and the footer) are not counted in the length.
+Note that the limit applies only to the first line of message.
+Specifically, for `ConventionalCommitsCz` the length only counts from the type of change to the subject,
+while the body and the footer are not counted.

docs/customization.md

@@ -227,11 +227,9 @@ class JiraCz(BaseCommitizen):
         ]
         return questions
 
-    def message(self, answers: dict, message_length_limit: int = 0) -> str:
+    def message(self, answers: dict) -> str:
         """Generate the message with the given answers."""
-        message = "{0} (#{1})".format(answers["title"], answers["issue"])
-        self._check_message_length_limit(message, message_length_limit)
-        return message
+        return "{0} (#{1})".format(answers["title"], answers["issue"])
 
     def example(self) -> str:
         """Provide an example to help understand the style (OPTIONAL)

tests/commands/test_commit_command.py

@@ -9,6 +9,7 @@
 from commitizen.cz.utils import get_backup_file_path
 from commitizen.exceptions import (
     CommitError,
+    CommitMessageLengthExceededError,
     CustomError,
     DryRunExit,
     NoAnswersError,
@@ -379,3 +380,29 @@ def test_commit_command_with_extra_args(config, mocker: MockFixture):
     commands.Commit(config, {"extra_cli_args": "-- -extra-args1 -extra-arg2"})()
     commit_mock.assert_called_once_with(ANY, args="-- -extra-args1 -extra-arg2")
     success_mock.assert_called_once()
+
+
+@pytest.mark.usefixtures("staging_is_clean")
+def test_commit_command_with_message_length_limit(config, mocker: MockFixture):
+    prompt_mock = mocker.patch("questionary.prompt")
+    prefix = "feat"
+    subject = "random subject"
+    message_length = len(prefix) + len(": ") + len(subject)
+    prompt_mock.return_value = {
+        "prefix": prefix,
+        "subject": subject,
+        "scope": "",
+        "is_breaking_change": False,
+        "body": "random body",
+        "footer": "random footer",
+    }
+
+    commit_mock = mocker.patch("commitizen.git.commit")
+    commit_mock.return_value = cmd.Command("success", "", b"", b"", 0)
+    success_mock = mocker.patch("commitizen.out.success")
+
+    commands.Commit(config, {"message_length_limit": message_length})()
+    success_mock.assert_called_once()
+
+    with pytest.raises(CommitMessageLengthExceededError):
+        commands.Commit(config, {"message_length_limit": message_length - 1})()

tests/conftest.py

@@ -204,12 +204,10 @@ def questions(self) -> list:
             },
         ]
 
-    def message(self, answers: dict, message_length_limit: int = 0) -> str:
+    def message(self, answers: dict) -> str:
         prefix = answers["prefix"]
         subject = answers.get("subject", "default message").trim()
-        message = f"{prefix}: {subject}"
-        self._check_message_length_limit(message, message_length_limit)
-        return message
+        return f"{prefix}: {subject}"
 
 
 @pytest.fixture()
@@ -222,7 +220,7 @@ class MockPlugin(BaseCommitizen):
     def questions(self) -> defaults.Questions:
         return []
 
-    def message(self, answers: dict, message_length_limit: int = 0) -> str:
+    def message(self, answers: dict) -> str:
         return ""
 
 

tests/test_cz_base.py

@@ -1,17 +1,14 @@
 import pytest
 
 from commitizen.cz.base import BaseCommitizen
-from commitizen.exceptions import CommitMessageLengthExceededError
 
 
 class DummyCz(BaseCommitizen):
     def questions(self):
         return [{"type": "input", "name": "commit", "message": "Initial commit:\n"}]
 
-    def message(self, answers: dict, message_length_limit: int = 0):
-        message = answers["commit"]
-        self._check_message_length_limit(message, message_length_limit)
-        return message
+    def message(self, answers: dict):
+        return answers["commit"]
 
 
 def test_base_raises_error(config):
@@ -51,16 +48,3 @@ def test_process_commit(config):
     cz = DummyCz(config)
     message = cz.process_commit("test(test_scope): this is test msg")
     assert message == "test(test_scope): this is test msg"
-
-
-def test_message_length_limit(config):
-    cz = DummyCz(config)
-    commit_message = "123456789"
-    message_length = len(commit_message)
-    assert cz.message({"commit": commit_message}) == commit_message
-    assert (
-        cz.message({"commit": commit_message}, message_length_limit=message_length)
-        == commit_message
-    )
-    with pytest.raises(CommitMessageLengthExceededError):
-        cz.message({"commit": commit_message}, message_length_limit=message_length - 1)