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: Introduce markdown renderer #480

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

miguelcrespo
Copy link

Markdown is one of the most ubiquitous formats to share content, many people rely on it to power their blogs and other tools, with this PR we introduce an official markdown renderer

@miguelcrespo miguelcrespo requested a review from a team June 22, 2023 15:26
[MARKS.BOLD]: (text: any) => `**${text}**`,
[MARKS.ITALIC]: (text: any) => `*${text}*`,
[MARKS.UNDERLINE]: (text: any) => `~~${text}~~`,
[MARKS.CODE]: (text: any) => `\`${text}\``,
Copy link
Contributor

Choose a reason for hiding this comment

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

Does markdown support superscript and subscript?

Copy link
Contributor

Choose a reason for hiding this comment

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

https://www.w3schools.io/file/markdown-super-sub-script/

Looks like it's just the html tags sub & sup

Copy link
Author

Choose a reason for hiding this comment

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

Yes, it indeed supports sub and sup but by using the HTML tags.

I'm not sure if we should try to address this here or leave it to users since it's up to the users if they want to support HTML tags

Copy link
Contributor

Choose a reason for hiding this comment

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

What happens with Tables?

},
[BLOCKS.QUOTE]: (node: any, next: any) => `> ${next(node.content)}\n`,
[BLOCKS.HR]: () => '\n---\n',
[BLOCKS.EMBEDDED_ENTRY]: () => '', // You can customize embedded entries if needed
Copy link
Contributor

Choose a reason for hiding this comment

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

What happens INLINES.EMBEDDED_ENTRY, INLINES.ENTRY_HYPERLINK, INLINES.ASSET_HYPERLINK?

I wonder would it be useful here to have some placeholder text for rendering these?

Something like <-- inline entry: abc123 --> what do you think?

"access": "public"
},
"scripts": {
"build": "tsc --module commonjs && rollup -c rollup.config.js",
Copy link
Contributor

Choose a reason for hiding this comment

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

Not for now but we should really migrate all the builds in this repo to SWC

...options.renderMark,
},
renderNode: {
[BLOCKS.PARAGRAPH]: (node: any, next: any) => `${next(node.content)}\n\n`,
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a type you can use here instead of any for node and next?

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