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

Fix: Ignore words in info string after syntax (fixes #166) #167

Merged
merged 3 commits into from Dec 20, 2020
Merged

Conversation

btmills
Copy link
Member

@btmills btmills commented Dec 16, 2020

This was working correctly in v1 but stopped working when v2 switched to the new processor API. The generated filename was 0.js more words are ignored. The test case for this was insufficiently strict: it asserted that the code block was extracted, which was sufficient with hard-coded extensions in v1, but it didn't assert the generated name was 0.js now that we're taking the extension from the info string.

Per CommonMark 0.29:

The line with the opening code fence may optionally contain some text following the code fence; this is trimmed of leading and trailing whitespace and called the info string.

I've strengthened the tests for generated filename extensions and added a new test to ensure that leading whitespace is trimmed correctly.

This was working correctly in v1 but stopped working when v2 switched to
the new processor API. The generated filename was `0.js more words are
ignored`. The test case for this was insufficiently strict: it asserted
that the code block was extracted, which was sufficient with hard-coded
extensions in v1, but it didn't assert the generated name was `0.js` now
that we're taking the extension from the info string.

Per CommonMark 0.29 [1]:

> The line with the opening code fence may optionally contain some text
> following the code fence; this is trimmed of leading and trailing
> whitespace and called the info string.

I've strengthened the tests for generated filename extensions and added
a new test to ensure that leading whitespace is trimmed correctly.

[1]: https://spec.commonmark.org/0.29/#info-string
lib/processor.js Outdated Show resolved Hide resolved
Markdown info strings permit trimming both leading and trailing
whitespace, so this shouldn't cause issues. `.trim()` is much nicer than
a regex `.replace()`.
Copy link
Member

@nzakas nzakas left a comment

Choose a reason for hiding this comment

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

LGTM

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.

None yet

3 participants