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

[commonmark] make code fences compliant (+ fix 1058) #1387

Merged
merged 5 commits into from Dec 10, 2018

Conversation

@Feder1co5oave
Copy link
Contributor

commented Dec 10, 2018

Code fences containing a matching fence not on a new line are terminated incorrectly

  • fixes #1058 (comment)
  • allow longer closing fences
  • require indentation up to 3 spaces
  • fixes #1325
  • fixes #1314

This gets up to 100% compliance with commonmark fenced code blocks

  • Test(s) exist to ensure functionality and minimize regression (if no tests added, list tests covering this PR); or,

  • Draft GitHub release notes have been updated.

  • CI is green (no forced merge required).

  • Merge PR

@Feder1co5oave Feder1co5oave changed the title [cm] make code fences compliant (+ fix 1058) [commonmark] make code fences compliant (+ fix 1058) Dec 10, 2018

lib/marked.js Show resolved Hide resolved
lib/marked.js Show resolved Hide resolved
@styfle

styfle approved these changes Dec 10, 2018

Copy link
Member

left a comment

This is awesome! I made a suggestion to change the infostring to language but I see now that the spec actually refers to it as info string so I'm okay with either name.

@Feder1co5oave

This comment has been minimized.

Copy link
Contributor Author

commented Dec 10, 2018

@styfle exactly, I changed the wording to better adhere to commonmark, see the paragraph and the examples below example 110. The language is the first word of the info string, which may contain other information.

@Feder1co5oave

This comment has been minimized.

Copy link
Contributor Author

commented Dec 10, 2018

Fix list updated.

@styfle styfle requested review from UziTech, joshbruce and davisjam Dec 10, 2018

@@ -91,7 +91,7 @@ block.normal = merge({}, block);
*/

block.gfm = merge({}, block.normal, {
fences: /^ *(`{3,}|~{3,})[ \.]*(\S+)? *\n([\s\S]*?)\n? *\1 *(?:\n+|$)/,
fences: /^ {0,3}(`{3,}|~{3,})([^`\n]*)\n(?:|([\s\S]*?)\n)(?: {0,3}\1[~`]* *(?:\n+|$)|$)/,

This comment has been minimized.

Copy link
@davisjam

davisjam Dec 10, 2018

Contributor

This is no more vulnerable than it previously was.

@davisjam
Copy link
Contributor

left a comment

This introduces no further vulnerabilities.

@UziTech
Copy link
Member

left a comment

We are getting closer to 100% spec compliance. Keep up the good work. 🎉

@styfle styfle merged commit 76d3733 into markedjs:master Dec 10, 2018

3 checks passed

WIP Ready for review
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
security/snyk - package.json (markedjsbot) No known issues
Details

@Feder1co5oave Feder1co5oave deleted the Feder1co5oave:fix-1058 branch Jan 4, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.