Skip to content
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.

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

fix: handle commit message starting with # #59

Merged
merged 1 commit into from
Jul 24, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
8 changes: 7 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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
Expand Down
7 changes: 0 additions & 7 deletions codecov.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,3 @@

# Disabling comments
comment: false

# Disabling Github checks
github_checks: false
coverage:
status:
project: off
patch: off
28 changes: 16 additions & 12 deletions src/commitlint/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -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


Expand Down Expand Up @@ -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.
Expand All @@ -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")

Expand Down Expand Up @@ -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)


Expand All @@ -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:
Expand All @@ -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)
Expand Down
9 changes: 6 additions & 3 deletions src/commitlint/linter/_linter.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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
Expand All @@ -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")
Expand Down
39 changes: 25 additions & 14 deletions src/commitlint/linter/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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()
7 changes: 0 additions & 7 deletions tests/fixtures/linter.py
Original file line number Diff line number Diff line change
Expand Up @@ -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, []),
Expand Down
14 changes: 14 additions & 0 deletions tests/test_cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
25 changes: 21 additions & 4 deletions tests/test_linter/test__linter.py
Original file line number Diff line number Diff line change
@@ -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


Expand All @@ -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)
Expand Down
Original file line number Diff line number Diff line change
@@ -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
Loading