diff --git a/src/components/terminal/TerminalGutter.test.tsx b/src/components/terminal/TerminalGutter.test.tsx index 8151b9f5..f72110aa 100644 --- a/src/components/terminal/TerminalGutter.test.tsx +++ b/src/components/terminal/TerminalGutter.test.tsx @@ -1,5 +1,6 @@ import { act, render } from "@testing-library/react"; -import type { Terminal } from "@xterm/xterm"; +import { Terminal } from "@xterm/xterm"; +import { Profiler } from "react"; import { afterEach, expect, it, vi } from "vitest"; import TerminalGutter from "./TerminalGutter"; @@ -7,62 +8,304 @@ afterEach(() => { vi.unstubAllGlobals(); }); -it("keeps the line number gutter wide after scrolling back across a digit boundary", () => { - vi.stubGlobal("requestAnimationFrame", (callback: FrameRequestCallback) => { - callback(0); - return 1; +function createHarness(initialBuffer: "normal" | "alternate" = "normal") { + let frameId = 0; + const frames = new Map(); + const requestFrame = vi.fn((callback: FrameRequestCallback) => { + frames.set(++frameId, callback); + return frameId; }); - vi.stubGlobal("cancelAnimationFrame", vi.fn()); + vi.stubGlobal("requestAnimationFrame", requestFrame); + vi.stubGlobal("cancelAnimationFrame", (id: number) => frames.delete(id)); + const listeners = { + render: new Set<() => void>(), + write: new Set<() => void>(), + resize: new Set<() => void>(), + scroll: new Set<(viewportY: number) => void>(), + buffer: new Set<() => void>(), + }; + const subscribe = (set: Set, listener: T) => { + set.add(listener); + return { dispose: () => set.delete(listener) }; + }; const element = document.createElement("div"); element.innerHTML = '
'; const buffer = { baseY: 100, cursorY: 0, viewportY: 98, - type: "normal", - getLine: () => ({ isWrapped: false }), + type: initialBuffer, + getLine: vi.fn(() => ({ isWrapped: false })), }; - let onScroll: (viewportY: number) => void = () => {}; - const disposable = { dispose: vi.fn() }; const terminal = { element, - buffer: { active: buffer }, + buffer: { + active: buffer, + onBufferChange: (listener: () => void) => subscribe(listeners.buffer, listener), + }, rows: 1, options: { fontSize: 12, fontFamily: "monospace" }, _core: { _renderService: { dimensions: { css: { cell: { height: 18, width: 10 } } } } }, - onRender: () => disposable, - onWriteParsed: () => disposable, - onResize: () => disposable, - onScroll: (listener: (viewportY: number) => void) => { - onScroll = listener; - return disposable; + onRender: (listener: () => void) => subscribe(listeners.render, listener), + onWriteParsed: (listener: () => void) => subscribe(listeners.write, listener), + onResize: (listener: () => void) => subscribe(listeners.resize, listener), + onScroll: (listener: (viewportY: number) => void) => subscribe(listeners.scroll, listener), + }; + const timestamps = new Map([[98, 1000]]); + const props = { + terminalRef: { current: terminal as unknown as Terminal }, + showLineNumbers: true, + showTimestamps: false, + timestampFormat: "[HH:mm:ss]", + lineTimestamps: timestamps, + getLineOffset: vi.fn(() => 0), + sessionId: "session-1", + }; + return { + props, + buffer, + terminal, + timestamps, + listeners, + frames, + requestFrame, + flush() { + act(() => { + const pending = [...frames.values()]; + frames.clear(); + for (const callback of pending) callback(0); + }); }, - } as unknown as Terminal; + repaint() { + act(() => { + for (const callback of listeners.write) callback(); + for (const callback of listeners.render) callback(); + }); + }, + scroll(viewportY: number) { + act(() => { + buffer.viewportY = viewportY; + for (const callback of listeners.scroll) callback(viewportY); + }); + }, + switchBuffer(type: "normal" | "alternate") { + act(() => { + buffer.type = type; + for (const callback of listeners.buffer) callback(); + }); + }, + refresh() { + act(() => { + window.dispatchEvent( + new CustomEvent("nyaterm:refresh-gutter", { detail: { sessionId: props.sessionId } }), + ); + }); + }, + }; +} - const { container } = render( - 0} - sessionId="session-1" - />, - ); +it("keeps the line number gutter wide after scrolling back across a digit boundary", () => { + const harness = createHarness(); + const { container } = render(); + harness.flush(); const gutter = container.firstElementChild as HTMLElement; expect(gutter.style.width).toBe("32px"); - act(() => { - buffer.viewportY = 99; - onScroll(99); - }); + harness.scroll(99); + harness.flush(); expect(gutter.style.width).toBe("40px"); - act(() => { - buffer.viewportY = 98; - onScroll(98); - }); + harness.scroll(98); + harness.flush(); expect(gutter.style.width).toBe("40px"); }); + +it("coalesces repaint events and skips React commits when visible gutter data is unchanged", () => { + const harness = createHarness(); + const onCommit = vi.fn(); + render( + + + , + ); + harness.flush(); + onCommit.mockClear(); + harness.requestFrame.mockClear(); + + for (let i = 0; i < 20; i += 1) harness.repaint(); + harness.refresh(); + expect(harness.requestFrame).toHaveBeenCalledTimes(1); + harness.flush(); + expect(onCommit).not.toHaveBeenCalled(); + + harness.repaint(); + harness.flush(); + expect(onCommit).not.toHaveBeenCalled(); +}); + +it("updates timestamps, wrapped rows, and dimensions even when the viewport does not move", () => { + const harness = createHarness(); + const { container } = render(); + harness.flush(); + const gutter = container.firstElementChild as HTMLElement; + const originalTimestamp = gutter.firstElementChild?.firstElementChild?.textContent; + + harness.timestamps.set(98, 5000); + harness.refresh(); + harness.flush(); + expect(gutter.firstElementChild?.firstElementChild?.textContent).not.toBe(originalTimestamp); + + harness.buffer.getLine.mockReturnValue({ isWrapped: true }); + harness.repaint(); + harness.flush(); + expect(gutter.textContent).toBe(""); + + harness.buffer.getLine.mockReturnValue({ isWrapped: false }); + harness.terminal._core._renderService.dimensions.css.cell = { height: 20, width: 12 }; + harness.terminal.options.fontFamily = "serif"; + harness.terminal.options.fontSize = 14; + act(() => { + for (const callback of harness.listeners.resize) callback(); + }); + harness.flush(); + expect(gutter.style.fontFamily).toBe("serif"); + expect(gutter.style.fontSize).toBe("14px"); + expect((gutter.firstElementChild as HTMLElement).style.height).toBe("20px"); + expect(gutter.textContent).toContain("99"); +}); + +it("cancels pending work and detaches repaint listeners in alternate screen, then resumes", () => { + const harness = createHarness(); + const { container } = render(); + harness.flush(); + const gutter = container.firstElementChild as HTMLElement; + const width = gutter.style.width; + const originalTimestamp = gutter.firstElementChild?.firstElementChild?.textContent; + + harness.repaint(); + expect(harness.frames.size).toBe(1); + harness.switchBuffer("alternate"); + expect(harness.frames.size).toBe(0); + expect(harness.listeners.render.size).toBe(0); + expect(harness.listeners.write.size).toBe(0); + expect(harness.listeners.scroll.size).toBe(0); + expect(harness.listeners.resize.size).toBe(0); + expect(harness.listeners.buffer.size).toBe(1); + expect(gutter.style.visibility).toBe("hidden"); + expect(gutter.style.width).toBe(width); + harness.buffer.getLine.mockClear(); + harness.props.getLineOffset.mockClear(); + harness.requestFrame.mockClear(); + + harness.timestamps.set(98, 5000); + harness.repaint(); + harness.refresh(); + harness.flush(); + expect(harness.requestFrame).not.toHaveBeenCalled(); + expect(harness.buffer.getLine).not.toHaveBeenCalled(); + expect(harness.props.getLineOffset).not.toHaveBeenCalled(); + + harness.switchBuffer("normal"); + harness.flush(); + expect(harness.listeners.render.size).toBe(1); + expect(harness.listeners.write.size).toBe(1); + expect(harness.listeners.scroll.size).toBe(1); + expect(harness.listeners.resize.size).toBe(1); + expect(gutter.style.visibility).toBe(""); + expect(gutter.style.width).toBe(width); + expect(gutter.firstElementChild?.firstElementChild?.textContent).not.toBe(originalTimestamp); +}); + +it("does not schedule gutter work when mounted in alternate screen", () => { + const harness = createHarness("alternate"); + const { container } = render(); + harness.refresh(); + expect(harness.requestFrame).not.toHaveBeenCalled(); + expect(harness.buffer.getLine).not.toHaveBeenCalled(); + expect((container.firstElementChild as HTMLElement).style.visibility).toBe("hidden"); + + harness.switchBuffer("normal"); + harness.flush(); + expect(container.textContent).toBe("99"); +}); + +it("follows real xterm alternate-screen escape sequences without losing normal-buffer timestamps", async () => { + const harness = createHarness(); + const terminal = new Terminal({ cols: 80, rows: 2 }); + // Supply only the measurement surface; xterm's parser and buffers remain real. + Object.defineProperty(terminal, "element", { value: harness.terminal.element }); + const write = (data: string) => new Promise((resolve) => terminal.write(data, resolve)); + const timestamps = new Map([[0, 1000]]); + const { container, unmount } = render( + , + ); + + try { + await act(async () => { + await write("shell prompt"); + }); + harness.flush(); + const gutter = container.firstElementChild as HTMLElement; + const normalContent = gutter.textContent; + const width = gutter.style.width; + + await act(async () => { + await write("\x1b[?1049h\x1b[2J\x1b[HTUI"); + }); + expect(terminal.buffer.active.type).toBe("alternate"); + expect(harness.frames.size).toBe(0); + expect(gutter.style.visibility).toBe("hidden"); + expect(gutter.style.width).toBe(width); + harness.requestFrame.mockClear(); + await act(async () => { + await write("\x1b[Hredraw\x1b[2;1Hcursor movement"); + }); + harness.refresh(); + expect(harness.requestFrame).not.toHaveBeenCalled(); + + await act(async () => { + await write("\x1b[?1049l"); + }); + harness.flush(); + expect(terminal.buffer.active.type).toBe("normal"); + expect(gutter.style.visibility).toBe(""); + expect(gutter.style.width).toBe(width); + expect(gutter.textContent).toBe(normalContent); + expect([...timestamps]).toEqual([[0, 1000]]); + } finally { + unmount(); + terminal.dispose(); + } +}); + +it("resumes with the current buffer after suspension and disposes all listeners on unmount", () => { + const harness = createHarness(); + const { container, rerender, unmount } = render(); + harness.flush(); + rerender(); + expect(container.firstElementChild).toBeNull(); + expect(harness.frames.size).toBe(0); + for (const listeners of Object.values(harness.listeners)) expect(listeners.size).toBe(0); + + harness.switchBuffer("alternate"); + rerender(); + expect(harness.frames.size).toBe(0); + expect((container.firstElementChild as HTMLElement).style.visibility).toBe("hidden"); + harness.switchBuffer("normal"); + harness.flush(); + expect(container.textContent).toBe("99"); + + harness.repaint(); + unmount(); + expect(harness.frames.size).toBe(0); + for (const listeners of Object.values(harness.listeners)) expect(listeners.size).toBe(0); + harness.requestFrame.mockClear(); + harness.refresh(); + expect(harness.requestFrame).not.toHaveBeenCalled(); +}); diff --git a/src/components/terminal/TerminalGutter.tsx b/src/components/terminal/TerminalGutter.tsx index 093b53e4..2c82ccaf 100644 --- a/src/components/terminal/TerminalGutter.tsx +++ b/src/components/terminal/TerminalGutter.tsx @@ -30,17 +30,30 @@ interface GutterLayout { cellWidth: number; } +function layoutsEqual(previous: GutterLayout, next: GutterLayout): boolean { + return ( + previous.maxLineNumberDigits === next.maxLineNumberDigits && + previous.sessionId === next.sessionId && + previous.rowHeight === next.rowHeight && + previous.topPadding === next.topPadding && + previous.fontFamily === next.fontFamily && + previous.fontSize === next.fontSize && + previous.cellWidth === next.cellWidth && + previous.lines.length === next.lines.length && + previous.lines.every((line, index) => { + const nextLine = next.lines[index]; + return ( + line.key === nextLine.key && + line.lineNumber === nextLine.lineNumber && + line.timestamp === nextLine.timestamp + ); + }) + ); +} + const DEFAULT_TIMESTAMP_FORMAT = "[HH:mm:ss]"; const MAX_TIMESTAMP_FORMAT_LENGTH = 64; -const TIMESTAMP_WIDTH_SAMPLE_MS = new Date( - 2099, - 11, - 28, - 23, - 59, - 59, - 999, -).getTime(); +const TIMESTAMP_WIDTH_SAMPLE_MS = new Date(2099, 11, 28, 23, 59, 59, 999).getTime(); const GUTTER_COLUMN_GAP = 12; const GUTTER_RIGHT_PADDING = 8; const MAX_TIMESTAMP_LOOKBACK_ROWS = 512; @@ -133,8 +146,11 @@ export default function TerminalGutter({ sessionId, suspended = false, }: TerminalGutterProps) { - const rafRef = useRef(0); + const rafRef = useRef(null); const viewportYRef = useRef(0); + const [alternateScreen, setAlternateScreen] = useState( + terminalRef.current?.buffer.active.type === "alternate", + ); const [layout, setLayout] = useState({ lines: [], @@ -146,6 +162,7 @@ export default function TerminalGutter({ fontSize: 12, cellWidth: 8, }); + const layoutRef = useRef(layout); const computeLines = useCallback(() => { if (suspended) return; @@ -154,11 +171,11 @@ export default function TerminalGutter({ const el = terminal.element; const buf = terminal.buffer.active; + if (buf.type === "alternate") return; const viewport = el.querySelector(".xterm-viewport") as HTMLElement | null; const screen = el.querySelector(".xterm-screen") as HTMLElement | null; - const screenHeight = - viewport?.clientHeight ?? screen?.clientHeight ?? el.clientHeight; + const screenHeight = viewport?.clientHeight ?? screen?.clientHeight ?? el.clientHeight; const topPadding = screen?.offsetTop ?? 0; const core = (terminal as Terminal & XTermCoreWithRenderDimensions)._core; @@ -171,25 +188,17 @@ export default function TerminalGutter({ : 18; const fontSize = Number(terminal.options.fontSize ?? 12); const cellWidth = - measuredCell?.width && measuredCell.width > 0 - ? measuredCell.width - : fontSize * 0.62; - const viewportY = Math.max( - 0, - Math.min(buf.baseY, viewportYRef.current || buf.viewportY), - ); + measuredCell?.width && measuredCell.width > 0 ? measuredCell.width : fontSize * 0.62; + const viewportY = Math.max(0, Math.min(buf.baseY, viewportYRef.current || buf.viewportY)); const rows = terminal.rows; const cursorAbsoluteY = buf.baseY + buf.cursorY; - const lineOffset = buf.type === "alternate" ? 0 : getLineOffset(); + const lineOffset = getLineOffset(); const resolveTimestamp = (bufferLine: number): number | undefined => { let y = bufferLine; while (y >= 0 && bufferLine - y < MAX_TIMESTAMP_LOOKBACK_ROWS) { - const ts = - buf.type === "alternate" - ? undefined - : lineTimestamps.get(lineOffset + y); + const ts = lineTimestamps.get(lineOffset + y); if (ts) return ts; const line = buf.getLine(y); @@ -216,10 +225,7 @@ export default function TerminalGutter({ showTimestamps && hasRenderedRow && !isWrapped && ts ? formatTimestamp(ts, timestampFormat) : "", - lineNumber: - showLineNumbers && hasRenderedRow && !isWrapped - ? String(logicalLine + 1) - : "", + lineNumber: showLineNumbers && hasRenderedRow && !isWrapped ? String(logicalLine + 1) : "", }); } @@ -228,7 +234,8 @@ export default function TerminalGutter({ 1, ); - setLayout((previous) => ({ + const previous = layoutRef.current; + const nextLayout: GutterLayout = { lines: nextLines, maxLineNumberDigits: previous.sessionId === sessionId @@ -240,7 +247,11 @@ export default function TerminalGutter({ fontFamily: String(terminal.options.fontFamily ?? "inherit"), fontSize, cellWidth, - })); + }; + if (!layoutsEqual(previous, nextLayout)) { + layoutRef.current = nextLayout; + setLayout(nextLayout); + } }, [ suspended, terminalRef, @@ -253,52 +264,77 @@ export default function TerminalGutter({ ]); const scheduleUpdate = useCallback(() => { - cancelAnimationFrame(rafRef.current); + if ( + suspended || + terminalRef.current?.buffer.active.type === "alternate" || + rafRef.current !== null + ) { + return; + } rafRef.current = requestAnimationFrame(() => { + rafRef.current = null; computeLines(); }); - }, [computeLines]); + }, [computeLines, suspended, terminalRef]); useEffect(() => { if (suspended) { - cancelAnimationFrame(rafRef.current); return; } let disposed = false; let handleExternalRefresh: ((event: Event) => void) | null = null; let disposables: Array<{ dispose: () => void }> = []; + let bufferDisposable: { dispose: () => void } | undefined; + const cancelUpdate = () => { + if (rafRef.current !== null) { + cancelAnimationFrame(rafRef.current); + rafRef.current = null; + } + }; + const disposeRefreshListeners = () => { + disposables.forEach((d) => { + d.dispose(); + }); + disposables = []; + }; const attach = () => { if (disposed) return; const terminal = terminalRef.current; if (!terminal) { - rafRef.current = requestAnimationFrame(attach); + rafRef.current = requestAnimationFrame(() => { + rafRef.current = null; + attach(); + }); return; } - viewportYRef.current = terminal.buffer.active.viewportY; - scheduleUpdate(); + const syncBuffer = () => { + const isAlternate = terminal.buffer.active.type === "alternate"; + setAlternateScreen(isAlternate); + disposeRefreshListeners(); + cancelUpdate(); + if (isAlternate) return; - disposables = [ - terminal.onRender(() => { + const refresh = () => { viewportYRef.current = terminal.buffer.active.viewportY; scheduleUpdate(); - }), - terminal.onWriteParsed(() => { - viewportYRef.current = terminal.buffer.active.viewportY; - scheduleUpdate(); - }), - terminal.onScroll((viewportY) => { - viewportYRef.current = viewportY; - scheduleUpdate(); - }), - terminal.onResize(() => { - viewportYRef.current = terminal.buffer.active.viewportY; - scheduleUpdate(); - }), - ]; + }; + disposables = [ + terminal.onRender(refresh), + terminal.onWriteParsed(refresh), + terminal.onScroll((viewportY) => { + viewportYRef.current = viewportY; + scheduleUpdate(); + }), + terminal.onResize(refresh), + ]; + refresh(); + }; + bufferDisposable = terminal.buffer.onBufferChange(syncBuffer); + syncBuffer(); handleExternalRefresh = (event: Event) => { const customEvent = event as CustomEvent<{ sessionId?: string }>; @@ -318,24 +354,15 @@ export default function TerminalGutter({ return () => { disposed = true; - cancelAnimationFrame(rafRef.current); - disposables.forEach((d) => { - d.dispose(); - }); + cancelUpdate(); + disposeRefreshListeners(); + bufferDisposable?.dispose(); if (handleExternalRefresh) { - window.removeEventListener( - "nyaterm:refresh-gutter", - handleExternalRefresh, - ); + window.removeEventListener("nyaterm:refresh-gutter", handleExternalRefresh); } }; }, [suspended, terminalRef, scheduleUpdate, sessionId]); - useEffect(() => { - if (suspended) return; - scheduleUpdate(); - }, [scheduleUpdate, suspended]); - if (suspended || (!showLineNumbers && !showTimestamps)) { return null; } @@ -343,13 +370,8 @@ export default function TerminalGutter({ const lineNumWidth = showLineNumbers ? Math.max(Math.ceil(layout.cellWidth * layout.maxLineNumberDigits) + 2, 24) : 0; - const timestampTemplate = formatTimestamp( - TIMESTAMP_WIDTH_SAMPLE_MS, - timestampFormat, - ); - const tsWidth = showTimestamps - ? Math.ceil(layout.cellWidth * timestampTemplate.length) + 2 - : 0; + const timestampTemplate = formatTimestamp(TIMESTAMP_WIDTH_SAMPLE_MS, timestampFormat); + const tsWidth = showTimestamps ? Math.ceil(layout.cellWidth * timestampTemplate.length) + 2 : 0; const columnGap = showLineNumbers && showTimestamps ? GUTTER_COLUMN_GAP : 0; const gutterWidth = tsWidth + lineNumWidth + columnGap + GUTTER_RIGHT_PADDING; @@ -359,6 +381,8 @@ export default function TerminalGutter({ style={{ boxSizing: "content-box", width: gutterWidth, + // Keep the terminal width stable while full-screen TUIs own the buffer. + visibility: alternateScreen ? "hidden" : undefined, paddingTop: layout.topPadding, borderColor: "color-mix(in srgb, var(--df-terminal-fg, var(--df-text)) 18%, var(--df-terminal-surface-bg))", diff --git a/src/components/terminal/XTerminal.tsx b/src/components/terminal/XTerminal.tsx index 40f77dd7..5b8b4bda 100644 --- a/src/components/terminal/XTerminal.tsx +++ b/src/components/terminal/XTerminal.tsx @@ -1790,6 +1790,7 @@ export default function XTerminal({ } const terminalSettings = terminalAppSettingsRef.current?.terminal; if ( + terminal.buffer.active.type === "normal" && performanceModeRef.current === "normal" && (terminalSettings?.show_line_numbers || terminalSettings?.show_timestamps)