fix: allow one more level of nested brackets in a link label - #4064
Open
luantaraschi wants to merge 1 commit into
Open
fix: allow one more level of nested brackets in a link label#4064luantaraschi wants to merge 1 commit into
luantaraschi wants to merge 1 commit into
Conversation
CommonMark lets the brackets in a link label nest to any depth. The label pattern only allowed a single level, so a link whose text carried a nested pair was not read as a link at all. Fixes CommonMark examples 512, 520 and 528.
|
@luantaraschi is attempting to deploy a commit to the MarkedJS Team on Vercel. A member of the Team first needs to authorize it. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Part of the "nested brackets in link text" group in #4050.
CommonMark says the brackets in a link label may nest, with no limit on the depth:
_inlineLabelallowed one level. The nested alternative it carries,\[(?:\[\s\S]|[^\[\]\])*\], matches a pair of brackets whose contents hold no bracket at all, so a second pair inside made the whole label fail to match and the text stayed literal:The nested alternative now accepts one such pair itself. Everything else about the label is unchanged, including the code-span alternatives that keep a backtick span from swallowing a bracket.
Why two levels and not more
A regex cannot follow arbitrary nesting, so the depth is a number someone has to pick. I built the pattern at depths 1 through 4 and ran the CommonMark suite at each:
The third and the fourth level pay for nothing: the spec has no case that needs them. So this stops at two, and the comment in the source says so, since the next person to look at that line will ask the same question.
Example 520,
](uri2)](uri3), comes along for the ride. It needs the same second level, and the link-inside-a-link rejection from #4051 already does the rest.Cost
link,reflinkand their gfm and pedantic variants each grow by 28 characters of source. Parsing 200 documents holding a 2000 character label measured the same before and after, at 14ms.npm run test:redosis clean: 148 patterns analysed, all reported safe, including the deeperinline.normal.linkandinline.normal.reflink.Tests
The three examples are covered by the spec suite itself, so this drops their
shouldFailflags intest/specs/commonmark/commonmark.0.31.2.jsonand in the gfm copy of the same file, which flagged them separately.npm testis green: 1783 spec tests, 191 unit tests, plus umd, cjs, types and lint.What is still failing in that group
524, 526, 536 and 538 are a different root cause, the one about raw HTML and autolinks binding more tightly than the brackets of a link text, and are untouched here.