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

Allow user-defined patterns #44

Closed
gibson042 opened this issue Nov 2, 2015 · 6 comments
Closed

Allow user-defined patterns #44

gibson042 opened this issue Nov 2, 2015 · 6 comments

Comments

@gibson042
Copy link

For example, to require subject lines consisting of semver level, one or more repository-specific components (separated by commas or comma-space pairs and ending with a colon), and a non-empty sentence-case message, all of which must be separated by spaces:

"commitplease": {
  // Braces like http://tools.ietf.org/html/rfc6570 URI Template expressions
  // Expression syntax like https://tools.ietf.org/html/rfc5234#section-3.6 ABNF repetition
  // …with introduction of optional separator definition in ECMAScript RegExp syntax
  "subjectPattern": "{semver-level} {1* ,[\\x20]? components}: {message}",

  // Variables are usually arrays of acceptable values
  "semver-level": [ "major", "minor", "patch" ],
  "components": [ "Build", "Test", "Core", "Legacy" ],

  // Variables can also be ECMAScript regular expressions in ESTree format
  // https://github.com/estree/estree/blob/master/spec.md#regexpliteral
  "message": {
    "regex": { "pattern": "^[^a-z\\s].*" },

    // …plus an optional human-readable description
    "description": "non-empty and sentence case (initial letter capitalized)"
  }
}

Another possibility is dropping the RegExp/variable expansion options in favor of pure RFC 5234 + RFC 7405 ABNF.

@mgol
Copy link

mgol commented Nov 2, 2015

Note that for the jQuery use case we might not want to prevent contributors from commiting if they don't include the semver level, this may be a barrier to contribute. Unless we have a commit template they just modify.

@gibson042
Copy link
Author

I agree, but consider that a separate issue. Ideally, we'll get to a point where things like commit message formatting is purely an issue for maintainers.

@mgol
Copy link

mgol commented Nov 2, 2015

Sounds good, just wanted to have people keep that in mind.

@jzaefferer
Copy link
Owner

For the record, I'm interested in improving the validation, but will wait for the parent discussion on contribute to get resolved.

@alisianoi
Copy link
Collaborator

alisianoi commented Jun 22, 2016

Initially, I was thinking that specifying a commit-message style with a context-free grammar is a great idea. However, after playing a tiny bit with an implementation of an ABNF parser (its homepage, I looked at JavaScript APG: Version 2.0 and JavaScript APG: Examples), here are some challenges that come up:

  1. I do not know how to efficiently communicate what is wrong with the message to the end user. By default the parser will only give the line numbers of lines with mistakes. You could register callbacks for Rules (as described here) that would get some details. However, since these rules are created by the user (in the package.json, as is suggested at the top), there is no way to prepare the callback functions in advance. So, if the user wants better error messages, then it looks like they have to write callbacks themselves too.
  2. The "semver is obligatory somewhere in the body" logic discussed above is not possible with pure context-free grammar (I am pretty sure) and needs a non-greedy extension to it:
"grammar": {
    "commit": "{header}\n\n{body}\n\n{footer}}",
    "header": ...
    "body": "{semver}/{text}" <-- allows a body without a semver
    "body": "{text}{semver}{text}" <-- is not a context-free rule (semver is surrounded by text) 
}

That parser supports such an extension. However, the user must first understand the limitation, then must configure the parser. That is in my opinion a lot more work than opening up commitplease and fixing it by hand.

@jzaefferer
Copy link
Owner

Thanks for investigating this. Since helpful error messages are central to commitplease, I don't think this is a direction we should explore further.

@gibson042 if validating/warning for semver lines is still interesting to you, please create a separate issue for that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants