Opt-in CommonMark link parsing in Banner-rendered plain strings#7458
Draft
shopify-river[bot] wants to merge 2 commits intomainfrom
Draft
Opt-in CommonMark link parsing in Banner-rendered plain strings#7458shopify-river[bot] wants to merge 2 commits intomainfrom
shopify-river[bot] wants to merge 2 commits intomainfrom
Conversation
Long URLs embedded as plain text in CLI error banners wrap across the bordered box at terminal width, splitting the URL with the border's `│` characters and producing output that is neither clickable nor copy-pasteable. Auto-detecting bare URLs is fragile — some error messages legitimately echo a user-supplied URL (e.g. `--tunnel-url https://wrong`), and turning that into a clickable OSC 8 escape misleads the user. Switch to opt-in: inside a `LinksContext` (Banner / Alert / FatalError), `TokenizedText` now recognizes CommonMark `[label](url)` inline links and `<url>` autolinks — both required to carry an explicit `http(s)://` scheme — and routes them through the existing `<Link>` component. Plain prose with bare URLs is left untouched, so callers who want a URL clickable have to mark it up explicitly. Two adjacent fixes inside the same code path: - `Link` inside a `LinksContext` now renders label-less links as a bare `[N]` anchor instead of `url [N]`, so the long URL never appears inside the bordered box; the footnote remains the single source of truth for the URL. - `FatalError` routes plain-string `error.message` through `TokenizedText` (instead of a bare `<Text>`), so opt-in markdown in a plain string `AbortError` / `BugError` message gets the same footnote treatment. Supersedes the auto-URL-detection approach in #7384 per Isaac's review feedback in #help-dev-platform: the CLI shouldn't have to guess whether a URL in an error string is meant to be clickable. Requested by Alfonso Noriega <[email protected]>
This change is internal CLI-kit plumbing — without a downstream caller emitting the opt-in markdown, the rendered output for every existing caller is unchanged. No user-facing release note needed. Requested by Alfonso Noriega <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
WHY are these changes introduced?
Long URLs embedded as plain text in CLI error banners wrap across the bordered box at terminal width (~78 cols on an 80-col terminal), splitting the URL with the border's
│characters and producing output that is neither clickable nor copy-pasteable as a single URL.Auto-detecting bare URLs in plain strings (the approach in #7384) cannot disambiguate a URL the message is describing from a URL the message wants the user to click. A common case: validation errors that echo a user-supplied URL back to the user (e.g.
Invalid tunnel URL: https://wrong) — turning that into a clickable OSC 8 escape misleads the user.WHAT is this pull request doing?
Adds opt-in CommonMark link parsing inside
LinksContext-aware components (Banner/Alert/FatalError) so callers can mark a URL up explicitly when they want it linkified.Three changes in
packages/cli-kit:TokenizedText.tsx— Plain-string tokens are scanned for two opt-in shapes:[label](url)<url>Both shapes require an explicit
http(s)://scheme. Parsing is gated onLinksContext, so it only runs inside Banner-rendered components that own a footnote slot. Bare URLs in plain prose are never linkified.Link.tsx— Inside aLinksContext, label-less links render as a bare[N]anchor instead ofurl [N], so the long URL never appears inside the bordered box; the footnote becomes the single source of truth for the URL.FatalError.tsx— Plain-stringerror.messageis now routed throughTokenizedText(instead of a bare<Text>), so anAbortError/BugErrorconstructed from a plain string also gets the opt-in treatment.Net effect for a message of
See specification requirements: [docs](https://example.com/very/long/path/with/anchor#specification-properties):Before:
After:
On OSC 8-capable terminals (iTerm2, modern Terminal.app, VS Code, Warp, Kitty, WezTerm, Ghostty) the
docs [1]label is a clickable hyperlink. On terminals without OSC 8 support, the URL appears outside the bordered box in the footnote block, where it can wrap freely without│interleaving and copy-paste recovers the full URL.A bare URL in the same body — e.g. echoed back from user input — is left alone:
Only
tunnel docsbecomes clickable;https://wrongstays plain text.This supersedes #7384.
No changeset: this is internal
cli-kitplumbing — no existing caller emits the opt-in markup, so the rendered output for every current caller is unchanged. A changeset can be added by the first downstream PR that adopts the markup.How to test your changes?
Unit coverage added (13 new tests):
TokenizedText.test.tsx— 10 tests covering: no-link passthrough, bare-URL passthrough (regression for the user-input-echo case),[label](url)with and without OSC 8,<url>autolinks, multiple links, back-to-back links, scheme-less markdown is ignored, noLinksContextis a no-op, and a tunnel-URL echo regression.Link.test.tsx— 2 tests:label [N]anchor + footnote registration; bare[N]anchor for label-less links inside aLinksContext.Alert.test.tsx— 1 integration test: long-URL message inside a realAlertlocks in "URL never inside the bordered box, URL appears in footnote, body showsdocs [1]anchor."FatalError.test.tsx— 1 integration test for the same shape via a plain-stringAbortError.pnpm vitest run packages/cli-kit/src/private/node/ui→ all UI tests passing locally. Lint + type-check clean.Visual QA in a real terminal
pnpm --filter @shopify/cli-kit build│, thedocs [1]anchor is clickable in OSC 8 terminals, and the URL in the footnote copy-pastes cleanly.FORCE_HYPERLINK=0and confirm the same[N]+ footnote layout still renders cleanly.Post-release steps
None.
Checklist
supports-hyperlinksdependency.Requested by Alfonso Noriega [email protected].