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.

Sign up for GitHub

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

feat: support BREAKING CHANGE in body #30

Merged
merged 3 commits into from
Dec 29, 2020
Merged

feat: support BREAKING CHANGE in body #30

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1d5a798
feat: support BREAKING CHANGE in body
bcoe Dec 29, 2020
17fc66d
docs: remove punctuation
bcoe Dec 29, 2020
451dde8
Merge branch 'main' into implement-breaking
bcoe Dec 29, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
90 changes: 47 additions & 43 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,49 +42,53 @@ in sync with the written specification on conventionalcommits.org.

```ebnf
/* See: https://tools.ietf.org/html/rfc3629#section-4 */
<UTF8-char> ::= "Placeholder for UTF-8 grammar"
<UTF8-octets> ::= <UTF8char>+

<CR> ::= "0x000D"
<LF> ::= "0x000A"
<newline> ::= [<CR>], <LF>
<parens> ::= "(" | ")"
<ZWNBSP> ::= "U+FEFF"
<TAB> ::= "U+0009"
<VT> ::= "U+000B"
<FF> ::= "U+000C"
<SP> ::= "U+0020"
<NBSP> ::= "U+00A0"
<UTF8-char> ::= "Placeholder for UTF-8 grammar"
<UTF8-octets> ::= <UTF8char>+

<CR> ::= "0x000D"
<LF> ::= "0x000A"
<newline> ::= [<CR>], <LF>
<parens> ::= "(" | ")"
<ZWNBSP> ::= "U+FEFF"
<TAB> ::= "U+0009"
Copy link
Member Author

Choose a reason for hiding this comment

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

adding the breaking-change node forced me to indent for consistency.

<VT> ::= "U+000B"
<FF> ::= "U+000C"
<SP> ::= "U+0020"
<NBSP> ::= "U+00A0"
/* See: https://www.ecma-international.org/ecma-262/11.0/index.html#sec-white-space */
<USP> ::= "Any other Unicode 'Space_Separator' code point"
<USP> ::= "Any other Unicode 'Space_Separator' code point"
/* Any non-newline whitespace: */
<whitespace> ::= <ZWNBSP> | <TAB> | <VT> | <FF> | <SP> | <NBSP> | <USP>

<message> ::= <summary>, <newline>+, <body>, <newline>*, <footer>+
| <summary>, <newline>*, <footer>+
| <summary>, <newline>*

<summary> ::= <type>, "(", <scope>, ")", ["!"], ":", <whitespace>*, <text>
| <type>, ["!"], ":", <whitespace>*, <text>
<type> ::= <any UTF8-octets except newline or parens or ":" or "!:" or whitespace>+
<scope> ::= <any UTF8-octets except newline or parens>+
<text> ::= <any UTF8-octets except newline>*

/*
* Note: if the first <body> node starts with "BREAKING CHANGE:" this should
* be treated by parsers as a breaking change marker upstream:
*/
<body> ::= [<any text except pre-footer>], <newline>, <body>*
| [<any text except pre-footer>]
/* Note: <pre-footer> is used during parsing, but never returned in the AST. */
<pre-footer> ::= <newline>*, <footer>+

<footer> ::= <token>, <separator>, <whitespace>*, <value>, [<newline>]
<token> ::= "BREAKING CHANGE"
| <type>, "(" <scope> ")", ["!"]
| <type>, ["!"]
<separator> ::= ":" | " #"
<value> ::= <text>, <continuation>+
| <text>
<continuation> ::= <newline>, <whitespace>+, <text>
<whitespace> ::= <ZWNBSP> | <TAB> | <VT> | <FF> | <SP> | <NBSP> | <USP>

<message> ::= <summary>, <newline>+, <body>, <newline>*, <footer>+
| <summary>, <newline>*, <footer>+
| <summary>, <newline>*

/* "!" should be added to the AST as a <breaking-change> node with the value "!" */
Copy link
Member Author

Choose a reason for hiding this comment

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

Added note that ! should be added as a breaking-change node to the AST (this is already the behavior).

<summary> ::= <type>, "(", <scope>, ")", ["!"], ":", <whitespace>*, <text>
| <type>, ["!"], ":", <whitespace>*, <text>
<type> ::= <any UTF8-octets except newline or parens or ":" or "!:" or whitespace>+
<scope> ::= <any UTF8-octets except newline or parens>+
<text> ::= <any UTF8-octets except newline>*


<body> ::= [<any body-text except pre-footer>], <newline>, <body>*
Copy link
Member Author

Choose a reason for hiding this comment

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

introduced a <body-text> node, which is <text> with an optional BREAKING CHANGE prefix.

| [<any body-text except pre-footer>]
/* For convenience the <breaking-change>, <separator>, <whitespace>, and
Copy link
Member Author

Choose a reason for hiding this comment

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

Note for implementers that we don't want to add a <body-text> node to the AST.

* <text> tokens of <body-text> should be appended as children to <body> */
<body-text> ::= [<breaking-change>, ":", <whitespace>*], text
/* Note: <pre-footer> is used during parsing, but not returned in the AST. */
<pre-footer> ::= <newline>*, <footer>+

<footer> ::= <token>, <separator>, <whitespace>*, <value>, [<newline>]
/* "!" should be added to the AST as a <breaking-change> node with the value "!" */
<token> ::= <breaking-change>
| <type>, "(" <scope> ")", ["!"]
| <type>, ["!"]
<separator> ::= ":" | " #"
<value> ::= <text>, <continuation>+
| <text>
<continuation> ::= <newline>, <whitespace>+, <text>

<breaking-change> ::= "BREAKING CHANGE" | "BREAKING-CHANGE"
Copy link
Member Author

Choose a reason for hiding this comment

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

The text on conventionalcommits.org indicates that we should support BREAKING-CHANGE.

```
26 changes: 19 additions & 7 deletions lib/parser.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -166,15 +166,25 @@ function scope (scanner) {
}

/*
* <body> ::= [<any text except pre-footer>], <newline>, <body>*
* | [<any text except pre-footer>]
* <body> ::= [<any body-text except pre-footer>], <newline>, <body>*
* | [<any body-text except pre-footer>]
*/
function body (scanner) {
const node = scanner.enter('body', [])
// <any text except pre-footer>

// check except <pre-footer> condition:
const pf = preFooter(scanner)
if (!(pf instanceof Error)) return scanner.abort(node)

// ["BREAKING CHANGE", ":", <whitespace>*]
const b = breakingChange(scanner, false)
if (!(b instanceof Error) && scanner.peek() === ':') {
node.children.push(b)
node.children.push(separator(scanner))
const w = whitespace(scanner)
if (!(w instanceof Error)) node.children.push(w)
}

// [<text>]
const t = text(scanner)
node.children.push(t)
Expand Down Expand Up @@ -282,13 +292,15 @@ function token (scanner) {
}

/*
* <breaking-change> ::= "!" | "BREAKING CHANGE"
* <breaking-change> ::= "!" | "BREAKING CHANGE" | "BREAKING-CHANGE"
*
* Note: "!" is only allowed in <footer> and <summary>, not <body>.
*/
function breakingChange (scanner) {
function breakingChange (scanner, allowBang = true) {
const node = scanner.enter('breaking-change', '')
if (scanner.peek() === '!') {
if (scanner.peek() === '!' && allowBang) {
node.value = scanner.next()
} else if (scanner.peekLiteral('BREAKING CHANGE')) {
} else if (scanner.peekLiteral('BREAKING CHANGE') || scanner.peekLiteral('BREAKING-CHANGE')) {
node.value = scanner.next('BREAKING CHANGE'.length)
}
if (node.value === '') {
Expand Down
18 changes: 17 additions & 1 deletion test/parser.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,9 @@ describe('<message>', () => {
parsed.should.matchSnapshot()
})
it('parses BREAKING CHANGE literal as <token>', () => {
const parsed = parser('fix: address major bug\nBREAKING CHANGE: this change is breaking')
let parsed = parser('fix: address major bug\nBREAKING CHANGE: this change is breaking')
parsed.should.matchSnapshot()
parsed = parser('fix: address major bug\nBREAKING-CHANGE: this change is breaking')
parsed.should.matchSnapshot()
})
it('supports multiline BREAKING CHANGES, via continuation', () => {
Expand Down Expand Up @@ -102,11 +104,25 @@ describe('<message>', () => {
assertNodePositions('fix: address major bug\n\nthis is a free form body of text')
})
})
describe('<body>', () => {
it('parses BREAKING CHANGE at start of body', () => {
const parsed = parser('feat: breaking change\n\nBREAKING CHANGE: introduces breaking change\nsecond line')
parsed.should.matchSnapshot()
})
})
describe('<body>, <newline>*, <footer>+', () => {
it('parses footer after body', () => {
const parsed = parser('fix: address major bug\n\nthis is a free form body of text\nAuthor: @bcoe\nRefs #392')
parsed.should.matchSnapshot()
})
it('parses footer after body containing BREAKING CHANGE', () => {
const parsed = parser('fix: address major bug\n\nBREAKING CHANGE: this is breaking.\nthis is a free form body of text\nAuthor: @bcoe\nRefs #392')
parsed.should.matchSnapshot()
})
it('parses BREAKING CHANGE footers with higher precedence than body', () => {
const parsed = parser('fix: address major bug\n\nBREAKING CHANGE: this is breaking.\n\nAuthor: @bcoe\nRefs #392')
parsed.should.matchSnapshot()
})
it('parses footer after multi-line body', () => {
const parsed = parser('fix: address major bug\n\nthis is the first line of the body\n\nthis is the second line of body\n\nAuthor: @bcoe\nRefs #392')
parsed.should.matchSnapshot()
Expand Down
Loading