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

feat: add greedy whitespace tokenizer #24

Merged
merged 5 commits into from
Dec 27, 2020
Merged

Conversation

byCedric
Copy link
Contributor

@byCedric byCedric commented Dec 25, 2020

This adds the whitespace parsing (as mentioned in #16), it's set (by default) to greedy. I think that's great and here is why:

  • When multiple whitespaces exist, it combines them into a single whitespace node (with exact value ofc)
  • When no whitespace token exist at the current position, it still returns an error. The method where this was invoked from can either decide to abort or continue.

Some good examples

Screenshot 2020-12-25 at 03 38 11

Screenshot 2020-12-25 at 03 38 30

Screenshot 2020-12-25 at 03 38 58

But not too permissive (yet)

Screenshot 2020-12-25 at 03 40 50

Screenshot 2020-12-25 at 03 41 52

@@ -26,6 +26,14 @@ describe('<message>', () => {
parsed = parser('feat(http parser)!: add support for scopes')
parsed.should.matchSnapshot()
})
it('parses summary with multiple spaces after separator', () => {
Copy link
Member

Choose a reason for hiding this comment

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

I think this change to the parser is great.

Mind updating the grammar in the README too, adding the permissive <whitespace>* marker wherever it's now supported?

Copy link
Contributor Author

@byCedric byCedric Dec 27, 2020

Choose a reason for hiding this comment

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

It was already in the grammar actually @bcoe

The only change I made was switching <continuation> from <whitespace> to <whitespace>+. (requires at least 1 whitespace)

@byCedric byCedric force-pushed the @bycedric/feat/whitespace branch from d4c8cf7 to 1529eb1 Compare December 27, 2020 15:03
@bcoe bcoe merged commit b0c64f9 into main Dec 27, 2020
@bcoe bcoe deleted the @bycedric/feat/whitespace branch December 27, 2020 23:41
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

Successfully merging this pull request may close these issues.

2 participants