Skip to content

Conversation

@joek13
Copy link
Contributor

@joek13 joek13 commented Jan 5, 2022

Summary

Fixes #695. On current master, calling escape_markdown with default parameters on something like the following string

[link 1](https://example.org)
[link 2](https://example.org)
[link 3](https://example.org), [link 4](https://example.org)

results in this escaping:

\[link 1](https://example.org)
\[link 2](https://example.org)
\[link 3](https://example.org), [link 4](https://example.org)

This is because the _MARKDOWN_ESCAPE_COMMON regular expression consumes greedily, and considers the third line a single link with text link 3](https://example.org), [link 4 and destination https://example.org. We fix this by disallowing [, ] characters in link text and (, ) characters in link destinations.

Under this PR, the above string escapes to:

\[link 1](https://example.org)
\[link 2](https://example.org)
\[link 3](https://example.org), \[link 4](https://example.org)

Worth noting is that the GitHub Flavored Markdown standard actually allows for brackets in link text, as well as parentheses in link destination/titles. This blog post provides a good reference on solving this intelligently, but unfortunately some of the regular expression features used (namely, recursive patterns) aren't included in Python's native re library. (As an aside, the language of finite strings with balanced parentheses is provably non-regular, so we shouldn't hope for an easy cheat here.)

Checklist

  • If code changes were made then they have been tested.
    • I have updated the documentation to reflect the changes.
  • If type: ignore comments were used, a comment is also left explaining why
  • This PR fixes an issue.
  • This PR adds something new (e.g. new method or parameters).
  • This PR is a breaking change (e.g. methods or parameters removed/renamed)
  • This PR is not a code change (e.g. documentation, README, typehinting, examples, ...)

@Lulalaby Lulalaby added this to the v2.1 milestone Jan 7, 2022
@Lulalaby Lulalaby added bug Something isn't working priority: medium Medium Priority Merge with squash labels Jan 7, 2022
@BobDotCom BobDotCom modified the milestones: v2.1, v2.0 Jan 11, 2022
@BobDotCom BobDotCom enabled auto-merge January 11, 2022 19:29
@krittick krittick self-requested a review January 18, 2022 05:41
@BobDotCom BobDotCom merged commit adfb0c6 into Pycord-Development:master Jan 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working priority: medium Medium Priority

Projects

None yet

Development

Successfully merging this pull request may close these issues.

discord.utils.escape_markdown not escaping all links

4 participants