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(message-parser): Parser options #739

Merged
merged 15 commits into from
Jun 15, 2022
Merged

feat(message-parser): Parser options #739

merged 15 commits into from
Jun 15, 2022

Conversation

tassoevan
Copy link
Collaborator

@tassoevan tassoevan commented Jun 8, 2022

Proposed changes (including videos or screenshots)

Add some grammar rules activated by options, like colors and katex.

Issue(s)

Further comments

@tassoevan tassoevan marked this pull request as ready for review June 9, 2022 22:00
@tassoevan tassoevan requested a review from a team June 9, 2022 22:00
@hugocostadev
Copy link
Contributor

The tokens and tests seem OK... @ggazzo do you want to check also?

@ggazzo
Copy link
Member

ggazzo commented Jun 14, 2022

just wondering how much this changes adds terms of sizing and if brings any performance issue

@tassoevan
Copy link
Collaborator Author

just wondering how much this changes adds terms of sizing and if brings any performance issue

Before:

56K    packages/message-parser/dist/messageParser.production.js

After:

76K    packages/message-parser/dist/messageParser.production.js

@hugocostadev
Copy link
Contributor

@ggazzo and @tassoevan
I made a benchmark between this grammar and develop branch grammar.
The markdown file used has all type blocks/elements and more than 2k lines of markdown text.

Here it is the results:

image

Considering that includes more functionalities I think that it is expected an increase of Avg. Time to parse a file.

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

Successfully merging this pull request may close these issues.

3 participants