Skip to content

Conversation

KTibow
Copy link

@KTibow KTibow commented Aug 26, 2025

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:

  • Code changes but no Discord API related changes
  • Typings don't need updating

Copy link

vercel bot commented Aug 26, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

2 Skipped Deployments
Project Deployment Preview Comments Updated (UTC)
discord-js Ignored Ignored Preview Aug 29, 2025 7:17pm
discord-js-guide Ignored Ignored Preview Aug 29, 2025 7:17pm

Copy link
Member

@Jiralite Jiralite left a comment

Choose a reason for hiding this comment

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

Please add a test or tests for this!

@github-project-automation github-project-automation bot moved this from Todo to Review in Progress in discord.js Aug 26, 2025
Copy link

codecov bot commented Aug 26, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 44.13%. Comparing base (7710dec) to head (1a1eb11).
⚠️ Report is 12 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #11064      +/-   ##
==========================================
+ Coverage   44.06%   44.13%   +0.06%     
==========================================
  Files         310      310              
  Lines       17755    17776      +21     
  Branches     1755     1760       +5     
==========================================
+ Hits         7824     7845      +21     
  Misses       9919     9919              
  Partials       12       12              
Flag Coverage Δ
formatters 99.66% <100.00%> (+0.02%) ⬆️
structures 85.19% <ø> (ø)
utilities 6.32% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@KTibow
Copy link
Author

KTibow commented Aug 26, 2025

Added

Edit: I forgot to test locally, give me a moment to set up my env

@almeidx almeidx changed the title fix(formatters): don't escape * in links fix: don't escape asterisks in links Aug 26, 2025
@KTibow
Copy link
Author

KTibow commented Aug 26, 2025

Patched

@Jiralite Jiralite dismissed their stale review August 26, 2025 23:13

Resolved.

@Jiralite Jiralite self-requested a review August 26, 2025 23:13
@Qjuh
Copy link
Member

Qjuh commented Aug 29, 2025

This fixes that but opens another can of worms (which also is an issue with underscore formatter then). The following markdown <http://*.example.com/globber/*>*Hi* is valid and renders as
IMG_6871
But because there is no space between the > and the * your PR would escape it to not be italic Hi anymore.

@KTibow
Copy link
Author

KTibow commented Aug 29, 2025

But because there is no space between the > and the * your PR would escape it to not be italic Hi anymore.

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 KTibow changed the title fix: don't escape asterisks in links fix: improve handling of italics in the presence of links Aug 29, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Review in Progress
Development

Successfully merging this pull request may close these issues.

Italics shouldn't be escaped in links
4 participants