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

Inline tokens #1627

Merged
merged 11 commits into from Apr 8, 2020
Merged

Inline tokens #1627

merged 11 commits into from Apr 8, 2020

Conversation

UziTech
Copy link
Member

@UziTech UziTech commented Mar 25, 2020

Description

Right now the Lexer returns the block tokens and the Parser sends the text to the InlineLexer which sends the information to the Renderer. There is no way to get the inline tokens before sending them to the Renderer.

This PR changes the Lexer to return all tokens with block tokens that have tokens inside of them in a tokens property on the token and changes the Parser to act on all tokens instead of the text from the block tokens.

Each token that has inline or block tokens will have a tokens property that is an array of tokens.

The list token will have an items property which will be an array of list items. Each list item will have a tokens property with the tokens inside the item.

This PR also adds a raw property to the tokens so a user can get the markdown that created that token.

fixes #1626
fixes #361

TODO:

  • Write unit tests for Lexer, and Parser
  • Make sure demo works
    • You can see the new tokens here
  • Update docs
  • Refactor to make benchmarks faster
    • About 5-10% faster than master
    • It looks like this sped the compiled es5 version (lib/marked.js) up even more.

Contributor

  • Test(s) exist to ensure functionality and minimize regression.
  • If submitting new feature, it has been documented in the appropriate places.

Committer

In most cases, this should be a different person than the contributor.

  • Draft GitHub release notes have been updated.
  • CI is green (no forced merge required).
  • Merge PR

@vercel
Copy link

vercel bot commented Mar 25, 2020

This pull request is being automatically deployed with ZEIT Now (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://zeit.co/markedjs/markedjs/qba5ws1f2
✅ Preview: https://markedjs-git-fork-uzitech-inline-tokens.markedjs.now.sh

@UziTech
Copy link
Member Author

UziTech commented Mar 25, 2020

Each token that has inline tokens will have a tokens property that is an array of inline tokens.

I wasn't sure about how to add the inline tokens to the tokens array. Some tokens (like lists and blockquotes) have other tokens inside of them already so they have start and end tokens (e.g. list_start, list_end) before and after the tokens inside of them. I figured we could go one of three ways:

  1. Change all tokens that have inline tokens to have start and end tokens (e.g. change header tokens to be header_start and header_end with the inline tokens between them.)
  2. Give all tokens that have inline tokens a tokens property that contains their inline tokens.
  3. Keep block tokens that can have other block tokens inside of them with start and end tokens and have all inline tokens added to a tokens property.

This PR currently does #3 but I am leaning toward #2. I think #2 is the cleanest but it would create major breaking changes for any dependents currently using the tokens from marked.lexer.

@UziTech
Copy link
Member Author

UziTech commented Mar 27, 2020

One thing I noticed while writing tests for the InlineLexer is that the mangle options mangles autolinks (e.g. <test@example.com>) but not plain email in gfm (e.g. test@example.com) demo

So I fixed it. demo

@UziTech
Copy link
Member Author

UziTech commented Mar 28, 2020

I'm not sure why travis ci isn't reporting but the tests are passing. travis ci log

.travis.yml Outdated Show resolved Hide resolved
docs/USING_PRO.md Outdated Show resolved Hide resolved
Copy link
Member

@styfle styfle left a comment

Choose a reason for hiding this comment

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

I didn't review all the tests thoroughly but looks good! 🥇

@UziTech
Copy link
Member Author

UziTech commented Mar 31, 2020

@styfle what do you think about #1627 (comment)?

@styfle
Copy link
Member

styfle commented Mar 31, 2020

Can you give an example of what the token output would look like for the 3 examples?

Also, our PR demo doesn't seem to work anymore

image

@UziTech
Copy link
Member Author

UziTech commented Apr 2, 2020

I have rebased and implemented option 2 from #1627 (comment)

It didn't make sense to keep the InlineLexer so I just merged it into the Lexer

@UziTech
Copy link
Member Author

UziTech commented Apr 7, 2020

I renamed the lexer functions and reordered the parameters for the lexer so they are consistent.

@styfle I think this is ready to merge if you approve.

Copy link
Member

@styfle styfle left a comment

Choose a reason for hiding this comment

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

Great work!

@UziTech UziTech merged commit 743ec55 into markedjs:master Apr 8, 2020
@UziTech UziTech deleted the inline-tokens branch April 8, 2020 17:54
@experimentalPrivateRepos

Hey @UziTech, thank you for the great work.
I would like to use the updated Lexer in a personal project of mine.
Do you have any idea when this new version will be available on npm?

@UziTech
Copy link
Member Author

UziTech commented Apr 9, 2020

@experimentalPrivateRepos We have a few more changes to get into the next release. I would estimate it will be about a month before the next release.

@experimentalPrivateRepos

@UziTech Alright, thank you for the quick response!

@upupming
Copy link

upupming commented Dec 5, 2020

I have rebased and implemented option 2 from #1627 (comment)

It didn't make sense to keep the InlineLexer so I just merged it into the Lexer

Hi, since the InlineLexer class has been deleted, can you delete the type definitions in @types/marked too, thanks!
image

zhenalexfan pushed a commit to zhenalexfan/MarkdownHan that referenced this pull request Nov 8, 2021
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.

[Question] Can marked return JSON instead of HTML? Attach original text to tokens
4 participants