fix(parser): require whitespace after the ":" footer separator - #57
Open
svyatov wants to merge 1 commit into
Open
fix(parser): require whitespace after the ":" footer separator#57svyatov wants to merge 1 commit into
svyatov wants to merge 1 commit into
Conversation
Spec rule 8 allows only the ":" <space> and <space> "#" separator forms, but footer() accepted a bare ":", so "External-Id:1337" parsed as a footer. It now parses as body text. <pre-footer> is all or nothing, so one disqualified line demotes the whole block: in "Author: @bcoe\nExternal-Id:1337" the Author footer becomes body text as well, and its references entry is lost. Nothing disappears from the tree, and BREAKING CHANGE notes survive the demotion because toConventionalChangelogFormat reads the marker under <body> as well as under <token>. summary() keeps its ":" <whitespace>* lenience. Rules 1 and 5 impose the same requirement there, but tightening it would reject "feat:no space", which conventional-commits#47 does not ask for. Closes conventional-commits#47
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Spec rule 8 allows only the
:<space>and<space>#separator forms, butfooter()accepts a bare:, soExternal-Id:1337parses as a footer. #47 reports the gap in the README grammar; this fixes the grammar and the parser together, the way #39 was scoped.If you would rather not move parser behavior, say so, and I will cut this down to the README change alone. That is up to you.
The change
The
<whitespace>*step infooter()becomes required for":"and stays optional for" #", which carries its own space. Returning anErroris what the callers already expect:preFooter()backtracks andbody()reads the line as<text>.The AST is untouched.
<separator>keeps the value":"with<whitespace>as a sibling node, so every currently-valid parse is byte-identical,toConventionalChangelogFormatkeeps working, and the multiple-space case does not split awkwardly.In the README,
<footer>becomes two productions so the" #"asymmetry is explicit, matching how<summary>at line 68 already inlines its literal":".Behavior change
Input that parsed as a footer now parses as body text:
<pre-footer>is all or nothing, so one disqualified line demotes the whole block:That demotion is existing behavior, this just widens what triggers it. Nothing disappears from the tree.
BREAKING CHANGEnotes survive it, becausetoConventionalChangelogFormatreads the marker under<body>as well as under<token>, soBREAKING CHANGE:no spacestill reachescc.notes.Filed as plain
fix, giving 0.4.2 underbump-minor-pre-major: true: no API, node type, or field changed, only which strings qualify. Re-tag itfix!if you would rather have the minor bump.What deliberately did not change
summary()keeps its":" <whitespace>*lenience, sofeat(tree):no whitespaces herestill parses. Rules 1 and 5 impose the same "colon and space" there, but that is a separate question with much larger fallout, and #47 does not ask for it. TheBREAKING CHANGE:branch inbody()is likewise untouched, being a body-level extension rather than a footer.Also unrelated to #41: this is the
:<space>requirement, not the git-trailer whitespace rules.Tests
parses footer tokens without whitespaces after separatoris the one existing test whose verdict flips. It is renamed, converted to plain assertions, and its 218-line snapshot block deleted by hand, since chai-jest-snapshot does not prune orphans. I did not runtest:snap, so the other 20 blocks are byte-identical. Two footer tests are added: the" #"form, and the block-demotion case above.Locally, 36 passing,
standardclean, andnpm run coverageclears the.nycrcthresholds. Two of the three new assertions fail without the parser change.Closes #47