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

Valid complex regex doesn't match valid commit message #554

Closed
1 task done
ArtemkaKun opened this issue Mar 14, 2024 · 4 comments · Fixed by #573
Closed
1 task done

Valid complex regex doesn't match valid commit message #554

ArtemkaKun opened this issue Mar 14, 2024 · 4 comments · Fixed by #573
Assignees
Labels
bug Something isn't working

Comments

@ArtemkaKun
Copy link

Is there an existing issue for this?

  • I have searched the existing issues

Description of the bug

I have a complex regex ^(fix|feat|setup|doc|refactor|test|optimization)\([A-Za-z0-9_-]+\)(, (fix|feat|setup|doc|refactor|test|optimization)\([A-Za-z0-9_-]+?\))+(:\ .*)$ that matches commit message doc(repo), setup(ci): improve PR naming section in CONTRIBUTING

image

But for some reason, commit parser { message = '^(fix|feat|setup|doc|refactor|test|optimization)\([A-Za-z0-9_-]+\)(, (fix|feat|setup|doc|refactor|test|optimization)\([A-Za-z0-9_-]+?\))+(:\ .*)$', group = "TEST", default_scope = "other"} doesn't match this commit message when using this regex.

Steps To Reproduce

Config

[changelog]
body = """
{% if version %}\
    # [{{ version | trim_start_matches(pat="v") }}] - {{ timestamp | date(format="%Y-%m-%d") }}\
{% else %}\
    # [unreleased]\
{% endif %}\
{% for group, commits in commits | group_by(attribute="group") %}\
    \n
    ## {{ group | upper_first }}\
    {% for group, commits in commits | group_by(attribute="scope") %}\
    	\n
        ### {{ group | upper_first }}\n\
        {% for commit in commits | unique(attribute="message") %}\
            \n- {{ commit.message | split(pat="\n") | first | trim }}\
              {% if commit.github.username %} by @{{ commit.github.username }}{% endif %}\
              {% if commit.github.pr_number %} in #{{ commit.github.pr_number }}{% endif %}\
    	{% endfor %}\
    {% endfor %}\
{% endfor %}\
\n
## First-time contributors 🥳\n\
{% for contributor in github.contributors | filter(attribute="is_first_time", value=true) %}\
    \n- @{{ contributor.username }} made their first contribution in #{{ contributor.pr_number }}\
{% endfor %}\
"""

trim = true

[git]
conventional_commits = true
filter_unconventional = false
split_commits = false

commit_parsers = [
    { message = '^(fix|feat|setup|doc|refactor|test|optimization)\([A-Za-z0-9_-]+\)(, (fix|feat|setup|doc|refactor|test|optimization)\([A-Za-z0-9_-]+?\))+(:\ .*)$', group = "TEST", default_scope = "other"},
    { message = "^feat", group = "✨ Features" },
    { message = "^fix", group = "🪲 Bug Fixes" },
    { message = "^doc", group = "📚 Documentation" },
    { message = "^optimization", group = "🚀 Performance" },
    { message = "^refactor", group = "🧹 Refactor" },
    { message = "^setup", group = "⚙️ Configuration" },
    { message = "^test", group = "✅ Testing" }, 
]

protect_breaking_commits = false
filter_commits = false
tag_pattern = "v[0-9].*"
skip_tags = ""
ignore_tags = ""
topo_order = false
sort_commits = "oldest"

Expected behavior

The commit message doc(repo), setup(ci): improve PR naming section in CONTRIBUTING is visible in TEST section in the Other group

Screenshots / Logs

No response

Software information

Additional context

No response

@ArtemkaKun
Copy link
Author

If I remove $ end match symbol from regex - it matches the commit message as expected 🤔 Is it related to the additional \n or spaces at the end of the commit message?

@orhun
Copy link
Owner

orhun commented Mar 15, 2024

If I remove $ end match symbol from regex - it matches the commit message as expected 🤔 Is it related to the additional \n or spaces at the end of the commit message?

Yes, I just tested as well and that's the issue.

We can simply apply this patch:

diff --git a/git-cliff-core/src/commit.rs b/git-cliff-core/src/commit.rs
index fbb3ba1..3b00564 100644
--- a/git-cliff-core/src/commit.rs
+++ b/git-cliff-core/src/commit.rs
@@ -309,7 +309,7 @@ impl Commit<'_> {
 				}
 			}
 			for (regex, text) in regex_checks {
-				if regex.is_match(&text) {
+				if regex.is_match(&text.trim_end()) {
 					if self.skip_commit(parser, protect_breaking) {
 						return Err(AppError::GroupError(String::from(
 							"Skipping commit",

Do you think it makes sense or should we add a note about the usage of $ to the documentation?

@ArtemkaKun
Copy link
Author

Hi, thanks for the response. It's actually the tough one.

I think the process should start from research why there is \n at the end of common commit message without body and without \n at the end. Maybe this is how GitHub sends it. If so, probably the tool should trim this kind of new line character.

If this is something that Git do for some reasons and the tool can't automatically remove new line character because of reasons or edgecases or complex implementation - it should definitely appear in the documentation so users be aware of it.

@orhun
Copy link
Owner

orhun commented Mar 23, 2024

I decided to always trim it: #573

The commit message originally has a newline at the end because of git2 and I already trimmed that in #403

So doing this won't hurt I reckon :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants