Skip to content

Conversation

@benediktziegler
Copy link

Description

This PR adds functionality to customise the commit message validation and to format the InvalidCommitMessageError to give better/more detailed feedback to the user.

Checklist

  • Add test cases to all the changes you introduce
  • Run ./scripts/format and ./scripts/test locally to ensure this change passes linter check and test
  • Test the changes on the local machine manually
  • Update the documentation for the changes

Expected behavior

The developer of a custom commitizen class can override the validate_commit_message and format_error_message methods to perform more complex commit message format checks then just a regex match and give more detailed feedback on failure.

Steps to Test This Pull Request

Run the the test_check_command_with_custom_validator_succeed and test_check_command_with_custom_validator_fail tests in test_check_command.py.

Additional context

This PR implements and fixes the comments from #648.

@codecov
Copy link

codecov bot commented Sep 13, 2024

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 97.93%. Comparing base (120d514) to head (e5805b6).
⚠️ Report is 807 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1236      +/-   ##
==========================================
+ Coverage   97.33%   97.93%   +0.59%     
==========================================
  Files          42       58      +16     
  Lines        2104     2713     +609     
==========================================
+ Hits         2048     2657     +609     
  Misses         56       56              
Flag Coverage Δ
unittests 97.93% <100.00%> (+0.59%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@benediktziegler benediktziegler force-pushed the feature/custom-validation branch from fbf3813 to 7bd16c3 Compare September 13, 2024 13:57
@benediktziegler
Copy link
Author

@Lee-W I was wondering if you had any input to this PR?

@Lee-W
Copy link
Member

Lee-W commented Sep 25, 2024

I'll be mostly out till at least mid-Oct, will try to check in depth after that. Thanks!

Copy link
Member

@Lee-W Lee-W left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall, I love this idea! left a few improvement suggestions

@Lee-W
Copy link
Member

Lee-W commented Dec 6, 2024

I'll be mostly out for the following month. Will try to take a look when I'm back.

Copy link
Member

@Lee-W Lee-W left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for taking so long. Some nitpicks. but we're close to merge I think!

if max_msg_length:
msg_len = len(commit_msg.partition("\n")[0].strip())
if msg_len > max_msg_length:
return ValidationResult(False, [])
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We probably would like to add errors here?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added an error message here and append all errors in InvalidCommitMessageError.

Let me know if that was your intention.

msg_len = len(commit_msg.partition("\n")[0].strip())
if msg_len > max_msg_length:
return ValidationResult(False, [])
return ValidationResult(bool(re.match(pattern, commit_msg)), [])
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and probably here as well?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added error message here too.

@Lee-W Lee-W added type: feature A new enhacement proposal and removed pr-status: wait-for-review labels Jan 30, 2025
@Lee-W Lee-W added this to the 4.10.0 milestone Sep 9, 2025
if any(map(commit_msg.startswith, allowed_prefixes)):
return ValidationResult(True, [])
if max_msg_length:
msg_len = len(commit_msg.partition("\n")[0].strip())
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we want user perceived length? Unicode characters may take more than 1 char, if we want the user perceived length we'll need to measure "graphemes". For example, an emoji like 🎏 may take up to 4 chars. And languages outside english may have the same problem.

What do you think @Lee-W ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are not many library options in python:

  • uniseg-py, last release: Jan 23, 2025, version 0.10.0, no dependencies, typed, supports Unicode 16
  • grapheme, relatively popular, last release 2020, no dependencies, no typed, supports Unicode 13
  • pyuegc, last release Jan 14, 2025, version 16.0.3, not sure I like the code, no typing. Supports Unicode 16.0.3
  • unicode-segmentation-py bindings to the well-maintained rust version, last release Mar 22, 2025

Tracking issue on cpython:
python/cpython#74902

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants