diff --git a/mobile/src/terminal/document/surface-tap.ts b/mobile/src/terminal/document/surface-tap.ts index 38da7ae0fe6..2f0c5fdf81f 100644 --- a/mobile/src/terminal/document/surface-tap.ts +++ b/mobile/src/terminal/document/surface-tap.ts @@ -39,10 +39,10 @@ export function notifyTerminalSurfaceTap( notify(scope, { type: 'open-url', url }) return } - const candidates = uniqueFileTapCandidates( - findTerminalFileLinksAtBufferPosition(buffer, position, term.cols) - ) - if (candidates.length > 0) { + const { links, guesses } = findTerminalFileLinksAtBufferPosition(buffer, position, term.cols) + // Why: a tap decides now, so only a path as written makes it a file tap; the host checks the guesses. + if (links.length > 0) { + const candidates = uniqueFileTapCandidates([...links, ...guesses]) notify(scope, { type: 'terminal-file-tap', candidates }) return } diff --git a/mobile/src/terminal/terminal-webview-tap-routing.test.ts b/mobile/src/terminal/terminal-webview-tap-routing.test.ts index f6deaf21740..2bca5be1b5d 100644 --- a/mobile/src/terminal/terminal-webview-tap-routing.test.ts +++ b/mobile/src/terminal/terminal-webview-tap-routing.test.ts @@ -267,6 +267,32 @@ describe('terminal WebView tap routing', () => { }) }) + it('keeps a tap on prose beside a path an ordinary tap', async () => { + const line = 'me@host:~/proj$ git status' + const { posted } = boot(line) + await settle() + + fireTouch('touchstart', [{ x: screenXForCol(line.indexOf('git') + 1), y: tapY }]) + fireTouch('touchend', []) + + expect(posted.find((m) => m.type === 'terminal-file-tap')).toBeUndefined() + expect(posted.find((m) => m.type === 'terminal-tap')).toBeDefined() + }) + + it('opens a markdown link whose destination has spaces', async () => { + const line = 'see [PDF]()' + const { posted } = boot(line) + await settle() + + fireTouch('touchstart', [{ x: screenXForCol(line.indexOf('with') + 1), y: tapY }]) + fireTouch('touchend', []) + + expect(posted.find((m) => m.type === 'terminal-file-tap')).toEqual({ + type: 'terminal-file-tap', + candidates: [{ pathText: '/tmp/folder with spaces/report.pdf', line: null, column: null }] + }) + }) + it('opens plain file URLs through terminal file taps', async () => { const line = 'open file:///tmp/result.json#L12C3 now' const { posted } = boot(line) diff --git a/src/renderer/src/components/sidebar/comment-markdown-native-chat-file-links.ts b/src/renderer/src/components/sidebar/comment-markdown-native-chat-file-links.ts index 22c71b52bdd..8a49bf664e6 100644 --- a/src/renderer/src/components/sidebar/comment-markdown-native-chat-file-links.ts +++ b/src/renderer/src/components/sidebar/comment-markdown-native-chat-file-links.ts @@ -7,9 +7,11 @@ import { parseFileLinkLocation } from '../../../../shared/file-link-location' import { + extractTerminalFileLinkReadings, extractTerminalFileLinks, type ParsedTerminalFileLink } from '../../../../shared/terminal-links' +import { ROOTED_PATH_START_PATTERN } from '../../../../shared/terminal-file-link-detection-ranges' import { preferLongestNonOverlappingMatches } from '@/lib/longest-non-overlapping-matches' type MarkdownNode = { @@ -19,13 +21,11 @@ type MarkdownNode = { children?: MarkdownNode[] } -const ROOTED_PATH_PREFIX_PATTERN = /^(?:~[\\/]|\.{1,2}[\\/]|[\\/]|[A-Za-z]:[\\/])/ - /** Whether a detected path names a file on the chat's host; the terminal's own check. */ export type FileLinkExists = (link: ParsedTerminalFileLink) => boolean function looksLikeFilePath(link: ParsedTerminalFileLink, isProse: boolean): boolean { - const hasRootedPrefix = ROOTED_PATH_PREFIX_PATTERN.test(link.pathText) + const hasRootedPrefix = ROOTED_PATH_START_PATTERN.test(link.pathText) const hasLineSuffix = link.line !== null || link.column !== null const hasAlphabeticExtension = /\.[\p{L}][\p{L}\p{N}\p{M}_+-]*$/u.test(link.pathText) const hasPathExtension = /\.[\p{L}\p{N}][\p{L}\p{N}\p{M}_+-]*$/u.test(link.pathText) @@ -110,54 +110,12 @@ function createFileLinkNode(link: ParsedTerminalFileLink, child: MarkdownNode): } } -function isTrailingPunctuation(token: string, endIndex: number): boolean { - return isSafeTrailingBoundary(token, endIndex) && /^\p{P}+$/u.test(token.slice(endIndex)) -} - -/** `endsSentence`: a token may end in sentence punctuation, which stays outside the link. */ -function linkableTokens( - link: ParsedTerminalFileLink, - endsSentence: boolean -): ParsedTerminalFileLink[] { - const tokenLinks: ParsedTerminalFileLink[] = [] - for (const match of link.displayText.matchAll(/\S+/g)) { - const token = match[0] - const exactLink = extractTerminalFileLinks(token).find( - (candidate) => - candidate.startIndex === 0 && - (candidate.endIndex === token.length || - (endsSentence && isTrailingPunctuation(token, candidate.endIndex))) - ) - if (exactLink && isLinkifiableFile(exactLink, true)) { - const startIndex = link.startIndex + (match.index ?? 0) - tokenLinks.push({ ...exactLink, startIndex, endIndex: startIndex + exactLink.endIndex }) - } - } - return tokenLinks -} - -// Prose can join paths into one extracted span; split path-like tokens while letting longer real spaced paths win existence checks. -function splitProseJoinedLinks(link: ParsedTerminalFileLink): ParsedTerminalFileLink[] { - if (!/\s/.test(link.displayText)) { - return [link] - } - if (ROOTED_PATH_PREFIX_PATTERN.test(link.pathText)) { - return [link, ...linkableTokens(link, true)] - } - // Preserve unrooted token punctuation to avoid linking cases the extractor intentionally leaves plain. - const tokenLinks = linkableTokens(link, false) - const hasBareWord = Array.from(link.displayText.matchAll(/\S+/g)).some( - (match) => !/[\\/.]/.test(match[0]) - ) - return hasBareWord || tokenLinks.length > 1 ? tokenLinks : [link] -} - function splitTextSegment(value: string, exists: FileLinkExists): MarkdownNode[] { + const { links: textLinks, guesses } = extractTerminalFileLinkReadings(value) const links = selectExistingLinks( - extractTerminalFileLinks(value) + [...textLinks, ...guesses] .filter((link) => !hasPartialPathBoundary(value, link)) - .filter((link) => isLinkifiableFile(link, true)) - .flatMap(splitProseJoinedLinks), + .filter((link) => isLinkifiableFile(link, true)), exists ) if (links.length === 0) { @@ -213,7 +171,7 @@ function exactFileLink(value: string, exists: FileLinkExists): ParsedTerminalFil return null } const looksLikePath = - ROOTED_PATH_PREFIX_PATTERN.test(parsed.pathText) || + ROOTED_PATH_START_PATTERN.test(parsed.pathText) || /[\\/]/.test(parsed.pathText) || /\.[\p{L}][\p{L}\p{N}\p{M}_+-]*$/u.test(parsed.pathText) if (!looksLikePath) { diff --git a/src/renderer/src/components/terminal-pane/terminal-file-link-hit-testing.ts b/src/renderer/src/components/terminal-pane/terminal-file-link-hit-testing.ts index 0b357ef0702..6f027972a45 100644 --- a/src/renderer/src/components/terminal-pane/terminal-file-link-hit-testing.ts +++ b/src/renderer/src/components/terminal-pane/terminal-file-link-hit-testing.ts @@ -42,6 +42,8 @@ type ClickedFileLink = FileLinkTarget & { pathText: string cachedExists: boolean | undefined hasOscLink: boolean + /** Opens only once the host has found its file. */ + guess: boolean } // Why: a slow probe from an earlier click must not open its file after a later click. @@ -54,25 +56,30 @@ export function openFilePathLinkAtBufferPosition( deps: FileLinkHitTestDeps ): boolean { const gesture = ++latestClickGesture + const host: FileLinkHost = { ...deps, cwd: deps.startupCwd, homePath: deps.terminalHomePath } + const readings = findTerminalFileLinksAtBufferPosition(buffer, position, terminalColumns) const links: ClickedFileLink[] = [] - for (const parsed of findTerminalFileLinksAtBufferPosition(buffer, position, terminalColumns)) { - const target = resolveFileLinkTarget(parsed, { - cwd: deps.startupCwd, - homePath: deps.terminalHomePath, - worktreeId: deps.worktreeId, - worktreePath: deps.worktreePath, - runtimeEnvironmentId: deps.runtimeEnvironmentId, - wslDistro: deps.wslDistro - }) - if (!target) { + for (const [parsed, guess] of [ + ...readings.links.map((link) => [link, false] as const), + ...readings.guesses.map((link) => [link, true] as const) + ]) { + const target = resolveFileLinkTarget(parsed, host) + const cachedExists = + target && deps.pathExistsCache + ? readTerminalPathExistence(deps.pathExistsCache, target.cacheKey) + : undefined + // Why: a guess the host may not be asked about (an external share) can never be confirmed. + if ( + !target || + (guess && cachedExists !== true && !mayCheckFileLinkTargetUnprompted(target, host)) + ) { continue } links.push({ ...target, pathText: parsed.pathText, - cachedExists: deps.pathExistsCache - ? readTerminalPathExistence(deps.pathExistsCache, target.cacheKey) - : undefined, + cachedExists, + guess, hasOscLink: terminalLogicalLinkHasOscLink( buffer, parsed.logicalLine, @@ -82,8 +89,12 @@ export function openFilePathLinkAtBufferPosition( }) } - // Why: a failure names the path the user sees, not a longer joined guess. - const shown = shortestPath(links) + // Why: only a written path or a confirmed guess takes the click; prose beside a path stays ordinary text. + if (!links.some((link) => !link.guess || link.cachedExists === true)) { + return false + } + // Why: a failure names the path the user sees, never a guess. + const shown = shortestPath(links.filter((link) => !link.guess)) const open = (link: ClickedFileLink, verified = link.cachedExists === true): void => { const { onOpenFailure } = deps // Why: a bare dotted word (`console.log`, `example.com`) is not a claim that a file exists. @@ -97,8 +108,7 @@ export function openFilePathLinkAtBufferPosition( onOpenFailure: reportsFailure ? (failure) => onOpenFailure(reportedPath, failure) : undefined }) } - const host: FileLinkHost = { ...deps, cwd: deps.startupCwd, homePath: deps.terminalHomePath } - // Never stat external shares unprompted; open the guess as-is and let the host resolve it. + // Never stat external shares unprompted; open the path as written and let the host resolve it. const settled = (link: ClickedFileLink): boolean => link.cachedExists === true || link.isKnownWorktreeRoot || @@ -114,19 +124,17 @@ export function openFilePathLinkAtBufferPosition( !settled(link) && (!best || link.pathText.length > best.pathText.length) ) - if (toProbe.length === 0 || (!best && toProbe.length === 1)) { - // Why the visible path last: a click on a path already known missing still says so. - // An OSC 8 hyperlink there is xterm's to open. - const knownMissing = - shown && !shown.hasOscLink && /[\\/]/.test(shown.pathText) ? shown : undefined - const link = best ?? toProbe[0] ?? knownMissing + // Why the visible path last: a click on a missing path still says so. An OSC 8 hyperlink there is xterm's to open. + const visible = shown && !shown.hasOscLink && /[\\/]/.test(shown.pathText) ? shown : undefined + if (toProbe.length === 0 || (!best && toProbe.length === 1 && !toProbe[0].guess)) { + const link = best ?? toProbe[0] ?? visible if (link) { open(link) } return link !== undefined } - // Wait for hover probes before choosing longer hard-wrap guesses; otherwise a bad join can hide the real path. + // Probe before choosing a longer hard-wrap or spaced reading; otherwise a bad join can hide the real path. const isLatestOpen = reserveDetectedFilePathOpenTurn() const clickedHost = fileHostFingerprint(toProbe[0].fileContext) void probeClickedFileLinks(toProbe, deps).then((existing) => { @@ -138,8 +146,8 @@ export function openFilePathLinkAtBufferPosition( return } const found = longestPath(existing) - // Without a verified candidate, let opening the shortest report the real failure instead of guessing absence. - const link = found ?? best ?? shortestPath(toProbe) + // Without a verified reading, opening the shortest unchecked path reports the real failure. + const link = found ?? best ?? shortestPath(toProbe.filter((link) => !link.guess)) ?? visible if (link) { open(link, link === found || link.cachedExists === true) } diff --git a/src/renderer/src/components/terminal-pane/terminal-link-handlers.ts b/src/renderer/src/components/terminal-pane/terminal-link-handlers.ts index dfce319283c..ed53da39150 100644 --- a/src/renderer/src/components/terminal-pane/terminal-link-handlers.ts +++ b/src/renderer/src/components/terminal-pane/terminal-link-handlers.ts @@ -1,11 +1,7 @@ -import { dedupeLogicalLines } from '../../../../shared/terminal-link-logical-line-deduplication' import type { TerminalPathExistsCache } from './terminal-path-exists-cache' import { createTerminalPathExistenceBatch } from './terminal-path-existence-batch' import type { IDisposable, ILink, ILinkProvider, Terminal } from '@xterm/xterm' -import { - extractTerminalFileLinkCandidates, - extractTerminalFileLinks -} from '../../../../shared/terminal-links' +import { extractTerminalFileLinkReadings } from '../../../../shared/terminal-links' import { preferLongestNonOverlappingMatches } from '@/lib/longest-non-overlapping-matches' import type { PaneManager } from '@/lib/pane-manager/pane-manager' import { buildCandidateLogicalLinesForBufferPosition } from '../../../../shared/terminal-link-buffer-hit-testing' @@ -14,8 +10,6 @@ import { shouldOpenTerminalFileWithSystemDefault } from './terminal-file-open-ro import { isHtmlFilePath, localHtmlFileDestinations } from './local-html-file-destinations' import { fileLinkTargetExists, resolveFileLinkTarget } from './terminal-file-link-target' import { - buildHardWrappedPathLogicalLineCandidates, - buildWrappedLogicalLine, rangeForParsedFileLink, type WrappedLogicalLine } from '../../../../shared/wrapped-terminal-link-ranges' @@ -95,19 +89,21 @@ export function createFilePathLinkProvider( } const buffer = pane.terminal.buffer.active - const softWrappedLogicalLine = buildWrappedLogicalLine(buffer, bufferLineNumber) - const logicalLines = dedupeLogicalLines([ - ...buildHardWrappedPathLogicalLineCandidates(buffer, bufferLineNumber), - ...(softWrappedLogicalLine ? [softWrappedLogicalLine] : []) - ]) - if (logicalLines.every((logicalLine) => !logicalLine.text)) { - callback(undefined) - return - } - - if ( - logicalLines.every((logicalLine) => extractTerminalFileLinks(logicalLine.text).length === 0) - ) { + // Why: xterm hit-tests only the hovered row, so checking the rest of a long line wastes host stats. + const rowLinks = buildCandidateLogicalLinesForBufferPosition( + buffer, + bufferLineNumber + ).flatMap((logicalLine) => { + const { links, guesses } = extractTerminalFileLinkReadings(logicalLine.text) + // Why guesses too: every reading below is shown only once the host finds its file. + return [...links, ...guesses].flatMap((parsed) => { + const range = rangeForParsedFileLink(logicalLine, parsed.startIndex, parsed.endIndex) + return range && range.start.y <= bufferLineNumber && range.end.y >= bufferLineNumber + ? [{ logicalLine, parsed, range }] + : [] + }) + }) + if (rowLinks.length === 0) { callback(undefined) return } @@ -115,86 +111,78 @@ export function createFilePathLinkProvider( const pathExists = createTerminalPathExistenceBatch() // A rejected wrapped-path guess must not hide verified sibling paths. void Promise.allSettled( - logicalLines.flatMap((logicalLine) => - extractTerminalFileLinkCandidates(logicalLine.text).map( - async (parsed): Promise => { - const runtimeEnvironmentId = - deps.getRuntimeEnvironmentIdForPane?.(paneId) ?? deps.runtimeEnvironmentId ?? null - const target = resolveFileLinkTarget(parsed, { - cwd: deps.getPaneLinkCwd?.(paneId) ?? startupCwd, - homePath: deps.terminalHomePath, - worktreeId, - worktreePath, - runtimeEnvironmentId, - wslDistro: deps.wslDistro - }) - const range = rangeForParsedFileLink(logicalLine, parsed.startIndex, parsed.endIndex) - if (!target || !range) { - return null - } - const terminalHandle = deps.getRuntimeTerminalHandleForPane?.(paneId) ?? null - if ( - !(await fileLinkTargetExists(target, pathExistsCache, pathExists, terminalHandle)) - ) { - return null - } - const { absolutePath: mappedPath, fileContext } = target + rowLinks.map(async ({ logicalLine, parsed, range }): Promise => { + const runtimeEnvironmentId = + deps.getRuntimeEnvironmentIdForPane?.(paneId) ?? deps.runtimeEnvironmentId ?? null + const target = resolveFileLinkTarget(parsed, { + cwd: deps.getPaneLinkCwd?.(paneId) ?? startupCwd, + homePath: deps.terminalHomePath, + worktreeId, + worktreePath, + runtimeEnvironmentId, + wslDistro: deps.wslDistro + }) + if (!target) { + return null + } + const terminalHandle = deps.getRuntimeTerminalHandleForPane?.(paneId) ?? null + if (!(await fileLinkTargetExists(target, pathExistsCache, pathExists, terminalHandle))) { + return null + } + const { absolutePath: mappedPath, fileContext } = target - return { - logicalLine, - link: { - range, - text: parsed.displayText, - activate: (event) => { - if ( - handleTerminalFileLink( - mappedPath, - target.line, - target.column, - event, - { - worktreeId, - worktreePath, - runtimeEnvironmentId, - wslDistro: deps.wslDistro, - terminalHandle, - sourceTabId: deps.sourceTabId, - onOpenFailure: (failure) => - showFileLinkOpenFailureToast(mappedPath, failure) - }, - deps.getLinkActionContext?.(paneId) - ) - ) { - pane.terminal.clearSelection?.() - } - }, - hover: () => { - // Only local paths can use the Shift+modifier OS escape hatch. - const canOpenWithSystemDefault = - shouldOpenTerminalFileWithSystemDefault(fileContext) - const showActions = deps.getLinkActionContext - ? deps.getLinkActionContext(paneId) !== null - : true - const hint = target.isKnownWorktreeRoot - ? getTerminalWorktreePathOpenHint(canOpenWithSystemDefault, showActions) - : canOpenWithSystemDefault - ? isHtmlFilePath(mappedPath) - ? getTerminalHtmlFileOpenHint(localHtmlFileDestinations(), showActions) - : showActions - ? openLinkHint - : getTerminalFileOpenHint(false) - : getTerminalOrcaFileOpenHint(showActions) - linkTooltip.textContent = `${mappedPath} (${hint})` - linkTooltip.style.display = '' - }, - leave: () => { - linkTooltip.style.display = 'none' - } + return { + logicalLine, + link: { + range, + text: parsed.displayText, + activate: (event) => { + if ( + handleTerminalFileLink( + mappedPath, + target.line, + target.column, + event, + { + worktreeId, + worktreePath, + runtimeEnvironmentId, + wslDistro: deps.wslDistro, + terminalHandle, + sourceTabId: deps.sourceTabId, + onOpenFailure: (failure) => showFileLinkOpenFailureToast(mappedPath, failure) + }, + deps.getLinkActionContext?.(paneId) + ) + ) { + pane.terminal.clearSelection?.() } + }, + hover: () => { + // Only local paths can use the Shift+modifier OS escape hatch. + const canOpenWithSystemDefault = + shouldOpenTerminalFileWithSystemDefault(fileContext) + const showActions = deps.getLinkActionContext + ? deps.getLinkActionContext(paneId) !== null + : true + const hint = target.isKnownWorktreeRoot + ? getTerminalWorktreePathOpenHint(canOpenWithSystemDefault, showActions) + : canOpenWithSystemDefault + ? isHtmlFilePath(mappedPath) + ? getTerminalHtmlFileOpenHint(localHtmlFileDestinations(), showActions) + : showActions + ? openLinkHint + : getTerminalFileOpenHint(false) + : getTerminalOrcaFileOpenHint(showActions) + linkTooltip.textContent = `${mappedPath} (${hint})` + linkTooltip.style.display = '' + }, + leave: () => { + linkTooltip.style.display = 'none' } } - ) - ) + } + }) ) .then((settledLinks) => { const latestFingerprints = new Set( diff --git a/src/renderer/src/components/terminal-pane/terminal-link-paired-click-fallback.test.ts b/src/renderer/src/components/terminal-pane/terminal-link-paired-click-fallback.test.ts index 8c85ff9afb9..3317f39fcac 100644 --- a/src/renderer/src/components/terminal-pane/terminal-link-paired-click-fallback.test.ts +++ b/src/renderer/src/components/terminal-pane/terminal-link-paired-click-fallback.test.ts @@ -341,6 +341,80 @@ describe('a modifier-click on a paired-server path followed by more paths', () = expect(asked).toEqual(['//server/share/other/report.txt']) }) + it.each([ + ['report', ['//server/share/report.md']], + ['details', []] + ])('never opens an unchecked guess past a share path when clicking %s', async (word, opened) => { + setPlatform('Windows') + runtimeEnvironmentCallMock.mockResolvedValue({ + id: 'rpc-1', + ok: true, + result: { worktree: 'wt-1', relativePath: null, absolutePath: null, exists: false }, + _meta: { runtimeId: 'remote-runtime' } + }) + const line = 'See //server/share/report.md for details' + const buffer = makeBuffer([makeBufferLine(line)]) + + openFilePathLinkAtBufferPosition(buffer, { x: line.indexOf(word) + 1, y: 1 }, 80, { + startupCwd: '/srv/repo', + worktreeId: 'wt-1', + worktreePath: '/srv/repo', + runtimeEnvironmentId: 'env-1', + terminalHandle: 'term-1', + pathExistsCache: new Map() + }) + await flushDoubleRaf() + + const asked = runtimeEnvironmentCallMock.mock.calls.map(([args]) => args.params?.pathText) + expect(asked).toEqual(opened) + }) + + it('leaves a click on prose beside a path to the terminal', async () => { + setPlatform('Macintosh') + const line = '/tmp/report.md completed successfully' + const buffer = makeBuffer([makeBufferLine(line)]) + + const handled = openFilePathLinkAtBufferPosition( + buffer, + { x: line.indexOf('completed') + 1, y: 1 }, + 80, + { + startupCwd: '/srv/repo', + worktreeId: 'wt-1', + worktreePath: '/srv/repo', + runtimeEnvironmentId: 'env-1', + terminalHandle: 'term-1', + pathExistsCache: new Map() + } + ) + await flushDoubleRaf() + + expect(handled).toBe(false) + expect(runtimeEnvironmentCallMock).not.toHaveBeenCalled() + }) + + it('opens the joined path when its checks fail and the visible prefix is known missing', async () => { + setPlatform('Macintosh') + hostAnswers((path) => ({ exists: path === 'report.txt' })) + const answerOpen = runtimeEnvironmentCallMock.getMockImplementation()! + runtimeEnvironmentCallMock.mockImplementation((args) => + args.method === 'files.pathsExist' ? Promise.reject(new Error('host busy')) : answerOpen(args) + ) + const buffer = makeBuffer([ + makeBufferLine('/srv/repo/report'), + makeBufferLine('.txt'), + makeBufferLine('.map') + ]) + + clickPairedPath(buffer, makeExistsCache([[runtimeKey('/srv/repo/report'), false]])) + await flushDoubleRaf() + + expect(openFileMock).toHaveBeenCalledWith( + expect.objectContaining({ filePath: '/srv/repo/report.txt' }), + expect.anything() + ) + }) + it('opens a wrapped share path whole even when its prefix is a known file', async () => { setPlatform('Windows') runtimeEnvironmentCallMock.mockResolvedValue({ diff --git a/src/shared/terminal-file-link-detection-ranges.ts b/src/shared/terminal-file-link-detection-ranges.ts index 3d3705f7017..c53887a7256 100644 --- a/src/shared/terminal-file-link-detection-ranges.ts +++ b/src/shared/terminal-file-link-detection-ranges.ts @@ -4,30 +4,33 @@ import type { ParsedTerminalFileLink } from './terminal-links' const LEADING_TRIM_CHARS = new Set(['(', '[', '{', '"', "'"]) const TRAILING_TRIM_CHARS = new Set([')', ']', '}', '"', "'", ',', ';', '.']) +export const ROOTED_PATH_START_PATTERN = /^(?:~[\\/]|[\\/]|\.{1,2}[\\/]|[A-Za-z]:[\\/])/ + export type DetectedTerminalFileLinkRange = { startIndex: number endIndex: number text: string } -function trimBoundaryPunctuation( +export function hasPathSeparator(text: string): boolean { + return text.includes('/') || text.includes('\\') +} + +export function trimTerminalPathBoundaryPunctuation( value: string, startIndex: number ): DetectedTerminalFileLinkRange | null { let start = 0 let end = value.length - while (start < end && LEADING_TRIM_CHARS.has(value[start])) { start += 1 } while (end > start && TRAILING_TRIM_CHARS.has(value[end - 1])) { end -= 1 } - if (start >= end) { return null } - return { text: value.slice(start, end), startIndex: startIndex + start, @@ -41,7 +44,7 @@ export function* detectTerminalFileLinkRanges( ): Generator { for (const match of lineText.matchAll(regex)) { const rawStart = match.index ?? 0 - const trimmed = trimBoundaryPunctuation(match[0], rawStart) + const trimmed = trimTerminalPathBoundaryPunctuation(match[0], rawStart) if (trimmed) { yield trimmed } @@ -85,30 +88,6 @@ export function terminalFileLinkRangesOverlap( return previous !== undefined && previous[1] > range.startIndex } -export function insertTerminalFileLinkClaimedRange( - claimedRanges: [number, number][], - range: [number, number] -): void { - // oxlint-disable-next-line unicorn/prefer-at -- the phone's terminal WebView (Chrome 74) has no Array#at - const last: [number, number] | undefined = claimedRanges[claimedRanges.length - 1] - if (!last || last[0] <= range[0]) { - claimedRanges.push(range) - return - } - - let low = 0 - let high = claimedRanges.length - while (low < high) { - const mid = Math.floor((low + high) / 2) - if (claimedRanges[mid][0] <= range[0]) { - low = mid + 1 - } else { - high = mid - } - } - claimedRanges.splice(low, 0, range) -} - export function toParsedTerminalFileLink( range: DetectedTerminalFileLinkRange ): ParsedTerminalFileLink | null { diff --git a/src/shared/terminal-link-buffer-hit-testing.ts b/src/shared/terminal-link-buffer-hit-testing.ts index 282ecadea36..d4ded4d51d6 100644 --- a/src/shared/terminal-link-buffer-hit-testing.ts +++ b/src/shared/terminal-link-buffer-hit-testing.ts @@ -2,7 +2,7 @@ import { buildTerminalHttpLogicalLines, hasHardWrappedRows } from './terminal-ht import { extractTerminalHttpLinks } from './terminal-http-url-extraction' import { dedupeLogicalLines } from './terminal-link-logical-line-deduplication' import { rangeForParsedLinkOnRow } from './terminal-link-row-range' -import { extractTerminalFileLinkCandidates, type ParsedTerminalFileLink } from './terminal-links' +import { extractTerminalFileLinkReadings, type ParsedTerminalFileLink } from './terminal-links' import { terminalLogicalLinkHasOscLink } from './terminal-osc-link-cells' import { buildHardWrappedPathLogicalLineCandidates, @@ -60,23 +60,28 @@ export function buildCandidateLogicalLinesForBufferPosition( return dedupeLogicalLines(candidates) } -/** Every file-link reading that covers the cell; callers choose among soft/hard-wrap guesses. */ +/** The readings covering the cell; a guess needs the host to find its file before anyone acts on it. */ export function findTerminalFileLinksAtBufferPosition( buffer: TerminalLinkBuffer, position: TerminalLinkBufferPosition, terminalColumns: number -): TerminalFileLinkAtBufferPosition[] { +): { links: TerminalFileLinkAtBufferPosition[]; guesses: TerminalFileLinkAtBufferPosition[] } { const links: TerminalFileLinkAtBufferPosition[] = [] + const guesses: TerminalFileLinkAtBufferPosition[] = [] for (const logicalLine of buildCandidateLogicalLinesForBufferPosition(buffer, position.y)) { - for (const parsed of extractTerminalFileLinkCandidates(logicalLine.text)) { + const readings = extractTerminalFileLinkReadings(logicalLine.text) + const covering = (parsed: ParsedTerminalFileLink): boolean => { const range = rangeForParsedFileLink(logicalLine, parsed.startIndex, parsed.endIndex) - if (!range || !rangeContainsBufferPosition(range, position, terminalColumns)) { - continue - } + return range !== null && rangeContainsBufferPosition(range, position, terminalColumns) + } + for (const parsed of readings.links.filter(covering)) { links.push({ ...parsed, logicalLine }) } + for (const parsed of readings.guesses.filter(covering)) { + guesses.push({ ...parsed, logicalLine }) + } } - return links + return { links, guesses } } function rangeContainsBufferPosition( diff --git a/src/shared/terminal-links-cjk.test.ts b/src/shared/terminal-links-cjk.test.ts index 0ac0e85a524..99c6c1b6519 100644 --- a/src/shared/terminal-links-cjk.test.ts +++ b/src/shared/terminal-links-cjk.test.ts @@ -1,5 +1,9 @@ import { describe, expect, it } from 'vitest' -import { extractTerminalFileLinks, resolveTerminalFileLink } from './terminal-links' +import { + extractTerminalFileLinkReadings, + extractTerminalFileLinks, + resolveTerminalFileLink +} from './terminal-links' describe('CJK terminal path names', () => { for (const character of ['・', '・', '〜', '~']) { @@ -13,9 +17,11 @@ describe('CJK terminal path names', () => { '/tmp/前CHAR後/space name/a.txt' ])(`keeps ${character} in %s`, (template) => { const filePath = template.replace('CHAR', character) - const links = extractTerminalFileLinks(filePath) - expect(links.map((link) => link.pathText)).toEqual([filePath]) - expect(links[0]).toMatchObject({ startIndex: 0, endIndex: filePath.length }) + const { links, guesses } = extractTerminalFileLinkReadings(filePath) + expect([...links, ...guesses].find((link) => link.pathText === filePath)).toMatchObject({ + startIndex: 0, + endIndex: filePath.length + }) }) it(`preserves location and normalized Windows target for ${character}`, () => { diff --git a/src/shared/terminal-links-redos.test.ts b/src/shared/terminal-links-redos.test.ts index ca8f35273e8..d441f9e301f 100644 --- a/src/shared/terminal-links-redos.test.ts +++ b/src/shared/terminal-links-redos.test.ts @@ -1,6 +1,6 @@ import { performance } from 'node:perf_hooks' import { describe, it, expect } from 'vitest' -import { extractTerminalFileLinks, extractTerminalFileLinkCandidates } from './terminal-links' +import { extractTerminalFileLinkReadings } from './terminal-links' // Large space-padded newline-free ConPTY dashboards once caused overlapping-whitespace backtracking (#5970); bound fixture size without hiding the regression. describe('terminal-links ReDoS guard (#5970)', () => { @@ -15,15 +15,14 @@ describe('terminal-links ReDoS guard (#5970)', () => { for (const [name, line] of cases) { it(`scans "${name}" in roughly linear time`, () => { const start = performance.now() - extractTerminalFileLinks(line) - extractTerminalFileLinkCandidates(line) + extractTerminalFileLinkReadings(line) const elapsedMs = performance.now() - start expect(elapsedMs).toBeLessThan(500) }) } it('still detects separator paths that contain spaces', () => { - const links = extractTerminalFileLinks('/Users/a/Foo Bar/file.ts') - expect(links.some((link) => link.pathText === '/Users/a/Foo Bar/file.ts')).toBe(true) + const { guesses } = extractTerminalFileLinkReadings('/Users/a/Foo Bar/file.ts') + expect(guesses.some((link) => link.pathText === '/Users/a/Foo Bar/file.ts')).toBe(true) }) }) diff --git a/src/shared/terminal-links.test.ts b/src/shared/terminal-links.test.ts index d5b07a6404f..af94824c5f2 100644 --- a/src/shared/terminal-links.test.ts +++ b/src/shared/terminal-links.test.ts @@ -3,32 +3,41 @@ import { TERMINAL_FILE_LINK_TAP_CONFORMANCE_CASES, columnForTerminalFileLinkTap } from './terminal-file-link-conformance' +import { classifyTerminalOscLinkTarget } from './terminal-osc-link-target' import { + extractTerminalFileLinkReadings, extractTerminalFileLinks, - findExactTerminalFileLink, isPathInsideWorktree, resolveTerminalFileLink, toWorktreeRelativePath } from './terminal-links' -function extractTerminalFileLinkAtColumn(lineText: string, column: number) { - return ( - extractTerminalFileLinks(lineText).find( - (link) => column >= link.startIndex && column < link.endIndex - ) ?? null - ) +// Every reading a host that checks files is offered. +function allReadings(lineText: string) { + const { links, guesses } = extractTerminalFileLinkReadings(lineText) + return [...links, ...guesses] +} + +function readingsAt(lineText: string, column: number) { + return allReadings(lineText) + .filter((link) => column >= link.startIndex && column < link.endIndex) + .map(({ pathText, line, column }) => ({ pathText, line, column })) +} + +function pathTexts(lineText: string): string[] { + return allReadings(lineText).map((link) => link.pathText) } describe('terminal path helpers', () => { describe('shared terminal file-link tap conformance', () => { + // Desktop offers every reading at the tap and lets the host's existence check choose. it.each(TERMINAL_FILE_LINK_TAP_CONFORMANCE_CASES)('$name', (testCase) => { - const link = extractTerminalFileLinkAtColumn( - testCase.lineText, - columnForTerminalFileLinkTap(testCase) - ) - expect( - link ? { pathText: link.pathText, line: link.line, column: link.column } : null - ).toEqual(testCase.expected) + const readings = readingsAt(testCase.lineText, columnForTerminalFileLinkTap(testCase)) + if (testCase.expected) { + expect(readings).toContainEqual(testCase.expected) + } else { + expect(readings).toEqual([]) + } }) }) @@ -58,8 +67,9 @@ describe('terminal path helpers', () => { }) it('does not double-link bare tokens that are part of an already-matched path', () => { - const links = extractTerminalFileLinks('./src/file.ts is the entry point') - expect(links.map((link) => link.displayText)).toEqual(['./src/file.ts']) + const paths = pathTexts('./src/file.ts is the entry point') + expect(paths).toContain('./src/file.ts') + expect(paths).not.toContain('file.ts') }) }) @@ -73,18 +83,14 @@ describe('terminal path helpers', () => { ).toEqual(['docs/café/report.pdf']) }) - it('detects an extensionless absolute path ending in a spaced segment', () => { - const links = extractTerminalFileLinks('/Users/alice/My Folder') - expect(links).toHaveLength(1) - expect(links[0]).toMatchObject({ - pathText: '/Users/alice/My Folder', - displayText: '/Users/alice/My Folder' - }) + it('offers absolute paths with spaces before the filename', () => { + expect(pathTexts('/Users/Path/FolderName with Space/content.js')).toContain( + '/Users/Path/FolderName with Space/content.js' + ) }) - it('does not treat mid-line command arguments as line-ending spaced paths', () => { - const links = extractTerminalFileLinks('run /usr/bin/env node, then continue') - expect(links.map((link) => link.pathText)).not.toContain('/usr/bin/env node') + it('offers an extensionless absolute path ending in a spaced segment', () => { + expect(pathTexts('/Users/alice/My Folder')).toContain('/Users/alice/My Folder') }) it('does not linkify root-only or relative trailing separator tokens', () => { @@ -96,6 +102,44 @@ describe('terminal path helpers', () => { expect(extractTerminalFileLinks('C:\\')).toEqual([]) }) + it('offers an extensionless relative path ending in a spaced segment', () => { + expect(pathTexts('./My Folder')).toContain('./My Folder') + }) + + // A spaced guess must never hide the plain path; the host keeps whichever exists. + it.each([ + [ + '~/orca-lanes/webhooks-research/bots/bots-ux-outline.md. It covers setup and/or teardown.', + '~/orca-lanes/webhooks-research/bots/bots-ux-outline.md' + ], + ['Wrote ~/notes/plan.md. Next: src/foo.ts', '~/notes/plan.md'], + ['Wrote ~/notes/plan.md and src/foo.ts covers the rest', '~/notes/plan.md'], + ['See /Users/me/a/b.md. Then C:\\Users\\me\\x.md.', '/Users/me/a/b.md'], + ['See /home/me/x.md. Then the pane/tab work.', '/home/me/x.md'], + ['See C:\\Users\\me\\x.md. Then the pane/tab work.', 'C:\\Users\\me\\x.md'] + ])('offers the plain path in %j', (line, path) => { + expect(pathTexts(line)).toContain(path) + }) + + it.each([ + '/Users/a/C# Projects/x.cs', + '/Users/a/Tom & Jerry/x.ts', + '/Users/a/Notes - Draft/x.md', + 'C:\\Users\\Me\\Q&A Docs\\x.txt', + '/tmp/Foo.bar Docs/report.md', + '/tmp/My File.123', + 'docs/My Archive.7z', + 'docs/My 檔案.文件' + ])('offers spaced folder names with symbols: %s', (path) => { + expect(pathTexts(`open ${path}, then continue`)).toContain(path) + }) + + it('still offers the spaced reading so an existing spaced path can win', () => { + expect(pathTexts('Saved /Users/me/My Docs/plan.md. Next: src/foo.ts')).toEqual( + expect.arrayContaining(['/Users/me/My', '/Users/me/My Docs/plan.md']) + ) + }) + it('detects framework route paths with bracket and paren segments', () => { const links = extractTerminalFileLinks( 'Error in app/(shop)/products/[productId]/page.tsx:42:7' @@ -108,6 +152,20 @@ describe('terminal path helpers', () => { displayText: 'app/(shop)/products/[productId]/page.tsx:42:7' }) }) + + it('handles large spaced path lists in linear time', () => { + const line = Array.from({ length: 20_000 }, () => '/tmp/Foo Bar/file').join(', ') + + const paths = pathTexts(line) + + expect(paths.filter((path) => path === '/tmp/Foo Bar/file')).toHaveLength(20_000) + }) + + it('grows a spaced reading over a bounded number of words', () => { + const paths = pathTexts(`/tmp/root ${'part/x '.repeat(20_000)}`) + + expect(paths.filter((path) => path.startsWith('/tmp/root'))).toHaveLength(11) + }) }) it('resolves tilde-prefixed paths against explicit terminal home when cwd is outside home', () => { @@ -131,16 +189,32 @@ describe('terminal path helpers', () => { }) }) - it('keeps line and column suffixes from exact OSC hyperlink text', () => { - expect(findExactTerminalFileLink('docs/README.md:12:3')).toMatchObject({ - pathText: 'docs/README.md', - line: 12, - column: 3 - }) + it('keeps a path spanning prose apart from the paths written in it', () => { + const { links, guesses } = extractTerminalFileLinkReadings( + '● Wrote ~/x/notes.md. It covers setup and/or teardown.' + ) + expect(links.map((link) => link.pathText)).toEqual(['~/x/notes.md', 'and/or']) + expect(guesses.map((link) => link.pathText)).toContain('~/x/notes.md. It covers setup') }) - it('does not take partial text as an OSC hyperlink target', () => { - expect(findExactTerminalFileLink('open docs/README.md')).toBeNull() + it.each([ + ['docs/README.md:12:3', 'docs/README.md', 12], + ['/Users/a/My Folder/file.ts:3', '/Users/a/My Folder/file.ts', 3], + ['docs/My Folder/file.ts', 'docs/My Folder/file.ts', null] + ])('takes OSC hyperlink target %s as written', (target, pathText, line) => { + const osc = classifyTerminalOscLinkTarget(target, { + windowsPathsFirst: false, + allowUncHost: false + }) + expect(osc).toMatchObject({ kind: 'path', link: { pathText, line } }) + }) + + it('does not read a malformed URL OSC hyperlink target as a path', () => { + const osc = classifyTerminalOscLinkTarget('https://exa mple.com', { + windowsPathsFirst: false, + allowUncHost: false + }) + expect(osc).toBeNull() }) describe('plain-text file:// URIs', () => { diff --git a/src/shared/terminal-links.ts b/src/shared/terminal-links.ts index 1e99f72e5d1..9211a6961ef 100644 --- a/src/shared/terminal-links.ts +++ b/src/shared/terminal-links.ts @@ -2,13 +2,13 @@ import { normalizeAbsolutePath } from './terminal-path-normalization' import { resolveExplicitFileLinkTarget } from './explicit-file-link-target' import { detectBareFilenameLinks } from './terminal-bare-file-link-detection' import { - detectTerminalFileLinkRanges, - insertTerminalFileLinkClaimedRange, + hasPathSeparator, mergeTerminalFileLinkRanges, - terminalFileLinkRangesOverlap, toParsedTerminalFileLink, + trimTerminalPathBoundaryPunctuation, type DetectedTerminalFileLinkRange } from './terminal-file-link-detection-ranges' +import { growSpacedPathRanges } from './terminal-spaced-path-candidates' import { detectTerminalFileUriLinks } from './terminal-file-uri-link' import { detectTerminalMarkdownFileLinks } from './terminal-markdown-file-links' @@ -34,62 +34,8 @@ export type ResolvedTerminalFileLink = Pick|`\r\n]+(?::\d+)?(?::\d+)?/g -// Why this shares the broad candidate shape: extension paths with prose after -// them still need trimming, but the whitespace/extension test stays in code. -const SPACED_PATH_WITH_EXTENSION_REGEX = - /(?:~[\\/]|[\\/]|\.{1,2}[\\/]|[A-Za-z]:[\\/]|[A-Za-z0-9._-]+[\\/])[^()[\]{}'",;<>|`\r\n]+(?::\d+)?(?::\d+)?/g -// Why this is also broad: the candidates path runs on hover, including huge -// space-padded TUI lines, so reject line-ending spaced paths outside the regex. -const LINE_ENDING_SPACED_PATH_REGEX = - /(?:~[\\/]|[\\/]|\.{1,2}[\\/]|[A-Za-z]:[\\/]|[A-Za-z0-9._-]+[\\/])[^()[\]{}'",;<>|`\r\n]+(?::\d+)?(?::\d+)?/g -const SPACED_LOCAL_PATH_REGEXES = [ - SPACED_PATH_WITH_SEPARATOR_REGEX, - SPACED_PATH_WITH_EXTENSION_REGEX, - LINE_ENDING_SPACED_PATH_REGEX -] - const URI_PREFIX_CHAR_PATTERN = /^[A-Za-z0-9+./:-]$/ -function hasPathSeparator(text: string): boolean { - return text.includes('/') || text.includes('\\') -} - -function hasSeparatorAfterWhitespace(text: string): boolean { - let sawWhitespace = false - for (const char of text) { - if (/\s/.test(char)) { - sawWhitespace = true - continue - } - if (sawWhitespace && (char === '/' || char === '\\')) { - return true - } - } - return false -} - -function hasInternalWhitespaceBeforeTrimmedEnd(text: string): boolean { - const trimmed = text.trimEnd() - return /\s/.test(trimmed) -} - -function isAtTrimmedLineEnd(lineText: string, endIndex: number): boolean { - return lineText.slice(endIndex).trim().length === 0 -} - -function hasSpacedPathExtension(text: string): boolean { - const trimmedRange = trimSpacedPathTrailingProse({ - text, - startIndex: 0, - endIndex: text.length - }) - const trimmedText = trimmedRange.text.trimEnd() - return /\s/.test(trimmedText) && /\.[A-Za-z0-9_+-]+(?::\d+)?(?::\d+)?$/.test(trimmedText) -} - function getImmediateUriPrefix(lineText: string, endIndex: number): string { let start = endIndex while (start > 0 && URI_PREFIX_CHAR_PATTERN.test(lineText[start - 1])) { @@ -108,183 +54,72 @@ function isInsideUriScheme(lineText: string, range: DetectedTerminalFileLinkRang ) } -function trimSpacedPathTrailingProse( - range: DetectedTerminalFileLinkRange -): DetectedTerminalFileLinkRange { - // Trim prose/unrelated paths after extension-terminated candidates. Extend only path-like added segments (v1.2 reports/result.json), never prose such as failed to start app.py. - let selected: string | null = null - const extensionPrefixPattern = /\.[A-Za-z0-9_+-]+(?::\d+)?(?::\d+)?(?=\s+|$)/g - const pathStartPattern = /(?:^|\s)(?:~[\\/]|[\\/]|\.{1,2}[\\/]|[A-Za-z]:[\\/])/g - let pathStartCount = 0 - let nextPathStart = pathStartPattern.exec(range.text) - let match: RegExpExecArray | null - while ((match = extensionPrefixPattern.exec(range.text)) !== null) { - const end = match.index + match[0].length - const text = range.text.slice(0, end) - while (nextPathStart && nextPathStart.index + nextPathStart[0].length <= end) { - pathStartCount += 1 - nextPathStart = pathStartPattern.exec(range.text) - } - if (pathStartCount > 1) { - continue - } - if ( - end < range.text.length || - selected === null || - /[\\/]/.test(range.text.slice(selected.length, end)) - ) { - selected = text - } - } - if (!selected) { - return range - } - return { - text: selected, - startIndex: range.startIndex, - endIndex: range.startIndex + selected.length - } -} - -function trimTrailingWhitespace( - range: DetectedTerminalFileLinkRange -): DetectedTerminalFileLinkRange { - const text = range.text.trimEnd() - return { - text, - startIndex: range.startIndex, - endIndex: range.startIndex + text.length - } -} - -function buildLineEndingSpacedPathPrefixRanges( - range: DetectedTerminalFileLinkRange -): DetectedTerminalFileLinkRange[] { - const ranges: DetectedTerminalFileLinkRange[] = [] - for (const match of range.text.matchAll(/\s+/g)) { - const endIndex = match.index ?? 0 - const text = range.text.slice(0, endIndex).trimEnd() - if (text.includes(' ')) { - ranges.push({ - text, - startIndex: range.startIndex, - endIndex: range.startIndex + text.length - }) - } - } - // oxlint-disable-next-line unicorn/no-array-reverse -- the phone's terminal WebView (Chrome 74) has no Array#toReversed; ranges is local - return ranges.reverse() -} - -/** Detect separator paths with optional :line:col, ported from VS Code TerminalLocalLinkDetector. */ -function detectLocalPathLinks( - lineText: string, - includeLineEndingPrefixCandidates = false -): ParsedTerminalFileLink[] { +/** Separator paths plus their spaced extensions, ported from VS Code TerminalLocalLinkDetector. */ +function detectLocalPathLinks(lineText: string): { + paths: ParsedTerminalFileLink[] + spacedPaths: ParsedTerminalFileLink[] +} { + const paths: ParsedTerminalFileLink[] = [] + const spacedPaths: ParsedTerminalFileLink[] = [] if (!hasPathSeparator(lineText)) { - return [] + return { paths, spacedPaths } } - - const links: ParsedTerminalFileLink[] = [] - const spacedLinks = detectSpacedLocalPathLinks(lineText, includeLineEndingPrefixCandidates) - const spacedRanges = mergeTerminalFileLinkRanges( - spacedLinks.map(({ startIndex, endIndex }): [number, number] => [startIndex, endIndex]) - ) - for (const link of spacedLinks) { - links.push(link) - } - for (const range of detectTerminalFileLinkRanges(lineText, LOCAL_PATH_REGEX)) { - if (terminalFileLinkRangesOverlap(range, spacedRanges)) { - continue - } - if (isInsideUriScheme(lineText, range)) { - continue - } - if (!/[\\/]/.test(range.text)) { + let grownWordEndIndex = 0 + for (const match of lineText.matchAll(LOCAL_PATH_REGEX)) { + const rawStart = match.index ?? 0 + const range = trimTerminalPathBoundaryPunctuation(match[0], rawStart) + if (!range || !hasPathSeparator(range.text) || isInsideUriScheme(lineText, range)) { continue } const link = toParsedTerminalFileLink(range) if (link) { - links.push(link) + paths.push(link) } - } - return links.sort((a, b) => a.startIndex - b.startIndex || b.endIndex - a.endIndex) -} - -function detectSpacedLocalPathLinks( - lineText: string, - includeLineEndingPrefixCandidates = false -): ParsedTerminalFileLink[] { - const links: ParsedTerminalFileLink[] = [] - const claimedRanges: [number, number][] = [] - for (const regex of SPACED_LOCAL_PATH_REGEXES) { - for (const range of detectTerminalFileLinkRanges(lineText, regex)) { - if (regex === SPACED_PATH_WITH_SEPARATOR_REGEX && !hasSeparatorAfterWhitespace(range.text)) { - continue - } - if (regex === SPACED_PATH_WITH_EXTENSION_REGEX && !hasSpacedPathExtension(range.text)) { - continue - } - if ( - regex === LINE_ENDING_SPACED_PATH_REGEX && - (!hasInternalWhitespaceBeforeTrimmedEnd(range.text) || - !isAtTrimmedLineEnd(lineText, range.endIndex)) - ) { - continue - } - if ( - terminalFileLinkRangesOverlap(range, claimedRanges) || - isInsideUriScheme(lineText, range) - ) { - continue - } - const candidateRanges = - includeLineEndingPrefixCandidates && regex === LINE_ENDING_SPACED_PATH_REGEX - ? [range, ...buildLineEndingSpacedPathPrefixRanges(range)] - : [range] - const candidateLinks = candidateRanges - .map((candidateRange) => - toParsedTerminalFileLink( - trimSpacedPathTrailingProse(trimTrailingWhitespace(candidateRange)) - ) - ) - .filter((link): link is ParsedTerminalFileLink => link !== null) - const link = candidateLinks[0] - if (link) { - for (const candidateLink of candidateLinks) { - links.push(candidateLink) - } - insertTerminalFileLinkClaimedRange(claimedRanges, [link.startIndex, link.endIndex]) + // Why: a path inside an earlier path's word (`a!b/c`) shares its growth; regrowing would rescan the word. + if (rawStart < grownWordEndIndex) { + continue + } + const grown = growSpacedPathRanges(lineText, range, rawStart + match[0].length) + grownWordEndIndex = grown.wordEndIndex + for (const spacedRange of grown.ranges) { + const spacedLink = toParsedTerminalFileLink(spacedRange) + if (spacedLink) { + spacedPaths.push(spacedLink) } } } - return links + return { paths, spacedPaths } } -/** Claim Markdown destinations/file URIs/paths before bare filenames so longer explicit links keep their ranges. */ -function assembleFileLinks( - lineText: string, - includeLineEndingPrefixCandidates: boolean -): ParsedTerminalFileLink[] { +export type TerminalFileLinkReadings = { + /** Paths as written: markdown destinations, file URIs, unspaced paths, bare filenames. */ + links: ParsedTerminalFileLink[] + /** A path continued across the following words; act on one only after the host finds it exists. */ + guesses: ParsedTerminalFileLink[] +} + +export function extractTerminalFileLinkReadings(lineText: string): TerminalFileLinkReadings { const markdown = detectTerminalMarkdownFileLinks(lineText) const uriLinks = detectTerminalFileUriLinks(markdown.remainingText) - const pathLinks = detectLocalPathLinks(markdown.remainingText, includeLineEndingPrefixCandidates) - const explicitLinks = [...markdown.links, ...uriLinks, ...pathLinks] + const { paths, spacedPaths } = detectLocalPathLinks(markdown.remainingText) + const claimingLinks = [...markdown.links, ...uriLinks, ...paths] const claimed = mergeTerminalFileLinkRanges( - explicitLinks.map(({ startIndex, endIndex }): [number, number] => [startIndex, endIndex]) + claimingLinks.map(({ startIndex, endIndex }): [number, number] => [startIndex, endIndex]) ) - for (const link of detectBareFilenameLinks(markdown.remainingText, claimed)) { - explicitLinks.push(link) + return { + links: [...claimingLinks, ...detectBareFilenameLinks(markdown.remainingText, claimed)].sort( + byStart + ), + guesses: spacedPaths } - return explicitLinks.sort((a, b) => a.startIndex - b.startIndex || b.endIndex - a.endIndex) } export function extractTerminalFileLinks(lineText: string): ParsedTerminalFileLink[] { - return assembleFileLinks(lineText, false) + return extractTerminalFileLinkReadings(lineText).links } -export function extractTerminalFileLinkCandidates(lineText: string): ParsedTerminalFileLink[] { - return assembleFileLinks(lineText, true) +function byStart(a: ParsedTerminalFileLink, b: ParsedTerminalFileLink): number { + return a.startIndex - b.startIndex || b.endIndex - a.endIndex } export function resolveTerminalFileLink( @@ -295,12 +130,6 @@ export function resolveTerminalFileLink( return resolveExplicitFileLinkTarget(parsed, cwd, homePath) } -/** The file link spanning all of `linkText`, as an OSC 8 target or other whole-text link names it. */ -export function findExactTerminalFileLink(linkText: string): ParsedTerminalFileLink | null { - const links = extractTerminalFileLinks(linkText) - return links.find((link) => link.startIndex === 0 && link.endIndex === linkText.length) ?? null -} - export function isPathInsideWorktree(filePath: string, worktreePath: string): boolean { const normalizedFile = normalizeAbsolutePath(filePath) const normalizedWorktree = normalizeAbsolutePath(worktreePath) diff --git a/src/shared/terminal-markdown-file-links.test.ts b/src/shared/terminal-markdown-file-links.test.ts index 39950654573..2de68f74732 100644 --- a/src/shared/terminal-markdown-file-links.test.ts +++ b/src/shared/terminal-markdown-file-links.test.ts @@ -1,90 +1,88 @@ import { describe, expect, it } from 'vitest' -import { extractTerminalFileLinkCandidates, extractTerminalFileLinks } from './terminal-links' +import { extractTerminalFileLinks } from './terminal-links' -for (const extract of [extractTerminalFileLinks, extractTerminalFileLinkCandidates]) { - describe(extract.name, () => { - it.each([ - [ - '[/tmp/Каталог/отчет.pdf](/tmp/Каталог/отчет.pdf)', - '/tmp/Каталог/отчет.pdf', - '/tmp/Каталог/отчет.pdf' - ], - ['[/wrong/file.pdf](/tmp/right.pdf)', '/tmp/right.pdf', '/tmp/right.pdf'], - [ - '[PDF](/tmp/folder%20with%20spaces/report.pdf)', - '/tmp/folder%20with%20spaces/report.pdf', - '/tmp/folder with spaces/report.pdf' - ], - [ - '[PDF](/tmp/literal%2520/report.pdf)', - '/tmp/literal%2520/report.pdf', - '/tmp/literal%20/report.pdf' - ], - [ - '[PDF]()', - '/tmp/folder with spaces/report.pdf', - '/tmp/folder with spaces/report.pdf' - ], - [ - '[route](app/(shop)/products/[id]/page.tsx:12:3 "source")', - 'app/(shop)/products/[id]/page.tsx:12:3', - 'app/(shop)/products/[id]/page.tsx' - ], - [ - '[route](/tmp/report\\(final\\).pdf)', - '/tmp/report\\(final\\).pdf', - '/tmp/report(final).pdf' - ], - [ - '[file](file:///tmp/folder%20name/report.pdf#L12C3)', - 'file:///tmp/folder%20name/report.pdf#L12C3', - '/tmp/folder name/report.pdf' - ], - [ - '[source](C:\\repo\\src\\file.ts:12:3)', - 'C:\\repo\\src\\file.ts:12:3', - 'C:\\repo\\src\\file.ts' - ], - [ - '[cfg](C:\\Users\\me\\.config\\app.json)', - 'C:\\Users\\me\\.config\\app.json', - 'C:\\Users\\me\\.config\\app.json' - ], - ['[ci](src\\.github\\ci.yml)', 'src\\.github\\ci.yml', 'src\\.github\\ci.yml'], - ['[source](README.md:12:3)', 'README.md:12:3', 'README.md'], - ['[PDF](/tmp/100%/report.pdf)', '/tmp/100%/report.pdf', '/tmp/100%/report.pdf'] - ])('extracts only destination from %s', (text, displayText, pathText) => { - const links = extract(`before ${text} after`) - expect(links).toHaveLength(1) - expect(links[0]).toMatchObject({ pathText, displayText }) - expect(`before ${text} after`.slice(links[0].startIndex, links[0].endIndex)).toBe(displayText) - if (text.includes(':12:3') || text.includes('#L12C3')) { - expect(links[0]).toMatchObject({ line: 12, column: 3 }) - } - }) - - it('preserves bare percent filenames and route punctuation beside Markdown', () => { - expect( - extract('/tmp/literal%20/file.pdf [PDF](/tmp/report.pdf) app/(shop)/[id]/page.tsx').map( - (link) => link.pathText - ) - ).toEqual(['/tmp/literal%20/file.pdf', '/tmp/report.pdf', 'app/(shop)/[id]/page.tsx']) - }) - - it('does not turn an external link label into a local file target', () => { - expect(extract('[/tmp/misleading.pdf](https://example.com/report.pdf)')).toEqual([]) - }) - - it('preserves adjacent route bracket and parenthesis segments in bare paths', () => { - expect(extract('/tmp/[id](group)/page.tsx').map((link) => link.pathText)).toEqual([ - '/tmp/[id](group)/page.tsx' - ]) - }) - - it('finds adjacent Markdown destinations independently', () => { - expect( - extract('[one](/tmp/one.pdf)[two](/tmp/two.pdf)').map((link) => link.pathText) - ).toEqual(['/tmp/one.pdf', '/tmp/two.pdf']) - }) +describe('extractTerminalFileLinks', () => { + it.each([ + [ + '[/tmp/Каталог/отчет.pdf](/tmp/Каталог/отчет.pdf)', + '/tmp/Каталог/отчет.pdf', + '/tmp/Каталог/отчет.pdf' + ], + ['[/wrong/file.pdf](/tmp/right.pdf)', '/tmp/right.pdf', '/tmp/right.pdf'], + [ + '[PDF](/tmp/folder%20with%20spaces/report.pdf)', + '/tmp/folder%20with%20spaces/report.pdf', + '/tmp/folder with spaces/report.pdf' + ], + [ + '[PDF](/tmp/literal%2520/report.pdf)', + '/tmp/literal%2520/report.pdf', + '/tmp/literal%20/report.pdf' + ], + [ + '[PDF]()', + '/tmp/folder with spaces/report.pdf', + '/tmp/folder with spaces/report.pdf' + ], + [ + '[route](app/(shop)/products/[id]/page.tsx:12:3 "source")', + 'app/(shop)/products/[id]/page.tsx:12:3', + 'app/(shop)/products/[id]/page.tsx' + ], + ['[route](/tmp/report\\(final\\).pdf)', '/tmp/report\\(final\\).pdf', '/tmp/report(final).pdf'], + [ + '[file](file:///tmp/folder%20name/report.pdf#L12C3)', + 'file:///tmp/folder%20name/report.pdf#L12C3', + '/tmp/folder name/report.pdf' + ], + [ + '[source](C:\\repo\\src\\file.ts:12:3)', + 'C:\\repo\\src\\file.ts:12:3', + 'C:\\repo\\src\\file.ts' + ], + [ + '[cfg](C:\\Users\\me\\.config\\app.json)', + 'C:\\Users\\me\\.config\\app.json', + 'C:\\Users\\me\\.config\\app.json' + ], + ['[ci](src\\.github\\ci.yml)', 'src\\.github\\ci.yml', 'src\\.github\\ci.yml'], + ['[source](README.md:12:3)', 'README.md:12:3', 'README.md'], + ['[PDF](/tmp/100%/report.pdf)', '/tmp/100%/report.pdf', '/tmp/100%/report.pdf'] + ])('extracts only destination from %s', (text, displayText, pathText) => { + const links = extractTerminalFileLinks(`before ${text} after`) + expect(links).toHaveLength(1) + expect(links[0]).toMatchObject({ pathText, displayText }) + expect(`before ${text} after`.slice(links[0].startIndex, links[0].endIndex)).toBe(displayText) + if (text.includes(':12:3') || text.includes('#L12C3')) { + expect(links[0]).toMatchObject({ line: 12, column: 3 }) + } }) -} + + it('preserves bare percent filenames and route punctuation beside Markdown', () => { + expect( + extractTerminalFileLinks( + '/tmp/literal%20/file.pdf [PDF](/tmp/report.pdf) app/(shop)/[id]/page.tsx' + ).map((link) => link.pathText) + ).toEqual(['/tmp/literal%20/file.pdf', '/tmp/report.pdf', 'app/(shop)/[id]/page.tsx']) + }) + + it('does not turn an external link label into a local file target', () => { + expect( + extractTerminalFileLinks('[/tmp/misleading.pdf](https://example.com/report.pdf)') + ).toEqual([]) + }) + + it('preserves adjacent route bracket and parenthesis segments in bare paths', () => { + expect( + extractTerminalFileLinks('/tmp/[id](group)/page.tsx').map((link) => link.pathText) + ).toEqual(['/tmp/[id](group)/page.tsx']) + }) + + it('finds adjacent Markdown destinations independently', () => { + expect( + extractTerminalFileLinks('[one](/tmp/one.pdf)[two](/tmp/two.pdf)').map( + (link) => link.pathText + ) + ).toEqual(['/tmp/one.pdf', '/tmp/two.pdf']) + }) +}) diff --git a/src/shared/terminal-osc-link-target.ts b/src/shared/terminal-osc-link-target.ts index e2313472123..5ba7d0b4d4c 100644 --- a/src/shared/terminal-osc-link-target.ts +++ b/src/shared/terminal-osc-link-target.ts @@ -3,7 +3,8 @@ import { resolveTerminalFileUrlTarget, type TerminalFileUrlTarget } from './terminal-file-url-target' -import { findExactTerminalFileLink, type ParsedTerminalFileLink } from './terminal-links' +import { toParsedTerminalFileLink } from './terminal-file-link-detection-ranges' +import type { ParsedTerminalFileLink } from './terminal-links' export type TerminalOscLinkTarget = | { kind: 'url'; url: string } @@ -21,8 +22,11 @@ export function classifyTerminalOscLinkTarget( rawText: string, options: TerminalOscLinkTargetOptions ): TerminalOscLinkTarget | null { + // Why: a hyperlink names its whole target, so the path is taken as written, never detected in it. const pathTarget = (): TerminalOscLinkTarget | null => { - const link = findExactTerminalFileLink(rawText) + const link = rawText.includes('://') + ? null + : toParsedTerminalFileLink({ text: rawText, startIndex: 0, endIndex: rawText.length }) return link ? { kind: 'path', link } : null } const windowsPath = diff --git a/src/shared/terminal-spaced-path-candidates.ts b/src/shared/terminal-spaced-path-candidates.ts new file mode 100644 index 00000000000..0ac6725b4ca --- /dev/null +++ b/src/shared/terminal-spaced-path-candidates.ts @@ -0,0 +1,67 @@ +import { + ROOTED_PATH_START_PATTERN, + hasPathSeparator, + trimTerminalPathBoundaryPunctuation, + type DetectedTerminalFileLinkRange +} from './terminal-file-link-detection-ranges' + +// Spaced readings are guesses the host's existence check settles, so no rule predicts where a path ends. +const MAX_SPACED_PATH_WORDS = 10 +// Why: the path regex stops at `#&=!`, which spaced folder names (`C# Projects`) may contain. +const PATH_WORD_CONTINUATION_REGEX = /[^\s()[\]{}'",;<>|`]*/y +// One single-spaced word; quotes and brackets end a spaced path. +const SPACED_PATH_WORD_REGEX = / ([^\s()[\]{}'",;<>|`]+)/y +const FILENAME_PATTERN = /\.[\p{L}\p{N}][\p{L}\p{N}\p{M}_+-]*(?::\d+)?(?::\d+)?[.,;:!?]*$/u + +/** Rooted: one reading per following word, up to ten; relative: its first filename. `wordEndIndex` ends the path's own word. */ +export function growSpacedPathRanges( + lineText: string, + path: DetectedTerminalFileLinkRange, + rawEndIndex: number +): { ranges: DetectedTerminalFileLinkRange[]; wordEndIndex: number } { + PATH_WORD_CONTINUATION_REGEX.lastIndex = rawEndIndex + const continuation = PATH_WORD_CONTINUATION_REGEX.exec(lineText)?.[0] ?? '' + const wordEndIndex = rawEndIndex + continuation.length + const ranges: DetectedTerminalFileLinkRange[] = [] + // Why: a trimmed closer (`(src/a.ts) now`) ends the path, a URL is a different link, and + // a word that continues the path past unsupported punctuation (`/tmp/$draft/a.md`) is never one link. + if ( + /[^.]/.test(lineText.slice(path.endIndex, rawEndIndex)) || + continuation.includes('://') || + hasPathSeparator(continuation) + ) { + return { ranges, wordEndIndex } + } + // Why: relative paths come in dense lists (`src/a.ts src/b.ts`), so they grow only to their first filename. + const relative = !ROOTED_PATH_START_PATTERN.test(path.text) + if (relative && FILENAME_PATTERN.test(path.text + continuation)) { + return { ranges, wordEndIndex } + } + SPACED_PATH_WORD_REGEX.lastIndex = wordEndIndex + let match: RegExpExecArray | null + for ( + let words = 0; + words < MAX_SPACED_PATH_WORDS && + (match = SPACED_PATH_WORD_REGEX.exec(lineText)) !== null && + // Why: `./a.ts ./b.ts` names two paths, and a `scheme://` word starts a URL. + !ROOTED_PATH_START_PATTERN.test(match[1]) && + !match[1].includes('://'); + words += 1 + ) { + const filename = FILENAME_PATTERN.test(match[1]) + if (relative && !filename) { + continue + } + const range = trimTerminalPathBoundaryPunctuation( + lineText.slice(path.startIndex, SPACED_PATH_WORD_REGEX.lastIndex), + path.startIndex + ) + if (range) { + ranges.push(range) + } + if (relative) { + break + } + } + return { ranges, wordEndIndex } +}