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

Improve @mention sanitizer for for verbatim backticks in code blocks #1427

Conversation

pedropombeiro
Copy link

This PR improves LinkAndMentionSanitizer so that it understands code fences with verbatim backticks which would normally derail the matching logic. Basically, the rule behind backtick code fences seems to be that the outermost delimiter is the one with most backticks, so we need to find the next occurrence with the same number of backticks in order to find the end of the code fence (see spec, confirmed in GitHub comment editor preview).

Example:

``` => Verbatim backticks

Related to #1421

cc @feelepxyz

@pedropombeiro pedropombeiro force-pushed the fix/verbatim-backticks-sanitization branch 3 times, most recently from 109f5a3 to 5c2e262 Compare October 3, 2019 15:08
@pedropombeiro
Copy link
Author

The CircleCI build is failing but looks unrelated to this PR.

@feelepxyz
Copy link
Contributor

@pombeirp thank you! Will take a look, did you rebase latest master?

@pedropombeiro
Copy link
Author

I did a couple of times to force a rebuild, and the build failed with different reasons each time.

@pedropombeiro pedropombeiro force-pushed the fix/verbatim-backticks-sanitization branch from 5c2e262 to ff0e556 Compare October 6, 2019 05:37
@pedropombeiro pedropombeiro force-pushed the fix/verbatim-backticks-sanitization branch from ff0e556 to e289ab2 Compare October 6, 2019 06:26
@pedropombeiro
Copy link
Author

@feelepxyz looks like rebasing on the fix in master did the trick, tests passed now.

Copy link
Contributor

@feelepxyz feelepxyz left a comment

Choose a reason for hiding this comment

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

Nice work on this! Sorry took a while to get around to reviewing 🙇

@feelepxyz feelepxyz merged commit 5b20bfc into dependabot:master Oct 8, 2019
@pedropombeiro
Copy link
Author

No problem, I know you guys are busy!

Sign up for free to 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.

2 participants