mirror of
https://github.com/stablyai/orca.git
synced 2026-09-22 00:02:31 +00:00
fix(terminal): re-linkify streamed URLs so links underline/click on first hover (#9320)
Invalidate xterm's linkifier hover cache (throttled) when output streams into a visible pane, so a URL printed under a stationary pointer underlines and becomes Cmd/Ctrl+clickable on the next pointer move instead of after several clicks. Skips while a link is hovered (no flicker) and re-arms so the reset is never dropped mid-hover. Reuses the visibility-resume primitive (#9061).
This commit is contained in:
@@ -515,6 +515,7 @@ describe('openTerminal — addon and provider wiring', () => {
|
||||
}
|
||||
}),
|
||||
attachCustomWheelEventHandler: vi.fn(),
|
||||
onWriteParsed: vi.fn(() => ({ dispose: vi.fn() })),
|
||||
write: vi.fn(() => {
|
||||
events.push('write')
|
||||
}),
|
||||
@@ -606,6 +607,22 @@ describe('openTerminal — addon and provider wiring', () => {
|
||||
expect(pane.arabicShapingJoinerCleanup).toBeNull()
|
||||
})
|
||||
|
||||
// Why: a link streamed under a stationary pointer must re-linkify on the next
|
||||
// move; openTerminal wires the hover-cache reset and disposePane must detach it.
|
||||
it('installs the streamed-output linkifier hover reset and disposes it', () => {
|
||||
const { pane } = createOpenTerminalHarness()
|
||||
|
||||
openTerminal(pane)
|
||||
const disposable = pane.linkifierHoverResetDisposable
|
||||
expect(disposable?.dispose).toBeTypeOf('function')
|
||||
expect(pane.terminal.onWriteParsed).toHaveBeenCalledTimes(1)
|
||||
|
||||
const disposeSpy = vi.spyOn(disposable!, 'dispose')
|
||||
disposePane(pane, new Map([[pane.id, pane]]))
|
||||
expect(disposeSpy).toHaveBeenCalledTimes(1)
|
||||
expect(pane.linkifierHoverResetDisposable).toBeNull()
|
||||
})
|
||||
|
||||
// Why: the DOM renderer misrenders joined spans (per-character
|
||||
// letter-spacing blowout), so the joiner must only join while this pane's
|
||||
// WebGL addon is live — locked here against the real openTerminal wiring.
|
||||
|
||||
@@ -9,6 +9,7 @@ import { cancelDeferredScrollRestore } from './pane-scroll'
|
||||
import { activateOrcaTerminalUnicodeProvider } from '../../../../shared/terminal-unicode-provider'
|
||||
import { attachTerminalMouseWheelMultiplier } from './pane-terminal-mouse-wheel'
|
||||
import { attachTerminalScrollIntentTracking } from './terminal-scroll-intent-dom-tracking'
|
||||
import { installTerminalLinkifierHoverResetOnWrite } from './terminal-linkifier-hover-reset-on-write'
|
||||
import { attachDomRendererFocusClassSync } from './pane-dom-focus-class-sync'
|
||||
import { attachWebgl, cancelPendingWebglRefresh, disposeWebgl } from './pane-webgl-renderer'
|
||||
import { configureLazyArabicShapingJoiner } from './terminal-arabic-shaping-joiner'
|
||||
@@ -56,6 +57,11 @@ export function openTerminal(pane: ManagedPaneInternal): void {
|
||||
xtermContainer,
|
||||
pane.leafId
|
||||
)
|
||||
// Why: a link streamed into a visible pane under a stationary pointer would
|
||||
// otherwise stay un-underlined/un-clickable until the mouse crosses to a new
|
||||
// line; invalidate the linkifier hover cache when output lands so the next
|
||||
// pointer move re-linkifies it.
|
||||
pane.linkifierHoverResetDisposable = installTerminalLinkifierHoverResetOnWrite(terminal)
|
||||
|
||||
// Activate Orca's Unicode 11 width shim *before* any caller-driven write. CJK / emoji /
|
||||
// ZWJ codepoints get baked into the buffer at the active unicode version on
|
||||
@@ -228,6 +234,8 @@ export function disposePane(
|
||||
pane.focusClassSyncCleanup = null
|
||||
pane.terminalScrollIntentDisposable?.dispose()
|
||||
pane.terminalScrollIntentDisposable = null
|
||||
pane.linkifierHoverResetDisposable?.dispose()
|
||||
pane.linkifierHoverResetDisposable = null
|
||||
// Deregister the RTL shaping joiner: terminal.dispose() below does not.
|
||||
try {
|
||||
pane.arabicShapingJoinerCleanup?.()
|
||||
|
||||
@@ -163,6 +163,9 @@ export type ManagedPaneInternal = {
|
||||
focusClassSyncCleanup?: (() => void) | null
|
||||
// Stored so disposePane() can remove user-scroll intent listeners.
|
||||
terminalScrollIntentDisposable?: IDisposable | null
|
||||
// Stored so disposePane() can detach the streamed-output hover-cache reset
|
||||
// that keeps freshly printed links linkifiable without a scroll.
|
||||
linkifierHoverResetDisposable?: IDisposable | null
|
||||
// Stored so disposePane() can deregister the joiner; terminal.dispose()
|
||||
// does not remove registered character joiners.
|
||||
arabicShapingJoinerCleanup?: (() => void) | null
|
||||
|
||||
@@ -0,0 +1,159 @@
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import type { Terminal } from '@xterm/xterm'
|
||||
import * as hoverReset from './terminal-linkifier-hover-reset'
|
||||
import { installTerminalLinkifierHoverResetOnWrite } from './terminal-linkifier-hover-reset-on-write'
|
||||
|
||||
// Spy on the reset primitive while keeping its real field-clearing behavior, so
|
||||
// tests can COUNT invocations — the throttle/coalesce properties are otherwise
|
||||
// invisible (the reset writes idempotent cache state).
|
||||
vi.mock('./terminal-linkifier-hover-reset', async (importOriginal) => {
|
||||
const actual = await importOriginal<typeof hoverReset>()
|
||||
return {
|
||||
...actual,
|
||||
resetTerminalLinkifierHoverState: vi.fn(actual.resetTerminalLinkifierHoverState)
|
||||
}
|
||||
})
|
||||
|
||||
type LinkifierCache = { _lastBufferCell?: unknown; _activeLine?: number; _currentLink?: unknown }
|
||||
|
||||
function createFakeTerminal(): {
|
||||
terminal: Terminal
|
||||
emitWriteParsed: () => void
|
||||
linkifier: LinkifierCache
|
||||
listenerDisposed: () => boolean
|
||||
} {
|
||||
const listeners = new Set<() => void>()
|
||||
let disposed = false
|
||||
const linkifier: LinkifierCache = {
|
||||
_lastBufferCell: { x: 3, y: 4 },
|
||||
_activeLine: 4,
|
||||
_currentLink: undefined
|
||||
}
|
||||
const terminal = {
|
||||
onWriteParsed: (handler: () => void) => {
|
||||
listeners.add(handler)
|
||||
return {
|
||||
dispose: () => {
|
||||
disposed = true
|
||||
listeners.delete(handler)
|
||||
}
|
||||
}
|
||||
},
|
||||
_core: { linkifier }
|
||||
} as unknown as Terminal
|
||||
return {
|
||||
terminal,
|
||||
emitWriteParsed: () => listeners.forEach((handler) => handler()),
|
||||
linkifier,
|
||||
listenerDisposed: () => disposed
|
||||
}
|
||||
}
|
||||
|
||||
const resetSpy = vi.mocked(hoverReset.resetTerminalLinkifierHoverState)
|
||||
|
||||
describe('installTerminalLinkifierHoverResetOnWrite', () => {
|
||||
beforeEach(() => {
|
||||
vi.useFakeTimers()
|
||||
resetSpy.mockClear()
|
||||
})
|
||||
afterEach(() => vi.useRealTimers())
|
||||
|
||||
it('clears the linkifier hover cache a throttle window after output lands', () => {
|
||||
const fake = createFakeTerminal()
|
||||
installTerminalLinkifierHoverResetOnWrite(fake.terminal)
|
||||
|
||||
fake.emitWriteParsed()
|
||||
// Not reset synchronously — throttled so streaming does not re-query per chunk.
|
||||
expect(resetSpy).not.toHaveBeenCalled()
|
||||
expect(fake.linkifier._lastBufferCell).toBeDefined()
|
||||
|
||||
vi.advanceTimersByTime(150)
|
||||
expect(resetSpy).toHaveBeenCalledTimes(1)
|
||||
expect(fake.linkifier._lastBufferCell).toBeUndefined()
|
||||
expect(fake.linkifier._activeLine).toBe(-1)
|
||||
})
|
||||
|
||||
it('coalesces a burst of writes into exactly one reset per window', () => {
|
||||
const fake = createFakeTerminal()
|
||||
installTerminalLinkifierHoverResetOnWrite(fake.terminal)
|
||||
|
||||
// 20 chunks inside one window must schedule only one reset — not 20. This
|
||||
// fails if the leading-edge throttle guard is removed.
|
||||
for (let i = 0; i < 20; i += 1) {
|
||||
fake.emitWriteParsed()
|
||||
vi.advanceTimersByTime(5)
|
||||
}
|
||||
vi.advanceTimersByTime(150)
|
||||
expect(resetSpy).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
|
||||
it('keeps resetting during continuous streaming instead of starving', () => {
|
||||
const fake = createFakeTerminal()
|
||||
installTerminalLinkifierHoverResetOnWrite(fake.terminal)
|
||||
|
||||
// A chunk every 50ms for 500ms. The throttle fires ~every 150ms; a debounce
|
||||
// (clearTimeout + reschedule per chunk) would never fire while the stream
|
||||
// continues — that regression is what this guards against.
|
||||
for (let i = 0; i < 10; i += 1) {
|
||||
fake.emitWriteParsed()
|
||||
vi.advanceTimersByTime(50)
|
||||
}
|
||||
expect(resetSpy.mock.calls.length).toBeGreaterThanOrEqual(3)
|
||||
})
|
||||
|
||||
it('does not disturb an actively hovered link, and resumes once it clears', () => {
|
||||
const fake = createFakeTerminal()
|
||||
installTerminalLinkifierHoverResetOnWrite(fake.terminal)
|
||||
|
||||
fake.linkifier._currentLink = { link: 'https://example.com' }
|
||||
fake.emitWriteParsed()
|
||||
vi.advanceTimersByTime(150)
|
||||
// Hovering: the cache is left intact so the underline/tooltip do not flicker.
|
||||
expect(resetSpy).not.toHaveBeenCalled()
|
||||
expect(fake.linkifier._lastBufferCell).toBeDefined()
|
||||
|
||||
fake.linkifier._currentLink = undefined
|
||||
fake.emitWriteParsed()
|
||||
vi.advanceTimersByTime(150)
|
||||
expect(resetSpy).toHaveBeenCalledTimes(1)
|
||||
expect(fake.linkifier._lastBufferCell).toBeUndefined()
|
||||
})
|
||||
|
||||
it('does not drop the reset when the stream goes quiet mid-hover', () => {
|
||||
const fake = createFakeTerminal()
|
||||
installTerminalLinkifierHoverResetOnWrite(fake.terminal)
|
||||
|
||||
// Last chunk of a burst lands while a link is hovered, then output stops.
|
||||
fake.linkifier._currentLink = { link: 'https://example.com' }
|
||||
fake.emitWriteParsed()
|
||||
// Many windows pass with NO further writes — the pending reset must survive.
|
||||
vi.advanceTimersByTime(600)
|
||||
expect(resetSpy).not.toHaveBeenCalled()
|
||||
|
||||
// Once the pointer leaves the link, the retry finally resets — without any
|
||||
// new output re-arming it.
|
||||
fake.linkifier._currentLink = undefined
|
||||
vi.advanceTimersByTime(150)
|
||||
expect(resetSpy).toHaveBeenCalledTimes(1)
|
||||
expect(fake.linkifier._lastBufferCell).toBeUndefined()
|
||||
})
|
||||
|
||||
it('cancels the pending reset and detaches the listener on dispose', () => {
|
||||
const fake = createFakeTerminal()
|
||||
const disposable = installTerminalLinkifierHoverResetOnWrite(fake.terminal)
|
||||
|
||||
fake.emitWriteParsed()
|
||||
disposable.dispose()
|
||||
expect(fake.listenerDisposed()).toBe(true)
|
||||
|
||||
vi.advanceTimersByTime(500)
|
||||
// Disposed before the timer fired: no reset, cache untouched.
|
||||
expect(resetSpy).not.toHaveBeenCalled()
|
||||
expect(fake.linkifier._lastBufferCell).toEqual({ x: 3, y: 4 })
|
||||
})
|
||||
|
||||
it('degrades to a no-op when the terminal lacks onWriteParsed', () => {
|
||||
const terminal = { _core: { linkifier: {} } } as unknown as Terminal
|
||||
expect(() => installTerminalLinkifierHoverResetOnWrite(terminal).dispose()).not.toThrow()
|
||||
})
|
||||
})
|
||||
@@ -0,0 +1,67 @@
|
||||
import type { IDisposable, Terminal } from '@xterm/xterm'
|
||||
import {
|
||||
isTerminalLinkifierHoverActive,
|
||||
resetTerminalLinkifierHoverState
|
||||
} from './terminal-linkifier-hover-reset'
|
||||
|
||||
// Why: coalesce bursts of streamed output into at most one hover-cache reset
|
||||
// per window so continuous agent output does not force a provider re-query on
|
||||
// every parsed chunk. 150ms keeps a freshly printed link responsive to the
|
||||
// user's next pointer move without measurable churn.
|
||||
const HOVER_RESET_THROTTLE_MS = 150
|
||||
|
||||
/**
|
||||
* Invalidate xterm's linkifier hover cache shortly after streamed output lands.
|
||||
*
|
||||
* Why: xterm re-runs link providers only on mousemove when the hovered buffer
|
||||
* cell changes, and it caches provider replies per line with no content-change
|
||||
* invalidation ({@link resetTerminalLinkifierHoverState} documents the fields).
|
||||
* A URL an agent streams into a visible pane under a stationary pointer is
|
||||
* therefore never underlined — and its native activation stays dead — until the
|
||||
* pointer crosses to a different line, which is the "click the terminal a few
|
||||
* times before the link works" symptom. Clearing the cell/line cache when new
|
||||
* content lands lets the very next pointer move re-linkify the fresh URL.
|
||||
*
|
||||
* Sibling of the visibility-resume reset (see terminal-visibility-resume.ts),
|
||||
* which only covers reveal — not output streaming into an already-visible pane.
|
||||
*/
|
||||
export function installTerminalLinkifierHoverResetOnWrite(terminal: Terminal): IDisposable {
|
||||
// Why: never let this break pane creation if a Terminal stub or a future
|
||||
// xterm build lacks onWriteParsed — links then recover on the next cell
|
||||
// change, as they did before this reset existed.
|
||||
if (typeof terminal.onWriteParsed !== 'function') {
|
||||
return { dispose: () => undefined }
|
||||
}
|
||||
let timer: ReturnType<typeof setTimeout> | null = null
|
||||
const flush = (): void => {
|
||||
// Why: never invalidate the cache while the user is hovering a link — it
|
||||
// would clear+re-query the active link (async for file paths), flickering
|
||||
// its underline/tooltip. Re-arm instead of dropping the pending reset: if
|
||||
// this was the last chunk of a burst and it appended a link to the hovered
|
||||
// line, dropping the reset would leave that link dead until a line change.
|
||||
// The retry performs the reset once the hover ends. (timer stays non-null
|
||||
// during the retry so a concurrent write does not stack a second timer.)
|
||||
if (isTerminalLinkifierHoverActive(terminal)) {
|
||||
timer = setTimeout(flush, HOVER_RESET_THROTTLE_MS)
|
||||
return
|
||||
}
|
||||
timer = null
|
||||
resetTerminalLinkifierHoverState(terminal)
|
||||
}
|
||||
const scheduleReset = (): void => {
|
||||
if (timer !== null) {
|
||||
return
|
||||
}
|
||||
timer = setTimeout(flush, HOVER_RESET_THROTTLE_MS)
|
||||
}
|
||||
const writeParsedDisposable = terminal.onWriteParsed(scheduleReset)
|
||||
return {
|
||||
dispose: () => {
|
||||
if (timer !== null) {
|
||||
clearTimeout(timer)
|
||||
timer = null
|
||||
}
|
||||
writeParsedDisposable.dispose()
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -1,8 +1,11 @@
|
||||
import { describe, expect, it } from 'vitest'
|
||||
import type { Terminal } from '@xterm/xterm'
|
||||
import { resetTerminalLinkifierHoverState } from './terminal-linkifier-hover-reset'
|
||||
import {
|
||||
isTerminalLinkifierHoverActive,
|
||||
resetTerminalLinkifierHoverState
|
||||
} from './terminal-linkifier-hover-reset'
|
||||
|
||||
type FakeLinkifier = { _lastBufferCell?: unknown; _activeLine?: number }
|
||||
type FakeLinkifier = { _lastBufferCell?: unknown; _activeLine?: number; _currentLink?: unknown }
|
||||
|
||||
function createTerminal(linkifier: FakeLinkifier | null | undefined, hasCore = true): Terminal {
|
||||
const core = hasCore ? { linkifier: linkifier ?? undefined } : undefined
|
||||
@@ -31,3 +34,18 @@ describe('resetTerminalLinkifierHoverState', () => {
|
||||
expect('_activeLine' in linkifier).toBe(false)
|
||||
})
|
||||
})
|
||||
|
||||
describe('isTerminalLinkifierHoverActive', () => {
|
||||
it('is true only while a link is currently hovered', () => {
|
||||
expect(isTerminalLinkifierHoverActive(createTerminal({ _currentLink: { link: 'x' } }))).toBe(
|
||||
true
|
||||
)
|
||||
expect(isTerminalLinkifierHoverActive(createTerminal({ _currentLink: undefined }))).toBe(false)
|
||||
expect(isTerminalLinkifierHoverActive(createTerminal({}))).toBe(false)
|
||||
})
|
||||
|
||||
it('degrades to false when linkifier internals are unavailable', () => {
|
||||
expect(isTerminalLinkifierHoverActive(createTerminal(null))).toBe(false)
|
||||
expect(isTerminalLinkifierHoverActive(createTerminal(undefined, false))).toBe(false)
|
||||
})
|
||||
})
|
||||
|
||||
@@ -3,6 +3,9 @@ import type { Terminal } from '@xterm/xterm'
|
||||
type LinkifierHoverCache = {
|
||||
_lastBufferCell?: unknown
|
||||
_activeLine?: number
|
||||
// Set while xterm is showing a hovered link; cleared on mouseleave / when the
|
||||
// pointer moves off the link (Linkifier `_clearCurrentLink`).
|
||||
_currentLink?: unknown
|
||||
}
|
||||
|
||||
type TerminalCoreWithLinkifier = {
|
||||
@@ -44,3 +47,22 @@ export function resetTerminalLinkifierHoverState(terminal: Terminal): void {
|
||||
/* linkifier internals unavailable — link recovers on the next cell change */
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* True while xterm is actively showing a hovered link.
|
||||
*
|
||||
* Why: callers that invalidate the hover cache on a timer (streamed output)
|
||||
* must skip while a link is hovered — clearing the cache makes the next
|
||||
* mousemove clear and (for async providers like file paths) re-query the active
|
||||
* link, flickering its underline/tooltip. Guarded like {@link
|
||||
* resetTerminalLinkifierHoverState} so a renamed field degrades to "not
|
||||
* hovering" rather than throwing.
|
||||
*/
|
||||
export function isTerminalLinkifierHoverActive(terminal: Terminal): boolean {
|
||||
try {
|
||||
const linkifier = (terminal as unknown as TerminalCoreWithLinkifier)._core?.linkifier
|
||||
return Boolean(linkifier && '_currentLink' in linkifier && linkifier._currentLink)
|
||||
} catch {
|
||||
return false
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user