Pre-commit hook error message does not state the correct commitMsgRegexPattern

[Mo0rBy]

Hey there,

I just realised this because I had my commit message slightly incorrect.

I have a custom regex pattern set (I suggested some changes a few days ago to add this nice config option to the documentation) and the pre-commit hook is showing the standard regex pattern, rather than the custom one.
Here is my custom regex which just includes a small addition to the beginning for a ticket/issue ID:

commitMessageRegexPattern: ^(?<ticket>[A-Z]*-[A-Za-z0-9]*) \| (?<type>\w+)(?:\((?<scope>[^()]+)\))?(?<breaking>!)?:\s*(?<description>.+)$

And here is the pre-commit error message stating to use the default format:

❯ gcmsg "[WM] DSO-108 | feat: add GenericWebhookTrigger and update DSOHub/build/tools-images to be configured with this trigger"
trim trailing whitespace.................................................Passed
fix end of files.........................................................Passed
Detect hardcoded secrets.................................................Passed
conventional-commits.....................................................Failed
- hook id: conventional-commits
- exit code: 2

[WARN] Invalid commit subject format: '[WM] DSO-108 | feat: add GenericWebhookTrigger and update DSOHub/build/tools-images to be configured with this trigger'
[ERROR] Invalid commit message
  Valid message pattern: <TYPE>[(<SCOPE>)]: <DESCRIPTION>

I think the final line that states the valid message pattern should show what is defined in the commitMessageRegexPattern config option.

[Mo0rBy]

Looks like it's this part of the code that needs to be updated: https://github.com/qoomon/git-conventional-commits/blob/master/lib/commands/commandCommitMessageHook.js#L29-L33

I'd be happy to take a look at some changes tomorrow, it is too late for me start looking right now.

[Mo0rBy]

I've had some time to look at this and I think the parseCommitSubject function needs to be changed up a bit.

I think there should be a place within the parsing functions that errors when the regex format does not fit.

Is there a difference between checking for things in gitCommandConvention.js - parseCommitSubject() & commandCommitMessageHook.js? It looks like there are some duplicate checks happening in both maybe? But only 1 of them gives back an error, the other just warns.

I'm still happy to do the actual changes. I just want to gather some info first.

Maybe parseCommitSubject() should:

1. Check the regex is valid - error if not valid
2. Check the type is a valid type - error or warn if not valid?
3. Check the scope is a valid scope (if given) - error or warn if not valid?
4. Check the description is valid (is this just that it's not empty?)
5. And then the merge/revert checks (I'm not quite sure what these are for)

And then maybe erroring/warning for valid types/scopes could be a configurable thing?
I personally would prefer a hard error, as the point of this is to stop commits being made if the message is not of the correct format, but maybe not all users would want that behaviour.

[Mo0rBy]

Here's what I've done so far on refactoring the parseCommitSubject() function: master...Mo0rBy:git-conventional-commits:update-regex-error-message

I need some help understanding what the msgMatchMerge and msgMatchRevert stuff does please.
I can't see anything that explains what they are supposed to do in the docs and there are merge & revert types in the default list of commitTypes, so I would think that the previous lines of code can unpack/parse those commitTypes just fine.

[qoomon]

@Mo0rBy thx a lot, I'll probably will have time to fix that during upcoming weekend.

[Mo0rBy]

I am looking at this again to add more tests to test this new regex checking and I've come into some trouble with loading custom configuration for tests. I've tried doing something like this:

    const conventionConfig = Config.defaultConfig().convention;
    conventionConfig.commitMessageRegexPattern = '/^custom prefix | (?<type>\w+)(?:\((?<scope>[^()]+)\))?(?<breaking>!)?:\s*(?<description>.+)/'
    const commitConvention = new CommitConvention(conventionConfig);

This doesn't quite work and raises a question. Most of the tests utilise the Config.defaultConfig() function, what is the point of this?
I think it would be better to load the config from the git-conventional-commits.default.yaml file, as then there is a single source for what the default configuration is.
It also then makes the tests slightly better because all users will load a config from a git-conventional-commits.yaml file within their repository, so loading a config file into the tests like simulates a real use case more closely.

The reason that the above unit test snippet does not work is because of this at the bottom of the Config.defaultConfig() function:

    // regex patterns
    config.convention.msgRegex = /^(?<type>\w+)(?:\((?<scope>[^()]+)\))?(?<breaking>!)?:\s*(?<description>.+)/i;
    config.convention.msgMergeRegex = /^Merge branch +["'](?<branch>.+)['"]/i;
    config.convention.msgRevertRegex = /^Revert +["'](?<subject>.+)['"]/i;
    config.convention.bodyRevertRegex = /(?<hash>\S+)\.+?$/im; // \S - no white spaces
    // match only release version e.g. 1.2.3 ((?:[^-]|$) ignores pre-release versions e.g. 1.2.3-SNAPSHOT)
    config.convention.semanticVersionRegex = /(?<major>\d+)\.(?<minor>\d+)\.(?<patch>\d+)(?:[^-]|$)/;

    config.changelog.commitIgnoreRegex = () => RegExp(config.changelog.commitIgnoreRegexPattern || '(?!)');

    config.convention.issueRegex = () => RegExp(
        "(?<=(^|\\s))" + (config.convention.issueRegexPattern || '(?!)') + "(?=(\\s|$))",
        "mg"
    );

Why are all these set here? Is there a specific reason I have overlooked?
If not, I think it makes more sense to have some sensible defaults in the git-conventional-commits.default.yaml file, then there is a single source of truth for the default config and it is more accessible for users to see.