Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions packages/cli/src/ui/utils/InlineMarkdownRenderer.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,22 @@ math then literal: $x^2\$$`;
expect(out).not.toContain(`\x1b]8;;${url}.\x07`);
});

it('stops the bare-URL hyperlink at glued-on full-width CJK punctuation', () => {
enableHyperlinks();
const url = 'https://github.com/QwenLM/qwen-code/pull/8742';
const { lastFrame } = renderWithProviders(
<RenderInline text={`PR:${url}(2 commits,等 CI)`} />,
);

const out = lastFrame() ?? '';
// OSC 8 target is exactly the URL — the (2 run is not swallowed into
// the link (https://github.com/QwenLM/qwen-code/issues/8750).
expect(out).toContain(`\x1b]8;;${url}\x07`);
expect(out).not.toContain(`\x1b]8;;${url}(`);
// The glued-on punctuation renders as plain text after the link.
expect(out.replace(/\s+/g, ' ')).toContain('(2 commits,等 CI)');
});

it('leaves bare URLs unwrapped when unsupported', () => {
const url = 'https://example.com/plain';
const { lastFrame } = renderWithProviders(
Expand Down
3 changes: 2 additions & 1 deletion packages/cli/src/ui/utils/InlineMarkdownRenderer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import stringWidth from 'string-width';
import { createDebugLogger } from '@qwen-code/qwen-code-core';
import { renderInlineLatex } from './latexRenderer.js';
import {
BARE_URL_PATTERN,
MD_LINK_CAPTURE,
MD_LINK_PATTERN,
isSafeOscScheme,
Expand Down Expand Up @@ -38,7 +39,7 @@ const UNDERLINE_TAG_START_LENGTH = 3; // For "<u>"
const UNDERLINE_TAG_END_LENGTH = 4; // For "</u>"
const INLINE_MARKDOWN_REGEX = new RegExp(
String.raw`(\*\*.*?\*\*|\*.*?\*|_.*?_|~~.*?~~|${MD_LINK_PATTERN}|` +
Comment thread
yiliang114 marked this conversation as resolved.
Resolved

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[P0 Critical] Re-tokenization: after BARE_URL_PATTERN (last in the alternation) truncates a URL at CJK punctuation, the tail is re-scanned by the earlier branches. Input 见 https://a.com/x(说明_1)和 https://b.com/y_。 produces tokens ["https://a.com/x", "_1)和 https://b.com/y_"] — the second URL loses its OSC 8 envelope, and the underscores are silently deleted from visible text.

Pre-PR \S+ kept both URLs inside one giant (broken) link; now the second URL is plain text with no clickable target at all.

One possible direction: add a negative lookbehind so _.*?_ / \*.*?\* / backtick spans cannot start inside text that was already part of a URL run — or ensure a bare-URL match followed by CJK punctuation is followed by a non-capturing word-boundary reset.

String.raw`${INLINE_CODE_SPAN_PATTERN_SOURCE}|<u>.*?<\/u>|https?:\/\/\S+)`,
String.raw`${INLINE_CODE_SPAN_PATTERN_SOURCE}|<u>.*?<\/u>|${BARE_URL_PATTERN})`,
Comment thread
yiliang114 marked this conversation as resolved.
Resolved
Comment thread
yiliang114 marked this conversation as resolved.
Resolved
'g',
);

Expand Down
10 changes: 10 additions & 0 deletions packages/cli/src/ui/utils/TableRenderer.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -649,6 +649,16 @@ describe('<TableRenderer />', () => {
expectAllLinesToHaveSameVisibleWidth(output);
});

it('stops a bare URL before glued-on CJK punctuation', () => {
enableHyperlinks();
const url = 'https://github.com/QwenLM/qwen-code/pull/8742';
const suffix = '(2 commits,等 CI)';
const output = renderTable(['PR'], [[`PR:${url}${suffix}`]], 100);
expect(output).toContain(`\x1b]8;;${url}\x07`);
expect(output).not.toContain(`\x1b]8;;${url}(`);
expect(stripAnsi(output)).toContain(suffix);
});

it('falls back to legacy `label (url)` in cells on unsupported terminals', () => {
// isTTY=false from the suite-wide beforeEach disables hyperlinks.
const url = 'https://example.com/page';
Expand Down
3 changes: 2 additions & 1 deletion packages/cli/src/ui/utils/TableRenderer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import {
unescapeMarkdownDollars,
} from './inline-math.js';
import {
BARE_URL_PATTERN,
MD_LINK_CAPTURE,
MD_LINK_PATTERN,
isSafeOscScheme,
Expand Down Expand Up @@ -50,7 +51,7 @@ const SAFETY_MARGIN = 4;

const INLINE_MARKDOWN_REGEX = new RegExp(
String.raw`(\*\*.*?\*\*|\*.*?\*|_.*?_|~~.*?~~|${MD_LINK_PATTERN}|` +

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Same re-tokenization issue as InlineMarkdownRenderer.tsx:41 — this is the identical INLINE_MARKDOWN_REGEX. See that thread for the repro and discussion.

String.raw`${INLINE_CODE_SPAN_PATTERN_SOURCE}|<u>.*?<\/u>|https?:\/\/\S+)`,
String.raw`${INLINE_CODE_SPAN_PATTERN_SOURCE}|<u>.*?<\/u>|${BARE_URL_PATTERN})`,
'g',
);

Expand Down
11 changes: 11 additions & 0 deletions packages/cli/src/ui/utils/osc8.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

import { afterEach, beforeEach, describe, expect, it } from 'vitest';
import {
BARE_URL_PATTERN,
HYPERLINK_ENV_KEYS,
isSafeOscScheme,
labelMayDeceive,
Expand Down Expand Up @@ -294,6 +295,16 @@ describe('osc8 helpers', () => {
});
});

describe('BARE_URL_PATTERN', () => {
it.each([
'https://ja.wikipedia.org/wiki/人々',
'https://example.com/二〇二六年報',
'https://zh.wikipedia.org/wiki/北京',
])('keeps word-forming CJK characters in %s', (url) => {
expect(new RegExp(BARE_URL_PATTERN).exec(url)?.[0]).toBe(url);
});
});

describe('supportsHyperlinks', () => {
function setTTY(value: boolean) {
Object.defineProperty(process.stdout, 'isTTY', {
Expand Down
21 changes: 18 additions & 3 deletions packages/cli/src/ui/utils/osc8.ts
Original file line number Diff line number Diff line change
Expand Up @@ -102,9 +102,9 @@ export function isSafeOscScheme(url: string): boolean {
* MSDN) aren't truncated.
*/
export function trimTrailingUrlPunctuation(url: string): string {
// Count `( [ {` opens once up-front; we then decrement running `)`/`]`/`}`
// close counts as we trim, keeping the whole trim O(n) instead of O(n²)
// for adversarial inputs like `https://x.com))))…`.
// Count `( [ {` opens once up-front; we then
// decrement running `)`/`]`/`}` close counts as we trim, keeping the whole
// trim O(n) instead of O(n²) for adversarial inputs like `https://x.com))))…`.
let openParen = 0;
let openBracket = 0;
let openBrace = 0;
Expand Down Expand Up @@ -178,6 +178,21 @@ export const MD_LINK_PATTERN = String.raw`\[.*?\]\((?:[^()]|\([^()]*\))*\)`;
*/
export const MD_LINK_CAPTURE = /^\[(.*?)\]\(((?:[^()]|\([^()]*\))*)\)$/;

/**
* Bare-URL pattern shared between the React and ANSI renderers. Unlike a
* plain `\S+` run it stops at CJK / full-width punctuation: Chinese prose
* routinely glues `(…)`/`。` onto a URL with no space
* (`https://x.com(2 commits)`), and `\S` swallows the punctuation plus
* everything up to the next ASCII space, turning the OSC 8 target into a
* 404. Raw CJK ideographs, U+3005 々, and U+3007 〇 stay in the match because
Comment thread
yiliang114 marked this conversation as resolved.
Resolved
* they are word-forming IRI characters. Typographic dashes and curly quotes
Comment thread
yiliang114 marked this conversation as resolved.
Resolved
* are excluded to terminate URLs in spaceless prose, so raw IRIs containing
* those characters are an accepted trade-off. ASCII trailing punctuation is
* still matched and left to `trimTrailingUrlPunctuation` so visible bytes stay
* unchanged on unsupported terminals.
*/
export const BARE_URL_PATTERN = String.raw`https?:\/\/[^\s\u00a0\u2013-\u2015\u2018-\u201f\u2026\u3000-\u3004\u3006\u3008-\u303f\uff01-\uff0f\uff1a-\uff20\uff3b-\uff40\uff5b-\uff65]+`;
Comment thread
yiliang114 marked this conversation as resolved.
Resolved

Comment thread
yiliang114 marked this conversation as resolved.
Resolved
/**
* Should the markdown renderers wrap a `[label](url)` token in an OSC 8
* envelope? Returns true only when (a) the host terminal advertises OSC 8,
Expand Down
Loading