diff --git a/mobile/src/components/MobileMarkdown.file-links.test.ts b/mobile/src/components/MobileMarkdown.file-links.test.ts index 962538a8842..28a3155203a 100644 --- a/mobile/src/components/MobileMarkdown.file-links.test.ts +++ b/mobile/src/components/MobileMarkdown.file-links.test.ts @@ -54,6 +54,21 @@ describe('MobileMarkdown file links', () => { return renderer! } + it('preserves a long unmatched bracket run as literal rendered text', () => { + const text = '['.repeat(60_000) + const tree = render(text) + expect(flattenText(tree.root)).toBe(text) + expect(pressables(tree)).toEqual([]) + }) + + it('preserves nested labels and empty image labels', () => { + const tree = render('[[nested](https://example.com) ![](https://example.com/image)') + pressByText(tree, '[nested') + expect(openURL).toHaveBeenLastCalledWith('https://example.com') + pressByText(tree, 'image') + expect(openURL).toHaveBeenLastCalledWith('https://example.com/image') + }) + it('opens a tapped POSIX absolute path in prose', () => { pressByText(render('Edit /Users/me/wt/src/app.tsx now'), '/Users/me/wt/src/app.tsx') expect(onOpenFile).toHaveBeenCalledWith('/Users/me/wt/src/app.tsx') diff --git a/mobile/src/components/MobileMarkdown.tsx b/mobile/src/components/MobileMarkdown.tsx index 7ecfab8c393..a2bff04cdcd 100644 --- a/mobile/src/components/MobileMarkdown.tsx +++ b/mobile/src/components/MobileMarkdown.tsx @@ -1,3 +1,4 @@ +import { createMarkdownInlineMatcher, type MarkdownInlineMatch } from './markdown-inline-matcher' import { MobileSelectableText } from './MobileSelectableText' import { Fragment, @@ -104,12 +105,15 @@ function renderTextRun( function renderInline(text: string, onOpenFile?: (pathText: string) => void): ReactNode[] { const parts: ReactNode[] = [] - const pattern = - /(!\[[^\]]*\]\([^)]+\)|`[^`]+`|~~[^~]+~~|\*\*[^*]+\*\*|__[^_]+__|\*[^*\n]+\*|_[^_\n]+_|\[[^\]]+\]\([^)]+\)|https?:\/\/[^\s<]+)/g + const pattern = createMarkdownInlineMatcher( + text, + /(`[^`]+`|~~[^~]+~~|\*\*[^*]+\*\*|__[^_]+__|\*[^*\n]+\*|_[^_\n]+_|https?:\/\/[^\s<]+)/g, + true + ) let pendingStart = 0 - let match: RegExpExecArray | null + let match: MarkdownInlineMatch | null - while ((match = pattern.exec(text))) { + while ((match = pattern.exec())) { const token = match[0] // Intraword `_` runs (snake_case, dunder tails) are literal text per // CommonMark; leaving them unflushed keeps surrounding file paths whole diff --git a/mobile/src/components/markdown-inline-matcher.test.ts b/mobile/src/components/markdown-inline-matcher.test.ts new file mode 100644 index 00000000000..b252a434b0c --- /dev/null +++ b/mobile/src/components/markdown-inline-matcher.test.ts @@ -0,0 +1,103 @@ +import { runInNewContext } from 'node:vm' +import { describe, expect, it } from 'vitest' +import { createMarkdownInlineMatcher } from './markdown-inline-matcher' +import { isIntrawordUnderscoreToken } from './markdown-inline-token-rules' +import { parseInline } from './pr-sidebar/markdown-blocks' + +const ORIGINAL_CHAT = + /(!\[[^\]]*\]\([^)]+\)|`[^`]+`|~~[^~]+~~|\*\*[^*]+\*\*|__[^_]+__|\*[^*\n]+\*|_[^_\n]+_|\[[^\]]+\]\([^)]+\)|https?:\/\/[^\s<]+)/g +const CHAT_OTHER = + /(`[^`]+`|~~[^~]+~~|\*\*[^*]+\*\*|__[^_]+__|\*[^*\n]+\*|_[^_\n]+_|https?:\/\/[^\s<]+)/g +const ORIGINAL_REVIEW = + /(`[^`]+`)|(\*\*[^*]+\*\*)|(__[^_]+__)|(\*[^*]+\*)|(_[^_]+_)|(\[[^\]]+\]\([^)]+\))/g +const REVIEW_OTHER = /(`[^`]+`)|(\*\*[^*]+\*\*)|(__[^_]+__)|(\*[^*]+\*)|(_[^_]+_)/g + +function tokens(text: string, chat: boolean, original: boolean) { + const pattern = new RegExp(chat ? ORIGINAL_CHAT : ORIGINAL_REVIEW) + const matcher = original + ? { + get lastIndex() { + return pattern.lastIndex + }, + set lastIndex(value) { + pattern.lastIndex = value + }, + exec: () => pattern.exec(text) + } + : createMarkdownInlineMatcher(text, new RegExp(chat ? CHAT_OTHER : REVIEW_OTHER), chat) + const result: Array<{ text: string; index: number; end: number }> = [] + let match + while ((match = matcher.exec())) { + if (chat && isIntrawordUnderscoreToken(text, match.index, match[0])) { + matcher.lastIndex = match.index + 1 + continue + } + result.push({ text: match[0], index: match.index, end: matcher.lastIndex }) + } + return result +} + +describe('mobile inline link scanning', () => { + it.each([false, true])('preserves original token order (chat=%s)', (chat) => { + const fragments = [ + '[', + ']', + '(', + ')', + '!', + 'a', + ' ', + '*', + '**', + '_', + '__', + '`', + '~', + '\n', + '[a](b)', + '![](x)', + 'https://x.y', + '[bad]', + 'foo_bar', + '[[nested](url)' + ] + let seed = 12345 + const random = () => { + seed = (Math.imul(seed, 1664525) + 1013904223) >>> 0 + return seed + } + for (let i = 0; i < 5000; i++) { + const text = Array.from( + { length: 1 + (random() % 40) }, + () => fragments[random() % fragments.length] + ).join('') + expect(tokens(text, chat, false), text).toEqual(tokens(text, chat, true)) + } + }) + + it.each([ + { name: 'unmatched labels', text: '['.repeat(60_000) }, + { name: 'unmatched destinations', text: '[a]('.repeat(12_000) } + ])('keeps $name literal within the parser deadline', ({ text }) => { + const result = runInNewContext('parse(text)', { parse: parseInline, text }, { timeout: 250 }) + expect(result).toEqual([{ kind: 'text', text }]) + }) + + it('does not repeatedly search the suffix for absent non-link tokens', () => { + const text = '[a](b)'.repeat(10_000) + const pattern = new RegExp(REVIEW_OTHER) + const originalExec = pattern.exec.bind(pattern) + let calls = 0 + pattern.exec = (value) => { + calls++ + return originalExec(value) + } + const matcher = createMarkdownInlineMatcher(text, pattern) + let count = 0 + while (matcher.exec()) { + count++ + } + expect(count).toBe(10_000) + expect(calls).toBe(1) + }) +}) diff --git a/mobile/src/components/markdown-inline-matcher.ts b/mobile/src/components/markdown-inline-matcher.ts new file mode 100644 index 00000000000..ba86597f32a --- /dev/null +++ b/mobile/src/components/markdown-inline-matcher.ts @@ -0,0 +1,72 @@ +export type MarkdownInlineMatch = { 0: string; index: number; end: number } + +/** Merge a global non-link regex with links; search starts must advance between calls. */ +export function createMarkdownInlineMatcher( + text: string, + nonLinkPattern: RegExp, + images = false +): { lastIndex: number; exec: () => MarkdownInlineMatch | null } { + let nextOther: MarkdownInlineMatch | null | undefined + let nextLink: MarkdownInlineMatch | null | undefined + let labelEnd = -1 + let destinationEnd = -1 + let noMoreLabels = false + let noMoreDestinations = false + + function findLink(from: number): MarkdownInlineMatch | null { + if (noMoreLabels || noMoreDestinations) { + return null + } + let open = text.indexOf('[', from) + while (open !== -1) { + if (labelEnd < open + 1) { + labelEnd = text.indexOf(']', open + 1) + } + if (labelEnd === -1) { + noMoreLabels = true + return null + } + const image = images && open > from && text[open - 1] === '!' + if ((image || labelEnd > open + 1) && text[labelEnd + 1] === '(') { + if (destinationEnd < labelEnd + 2) { + destinationEnd = text.indexOf(')', labelEnd + 2) + } + if (destinationEnd === -1) { + noMoreDestinations = true + return null + } + if (destinationEnd > labelEnd + 2) { + const index = image ? open - 1 : open + return { 0: text.slice(index, destinationEnd + 1), index, end: destinationEnd + 1 } + } + } + // Every opener before this closing bracket shares the same invalid suffix. + open = text.indexOf('[', labelEnd + 1) + } + return null + } + + const matcher = { + lastIndex: 0, + exec(): MarkdownInlineMatch | null { + const from = matcher.lastIndex + if (nextOther === undefined || (nextOther !== null && nextOther.index < from)) { + nonLinkPattern.lastIndex = from + const match = nonLinkPattern.exec(text) + nextOther = match + ? { 0: match[0], index: match.index, end: nonLinkPattern.lastIndex } + : null + } + if (nextLink === undefined || (nextLink !== null && nextLink.index < from)) { + nextLink = findLink(from) + } + const match = + nextLink && (!nextOther || nextLink.index < nextOther.index) ? nextLink : nextOther + if (match) { + matcher.lastIndex = match.end + } + return match + } + } + return matcher +} diff --git a/mobile/src/components/pr-sidebar/markdown-blocks.ts b/mobile/src/components/pr-sidebar/markdown-blocks.ts index 49d8e03e1f4..1f53bbdbae9 100644 --- a/mobile/src/components/pr-sidebar/markdown-blocks.ts +++ b/mobile/src/components/pr-sidebar/markdown-blocks.ts @@ -1,3 +1,5 @@ +import { createMarkdownInlineMatcher } from '../markdown-inline-matcher' + // Tiny, dependency-free markdown model for PR comment bodies. We render GitHub // markdown without a third-party RN markdown library (the previous dependency hung // the JS thread when a comment list mounted). Scope is deliberately small — the @@ -236,23 +238,25 @@ function parseAlignRow(line: string): CellAlign[] { // Inline emphasis/code/link tokenizer. Walks the string once, longest-match first, // emitting plain-text runs between matches. Unbalanced markers stay literal text. -const INLINE = /(`[^`]+`)|(\*\*[^*]+\*\*)|(__[^_]+__)|(\*[^*]+\*)|(_[^_]+_)|(\[[^\]]+\]\([^)]+\))/ +const INLINE = /(`[^`]+`)|(\*\*[^*]+\*\*)|(__[^_]+__)|(\*[^*]+\*)|(_[^_]+_)/g export function parseInline(text: string): InlineToken[] { const tokens: InlineToken[] = [] // Strip residual inline HTML tags (, , , …) so they don't render // literally; emphasis/code/links below are markdown, not HTML, so this is safe. - let rest = stripHtmlTags(text) + const plain = stripHtmlTags(text) + const matcher = createMarkdownInlineMatcher(plain, INLINE) + let cursor = 0 let guard = 0 - while (rest.length > 0 && guard < 5000) { + while (cursor < plain.length && guard < 5000) { guard += 1 - const m = INLINE.exec(rest) + const m = matcher.exec() if (!m || m.index === undefined) { - tokens.push({ kind: 'text', text: rest }) + tokens.push({ kind: 'text', text: plain.slice(cursor) }) break } - if (m.index > 0) { - tokens.push({ kind: 'text', text: rest.slice(0, m.index) }) + if (m.index > cursor) { + tokens.push({ kind: 'text', text: plain.slice(cursor, m.index) }) } const token = m[0] if (token.startsWith('`')) { @@ -269,7 +273,7 @@ export function parseInline(text: string): InlineToken[] { } else { tokens.push({ kind: 'italic', text: token.slice(1, -1) }) } - rest = rest.slice(m.index + token.length) + cursor = matcher.lastIndex } return tokens }