-
Notifications
You must be signed in to change notification settings - Fork 952
feat(rules): add scope-delimiter-style #4580
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
base: master
Are you sure you want to change the base?
Conversation
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. |
|
Pressed the copilot review. Have a look if any of that is valid or useful, thanks! |
|
Hey @escapedcat! Thanks a lot, some of Copilot's suggestions were pretty helpful. I've pushed the changes. Whenever you have a moment, could you take a look? |
|
lgtm, @JounQin would you mind having a look? |
|
Hey @escapedcat ! While we're waiting for additional review, I took another look at the code (with a fresh mind) and made a few small changes related to naming, to make it more consistent and unified (without affecting code quality). Hope that's okay 🙏 |
|
|
||
| const checks = (Array.isArray(value) ? value : [value]).map((check) => { | ||
| const checks = ( | ||
| isObjectBasedConfiguration |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we just check 'cases' in value?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wanted to do it like that initially, but it's not enough for TypeScript.
Type 'TargetCaseType | TargetCaseType[] | { cases: TargetCaseType[]; delimiters?: string[] }' is not assignable to type 'object'. Type 'string' is not assignable to type 'object'.
We could also do:
const checks = (
typeof value === "string"
? [value]
: Array.isArray(value)
? value
: value.cases
But to me, it's more verbose, because later, when we need to read delimiters, we'd need a similar check again. That's why I decided to introduce an extra variable.
| const delimiters = /\/|\\|, ?/g; | ||
| const scopeSegments = scope.split(delimiters); | ||
| const delimiters = | ||
| isObjectBasedConfiguration && value.delimiters?.length |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just check 'delimiters' in value instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Check the comments above please, same here
Description
This PR introduces a new rule,
scope-delimiter-style, which validates that commit scopes use only the allowed delimiters. This ensures consistent behavior when custom delimiters are configured.It also adds support for passing custom
delimitersto thescope-enumandscope-caserules, allowing users to redefine how multi-segment scopes are split and validated.The default delimiters (
/,\,,) remain unchanged unless explicitly overridden.Documentation has been updated to reflect:
scope-enumandscope-casescope-delimiter-styleruleThis brings delimiter handling across all scope-related rules to a unified and configurable model.
Motivation and Context
Resolves #701
Current scope-related rules (
scope-enum,scope-case) use hardcoded delimiters to split multi-segment scopes. This makes it impossible for users to adapt commitlint to projects that use different delimiter conventions.This PR introduces configurable delimiters and a dedicated
scope-delimiter-stylerule, enabling consistent validation across all scope rules. It provides a unified and flexible way to define how scopes should be parsed and validated, solving the long-standing limitation described in the linked issue.Usage examples
How Has This Been Tested?
All changes are covered by an extended test suite:
Updated existing tests for
scope-enumandscope-caseto ensure backward compatibility with default delimiters.Added new tests for object-based configuration, including custom delimiter lists and empty delimiter arrays.
Added a full test suite for the new
scope-delimiter-stylerule, covering:alwaysandneverconditions-and_Types of changes
Checklist: