3
\$\begingroup\$

I would like to create a grammar for the Conventional Commits spec and I would love to hear any feedback for what I wrote. The spec has some ambiguities, I think, hence my usage of "island grammars" to avoid writing predicates and such, which would limit the parser to a certain language target.

An example of a conventional commit message the parser should be able to handle

fix(some_module): this is a commit description
    
Some more in-depth description of what was fixed. This
can be a multi-line text, not only a one-liner.

Signed-off: [email protected]
Another-Key: another value
Some-Other-Key: some other value

The lexer:

lexer grammar ConventionalCommitLexer;
options { caseInsensitive = true; }
tokens { TEXT }

WS: [ \t]+ -> skip;
NEWLINE: ('\r')? '\n';

LPAREN: '(' -> mode(Scope);
RPAREN: ')';
SEMICOLON: ':' WS*-> mode(Description);

OTHER: 'other';
FEAT: 'feat';
FIX: 'fix';
DOCS: 'docs';
STYLE: 'style';
REFACTOR: 'refactor';
PERF: 'perf';
TEST: 'test';
CHORE: 'chore';
BUILD: 'build';
CI: 'ci';
BREAKING: 'breaking';
SECURITY: 'security';
REVERT: 'revert';
CONFIG: 'config';
UPGRADE: 'upgrade';
DOWNGRADE: 'downgrade';
PIN: 'pin';

IDENTIFIER: [a-z][a-z0-9_-]*;

mode Scope;
WS_SCOPE: [ \t] -> skip;
SCOPE: IDENTIFIER -> type(IDENTIFIER);
END_OF_SCOPE: RPAREN -> type(RPAREN), mode(DEFAULT_MODE);

mode Description;
DESCRIPTION: ~[\n]+ -> type(TEXT);
END_OF_TEXT: (WS | NEWLINE)+ -> mode(Body), type(NEWLINE);

mode Body;
END_OF_BODY: (WS | NEWLINE)+ -> mode(Footer), type(NEWLINE);
BODY: (SINGLE_LINE |  MULTI_LINE) -> type(TEXT);

SINGLE_LINE
  :  ~[\n]+
  ;

MULTI_LINE
  :  (SINGLE_LINE | (~[\n]+ NEWLINE))+ -> mode(Footer)
  ;

mode Footer;
WS_FOOTER: ' ' -> skip;
KEY: IDENTIFIER -> type(IDENTIFIER);
SEPARATOR: WS_FOOTER* SEMICOLON WS_FOOTER* -> type(SEMICOLON), mode(FooterValue);

mode FooterValue;
NEWLINE_FOOTER: NEWLINE -> type(NEWLINE), mode(Footer);
VALUE: ~[\n]+ -> type(TEXT);

The parser:

parser grammar ConventionalCommitParser;
options { tokenVocab = ConventionalCommitLexer; }

type: (OTHER
  |  FEAT
  |  FIX
  |  DOCS
  |  STYLE
  |  REFACTOR
  |  PERF
  |  TEST
  |  CHORE
  |  BUILD
  |  CI
  |  BREAKING
  |  SECURITY
  |  REVERT
  |  CONFIG
  |  UPGRADE
  |  DOWNGRADE
  |  PIN)           #RecognizedType

  | IDENTIFIER      #OtherType
  ;

footerKeyValue: key = IDENTIFIER SEMICOLON value = TEXT NEWLINE?;
commitMessage: type LPAREN scope = IDENTIFIER RPAREN SEMICOLON description = TEXT (NEWLINE body = TEXT)? (NEWLINE values += footerKeyValue+)? EOF;
\$\endgroup\$
1
  • \$\begingroup\$ I think you should use fragment more. Lexical symbols included in other lexical symbols should generally be fragments, if at all possible. \$\endgroup\$
    – rici
    Feb 4 at 2:52

1 Answer 1

1
\$\begingroup\$

Nice idea.

Obtaining conformant commit messages will be easier if much of a project's developer community installs a validating .git/hooks/pre-commit script.

I think a giant regex could handle the task, but this is far more readable!


SEMICOLON: ':' WS*-> mode(Description);

nit: Whitespace around the -> arrow, please, for readability.

Bigger item: Surely this is a COLON token, no?


{FEAT, FIX} are canonical, and then there are multiple vocabularies that a project could choose to adopt, such as Angular's {BUILD, CHORE, ...}. For such words, I feel you should

  1. Separate them out with a blank line.
  2. Cite the URL of your reference (as you nicely did in this question).
  3. Maybe alphabetize, even if (as here) the cited reference does not.

What we're shooting for is traceability, and ease of editing when the upstream reference inevitably changes the list of words.

For the same reason, consider putting BREAKING right after {FEAT, FIX}, to match the cited reference.


WS_SCOPE: [ \t] -> skip;

I am slightly sad that we didn't manage to recycle WS from above. Whatever.


I get the sense that you feel CRLF could potentially be a Problem.

DESCRIPTION: ~[\n]+ -> type(TEXT);

Soooo, that's going to lump \r CR in with the TEXT, right? Is that OK?

Both SINGLE_LINE & MULTI_LINE wrestle with the same detail. Oh, wait. Could DESCRIPTION take advantage of SINGLE_LINE ?

Maybe we'd be better off insisting that a "strip all CRs!" preprocessing step shall happen before lexing?


END_OF_TEXT: (WS | NEWLINE)+ -> mode(Body), type(NEWLINE);

Pair of tiny nits:

  1. WS or NEWLINE is kind of funny, it is whitespace. But yeah, I get it. Consider using the name BLANK for WS? To free up the name for this expression?
  2. I would find these slightly easier to read and compare if we consistently mentioned type / mode in the same order.

WS_FOOTER: ' ' -> skip;

Pretty sure you wanted to handle TABs, as well. Maybe we could reuse WS?


  |  PIN)           #RecognizedType

  | IDENTIFIER      #OtherType

I found the first comment helpful, thank you. The second produced some cognitive dissonance with OTHER, sigh!


footerKeyValue: key = IDENTIFIER SEMICOLON value = TEXT NEWLINE?;

The NEWLINE? feels like it's possibly trouble. Imagine that an annoying commit message mentioned Another-Key: aaa Looks-Like-A-Header: bbb ccc on a single line. I'm concerned that TEXT would pick up just aaa. It doesn't seem hard to accidentally stumble upon, perhaps with Priority: time to implement: soon. Or even with a URL.

Again, insisting on a preprocessing step, in this case one which appends \n so we're sure the document ends with NEWLINE, seems fairly prudent.

commitMessage: type LPAREN scope = IDENTIFIER RPAREN SEMICOLON description = TEXT (NEWLINE body = TEXT)? (NEWLINE values += footerKeyValue+)? EOF;

Wrap this at 80 chars, please, to improve readability.


Overall?

All issues are minor. Looks fine to ship and get some testing feedback from real-world inputs.

\$\endgroup\$
1
  • 1
    \$\begingroup\$ Thank you so much for so detailed a review! It helped a lot! I find it really helps to receive a feedback from someone else :) Overall, I found a rather big issue with the implementation I posted, it doesn't allow only body and only footer, it requires both of them to exist. In the end, I implemented something inspired by stackoverflow.com/a/75352176/320103. \$\endgroup\$
    – Michael
    Feb 16 at 6:06

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.