Skip to content

update markdown rendering approach #1133

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

Merged
merged 2 commits into from
Feb 12, 2022

Conversation

miguelcobain
Copy link
Collaborator

@miguelcobain miguelcobain commented Feb 12, 2022

What this PR does:

  • updates marked and highlight.js to the latest versions
  • changes the approach to markdown parsing

Previously we were parsing the markdown with the vanilla tokenizer from marked. This meant that we had to make some massaging of the tokens to successfully remove extra paragraphs when inside a curly or angle bracket syntax (see compactParagraphs function).

This new proposal makes use of marked's extensions, trying to accomplish a similar same thing.

One advantage is that the content inside an html or hbs block now is preserved, including whitespace.
As an example, previously compiling this markdown:

{{#foo-bar}}

    hello

{{/foo-bar}}

resulted in the following hbs:

<div class="docs-md"><p>{{#foo-bar}} hello {{/foo-bar}}</p></div>

But now it results in the the following:

<div class="docs-md">
<p>{{#foo-bar}}

    hello

{{/foo-bar}}</p>
</div>

Notice how everything inside {{#foo-bar}} and {{/foo-bar}} is preserved. Same thing happens for html/angle bracket components.
This explains why I had to change tests.

Test coverage was good, but I went manually to every single page in the docs app and everything looks good.

Since @dfreeman seems to have worked on this code in the past, I'm going to mention him. Would be great to get another pair of eyes, at least in the test changes. :)

@@ -5,7 +5,6 @@ import hljs from 'highlight.js/lib/core';
import javascript from 'highlight.js/lib/languages/javascript';
import css from 'highlight.js/lib/languages/css';
import handlebars from 'highlight.js/lib/languages/handlebars';
import htmlbars from 'highlight.js/lib/languages/htmlbars';
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

htmlbars is no longer a thing in highlight.js, being replaced with just handlebars.

@@ -28,6 +27,6 @@ hljs.registerLanguage('ts', typescript);

export default class DocsCodeHighlight extends Component {
setupElement(element) {
hljs.highlightBlock(element);
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

highlightBlock method was deprecated, being replaced with highlightElement.

@SergeAstapov
Copy link
Collaborator

@miguelcobain This is great! Thanks for working on this!

Copy link
Member

@RobbieTheWagner RobbieTheWagner left a comment

Choose a reason for hiding this comment

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

This looks amazing to me, thanks for doing this @miguelcobain!

@RobbieTheWagner RobbieTheWagner merged commit e5b3145 into ember-learn:master Feb 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants