Fix: Only embed YouTube videos when URL is on its own line #14356
Fix: Only embed YouTube videos when URL is on its own line #14356Shresthap21 wants to merge 5 commits intolaurent22:devfrom
Conversation
…#14352) When markdown.plugin.externalEmbed is enabled, YouTube videos should only be embedded when the URL appears on its own line (standalone), not when inline with other text. Modified externalEmbed plugin to check for surrounding text content before/after the YouTube link. If any text exists in the same paragraph, the URL is rendered as a regular link instead of an embedded video. Added test cases for inline and standalone YouTube URLs.
|
CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅ |
WalkthroughUpdates add new markdown/HTML test fixtures for external-URL embedding edge cases and tighten the renderer's external-embed rule so YouTube links are embedded only when the link appears on its own line (standalone), not when inline with other text. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 6✅ Passed checks (6 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
No actionable comments were generated in the recent review. 🎉 🧹 Recent nitpick comments
Comment |
|
I have read the CLA Document and I hereby sign the CLA |
There was a problem hiding this comment.
Pull request overview
This PR updates the Markdown renderer’s externalEmbed rule to only embed YouTube videos when the URL appears standalone (no surrounding content in the same paragraph), and adds regression fixtures to cover inline vs standalone and edge scenarios.
Changes:
- Add surrounding-content checks before embedding YouTube links in
externalEmbedrenderer rule. - Add new md/html fixture pairs to validate inline URLs remain links while standalone URLs embed.
- Add additional edge-case fixtures (URL at start/end of paragraph, multiple URLs).
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/renderer/MdToHtml/rules/externalEmbed.ts | Adds logic to detect surrounding content and only emit embed markup when the YouTube URL is standalone. |
| packages/app-cli/tests/md_to_html/external_embed_inline.md | New fixture covering inline vs standalone YouTube URLs. |
| packages/app-cli/tests/md_to_html/external_embed_inline.html | Expected HTML output for external_embed_inline.md. |
| packages/app-cli/tests/md_to_html/external_embed_edges.md | New fixture covering edge cases and multiple-URL scenarios. |
| packages/app-cli/tests/md_to_html/external_embed_edges.html | Expected HTML output for external_embed_edges.md. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| But this URL on its own line should be embedded: | ||
|
|
||
| https://www.youtube.com/watch?v=standalone456 |
There was a problem hiding this comment.
standalone456 is not a valid 11-character YouTube video ID per extractVideoId, so the renderer won’t embed it and this fixture will fail. Replace with an 11-char ID and update the expected HTML snapshot accordingly.
| https://www.youtube.com/watch?v=standalone456 | |
| https://www.youtube.com/watch?v=abcdefghijk |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
Thanks for the submission. While this PR targets an existing issue, the proposed solution is not suitable for the project in its current form. Bringing it up to an acceptable standard would require significant redesign and guidance. For future contributions, please ensure changes are well-aligned with the existing architecture and sufficiently tested. Please do not open another PR for this issue. |
Fixes #14352
Problem
When
markdown.plugin.externalEmbedis enabled, YouTube videos were being embedded even when the URL appeared inline with other text, not just when standalone on its own line.Solution
Modified the
externalEmbedplugin to check for surrounding text content before/after YouTube links:Test Cases
Added comprehensive test cases:
external_embed_inline.md/html- Tests inline and standalone scenariosexternal_embed_edges.md/html- Edge cases (URL at start/end of paragraph, multiple URLs)Examples
Before: Inline URL would embed →
Text https://youtube.com/watch?v=xyz more text❌After: Inline URL shows as link →
Text https://youtube.com/watch?v=xyz more text✓Standalone still embeds: Empty line with just URL → Embeds video ✓
Summary by CodeRabbit
Tests
Improvements