feat: allow emojis (gitmoji) in messages #222
Conversation
WalkthroughThe changes update the commit message validation by modifying the regular expression in the default configuration to include a broader Unicode range, such as emojis and various symbols. Additionally, new tests have been added to ensure that commit messages containing emojis are correctly validated by the system. No modifications have been made to the public or exported entities. Changes
Sequence Diagram(s)sequenceDiagram
participant T as Test
participant R as CommitMsgReader
participant V as Validator
T->>+R: get_default_commit_msg_file()
R-->>-T: default commit message file
T->>+R: read_commit_msg(file)
R-->>-T: "fix: 🐛 sample message"
T->>+V: check_commit_msg("fix: 🐛 sample message")
V-->>-T: 0 (success)
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
Please mind, that the check fails due to my name. See #221. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
commit_check/__init__.py (1)
19-19: LGTM! Consider using theregexlibrary for better maintainability.The updated regex pattern correctly supports emojis while maintaining backward compatibility. However, the complex Unicode ranges could be simplified using the
regexlibrary's\p{Emoji}syntax.Consider transitioning to the
regexlibrary to simplify the pattern:- 'regex': r'^(build|chore|ci|docs|feat|fix|perf|refactor|revert|style|test){1}(\([\w\-\.]+\))?(!)?: ([\U0001F600-\U0001F64F\U0001F300-\U0001F5FF\U0001F680-\U0001F6FF\U0001F700-\U0001F77F\U0001F780-\U0001F7FF\U0001F800-\U0001F8FF\U0001F900-\U0001F9FF\U0001FA00-\U0001FA6F\U0001FA70-\U0001FAFF\u2600-\u26FF\u2700-\u27BF] )?([\w ])+([\s\S]*)|(Merge).*|(fixup!.*)', + 'regex': r'^(build|chore|ci|docs|feat|fix|perf|refactor|revert|style|test){1}(\([\w\-\.]+\))?(!)?: (\p{Emoji} )?([\w ])+([\s\S]*)|(Merge).*|(fixup!.*)',tests/commit_test.py (1)
51-67: LGTM! Consider adding more test cases for comprehensive coverage.The test correctly verifies emoji support in commit messages. However, consider expanding the test coverage:
Add more test cases to verify different emoji scenarios:
+def test_check_commit_msg_with_various_emoji_patterns(mocker): + mock_get_default_commit_msg_file = mocker.patch( + "commit_check.commit.get_default_commit_msg_file", + return_value=".git/COMMIT_EDITMSG" + ) + + test_cases = [ + "feat: ✨ new feature", # Sparkles + "refactor: ♻️ cleanup code", # Recycle with variation selector + "style: 💅 polish UI", # Nail polish + "docs: 📚 update readme", # Books + "test: multiple 🧪 ✅ emojis", # Multiple emojis + ] + + checks = [next(c for c in DEFAULT_CONFIG['checks'] if c['check'] == 'message')] + + for msg in test_cases: + mock_read_commit_msg = mocker.patch( + "commit_check.commit.read_commit_msg", + return_value=msg + ) + + result = check_commit_msg(checks, commit_msg_file="") + assert result == 0 + mock_read_commit_msg.assert_called_once_with(".git/COMMIT_EDITMSG") + mock_read_commit_msg.reset_mock() + + mock_get_default_commit_msg_file.assert_called()
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
commit_check/__init__.py(1 hunks)tests/commit_test.py(2 hunks)
🔇 Additional comments (1)
commit_check/__init__.py (1)
19-19:✅ Verification successful
Verify the performance impact of the complex regex pattern.
The extensive Unicode ranges in the pattern might affect performance. Let's verify the impact:
🏁 Script executed:
#!/bin/bash # Description: Test regex pattern performance with and without emoji ranges # Create test file with sample commit messages cat << 'EOF' > test_commits.txt feat: add new feature fix: 🐛 fix bug refactor: ♻️ improve code chore: 🔧 update dependencies style: 💄 format code test: ✅ add tests EOF # Test performance using Python python3 - << 'EOF' import re import time import statistics # Original pattern (simplified for testing) original = r'^(feat|fix|refactor|style|test){1}(\([\w\-\.]+\))?(!)?: ([\w ])+([\s\S]*)' # New pattern with emoji support new = r'^(feat|fix|refactor|style|test){1}(\([\w\-\.]+\))?(!)?: ([\U0001F600-\U0001F64F\U0001F300-\U0001F5FF\U0001F680-\U0001F6FF\U0001F700-\U0001F77F\U0001F780-\U0001F7FF\U0001F800-\U0001F8FF\U0001F900-\U0001F9FF\U0001FA00-\U0001FA6F\U0001FA70-\U0001FAFF\u2600-\u26FF\u2700-\u27BF] )?([\w ])+([\s\S]*)' def test_pattern(pattern, messages, iterations=1000): times = [] compiled = re.compile(pattern) for _ in range(iterations): start = time.perf_counter_ns() for msg in messages: compiled.match(msg) end = time.perf_counter_ns() times.append(end - start) return statistics.mean(times) / 1_000_000 # Convert to milliseconds with open('test_commits.txt', 'r') as f: messages = f.readlines() original_time = test_pattern(original, messages) new_time = test_pattern(new, messages) print(f'Original pattern average time: {original_time:.3f}ms') print(f'New pattern average time: {new_time:.3f}ms') print(f'Performance impact: {((new_time - original_time) / original_time * 100):.1f}%') EOFLength of output: 116
Performance Impact Acceptable – Change Approved
The added Unicode ranges for emoji support do introduce a relative performance overhead (a roughly 43.6% increase), but in absolute terms the difference is minimal (from ~0.002ms to ~0.003ms per match). This impact is negligible in the context of commit message validation. Overall, the change maintains backward compatibility while extending support for emojis. As a minor improvement for long-term maintainability, consider exploring a dedicated regex library or alternative approaches to simplify such complex patterns if further issues arise.
- Verified performance using a test script.
- The absolute cost per match is insignificant.
- Suggest evaluating regex library options for clarity and potential future maintenance.
There are emojis out there that consist of a Unicode code point for the emoji and a variation selector too (another code point). See related test.
|
|
Hi @kohanyirobert, Thanks for your pull request! You're right—the default regex only covers the general scenario. But you can add your own |
|
Alright, got it. I'll close this PR then. |
To allow commit messages like
refactor: ♻️ don't use a monorepoand so I use devmoji a tool to automate and ease gitmoji usage.However the default regex doesn't allow emojis in the message. I'm not sure whether this is an oversight or that the conventional commit "spec" (if it is a spec) says that this not allowed.
If it doesn't disallow this I would wager it can be allowed, hence my PR.
Since Python
redoesn't support using\p{Emoji}I added a long messy character class in__init__.py.If it could considered to update to
regexthis long regex could be shortened considerable ... not sure it's worth the effort tho' and the extra dependency to manage.Summary by CodeRabbit
New Features
Tests