Fix typographic substitution in (pre|code|kbd|script) blocks when smartypants=true #1335
Conversation
lib/marked.js
Outdated
| } else if (this.inRawBlock | ||
| && /^<\/pre>|^<\/code>|^<\/kbd>|^<\/script>/i.test(cap[0])) { | ||
| this.inRawBlock = false; | ||
| } |
There was a problem hiding this comment.
There's some duplication here: it'd be nice to just have a simple list of tags in which no substitution is to be performed -- ['pre', 'code', 'kbd', 'script'] -- 'cos that'd make it easy to add/remove tags; I did try that route but the regexes looked less "on the nose" than this version.
I also experimented with alternate names for the inRawBlock field: they were all too darn long and made the code less readable: forex, isBlockInWhichNoSubstitutionIsToBePerformed.
Happy to take a steer here 😄
There was a problem hiding this comment.
Could you change them to /^<(pre|code|kbd|script)/i and /^<\/(pre|code|kbd|script)>/i
I think that makes it a little easier to see what the regex is looking for
| smartypants: true | ||
| description: SmartyPants does not modify characters within <pre>, <code>, <kbd>, or <script> tag blocks. | ||
| spec: https://daringfireball.net/projects/smartypants/ | ||
| --- |
There was a problem hiding this comment.
Just trying to be helpful by linking to the "spec" that explains why.
| <pre>&</pre> | ||
| <code>--foo</code> | ||
| <kbd>---foo</kbd> | ||
| <script>--foo</script> |
There was a problem hiding this comment.
I was pleasantly surprised by the form the test suite takes: adding a specification was so easy, good work Team Marked.
😍
lib/marked.js
Outdated
| } else if (this.inRawBlock | ||
| && /^<\/pre>|^<\/code>|^<\/kbd>|^<\/script>/i.test(cap[0])) { | ||
| this.inRawBlock = false; | ||
| } |
There was a problem hiding this comment.
Could you change them to /^<(pre|code|kbd|script)/i and /^<\/(pre|code|kbd|script)>/i
I think that makes it a little easier to see what the regex is looking for
Agreed, great suggestion 👍 I have done so. |
lib/marked.js
Outdated
| @@ -730,6 +730,14 @@ InlineLexer.prototype.output = function(src) { | |||
| } else if (this.inLink && /^<\/a>/i.test(cap[0])) { | |||
| this.inLink = false; | |||
| } | |||
| if (!this.inRawBlock | |||
| && /^<(pre|code|kbd|script)/i.test(cap[0])) { | |||
There was a problem hiding this comment.
There should be a (\s|>) at the end to prevent matching other tags that start with these words
There was a problem hiding this comment.
Good one Tony; I've done so and expanded the tests to prove that.
Loving your attention to detail here, much appreciated 👍
styfle
changed the title
styfle
changed the title
styfle
changed the title
|
Great job! 🎉 |
richdouglasevans
deleted the
issue/1334-no-typo-subs-in-certain-blocks
branch
…typo-subs-in-certain-blocks Fix typographic substitution in (pre|code|kbd|script) blocks when smartypants=true
Marked version:
0.5.0(latest)Markdown flavor: all
Description
Contributor
Committer
In most cases, this should be a different person than the contributor.