fix: improve handling of italics in the presence of links #11064
Conversation
KTibow
requested review from
Jiralite,
almeidx,
iCrawl,
kyranet and
vladfrangu
|
The latest updates on your projects. Learn more about Vercel for GitHub. 2 Skipped Deployments
|
Jiralite
left a comment
Jiralite
left a comment
There was a problem hiding this comment.
Please add a test or tests for this!
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #11064 +/- ##
==========================================
- Coverage 44.39% 44.27% -0.13%
==========================================
Files 315 310 -5
Lines 18134 17659 -475
Branches 1805 1758 -47
==========================================
- Hits 8051 7818 -233
+ Misses 10071 9829 -242
Partials 12 12
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Added Edit: I forgot to test locally, give me a moment to set up my env |
almeidx
changed the title
|
Patched |
|
This fixes that but opens another can of worms (which also is an issue with underscore formatter then). The following markdown |
Do you mean that this PR would fail to escape the italics, introducing new problematic behavior? That's fair, I'll see what I can do about that. |
KTibow
changed the title
There was a problem hiding this comment.
Pull Request Overview
This PR enhances the escapeItalic function to better handle URLs containing asterisks and underscores. The regex patterns are updated to avoid escaping italic markdown characters (single asterisks and underscores) that appear within URLs, while still properly escaping them after angle-bracketed URLs.
Key changes:
- Updated regex patterns in
escapeItalicto add negative lookbehinds that detect URLs and prevent escaping of*and_characters within them - Added support for both plain URLs and angle-bracketed URLs (
<https://...>) - Added test cases for URLs containing asterisks (wildcards/globs) and verification that characters after angle-bracketed URLs are still properly escaped
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| packages/formatters/src/escapers.ts | Enhanced regex patterns in escapeItalic with nested negative lookbehinds to detect and skip URL patterns when escaping italic markdown characters |
| packages/formatters/tests/escapers.test.ts | Added test URLs with asterisks and new test case verifying that italic characters after angle-bracketed URLs are properly escaped |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
github-project-automation
bot
moved this from Review in Progress
to Review Approved
in discord.js
* fix(formatters): don't escape * in links * add test for * -> * in url * `\S+` -> `\S*` (tested locally) * @Qjuh: handle italics both inside+outside <url>s * more accurate <link> matcher --------- Co-authored-by: Qjuh <[email protected]>
5 participants
Please describe the changes this PR makes and why it should be merged:
The underscore matcher already has
(?<!<a?:.+|https?:\/\/\S+), which prevents escaping underscores in emojis, animated emojis, and links (from the https to the first whitespace character).This PR makes sure asterisks aren't escaped in links either. Fixes #11063.
Status and versioning classification: