Skip to content

Conversation

@richdouglasevans
Copy link
Contributor

@richdouglasevans richdouglasevans commented Sep 11, 2018

Right, I was sufficiently bugged by this to investigate it.

It looks to be a bug in the Markdown library that Hexo uses. (marked)

Even though the SmartyPants "spec" says that content inside a <code> block is not to be SmartyPants-ed, I have tested a small file against the library directly and it does SmartyPants-ify content in such blocks:

Input

--report <code>--report</code>

Output

<p>–report <code>–report</code></p>

Code that is fenced in backticks doesn't exhibit the same behaviour:

Input (some Markdown)

--report `--report`

Output

<p>–report <code>--report</code></p>

Right, onto the Cypress code. We have content with nested tags:

{% note info %}
`--report` {% url `--report` foo.html %}
{% endnote %}

What happens is that the url tag is processed first and it spits out HTML into the surrounding block:

{% note info %}
`--report` <a href="foo.html"><code>--report</code></a>
{% endnote %}

The entire content of the note block is then rendered into Markdown... and as we saw with my wee example at the start of this comment the -- inside the <code> block is SmartyPants-ed, resulting in:

<code>--report</code> <a href="foo.html"><code>–report</code></a>

I'll have a shuftie at sorting the issue at source in the underlying marked library.

In the interim, I submit a PR that addresses the issue tactically.

sweet

I feel kinda ill submitting this: it is my sincere hope that you'll be so outraged by how bleurgh it is that you'll submit a PR that fixes it in another brilliant way.

return Promise.resolve(
toMarkdown(content)
.replace('–', '--')
.replace('—', '---')
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The values to be replaced look exactly the same but they are distinct: an em-dash (mutton) and an n-dash (nut).

Copy link
Member

Choose a reason for hiding this comment

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

Oh boy 😂

@jennifer-shehane
Copy link
Member

I just feel like this is an external issue, their docs claim it will not affect blocks, but it does, so it's a bug. Did you happen to open an issue on marked?

What happens is that the url tag is processed first and it spits out HTML into the surrounding block:

We have run into issues with this previously, the order of the transformation of markdown to html, so maybe there is some better way to process tags within tags.

Can the replacement of the dashes be done at not such a global level? Like, only do replacements within processed note tags?

@richdouglasevans
Copy link
Contributor Author

I just feel like this is an external issue, their docs claim it will not affect blocks, but it does, so it's a bug. Did you happen to open an issue on marked?

No. It was late and series 3 of Fargo was a-callin' me 😄 I'll create a reproducible gist and then open an issue later today; I'll post a link on the issue that Amir opened.

Can the replacement of the dashes be done at not such a global level? Like, only do replacements within processed note tags?

Totes; the current implementation will only do the replacement within the note tag that is currently being processed. Typographic markup elsewhere in the content is unaffected.

I'm happy, of course, for this PR to be closed: the investigation was the important bit and my tactical fix is... well, yeah 😂

return Promise.resolve(
toMarkdown(content)
.replace('–', '--')
.replace('—', '---')
Copy link
Member

Choose a reason for hiding this comment

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

Maybe let's actually add a comment in the code here about how this is a temp fix and maybe link to this issue. I can see our future selves looking at this code like 😳 wat...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

😆

Yes, good suggestion Jennifer. I have done so.

@richdouglasevans
Copy link
Contributor Author

Did you happen to open an issue on marked?

I have now: markedjs/marked#1334

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