refactor(TerminalGutter): enhance performance and layout management with new harness and layout comparison logic (#742)

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<number, FrameRequestCallback>();
+  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 = <T,>(set: Set<T>, listener: T) => {
+    set.add(listener);
+    return { dispose: () => set.delete(listener) };
+  };
   const element = document.createElement("div");
   element.innerHTML = '<div class="xterm-viewport"></div><div class="xterm-screen"></div>';
   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;
-
-  const { container } = render(
-    <TerminalGutter
-      terminalRef={{ current: terminal }}
-      showLineNumbers
-      showTimestamps={false}
-      timestampFormat="[HH:mm:ss]"
-      lineTimestamps={new Map()}
-      getLineOffset={() => 0}
-      sessionId="session-1"
-    />,
+    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 } }),
         );
+      });
+    },
+  };
+}
+
+it("keeps the line number gutter wide after scrolling back across a digit boundary", () => {
+  const harness = createHarness();
+  const { container } = render(<TerminalGutter {...harness.props} />);
+  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(
+    <Profiler id="gutter" onRender={onCommit}>
+      <TerminalGutter {...harness.props} showTimestamps />
+    </Profiler>,
+  );
+  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(<TerminalGutter {...harness.props} showTimestamps />);
+  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(<TerminalGutter {...harness.props} showTimestamps />);
+  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(<TerminalGutter {...harness.props} />);
+  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<void>((resolve) => terminal.write(data, resolve));
+  const timestamps = new Map([[0, 1000]]);
+  const { container, unmount } = render(
+    <TerminalGutter
+      {...harness.props}
+      terminalRef={{ current: terminal }}
+      lineTimestamps={timestamps}
+      showTimestamps
+    />,
+  );
+
+  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(<TerminalGutter {...harness.props} />);
+  harness.flush();
+  rerender(<TerminalGutter {...harness.props} suspended />);
+  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(<TerminalGutter {...harness.props} />);
+  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();
+});

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<number | null>(null);
   const viewportYRef = useRef(0);
+  const [alternateScreen, setAlternateScreen] = useState(
+    terminalRef.current?.buffer.active.type === "alternate",
+  );
 
   const [layout, setLayout] = useState<GutterLayout>({
     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;
 
+        const refresh = () => {
+          viewportYRef.current = terminal.buffer.active.viewportY;
+          scheduleUpdate();
+        };
         disposables = [
-        terminal.onRender(() => {
-          viewportYRef.current = terminal.buffer.active.viewportY;
-          scheduleUpdate();
-        }),
-        terminal.onWriteParsed(() => {
-          viewportYRef.current = terminal.buffer.active.viewportY;
-          scheduleUpdate();
-        }),
+          terminal.onRender(refresh),
+          terminal.onWriteParsed(refresh),
           terminal.onScroll((viewportY) => {
             viewportYRef.current = viewportY;
             scheduleUpdate();
           }),
-        terminal.onResize(() => {
-          viewportYRef.current = terminal.buffer.active.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))",

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)