From 445d7389791b8e0678e2668aac55dab3c3cfc15d Mon Sep 17 00:00:00 2001 From: Ajesh Sen Thapa Date: Wed, 17 Jul 2024 22:20:14 +0545 Subject: [PATCH] fix: handle commit message starting with # - Removed comments only on `--file` (since the commit message from hash doesn't contain comments). This improves performance. - Displayed full commit on 'Input' section of output. - Created function `remove_diff_from_commit_message` for removing diff from the commits. - Updated README doc for not using commit messages that start with '#'. - Allowed codecov checks for coverage. --- README.md | 8 +++- codecov.yml | 7 ---- src/commitlint/cli.py | 28 +++++++------ src/commitlint/linter/_linter.py | 9 +++-- src/commitlint/linter/utils.py | 39 ++++++++++++------- tests/fixtures/linter.py | 7 ---- tests/test_cli.py | 14 +++++++ tests/test_linter/test__linter.py | 25 ++++++++++-- .../test_remove_diff_from_commit_message.py | 29 ++++++++++++++ 9 files changed, 118 insertions(+), 48 deletions(-) create mode 100644 tests/test_linter/test_utils/test_remove_diff_from_commit_message.py diff --git a/README.md b/README.md index ad71dab..3c37c2e 100644 --- a/README.md +++ b/README.md @@ -57,7 +57,10 @@ For more details, please refer to the Conventional Commits specification at http pre-commit install --hook-type commit-msg ``` -> **_NOTE:_** Installing using only `pre-commit install` will not work. + Installing using only `pre-commit install` will not work. + +> **_NOTE:_** Avoid using commit messages that start with '#'. +> This might result in unexpected behavior with commitlint. ### For GitHub Actions @@ -151,6 +154,9 @@ Check commit message from file: $ commitlint --file /foo/bar/commit-message.txt ``` +> **_NOTE:_** For `--file` option, avoid using commit messages that start with '#'. +> This might result in unexpected behavior with commitlint. + Check commit message of a hash: ```shell diff --git a/codecov.yml b/codecov.yml index fc1294e..2c2ad3c 100644 --- a/codecov.yml +++ b/codecov.yml @@ -2,10 +2,3 @@ # Disabling comments comment: false - -# Disabling Github checks -github_checks: false -coverage: - status: - project: off - patch: off diff --git a/src/commitlint/cli.py b/src/commitlint/cli.py index 5f736f3..52172aa 100644 --- a/src/commitlint/cli.py +++ b/src/commitlint/cli.py @@ -24,7 +24,7 @@ from .exceptions import CommitlintException from .git_helpers import get_commit_message_of_hash, get_commit_messages_of_hash_range from .linter import lint_commit_message -from .linter.utils import remove_comments +from .linter.utils import remove_diff_from_commit_message from .messages import VALIDATION_FAILED, VALIDATION_SUCCESSFUL @@ -93,9 +93,7 @@ def get_args() -> argparse.Namespace: def _show_errors( - commit_message: str, - errors: List[str], - skip_detail: bool = False, + commit_message: str, errors: List[str], skip_detail: bool = False ) -> None: """ Display a formatted error message for a list of errors. @@ -104,10 +102,10 @@ def _show_errors( commit_message (str): The commit message to display. errors (List[str]): A list of error messages to be displayed. skip_detail (bool): Whether to skip the detailed error message. - """ error_count = len(errors) - commit_message = remove_comments(commit_message) + + commit_message = remove_diff_from_commit_message(commit_message) console.error(f"⧗ Input:\n{commit_message}\n") @@ -141,24 +139,28 @@ def _get_commit_message_from_file(filepath: str) -> str: return commit_message -def _handle_commit_message(commit_message: str, skip_detail: bool) -> None: +def _handle_commit_message( + commit_message: str, skip_detail: bool, strip_comments: bool = False +) -> None: """ Handles a single commit message, checks its validity, and prints the result. Args: commit_message (str): The commit message to be handled. skip_detail (bool): Whether to skip the detailed error linting. + strip_comments (bool, optional): Whether to remove comments from the + commit message (default is False). Raises: SystemExit: If the commit message is invalid. """ - success, errors = lint_commit_message(commit_message, skip_detail=skip_detail) + success, errors = lint_commit_message(commit_message, skip_detail, strip_comments) if success: console.success(VALIDATION_SUCCESSFUL) return - _show_errors(commit_message, errors, skip_detail=skip_detail) + _show_errors(commit_message, errors, skip_detail) sys.exit(1) @@ -177,13 +179,13 @@ def _handle_multiple_commit_messages( has_error = False for commit_message in commit_messages: - success, errors = lint_commit_message(commit_message, skip_detail=skip_detail) + success, errors = lint_commit_message(commit_message, skip_detail) if success: console.verbose("lint success") continue has_error = True - _show_errors(commit_message, errors, skip_detail=skip_detail) + _show_errors(commit_message, errors, skip_detail) console.error("") if has_error: @@ -207,7 +209,9 @@ def main() -> None: if args.file: console.verbose("checking commit from file") commit_message = _get_commit_message_from_file(args.file) - _handle_commit_message(commit_message, skip_detail=args.skip_detail) + _handle_commit_message( + commit_message, skip_detail=args.skip_detail, strip_comments=True + ) elif args.hash: console.verbose("checking commit from hash") commit_message = get_commit_message_of_hash(args.hash) diff --git a/src/commitlint/linter/_linter.py b/src/commitlint/linter/_linter.py index 7370023..a2a3455 100644 --- a/src/commitlint/linter/_linter.py +++ b/src/commitlint/linter/_linter.py @@ -16,7 +16,7 @@ def lint_commit_message( - commit_message: str, skip_detail: bool = False + commit_message: str, skip_detail: bool = False, strip_comments: bool = False ) -> Tuple[bool, List[str]]: """ Lints a commit message. @@ -25,6 +25,8 @@ def lint_commit_message( commit_message (str): The commit message to be linted. skip_detail (bool, optional): Whether to skip the detailed error linting (default is False). + strip_comments (bool, optional): Whether to remove comments from the + commit message (default is False). Returns: Tuple[bool, List[str]]: Returns success as a first element and list of errors @@ -35,8 +37,9 @@ def lint_commit_message( # perform processing and pre checks # removing unnecessary commit comments - console.verbose("removing comments from the commit message") - commit_message = remove_comments(commit_message) + if strip_comments: + console.verbose("removing comments from the commit message") + commit_message = remove_comments(commit_message) # checking if commit message should be ignored console.verbose("checking if the commit message is in ignored list") diff --git a/src/commitlint/linter/utils.py b/src/commitlint/linter/utils.py index 5bd2aa1..ef4c0ca 100644 --- a/src/commitlint/linter/utils.py +++ b/src/commitlint/linter/utils.py @@ -24,10 +24,29 @@ def is_ignored(commit_message: str) -> bool: return bool(re.match(IGNORE_COMMIT_PATTERNS, commit_message)) -def remove_comments(msg: str) -> str: +def remove_comments(commit_message: str) -> str: """Removes comments from the commit message. - For `git commit --verbose`, excluding the diff generated message, + Args: + commit_message(str): The commit message to remove comments. + + Returns: + str: The commit message without comments. + """ + commit_message = remove_diff_from_commit_message(commit_message) + + lines: List[str] = [] + for line in commit_message.split("\n"): + if not line.startswith("#"): + lines.append(line) + + return "\n".join(lines) + + +def remove_diff_from_commit_message(commit_message: str) -> str: + """Removes commit diff from the commit message. + + For `git commit --verbose`, removing the diff generated message, for example: ```bash @@ -40,18 +59,10 @@ def remove_comments(msg: str) -> str: ``` Args: - msg(str): The commit message to remove comments. + commit_message (str): The commit message to remove diff. Returns: - str: The commit message without comments. + str: The commit message without diff. """ - - lines: List[str] = [] - for line in msg.split("\n"): - if "# ------------------------ >8 ------------------------" in line: - # ignoring all the verbose message below this line - break - if not line.startswith("#"): - lines.append(line) - - return "\n".join(lines) + verbose_commit_separator = "# ------------------------ >8 ------------------------" + return commit_message.split(verbose_commit_separator)[0].strip() diff --git a/tests/fixtures/linter.py b/tests/fixtures/linter.py index 6d4443d..4e2bcee 100644 --- a/tests/fixtures/linter.py +++ b/tests/fixtures/linter.py @@ -44,13 +44,6 @@ ("feat: add new feature\n\nthis is body\n\ntest", True, []), ("feat: add new feature\n", True, []), ("build(deps-dev): bump @babel/traverse from 7.22.17 to 7.24.0", True, []), - ("feat(scope): add new feature\n#this is a comment", True, []), - ( - "fix: fixed a bug\n\nthis is body\n" - "# ------------------------ >8 ------------------------\nDiff message", - True, - [], - ), ("feat!: breaking feature", True, []), # ignored commits (success) ("Merge pull request #123", True, []), diff --git a/tests/test_cli.py b/tests/test_cli.py index 35a78e5..a047cb1 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -189,6 +189,20 @@ def test__main__valid_commit_message_with_file( main() mock_output_success.assert_called_with(f"{VALIDATION_SUCCESSFUL}") + @patch( + "commitlint.cli.get_args", + return_value=MagicMock(file="path/to/file.txt", skip_detail=False, quiet=False), + ) + @patch( + "builtins.open", + mock_open(read_data="feat: valid commit message 2\n#this is a comment"), + ) + def test__main__valid_commit_message_and_comments_with_file( + self, _mock_get_args, _mock_output_error, mock_output_success + ): + main() + mock_output_success.assert_called_with(f"{VALIDATION_SUCCESSFUL}") + @patch( "commitlint.cli.get_args", return_value=MagicMock(file="path/to/file.txt", skip_detail=False, quiet=False), diff --git a/tests/test_linter/test__linter.py b/tests/test_linter/test__linter.py index ffa1620..e994f64 100644 --- a/tests/test_linter/test__linter.py +++ b/tests/test_linter/test__linter.py @@ -1,14 +1,14 @@ # type: ignore # pylint: disable=all +from unittest.mock import patch + import pytest from commitlint.constants import COMMIT_HEADER_MAX_LENGTH from commitlint.linter import lint_commit_message -from commitlint.messages import ( - HEADER_LENGTH_ERROR, - INCORRECT_FORMAT_ERROR, -) +from commitlint.messages import HEADER_LENGTH_ERROR, INCORRECT_FORMAT_ERROR + from ..fixtures.linter import LINTER_FIXTURE_PARAMS @@ -30,6 +30,23 @@ def test__lint_commit_message__skip_detail(fixture_data): assert success == expected_success +def test__lint_commit_message__remove_comments_if_strip_comments_is_True(): + commit_message = "feat(scope): add new feature\n#this is a comment" + success, errors = lint_commit_message(commit_message, strip_comments=True) + assert success is True + assert errors == [] + + +@patch("commitlint.linter._linter.remove_comments") +def test__lint_commit_message__calls_remove_comments_if_strip_comments_is_True( + mock_remove_comments, +): + commit_message = "feat(scope): add new feature" + mock_remove_comments.return_value = commit_message + lint_commit_message(commit_message, strip_comments=True) + mock_remove_comments.assert_called_once() + + def test__lint_commit_message__skip_detail_returns_header_length_error_message(): commit_message = "Test " + "a" * (COMMIT_HEADER_MAX_LENGTH + 1) success, errors = lint_commit_message(commit_message, skip_detail=True) diff --git a/tests/test_linter/test_utils/test_remove_diff_from_commit_message.py b/tests/test_linter/test_utils/test_remove_diff_from_commit_message.py new file mode 100644 index 0000000..08f499d --- /dev/null +++ b/tests/test_linter/test_utils/test_remove_diff_from_commit_message.py @@ -0,0 +1,29 @@ +# type: ignore +# pylint: disable=all + +from commitlint.linter.utils import remove_diff_from_commit_message + + +def test__remove_diff_from_commit_message__without_diff(): + input_msg = "Commit message without diff" + expected_output = "Commit message without diff" + result = remove_diff_from_commit_message(input_msg) + assert result == expected_output + + +def test__remove_diff_from_commit_message__with_diff(): + input_msg = ( + "Fix a bug\n" + "# ------------------------ >8 ------------------------\n" + "Diff message" + ) + expected_output = "Fix a bug" + result = remove_diff_from_commit_message(input_msg) + assert result == expected_output + + +def test__remove_diff_from_commit_message__strips_commit_message(): + input_msg = "Commit message\n " + expected_output = "Commit message" + result = remove_diff_from_commit_message(input_msg) + assert result == expected_output