Skip to content

Link redos - #1426

Merged
joshbruce merged 2 commits into
markedjs:masterfrom
UziTech:link-code
Feb 21, 2019
Merged

Link redos#1426
joshbruce merged 2 commits into
markedjs:masterfrom
UziTech:link-code

Conversation

@UziTech

@UziTechUziTech commented Feb 20, 2019

Copy link
Copy Markdown
Member

Marked version: master

Description

Fixes redos when code blocks are in square brackets that are not links.

Contributor

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

Committer

In most cases, this should be a different person than the contributor.

  • Draft GitHub release notes have been updated.
  • CI is green (no forced merge required).
  • Merge PR

@UziTech

Copy link
Copy Markdown
MemberAuthor

/cc @Feder1co5oave@davisjam

I'm not sure this is a great fix. The test still takes a few hundred milliseconds.

@UziTech
UziTech marked this pull request as ready for review February 20, 2019 19:30
@Feder1co5oave

Feder1co5oave commented Feb 20, 2019

Copy link
Copy Markdown
Contributor

I tried a different approach but this seems better! It's far from perfect though. I think this issue is inherent in the way links and code spans bind.

So, for example:

[foo`](url)` <-- this is not a link
[`foo]`](url) <-- neither is this
[foo`](url) <-- but this is
[foo`](url)`` code `` <-- and this too.
[foo`](url)``code` <-- these are just two code spans sitting back-to-back

Whenever we try to match a link, if we find a ` inside the text part, we cannot know if it is a lonely backtick, the start of a codespan contained within the link text, or the start of a codespan which invalidates the link-like part we matched up at that point.
Also, code spans by themselves are hard to parse!:

code: /^(`+)([^`]|[^`][\s\S]*?[^`])\1(?!`)/,

I think we should consider trying to resemble the codespan matching regex to avoid backtracking too much. Something along the lines of (`+) ... \2...

If you want to merge this fix in the mean time, that's fine too!

@styflestyfle left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good, besides adding butt to the repo 😆

@joshbruce
joshbruce merged commit 1f5b9a1 into markedjs:masterFeb 21, 2019
@UziTech
UziTech deleted the link-code branch March 12, 2019 17:05
@styflestyfle mentioned this pull request Mar 25, 2019
12 tasks
Sign up for freeto 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.

4 participants

@UziTech@Feder1co5oave@styfle@joshbruce