diff --git a/.claude/CLAUDE.md b/.claude/CLAUDE.md index 98d6b2d..b687577 100644 --- a/.claude/CLAUDE.md +++ b/.claude/CLAUDE.md @@ -72,8 +72,8 @@ Only update the `Status` field — do not modify any other frontmatter or prompt ## Current State -Branch: `fix/sticky-zone-viewport-overflow` -In-progress: Nothing. PR #145 (fix #134, sticky zone viewport overflow) open, auto-merge enabled. +Branch: `fix/terminal-rendering-redesign` +In-progress: All stages complete, PR #147 open. Resize corruption fix (narrow) and widen zone-anchoring fix both applied and pushed. Awaiting SC manual verification and merge. @@ -176,7 +176,7 @@ Opt-in via `shellicarMcp: true` config. Registers an in-process MCP server (`she 6. **Context thresholds hardcoded** — 85%/90% tool disable thresholds are not configurable. -7. **Cursor positioning is fragile** — `stickyLineCount` is a single point of truth and failure. Occasional off-by-1 documented but not reliably reproducible. +7. **Cursor positioning via Viewport**: `Viewport.scrollOffset` tracks visible window into layout rows. Off-by-1 errors at screen boundaries are possible but not yet observed post-Stage 5 rewrite. 8. **Null unsets in config merge are subtle** — `"model": null` in local config means "use home config's model", not "set to null". Easy to confuse. @@ -188,6 +188,7 @@ Opt-in via `shellicarMcp: true` config. Registers an in-process MCP server (`she - **Structured command execution via in-process MCP** (#99) — replaced freeform Bash with a structured Exec tool served by an in-process MCP server. Glob-based auto-approve (`execAutoApprove`) with custom zero-dep glob matcher (no minimatch dependency). - **Exec tool extracted to `@shellicar/mcp-exec`** — schema, executor, pipeline, validation rules, and ANSI stripping moved to a published package. CLI retains only `autoApprove.ts` (CLI-specific config concern). +- **ZWJ sanitisation in layout pipeline**: `sanitiseZwj` strips U+200D before `wrapLine` measures width. Terminals render ZWJ sequences as individual emojis; `string-width` assumes composed form. Stripping at the layout boundary removes the mismatch. diff --git a/.claude/sessions/2026-03-29.md b/.claude/sessions/2026-03-29.md index a48a3a8..3190be6 100644 --- a/.claude/sessions/2026-03-29.md +++ b/.claude/sessions/2026-03-29.md @@ -15,3 +15,83 @@ - Decisions: Phase 1 (implementation, type-check, build, test, supervisor verification) was completed before this session. This session handled commit, push, and PR only. - Next: Await PR #145 auto-merge. - Violations: None + +### 03:44 - fix/terminal-rendering-redesign (#146) Stage 1 + +- Did: Created branch `fix/terminal-rendering-redesign`. Built `Screen` interface and `StdoutScreen` in `src/Screen.ts`. Built `MockScreen` test double in `test/MockScreen.ts` with character-by-character ANSI parser (cursor movement, clearLine, clearDown, scrollback violation counting). Wrote 20 tests in `test/mock-screen.spec.ts`. All pass. +- Files: `src/Screen.ts`, `test/MockScreen.ts`, `test/mock-screen.spec.ts` +- Decisions: `MockScreen` placed in `test/` per spec (test double, not production code). Biome `useConsistentMemberAccessibility` required explicit `public` on all members (unsafe fix, done manually). Wrap at last column on last row treated as scrollback violation (matches real terminal scroll behavior). +- Next: Stage 2 (Layout pure function). +- Violations: None + +### 03:51 - fix/terminal-rendering-redesign (#146) Stage 2 + +- Did: Implemented `layout()` pure function in `src/Layout.ts` with `BuiltComponent`, `LayoutInput`, `LayoutResult` interfaces and a `wrapLine()` helper. Wrote 11 tests in `test/layout.spec.ts` covering all cases from the spec. All 140 tests pass. +- Files: `src/Layout.ts`, `test/layout.spec.ts` +- Decisions: `wrapLine` uses `stringWidth` char-by-char for correct visual width splitting. ANSI-escaped BuiltComponent rows that don't exceed `columns` pass through as-is (typical for status lines); ANSI-aware splitting for wider strings deferred to Stage 5 if needed. +- Next: Stage 3 (Viewport stateful class). +- Violations: None + +### 03:54 - fix/terminal-rendering-redesign (#146) Stage 3 + +- Did: Implemented `Viewport` class in `src/Viewport.ts` with `scrollOffset` state, cursor chasing (up/down), and resize capping. Wrote 11 tests in `test/viewport.spec.ts` covering all spec cases. All 151 tests pass. Committed and pushed. +- Files: `src/Viewport.ts`, `test/viewport.spec.ts` +- Decisions: Biome `useConsistentMemberAccessibility` required explicit `public` on `resolve()` (unsafe fix, applied manually, consistent with Stage 1 pattern). +- Next: Stage 4 (Renderer class). +- Violations: None + +### 05:29 - fix/terminal-rendering-redesign (#146) Stage 5 + +- Did: Wired Layout/Viewport/Renderer pipeline into Terminal class. Replaced `buildSticky()`/`clearStickyZone()` with `renderZone()` pipeline. Added `buildLayoutInput()` that maps existing builders to `BuiltComponent` format. Fixed two bugs found during manual testing: (1) render() wrote padded viewport rows causing scrollback violations on first render (fix: trim trailing empty rows before writing); (2) writeHistory() re-rendered stale `lastFrame` with cleared-question still visible (fix: split into `writeHistoryLine()` + fresh `renderZone()` for current component state). Added 8 integration tests in `test/terminal-integration.spec.ts`. All 167 tests pass. Type-check and build clean. +- Files: `src/terminal.ts`, `src/TerminalRenderer.ts`, `test/terminal-integration.spec.ts` +- Decisions: `writeHistoryLine()` added to Renderer to write a history line and reset cursor tracking without triggering a zone re-render; Terminal calls it then calls `renderZone()` separately for a fresh frame. Trailing empty row trim applied in both `render()` and `writeHistory()` paths to prevent padding rows from advancing cursor past screen bottom. No commit yet per Stage 5 instructions (do not commit; await supervisor manual verification). +- Next: Manual verification by Supreme Commander, then commit/push/PR with `Closes #146`. +- Violations: None + +### 16:15 - fix/terminal-rendering-redesign (#146) Grapheme fix + +- Did: Fixed `wrapLine` to iterate grapheme clusters via `Intl.Segmenter` instead of code points. Added test `multi-codepoint grapheme clusters wrap as single units` to `test/layout.spec.ts` covering ZWJ emoji sequences. All 169 tests pass. +- Files: `src/Layout.ts`, `test/layout.spec.ts` +- Decisions: `for (const char of line)` iterates Unicode code points; multi-codepoint grapheme clusters (e.g. 👨‍👩‍👦 = U+1F468 ZWJ U+1F469 ZWJ U+1F466) would be split mid-cluster, causing incorrect wrap counts. Fix mirrors the pattern already used in `src/editor.ts`. +- Next: Await PR auto-merge. +- Violations: None + +### 15:46 - fix/terminal-rendering-redesign (#146) Commit/Push/PR + +- Did: Committed Stage 5 changes after supervisor manual verification. Pushed branch. Created PR with `Closes #146`. +- Files: `src/terminal.ts`, `src/TerminalRenderer.ts`, `test/terminal-integration.spec.ts`, `.claude/CLAUDE.md`, `.claude/sessions/2026-03-29.md` +- Decisions: Session log and CLAUDE.md state updates included in the same commit as the code changes per session protocol. +- Next: Await PR auto-merge. +- Violations: None + +### 16:50 - fix/terminal-rendering-redesign: ZWJ sanitisation in layout pipeline + +- Did: Added `sanitiseZwj` to `src/sanitise.ts` (strips U+200D before width measurement). Wired it into `wrapLine` in `src/Layout.ts` so terminal rendering and `string-width` agree on emoji width. Updated tests to reflect decomposed behavior. +- Files: `src/sanitise.ts`, `src/Layout.ts`, `test/sanitise.spec.ts`, `test/layout.spec.ts` +- Decisions: ZWJ sequences render as individual emojis in terminals that do not support composed rendering; `string-width` reports them as composed (width 2). Stripping ZWJ at the layout boundary removes the ambiguity. The old "wrap as single unit" test was testing the wrong assumption and was updated. Manual verification by SC still pending. +- Next: SC to verify manually in terminal. Then fix cursor position bug. +- Violations: None + +### 18:30 - fix/terminal-rendering-redesign: resize corruption fix + +- Did: Diagnosed resize corruption: on terminal narrow, old zone content reflows to more rows at the new column width and can appear anywhere in the visible area. `lastVisibleCursorRow` is stale and `cursorUp` moves to the wrong row. Added TDD test to expose the bug (RED: scrollback violation). Implemented fix: `Renderer.notifyResize()` sets `pendingResize` flag; next `render()` clears the entire visible screen (`cursorAt(1,1)` + `clearDown`), then anchors the new zone at `screen.rows - zoneHeight`. Added `MockScreen` support for `ESC[row;colH` absolute positioning (required by fix). Wired `Terminal.notifyResize()` and `ClaudeCli` resize debounce. 184 tests pass. SC manual verification: fix partially resolved the issue; second iteration replaced partial clear (old zone area only) with full screen clear, which also handles the case where reflowed zone content spreads above the calculated old zone top. Build and type-check clean. +- Files: `src/TerminalRenderer.ts`, `src/terminal.ts`, `src/ClaudeCli.ts`, `test/MockScreen.ts`, `test/mock-screen.spec.ts`, `test/TerminalRenderer.spec.ts` +- Decisions: Full screen clear on resize is the correct approach because after terminal reflow the old zone can occupy an unknown number of rows anywhere in the visible area. Partial clear (based on stored zoneHeight) is unreliable. Trade-off: visible history above zone is momentarily blank after resize (still accessible in scrollback). +- Next: SC to verify manually with latest build. +- Violations: None + +### 19:41 - fix/terminal-rendering-redesign: widen resize zone-anchoring fix + +- Did: Diagnosed two widen-resize bugs via TDD. Bug 1: `cursorAt(1,1) + clearDown` in the `pendingResize` path erased all visible history above the zone when widening. Bug 2: `cursorAt(newZoneTop, 1)` always anchored the zone to the screen bottom, causing it to jump down even when the cursor had not moved. Fixed both by differentiating narrow vs widen in `pendingResize`: use `cursorAt(newZoneTop, 1)` only when `newZoneHeight > zoneHeight` (zone grew, cursor displaced by reflow); use `cursorUp(lastVisibleCursorRow)` otherwise (zone shrunk or same, cursor approximately in place). Added two regression tests; all 186 tests pass. +- Files: `src/TerminalRenderer.ts`, `test/TerminalRenderer.spec.ts` +- Decisions: Zone-grew heuristic (`newZoneHeight > zoneHeight`) is the narrowing signal: when the terminal narrows, wrapped content produces more rows; when it widens, fewer rows. The existing narrow-case test (`no scrollback violations when cursor displaced`) continues to pass because `newZoneHeight (4) > zoneHeight (2)` triggers the absolute path correctly. +- Next: SC to verify manually with latest build. Await PR #147 auto-merge. +- Violations: None + +### 04:29 - fix/terminal-rendering-redesign (#146) Stage 4 + +- Did: Implemented `Renderer` class in `src/TerminalRenderer.ts` (filename avoids macOS case-insensitive collision with existing `src/renderer.ts`). Wrote 8 tests in `test/renderer.spec.ts` covering all spec cases. All 159 tests pass. Committed and pushed. +- Files: `src/TerminalRenderer.ts`, `test/renderer.spec.ts` +- Decisions: File named `TerminalRenderer.ts` not `Renderer.ts` because macOS case-insensitive FS treats `Renderer.ts` and `renderer.ts` as the same file (existing `renderer.ts` contains `prepareEditor`). `clearDown` must precede the last row write, not follow it: MockScreen clears the entire current row on `ESC[J`, so issuing clearDown after writing the last row erases it. The fix issues clearDown when cursor is on the last row position before writing content there. +- Next: Stage 5 (Wire + Replace). +- Violations: None diff --git a/src/ClaudeCli.ts b/src/ClaudeCli.ts index 776545c..6a6f0a7 100644 --- a/src/ClaudeCli.ts +++ b/src/ClaudeCli.ts @@ -992,6 +992,7 @@ export class ClaudeCli { clearTimeout(this.resizeTimer); this.resizeTimer = setTimeout(() => { this.term.paused = false; + this.term.notifyResize(); this.redraw(); }, 300); }); diff --git a/src/Layout.ts b/src/Layout.ts new file mode 100644 index 0000000..f286bdf --- /dev/null +++ b/src/Layout.ts @@ -0,0 +1,97 @@ +import stringWidth from 'string-width'; +import type { EditorRender } from './renderer.js'; +import { sanitiseZwj } from './sanitise.js'; + +const segmenter = new Intl.Segmenter(undefined, { granularity: 'grapheme' }); + +/** + * Output from an existing builder (status, attachment, preview). + * `rows` are logical lines as the builder produces them (may be wider than columns). + * `height` is the visual height in terminal rows (accounts for wrapping). + */ +export interface BuiltComponent { + rows: string[]; + height: number; +} + +export interface LayoutInput { + editor: EditorRender; + status: BuiltComponent | null; + attachments: BuiltComponent | null; + preview: BuiltComponent | null; + question: BuiltComponent | null; + columns: number; +} + +/** + * Layout output. `buffer` contains one entry per visual (terminal) row. + * Layout is responsible for wrapping: a logical line wider than `columns` + * becomes multiple buffer entries. + */ +export interface LayoutResult { + buffer: string[]; + cursorRow: number; + cursorCol: number; + editorStartRow: number; +} + +/** + * Splits a logical line into visual rows by wrapping at `columns` visual width. + * Returns at least one entry (empty string for empty input). + */ +function wrapLine(line: string, columns: number): string[] { + const sanitised = sanitiseZwj(line); + if (stringWidth(sanitised) <= columns) { + return [sanitised]; + } + const segments: string[] = []; + let current = ''; + let currentWidth = 0; + for (const { segment } of segmenter.segment(sanitised)) { + const cw = stringWidth(segment); + if (currentWidth + cw > columns) { + segments.push(current); + current = segment; + currentWidth = cw; + } else { + current += segment; + currentWidth += cw; + } + } + if (current.length > 0 || segments.length === 0) { + segments.push(current); + } + return segments; +} + +/** + * Pure layout function. Takes all UI components and returns an unbounded + * buffer of visual rows with cursor position metadata. + * + * Buffer order (top to bottom): question, status, attachments, preview, editor. + */ +export function layout(input: LayoutInput): LayoutResult { + const { editor, status, attachments, preview, question, columns } = input; + const buffer: string[] = []; + + for (const component of [question, status, attachments, preview]) { + if (component !== null) { + for (const row of component.rows) { + buffer.push(...wrapLine(row, columns)); + } + } + } + + const editorStartRow = buffer.length; + + for (const line of editor.lines) { + buffer.push(...wrapLine(line, columns)); + } + + return { + buffer, + cursorRow: editorStartRow + editor.cursorRow, + cursorCol: editor.cursorCol, + editorStartRow, + }; +} diff --git a/src/Screen.ts b/src/Screen.ts new file mode 100644 index 0000000..f658a15 --- /dev/null +++ b/src/Screen.ts @@ -0,0 +1,26 @@ +export interface Screen { + readonly rows: number; + readonly columns: number; + write(data: string): void; + onResize(cb: (columns: number, rows: number) => void): () => void; +} + +export class StdoutScreen implements Screen { + public get rows(): number { + return process.stdout.rows ?? 24; + } + + public get columns(): number { + return process.stdout.columns ?? 80; + } + + public write(data: string): void { + process.stdout.write(data); + } + + public onResize(cb: (columns: number, rows: number) => void): () => void { + const handler = () => cb(process.stdout.columns ?? 80, process.stdout.rows ?? 24); + process.stdout.on('resize', handler); + return () => process.stdout.off('resize', handler); + } +} diff --git a/src/TerminalRenderer.ts b/src/TerminalRenderer.ts new file mode 100644 index 0000000..7caa4d2 --- /dev/null +++ b/src/TerminalRenderer.ts @@ -0,0 +1,132 @@ +import type { Screen } from './Screen.js'; +import type { ViewportResult } from './Viewport.js'; + +const ESC = '\x1B['; +const cursorUp = (n: number) => (n > 0 ? `${ESC}${n}A` : ''); +const cursorTo = (col: number) => `${ESC}${col + 1}G`; +const cursorAt = (row: number, col: number) => `${ESC}${row};${col}H`; // 1-based +const clearLine = `${ESC}2K`; +const clearDown = `${ESC}J`; +const showCursor = `${ESC}?25h`; +const hideCursor = `${ESC}?25l`; +const syncStart = '\x1B[?2026h'; +const syncEnd = '\x1B[?2026l'; + +export class Renderer { + public zoneHeight = 0; + private lastVisibleCursorRow = 0; + private lastFrame: ViewportResult | null = null; + private pendingResize = false; + + public constructor(private readonly screen: Screen) {} + + /** + * Call after a terminal resize. The physical cursor position is unknown after + * terminal content reflows, so lastVisibleCursorRow is stale. The next render() + * call will use absolute positioning to re-anchor the zone at the screen bottom + * instead of the stale relative cursorUp. + */ + public notifyResize(): void { + this.pendingResize = true; + } + + public render(frame: ViewportResult): void { + // Trim trailing empty rows from the Viewport-padded frame. Padding is correct + // for Viewport's contract but would cause the Renderer to write screenRows rows + // from wherever the cursor currently is, scrolling the terminal unnecessarily. + let trimEnd = frame.rows.length; + while (trimEnd > 1 && frame.rows[trimEnd - 1] === '') { + trimEnd--; + } + const renderFrame = + trimEnd === frame.rows.length + ? frame + : { + rows: frame.rows.slice(0, trimEnd), + visibleCursorRow: Math.min(frame.visibleCursorRow, trimEnd - 1), + visibleCursorCol: frame.visibleCursorCol, + }; + let out = syncStart + hideCursor; + if (this.pendingResize) { + this.pendingResize = false; + // After a resize, the terminal reflows content. Two cases: + // - Narrowing (zone grew): the terminal may displace the cursor significantly + // as content wraps onto more rows. Use absolute positioning anchored at the + // screen bottom so the render starts from a known location. + // - Widening (zone shrunk or same): the cursor stays approximately in place + // because content unwraps and the cursor doesn't travel far. Treat like a + // normal render (cursorUp) so the zone stays where it was instead of + // jumping to the screen bottom. + const newZoneHeight = renderFrame.rows.length; + if (newZoneHeight > this.zoneHeight) { + const newZoneTop = this.screen.rows - newZoneHeight + 1; // 1-based + out += cursorAt(newZoneTop, 1); + } else { + out += cursorUp(this.lastVisibleCursorRow); + } + } else { + out += cursorUp(this.lastVisibleCursorRow); + } + out += this.buildZoneOutput(renderFrame); + out += showCursor + syncEnd; + this.zoneHeight = renderFrame.rows.length; + this.lastVisibleCursorRow = renderFrame.visibleCursorRow; + this.lastFrame = frame; + this.screen.write(out); + } + + /** + * Writes a history line above the zone and resets cursor tracking so the + * CALLER can re-render the zone from the current position. Does NOT + * re-render the zone itself. Use this when the caller will supply a fresh + * frame (e.g. Terminal.writeHistory → renderZone). + */ + public writeHistoryLine(line: string): void { + const out = cursorUp(this.lastVisibleCursorRow) + '\r' + clearLine + line + '\n'; + this.lastVisibleCursorRow = 0; + this.screen.write(out); + } + + /** @deprecated Prefer writeHistoryLine + external render for fresh frames. */ + public writeHistory(line: string): void { + let out = cursorUp(this.lastVisibleCursorRow); + out += '\r' + line + '\n'; + this.screen.write(out); + if (this.lastFrame !== null) { + // Trim trailing empty rows so the re-rendered zone fits after the zone shifts + // down by one row. Viewport pads frames to screenRows; those empty rows are + // correct for render() but cause scrollback violations here. + let trimEnd = this.lastFrame.rows.length; + while (trimEnd > 1 && this.lastFrame.rows[trimEnd - 1] === '') { + trimEnd--; + } + const trimmedRows = this.lastFrame.rows.slice(0, trimEnd); + const trimmedFrame = { + rows: trimmedRows, + visibleCursorRow: Math.min(this.lastFrame.visibleCursorRow, trimmedRows.length - 1), + visibleCursorCol: this.lastFrame.visibleCursorCol, + }; + this.screen.write(this.buildZoneOutput(trimmedFrame)); + } + } + + private buildZoneOutput(frame: ViewportResult): string { + let out = ''; + // Write all rows except the last, each followed by \n to advance the cursor. + for (let i = 0; i < frame.rows.length - 1; i++) { + out += '\r' + clearLine + frame.rows[i] + '\n'; + } + // clearDown here clears leftover rows from a taller previous frame. + // It must come before the last row write so the last row's content is not erased. + out += clearDown; + // Write the last row without \n to avoid a scrollback violation on a full screen. + const lastRow = frame.rows[frame.rows.length - 1]; + if (lastRow !== undefined) { + out += '\r' + clearLine + lastRow; + } + const rowsFromBottom = frame.rows.length - 1 - frame.visibleCursorRow; + out += cursorUp(rowsFromBottom); + out += cursorTo(frame.visibleCursorCol); + return out; + } +} diff --git a/src/Viewport.ts b/src/Viewport.ts new file mode 100644 index 0000000..afaef57 --- /dev/null +++ b/src/Viewport.ts @@ -0,0 +1,31 @@ +export interface ViewportResult { + rows: string[]; + visibleCursorRow: number; + visibleCursorCol: number; +} + +export class Viewport { + private scrollOffset = 0; + + public resolve(buffer: string[], screenRows: number, cursorRow: number, cursorCol: number): ViewportResult { + // Cap scrollOffset after a potential resize + this.scrollOffset = Math.min(this.scrollOffset, Math.max(0, buffer.length - screenRows)); + + // Cursor chasing + if (cursorRow < this.scrollOffset) { + this.scrollOffset = cursorRow; + } else if (cursorRow >= this.scrollOffset + screenRows) { + this.scrollOffset = cursorRow - screenRows + 1; + } + + const slice = buffer.slice(this.scrollOffset, this.scrollOffset + screenRows); + const padding = screenRows - slice.length; + const rows = padding > 0 ? [...slice, ...Array(padding).fill('')] : slice; + + return { + rows, + visibleCursorRow: cursorRow - this.scrollOffset, + visibleCursorCol: cursorCol, + }; + } +} diff --git a/src/renderer.ts b/src/renderer.ts index 860dba1..1682de4 100644 --- a/src/renderer.ts +++ b/src/renderer.ts @@ -25,13 +25,14 @@ export function prepareEditor(editor: EditorState, prompt: string): EditorRender const cursorPrefix = editor.cursor.row === 0 ? prompt : CONTINUATION; const textBeforeCursor = editor.lines[editor.cursor.row].slice(0, editor.cursor.col); - const cursorCol = stringWidth(cursorPrefix) + stringWidth(textBeforeCursor); + let cursorCol = stringWidth(cursorPrefix) + stringWidth(textBeforeCursor); let cursorRow = 0; for (let i = 0; i < editor.cursor.row; i++) { cursorRow += Math.max(1, Math.ceil(stringWidth(lines[i]) / columns)); } cursorRow += Math.floor(cursorCol / columns); + cursorCol = cursorCol % columns; return { lines, cursorRow, cursorCol }; } diff --git a/src/sanitise.ts b/src/sanitise.ts index 456fecc..b383604 100644 --- a/src/sanitise.ts +++ b/src/sanitise.ts @@ -3,3 +3,7 @@ const LONE_SURROGATE_RE = /[\uD800-\uDBFF](?![\uDC00-\uDFFF])|(? (n > 0 ? `${ESC}${n}A` : ''); -const cursorDown = (n: number) => (n > 0 ? `${ESC}${n}B` : ''); -const cursorTo = (col: number) => `${ESC}${col + 1}G`; -const clearLine = `${ESC}2K`; -const clearDown = `${ESC}J`; -const showCursor = `${ESC}?25h`; const hideCursorSeq = `${ESC}?25l`; const resetStyle = `${ESC}0m`; const inverseOn = `${ESC}7m`; @@ -25,13 +25,13 @@ const bel = '\x07'; export class Terminal { private editorContent: EditorRender = { lines: [], cursorRow: 0, cursorCol: 0 }; - private stickyLineCount = 0; - private cursorLinesFromBottom = 0; private cursorHidden = false; - private scrollOffset = 0; private _paused = false; private pauseBuffer: string[] = []; private questionLines: string[] = []; + private readonly screen: Screen; + private readonly viewport: Viewport; + private readonly renderer: Renderer; public sessionId: string | undefined; public modelOverride: string | undefined; @@ -40,12 +40,20 @@ export class Terminal { private drowningThreshold: number | null, private readonly attachmentStore: AttachmentStore, private readonly commandMode: CommandMode, - ) {} + ) { + this.screen = new StdoutScreen(); + this.viewport = new Viewport(); + this.renderer = new Renderer(this.screen); + } public updateConfig(drowningThreshold: number | null): void { this.drowningThreshold = drowningThreshold; } + public notifyResize(): void { + this.renderer.notifyResize(); + } + public get paused(): boolean { return this._paused; } @@ -98,19 +106,6 @@ export class Terminal { } } - private clearStickyZone(): string { - if (this.stickyLineCount === 0) { - return ''; - } - let output = ''; - // Move cursor to bottom of sticky zone first, then up to top - output += cursorDown(this.cursorLinesFromBottom); - output += cursorUp(this.stickyLineCount - 1); - output += '\r'; - output += clearDown; - return output; - } - private buildStatusLine(columns: number, allowIdle: boolean): { line: string; screenLines: number } | null { const b = new StatusLineBuilder(); const phase = this.appState.phase; @@ -256,156 +251,58 @@ export class Terminal { return { lines, screenLines }; } - private buildSticky(): string { - const columns = process.stdout.columns || 80; - const terminalRows = process.stdout.rows || 24; - - const attachmentLine = this.buildAttachmentLine(columns, this.commandMode.active); - const statusLine = this.buildStatusLine(columns, !attachmentLine); + private buildLayoutInput(columns: number): LayoutInput { + const attachmentResult = this.buildAttachmentLine(columns, this.commandMode.active); + const statusResult = this.buildStatusLine(columns, !attachmentResult); + const previewResult = this.buildPreviewLines(columns); - // Pre-build each non-editor component into discrete parts (no leading/trailing newlines). + const statusComp: BuiltComponent | null = statusResult ? { rows: [statusResult.line], height: statusResult.screenLines } : null; + let attachComp: BuiltComponent | null = attachmentResult ? { rows: [attachmentResult.line], height: attachmentResult.screenLines } : null; + let previewComp: BuiltComponent | null = previewResult ? { rows: previewResult.lines, height: previewResult.screenLines } : null; - const questionParts: string[] = []; - let questionScreenLines = 0; + let questionHeight = 0; for (const line of this.questionLines) { - questionParts.push(clearLine + line); - questionScreenLines += Math.max(1, Math.ceil(stringWidth(line) / columns)); + questionHeight += Math.max(1, Math.ceil(stringWidth(line) / columns)); } + let questionComp: BuiltComponent | null = this.questionLines.length > 0 ? { rows: this.questionLines, height: questionHeight } : null; - let statusPart = ''; - let statusScreenLines = 0; - if (statusLine) { - statusPart = clearLine + statusLine.line; - statusScreenLines = statusLine.screenLines; - } - - let attachmentPart = ''; - let attachmentScreenLines = 0; - if (attachmentLine) { - attachmentPart = clearLine + attachmentLine.line; - attachmentScreenLines = attachmentLine.screenLines; - } - - const previewParts: string[] = []; - let previewScreenLines = 0; - const preview = this.buildPreviewLines(columns); - if (preview) { - for (const line of preview.lines) { - previewParts.push(clearLine + line); - } - previewScreenLines = preview.screenLines; - } - - // Budget allocation: reserve at least 1 row for the editor. Drop lowest-priority - // components (preview, then attachment, then question) if non-editor content alone - // would consume the entire terminal height. - let nonEditorRows = questionScreenLines + statusScreenLines + attachmentScreenLines + previewScreenLines; + const terminalRows = this.screen.rows; const minEditorRows = 1; + const maxNonEditor = terminalRows - minEditorRows; + let nonEditorRows = (statusComp?.height ?? 0) + (attachComp?.height ?? 0) + (previewComp?.height ?? 0) + (questionComp?.height ?? 0); - if (nonEditorRows > terminalRows - minEditorRows) { - nonEditorRows -= previewScreenLines; - previewParts.length = 0; - previewScreenLines = 0; + if (nonEditorRows > maxNonEditor) { + nonEditorRows -= previewComp?.height ?? 0; + previewComp = null; } - if (nonEditorRows > terminalRows - minEditorRows) { - nonEditorRows -= attachmentScreenLines; - attachmentPart = ''; - attachmentScreenLines = 0; + if (nonEditorRows > maxNonEditor) { + nonEditorRows -= attachComp?.height ?? 0; + attachComp = null; } - if (nonEditorRows > terminalRows - minEditorRows) { - nonEditorRows -= questionScreenLines; - questionParts.length = 0; - questionScreenLines = 0; + if (nonEditorRows > maxNonEditor) { + questionComp = null; } - const availableRows = Math.max(minEditorRows, terminalRows - nonEditorRows); - - // Assemble non-editor output. Preview always uses a leading newline (matches original behaviour). - const topParts = [...questionParts]; - if (statusPart) { - topParts.push(statusPart); - } - if (attachmentPart) { - topParts.push(attachmentPart); - } - - let output = ''; - let hasOutput = false; - for (const part of topParts) { - if (hasOutput) { - output += '\n'; - } - output += part; - hasOutput = true; - } - for (const part of previewParts) { - output += '\n'; - output += part; - } - - // Build a map from logical line index to its starting terminal row within the editor. - const lineStartRow: number[] = []; - let nextStartRow = 0; - for (let i = 0; i < this.editorContent.lines.length; i++) { - lineStartRow.push(nextStartRow); - nextStartRow += Math.max(1, Math.ceil(stringWidth(this.editorContent.lines[i]) / columns)); - } - - // Adjust scrollOffset so the cursor row stays within the visible window. - const cursorRow = this.editorContent.cursorRow; - if (cursorRow < this.scrollOffset) { - this.scrollOffset = cursorRow; - } else if (cursorRow >= this.scrollOffset + availableRows) { - this.scrollOffset = cursorRow - availableRows + 1; - } - - // Cap scrollOffset so content is never scrolled past the end (no empty rows below content). - const maxScrollOffset = Math.max(0, nextStartRow - availableRows); - this.scrollOffset = Math.min(this.scrollOffset, maxScrollOffset); - - // Snap scrollOffset backward to the nearest logical line boundary so we - // never start rendering mid-way through a wrapped logical line. - let snapped = 0; - for (let i = 0; i < lineStartRow.length; i++) { - if (lineStartRow[i] <= this.scrollOffset) { - snapped = lineStartRow[i]; - } else { - break; - } - } - this.scrollOffset = snapped; - - // Render logical lines whose start terminal row falls within the visible window. - let editorScreenLines = 0; - for (let i = 0; i < this.editorContent.lines.length; i++) { - const start = lineStartRow[i]; - const rows = Math.max(1, Math.ceil(stringWidth(this.editorContent.lines[i]) / columns)); - if (start >= this.scrollOffset && start < this.scrollOffset + availableRows) { - output += '\n'; - output += clearLine + this.editorContent.lines[i]; - // Count how many of this line's terminal rows actually fit in the window. - const visibleRows = Math.min(rows, this.scrollOffset + availableRows - start); - editorScreenLines += visibleRows; - } - } - - // Clear any leftover lines from previous render - output += clearDown; + return { + editor: this.editorContent, + status: statusComp, + attachments: attachComp, + preview: previewComp, + question: questionComp, + columns, + }; + } - // Position cursor within the visible editor window. - // cursorRow is the absolute terminal row within the full editor. Subtract - // scrollOffset to get the row within the rendered window. - const visibleCursorRow = cursorRow - this.scrollOffset; - this.cursorLinesFromBottom = editorScreenLines - visibleCursorRow - 1; - if (this.cursorLinesFromBottom > 0) { - output += cursorUp(this.cursorLinesFromBottom); + private renderZone(): void { + const columns = this.screen.columns; + const rows = this.screen.rows; + const input = this.buildLayoutInput(columns); + const result = layout(input); + const frame = this.viewport.resolve(result.buffer, rows, result.cursorRow, result.cursorCol); + this.renderer.render(frame); + if (this.cursorHidden) { + this.screen.write(hideCursorSeq); } - output += cursorTo(this.editorContent.cursorCol % columns); - output += this.cursorHidden ? hideCursorSeq : showCursor; - - this.stickyLineCount = statusScreenLines + attachmentScreenLines + previewScreenLines + questionScreenLines + editorScreenLines; - - return output; } private writeHistory(line: string): void { @@ -413,13 +310,12 @@ export class Terminal { this.pauseBuffer.push(line); return; } - let output = ''; - output += this.clearStickyZone(); - output += line; - output += '\n'; - this.stickyLineCount = 0; - output += this.buildSticky(); - process.stdout.write(output); + // writeHistoryLine moves to zone top, writes the line, and resets cursor + // tracking. renderZone then re-renders with the CURRENT layout state so the + // zone reflects any changes that happened before this write (e.g. question + // cleared by clearQuestionLines before the history write). + this.renderer.writeHistoryLine(line); + this.renderZone(); } public setQuestionLines(lines: string[]): void { @@ -437,11 +333,7 @@ export class Terminal { if (this.paused) { return; } - let output = ''; - output += this.clearStickyZone(); - this.stickyLineCount = 0; - output += this.buildSticky(); - process.stdout.write(output); + this.renderZone(); } public log(message: string, ...args: unknown[]): void { @@ -457,24 +349,20 @@ export class Terminal { if (this.paused) { return; } - let output = ''; - output += this.clearStickyZone(); this.editorContent = prepareEditor(editor, prompt); this.cursorHidden = hideCursor; - this.stickyLineCount = 0; - output += this.buildSticky(); - process.stdout.write(output); + this.renderZone(); } public write(data: string): void { if (this.paused) { return; } - process.stdout.write(data); + this.screen.write(data); } public beep(): void { - process.stdout.write(bel); + this.screen.write(bel); } public error(message: string): void { diff --git a/test/MockScreen.ts b/test/MockScreen.ts new file mode 100644 index 0000000..0b81818 --- /dev/null +++ b/test/MockScreen.ts @@ -0,0 +1,139 @@ +import type { Screen } from '../src/Screen.js'; + +export class MockScreen implements Screen { + public readonly cells: string[][]; + public cursorRow = 0; + public cursorCol = 0; + public scrollbackViolations = 0; + private pendingWrap = false; + + public constructor( + public readonly columns: number, + public readonly rows: number, + ) { + this.cells = Array.from({ length: rows }, () => new Array(columns).fill('')); + } + + public write(data: string): void { + let i = 0; + while (i < data.length) { + const ch = data[i]; + if (ch === '\x1B') { + i++; + if (i < data.length && data[i] === '[') { + i++; + let param = ''; + while (i < data.length && !/[A-Za-z]/.test(data[i] as string)) { + param += data[i]; + i++; + } + if (i < data.length) { + this.handleCsi(param, data[i] as string); + i++; + } + } else { + i++; + } + } else if (ch === '\r') { + this.cursorCol = 0; + this.pendingWrap = false; + i++; + } else if (ch === '\n') { + this.cursorCol = 0; + this.pendingWrap = false; + if (this.cursorRow === this.rows - 1) { + this.scrollbackViolations++; + this.cells.shift(); + this.cells.push(new Array(this.columns).fill('')); + } else { + this.cursorRow++; + } + i++; + } else { + if (this.pendingWrap) { + this.pendingWrap = false; + this.cursorCol = 0; + if (this.cursorRow < this.rows - 1) { + this.cursorRow++; + } else { + this.scrollbackViolations++; + this.cells.shift(); + this.cells.push(new Array(this.columns).fill('')); + } + } + this.cells[this.cursorRow][this.cursorCol] = ch; + this.cursorCol++; + if (this.cursorCol >= this.columns) { + this.cursorCol = this.columns - 1; + this.pendingWrap = true; + } + i++; + } + } + } + + private handleCsi(param: string, final: string): void { + this.pendingWrap = false; + switch (final) { + case 'A': { + const n = parseInt(param || '1', 10); + this.cursorRow = Math.max(0, this.cursorRow - n); + break; + } + case 'G': { + const n = parseInt(param || '1', 10); + this.cursorCol = Math.min(this.columns - 1, Math.max(0, n - 1)); + break; + } + case 'K': { + if (param === '2') { + for (let c = 0; c < this.columns; c++) { + this.cells[this.cursorRow][c] = ''; + } + } + break; + } + case 'H': { + // Cursor position: ESC[row;colH (1-based, defaults to 1;1) + const parts = param.split(';'); + const row = parseInt(parts[0] || '1', 10); + const col = parseInt(parts[1] || '1', 10); + this.cursorRow = Math.max(0, Math.min(this.rows - 1, row - 1)); + this.cursorCol = Math.max(0, Math.min(this.columns - 1, col - 1)); + break; + } + case 'J': { + for (let r = this.cursorRow; r < this.rows; r++) { + for (let c = 0; c < this.columns; c++) { + this.cells[r][c] = ''; + } + } + break; + } + default: + break; + } + } + + public assertNoScrollbackViolations(): void { + if (this.scrollbackViolations > 0) { + throw new Error(`MockScreen: ${this.scrollbackViolations} scrollback violation(s) detected`); + } + } + + public getRow(r: number): string { + const row = this.cells[r]; + let lastNonEmpty = -1; + for (let c = row.length - 1; c >= 0; c--) { + if (row[c] !== '') { + lastNonEmpty = c; + break; + } + } + return row.slice(0, lastNonEmpty + 1).join(''); + } + + public onResize(_cb: (columns: number, rows: number) => void): () => void { + return () => {}; + } +} diff --git a/test/TerminalRenderer.spec.ts b/test/TerminalRenderer.spec.ts new file mode 100644 index 0000000..f180c36 --- /dev/null +++ b/test/TerminalRenderer.spec.ts @@ -0,0 +1,252 @@ +import { describe, expect, it } from 'vitest'; +import type { Screen } from '../src/Screen.js'; +import { Renderer } from '../src/TerminalRenderer.js'; +import type { ViewportResult } from '../src/Viewport.js'; +import { MockScreen } from './MockScreen.js'; + +function makeScreen(columns: number) { + const output: string[] = []; + const screen: Screen = { + columns, + rows: 24, + write(data: string) { + output.push(data); + }, + onResize() { + return () => {}; + }, + }; + return { screen, output }; +} + +function makeFrame(rows: string[], visibleCursorRow: number, visibleCursorCol: number): ViewportResult { + return { rows, visibleCursorRow, visibleCursorCol }; +} + +describe('Renderer', () => { + describe('lastVisibleCursorRow after render', () => { + it('does not overcount when a row above the cursor exactly fills terminal width', () => { + const cols = 10; + const { screen, output } = makeScreen(cols); + const renderer = new Renderer(screen); + + // Row 0 is exactly cols-wide; cursor sits on row 1 + const frame = { + rows: ['a'.repeat(cols), 'x'], + visibleCursorRow: 1, + visibleCursorCol: 1, + }; + + renderer.render(frame); + output.length = 0; + + // Second render: should move up by 1 (lastVisibleCursorRow = 1), not 2 + renderer.render(frame); + const all = output.join(''); + + expect(all).toContain('\x1B[1A'); // cursorUp(1): correct + expect(all).not.toContain('\x1B[2A'); // cursorUp(2): was the bug + }); + + it('moves up correctly when no row above the cursor fills terminal width', () => { + const cols = 10; + const { screen, output } = makeScreen(cols); + const renderer = new Renderer(screen); + + const frame = { + rows: ['abc', 'cursor'], + visibleCursorRow: 1, + visibleCursorCol: 6, + }; + + renderer.render(frame); + output.length = 0; + + renderer.render(frame); + const all = output.join(''); + + expect(all).toContain('\x1B[1A'); + expect(all).not.toContain('\x1B[2A'); + }); + }); + + it('single frame on 10-row screen: no scrollback violations', () => { + const screen = new MockScreen(80, 10); + const renderer = new Renderer(screen); + renderer.render(makeFrame(['line 0', 'line 1', 'line 2', 'line 3', 'line 4'], 2, 0)); + screen.assertNoScrollbackViolations(); + }); + + it('frame filling entire 10-row screen: exactly 10 rows written, no violations', () => { + const screen = new MockScreen(80, 10); + const renderer = new Renderer(screen); + const rows = Array.from({ length: 10 }, (_, i) => `row ${i}`); + renderer.render(makeFrame(rows, 9, 0)); + screen.assertNoScrollbackViolations(); + for (let i = 0; i < 10; i++) { + expect(screen.getRow(i)).toBe(`row ${i}`); + } + }); + + it('two consecutive frames: no violations, screen shows second frame content', () => { + const screen = new MockScreen(80, 10); + const renderer = new Renderer(screen); + renderer.render(makeFrame(['a0', 'a1', 'a2', 'a3', 'a4'], 2, 0)); + renderer.render(makeFrame(['b0', 'b1', 'b2', 'b3', 'b4'], 2, 0)); + screen.assertNoScrollbackViolations(); + expect(screen.getRow(0)).toBe('b0'); + expect(screen.getRow(4)).toBe('b4'); + }); + + it('shorter frame after taller frame: leftover rows cleared, no violations', () => { + const screen = new MockScreen(80, 10); + const renderer = new Renderer(screen); + renderer.render( + makeFrame( + Array.from({ length: 8 }, (_, i) => `long${i}`), + 4, + 0, + ), + ); + renderer.render(makeFrame(['short0', 'short1', 'short2'], 1, 0)); + screen.assertNoScrollbackViolations(); + expect(screen.getRow(0)).toBe('short0'); + expect(screen.getRow(2)).toBe('short2'); + // Leftover rows from 8-row frame should be cleared + expect(screen.getRow(3)).toBe(''); + expect(screen.getRow(7)).toBe(''); + }); + + it('writeHistory then render: history line above zone, zone re-rendered, no violations', () => { + const screen = new MockScreen(80, 20); + const renderer = new Renderer(screen); + renderer.render(makeFrame(['zone0', 'zone1', 'zone2', 'zone3', 'zone4'], 2, 0)); + renderer.writeHistory('history'); + screen.assertNoScrollbackViolations(); + expect(screen.getRow(0)).toBe('history'); + expect(screen.getRow(1)).toBe('zone0'); + expect(screen.getRow(5)).toBe('zone4'); + }); + + it('writeHistory preserves zoneHeight: zoneHeight unchanged after writeHistory', () => { + const screen = new MockScreen(80, 20); + const renderer = new Renderer(screen); + renderer.render(makeFrame(['z0', 'z1', 'z2', 'z3', 'z4'], 2, 0)); + const before = renderer.zoneHeight; + renderer.writeHistory('a line'); + expect(renderer.zoneHeight).toBe(before); + }); + + it('multiple writeHistory calls between renders: each pushes zone down, no violations', () => { + const screen = new MockScreen(80, 30); + const renderer = new Renderer(screen); + renderer.render(makeFrame(['z0', 'z1', 'z2', 'z3', 'z4'], 2, 0)); + for (let i = 0; i < 10; i++) { + renderer.writeHistory(`history ${i}`); + } + screen.assertNoScrollbackViolations(); + // Zone has drifted 10 rows down from its original position at rows 0-4 + expect(screen.getRow(10)).toBe('z0'); + expect(screen.getRow(14)).toBe('z4'); + }); + + it('cursor position: after render, cursor is at (visibleCursorRow, visibleCursorCol)', () => { + const screen = new MockScreen(80, 10); + const renderer = new Renderer(screen); + renderer.render(makeFrame(['a', 'b', 'c', 'd', 'e'], 3, 15)); + expect(screen.cursorRow).toBe(3); + expect(screen.cursorCol).toBe(15); + }); + + describe('resize: simulated terminal reflow', () => { + it('widen resize: does not clear rows above new zone top', () => { + // Bug: pendingResize path does cursorAt(1,1) + clearDown, erasing the entire + // visible screen. When widening, the zone shrinks and more rows above become + // visible (OS reflow). Those rows must NOT be cleared by the resize render. + + // 10-row screen. Initial zone: 5 rows at rows 5–9 (0-indexed). + const screen = new MockScreen(80, 10); + const renderer = new Renderer(screen); + + renderer.render(makeFrame(['zone0', 'zone1', 'zone2', 'zone3', 'zone4'], 0, 0)); + + // Simulate OS widen reflow: old 5-row zone reflowed to 3 rows at new width. + // Terminal now shows history at rows 0–6, old zone at rows 7–9. + // Write history content directly into cells to represent what is visible. + for (let r = 0; r < 7; r++) { + const text = `history ${r}`; + for (let c = 0; c < text.length; c++) { + screen.cells[r][c] = text[c] as string; + } + } + screen.cursorRow = 7; + screen.cursorCol = 0; + + renderer.notifyResize(); + + // New zone: 3 rows (wider terminal, less wrapping). newZoneTop = 10 - 3 + 1 = 8 (1-based) = row 7 (0-based). + renderer.render(makeFrame(['zone0', 'zone1', 'zone2'], 0, 0)); + + // History rows 0–6 must be preserved (not cleared by pendingResize). + expect(screen.getRow(0)).toBe('history 0'); + expect(screen.getRow(6)).toBe('history 6'); + + // New zone must be at rows 7–9. + expect(screen.getRow(7)).toBe('zone0'); + expect(screen.getRow(8)).toBe('zone1'); + expect(screen.getRow(9)).toBe('zone2'); + }); + + it('widen resize: zone stays in place instead of jumping to screen bottom', () => { + // Bug: pendingResize used cursorAt(newZoneTop) which always anchors the zone + // to the bottom of the screen. When widening, the cursor doesn't move, so the + // zone should stay where it was (rows 0–2), not jump to the bottom (rows 7–9). + + const screen = new MockScreen(80, 10); + const renderer = new Renderer(screen); + + // Initial render: 5-row zone at rows 0–4 (cursor at visibleCursorRow=0) + renderer.render(makeFrame(['z0', 'z1', 'z2', 'z3', 'z4'], 0, 0)); + expect(screen.cursorRow).toBe(0); + + renderer.notifyResize(); + + // Widen: new 3-row zone (less wrapping at wider columns) + renderer.render(makeFrame(['z0', 'z1', 'z2'], 0, 0)); + + // Zone must stay at rows 0–2, not jump to rows 7–9 + const actual0 = screen.getRow(0); + const actual2 = screen.getRow(2); + const actual7 = screen.getRow(7); + + expect(actual0).toBe('z0'); + expect(actual2).toBe('z2'); + expect(actual7).toBe(''); + + screen.assertNoScrollbackViolations(); + }); + + it('no scrollback violations when cursor displaced by terminal reflow before render', () => { + // Resize bug: when the terminal is resized, the physical cursor moves because + // content reflows. renderer.lastVisibleCursorRow is stale (based on the + // pre-resize frame). The next render does cursorUp(stale), which moves the + // cursor from the wrong row. A taller zone then overflows the screen bottom. + + const screen = new MockScreen(80, 5); + const renderer = new Renderer(screen); + + // Phase 1: render a 2-row zone; cursor lands on row 1 + renderer.render(makeFrame(['first0', 'first1'], 1, 0)); + expect(screen.cursorRow).toBe(1); + + // Simulate terminal resize+reflow: cursor has been displaced to row 4 + screen.cursorRow = 4; + renderer.notifyResize(); + + // Phase 2: render a 4-row zone (zone grew because the terminal is now narrower) + renderer.render(makeFrame(['new0', 'new1', 'new2', 'new3'], 3, 0)); + + screen.assertNoScrollbackViolations(); + }); + }); +}); diff --git a/test/layout.spec.ts b/test/layout.spec.ts new file mode 100644 index 0000000..a355f34 --- /dev/null +++ b/test/layout.spec.ts @@ -0,0 +1,235 @@ +import { describe, expect, it } from 'vitest'; +import type { BuiltComponent, LayoutInput } from '../src/Layout.js'; +import { layout } from '../src/Layout.js'; + +function makeEditor(lines: string[], cursorRow = 0, cursorCol = 0) { + return { lines, cursorRow, cursorCol }; +} + +function component(rows: string[], height: number): BuiltComponent { + return { rows, height }; +} + +describe('layout', () => { + it('editor only: 5 single-row lines produce buffer of 5 rows', () => { + const input: LayoutInput = { + editor: makeEditor(['line 1', 'line 2', 'line 3', 'line 4', 'line 5']), + status: null, + attachments: null, + preview: null, + question: null, + columns: 80, + }; + const result = layout(input); + expect(result.buffer.length).toBe(5); + expect(result.editorStartRow).toBe(0); + expect(result.cursorRow).toBe(0); + expect(result.cursorCol).toBe(0); + }); + + it('status + editor: status row before editor, editorStartRow = 1', () => { + const input: LayoutInput = { + editor: makeEditor(['hello']), + status: component(['status line'], 1), + attachments: null, + preview: null, + question: null, + columns: 80, + }; + const result = layout(input); + expect(result.buffer[0]).toBe('status line'); + expect(result.buffer[1]).toBe('hello'); + expect(result.editorStartRow).toBe(1); + expect(result.cursorRow).toBe(1); + }); + + it('all components present: buffer order is question, status, attachment, preview, editor', () => { + const input: LayoutInput = { + editor: makeEditor(['editor']), + status: component(['status'], 1), + attachments: component(['attachment'], 1), + preview: component(['preview'], 1), + question: component(['question'], 1), + columns: 80, + }; + const result = layout(input); + expect(result.buffer[0]).toBe('question'); + expect(result.buffer[1]).toBe('status'); + expect(result.buffer[2]).toBe('attachment'); + expect(result.buffer[3]).toBe('preview'); + expect(result.buffer[4]).toBe('editor'); + expect(result.editorStartRow).toBe(4); + expect(result.cursorRow).toBe(4); + }); + + it('null components skipped: no empty rows in buffer', () => { + const input: LayoutInput = { + editor: makeEditor(['editor']), + status: null, + attachments: null, + preview: null, + question: null, + columns: 80, + }; + const result = layout(input); + expect(result.buffer.length).toBe(1); + expect(result.buffer[0]).toBe('editor'); + }); + + it('long editor line wraps: 200 chars at 80 columns = 3 buffer rows', () => { + const longLine = 'A'.repeat(200); + const input: LayoutInput = { + editor: makeEditor([longLine], 0, 0), + status: null, + attachments: null, + preview: null, + question: null, + columns: 80, + }; + const result = layout(input); + expect(result.buffer.length).toBe(3); + expect(result.buffer[0]).toBe('A'.repeat(80)); + expect(result.buffer[1]).toBe('A'.repeat(80)); + expect(result.buffer[2]).toBe('A'.repeat(40)); + }); + + it('ZWJ sequence is decomposed: each emoji wraps independently', () => { + // ZWJ sequences are stripped before layout so string-width matches terminal rendering. + // 👨‍👩‍👦 (U+1F468 ZWJ U+1F469 ZWJ U+1F466) becomes 3 individual emojis, each width 2. + // Total display width: A(1)+B(1)+man(2)+woman(2)+girl(2)+C(1) = 9, not 5. + const familyEmoji = '\u{1F468}\u200D\u{1F469}\u200D\u{1F466}'; + const line = `AB${familyEmoji}C`; + const input: LayoutInput = { + editor: makeEditor([line], 0, 0), + status: null, + attachments: null, + preview: null, + question: null, + columns: 4, + }; + + const result = layout(input); + + // Row 0: A(1)+B(1)+man(2) = 4. Row 1: woman(2)+girl(2) = 4. Row 2: C(1). + expect(result.buffer).toHaveLength(3); + expect(result.buffer[0]).toBe('AB\u{1F468}'); + expect(result.buffer[1]).toBe('\u{1F469}\u{1F466}'); + expect(result.buffer[2]).toBe('C'); + }); + + it('50 editor lines: buffer has 50 rows, cursorRow accounts for non-editor rows', () => { + const lines = Array.from({ length: 50 }, (_, i) => `line ${i}`); + const input: LayoutInput = { + editor: makeEditor(lines, 25, 0), + status: component(['status'], 1), + attachments: null, + preview: null, + question: null, + columns: 80, + }; + const result = layout(input); + expect(result.buffer.length).toBe(51); // 1 status + 50 editor + expect(result.editorStartRow).toBe(1); + expect(result.cursorRow).toBe(26); // 1 (status) + 25 (editor cursor) + }); + + it('cursorCol passes through from EditorRender.cursorCol', () => { + const input: LayoutInput = { + editor: makeEditor(['hello world'], 0, 5), + status: null, + attachments: null, + preview: null, + question: null, + columns: 80, + }; + const result = layout(input); + expect(result.cursorCol).toBe(5); + }); + + it('editorStartRow correct with two non-editor components', () => { + const input: LayoutInput = { + editor: makeEditor(['e']), + status: component(['s'], 1), + attachments: component(['a'], 1), + preview: null, + question: null, + columns: 80, + }; + const result = layout(input); + expect(result.editorStartRow).toBe(2); + }); + + it('editorStartRow correct with question and preview but no status or attachment', () => { + const input: LayoutInput = { + editor: makeEditor(['e']), + status: null, + attachments: null, + preview: component(['p'], 1), + question: component(['q'], 1), + columns: 80, + }; + const result = layout(input); + expect(result.buffer[0]).toBe('q'); + expect(result.buffer[1]).toBe('p'); + expect(result.editorStartRow).toBe(2); + }); + + it('multi-row component contributes multiple buffer entries', () => { + const input: LayoutInput = { + editor: makeEditor(['e']), + status: null, + attachments: null, + preview: component(['preview line 1', 'preview line 2', 'preview line 3'], 3), + question: null, + columns: 80, + }; + const result = layout(input); + expect(result.buffer[0]).toBe('preview line 1'); + expect(result.buffer[1]).toBe('preview line 2'); + expect(result.buffer[2]).toBe('preview line 3'); + expect(result.editorStartRow).toBe(3); + expect(result.cursorRow).toBe(3); + }); + + it('ZWJ family emoji at col 79: wraps to next line (terminal renders 4 emojis x 2 = 8 cells)', () => { + // string-width reports the ZWJ sequence as width 2 (composed form). + // The terminal renders it as 4 separate emojis, each width 2 = 8 cells total. + // 78 ASCII + 8 emoji cells = 86 > 80, so the emoji must wrap. + // Without ZWJ stripping, layout sees 78+2=80 and returns 1 buffer row (wrong). + const familyEmoji = '\u{1F468}\u200D\u{1F469}\u200D\u{1F467}\u200D\u{1F466}'; + const line = 'A'.repeat(78) + familyEmoji; + const input: LayoutInput = { + editor: makeEditor([line], 0, 0), + status: null, + attachments: null, + preview: null, + question: null, + columns: 80, + }; + + const result = layout(input); + + // 78 A's + first emoji (2 cols) = 80 on row 0; remaining 3 emojis on row 1. + expect(result.buffer.length).toBe(2); + }); + + it('editor cursor on wrapped line: cursorRow offset by editorStartRow', () => { + // Editor has 1 wrapped line (160 chars = 2 visual rows at 80 cols) + // cursor is on visual row 1 of the editor (second wrap segment) + // with 1 status row above, buffer cursorRow = 1 (status) + 1 (visual row) = 2 + const wrappedLine = 'B'.repeat(160); + const input: LayoutInput = { + editor: makeEditor([wrappedLine], 1, 10), + status: component(['status'], 1), + attachments: null, + preview: null, + question: null, + columns: 80, + }; + const result = layout(input); + expect(result.buffer.length).toBe(3); // 1 status + 2 editor rows + expect(result.editorStartRow).toBe(1); + expect(result.cursorRow).toBe(2); // 1 + 1 + expect(result.cursorCol).toBe(10); + }); +}); diff --git a/test/mock-screen.spec.ts b/test/mock-screen.spec.ts new file mode 100644 index 0000000..64e8a13 --- /dev/null +++ b/test/mock-screen.spec.ts @@ -0,0 +1,194 @@ +import { describe, expect, it } from 'vitest'; +import { MockScreen } from './MockScreen.js'; + +const ESC = '\x1B['; +const cursorUp = (n: number) => `${ESC}${n}A`; +const cursorTo = (col: number) => `${ESC}${col + 1}G`; +const clearLine = `${ESC}2K`; + +describe('MockScreen', () => { + it('writes characters at correct cursor position and advances cursor', () => { + const screen = new MockScreen(80, 24); + screen.write('AB'); + expect(screen.cursorRow).toBe(0); + expect(screen.cursorCol).toBe(2); + expect(screen.getRow(0)).toBe('AB'); + }); + + it('writes characters at offset cursor position', () => { + const screen = new MockScreen(80, 24); + screen.write('\n\n'); // row 2 + screen.write('XY'); + expect(screen.cursorRow).toBe(2); + expect(screen.cursorCol).toBe(2); + expect(screen.getRow(2)).toBe('XY'); + expect(screen.getRow(0)).toBe(''); + }); + + it('cursorUp moves cursor up N rows', () => { + const screen = new MockScreen(80, 24); + screen.write('\n\n\n'); // row 3 + screen.write(cursorUp(2)); + expect(screen.cursorRow).toBe(1); + }); + + it('cursorUp clamped at row 0', () => { + const screen = new MockScreen(80, 24); + screen.write('\n\n'); // row 2 + screen.write(cursorUp(10)); + expect(screen.cursorRow).toBe(0); + }); + + it('cursorUp by 1 moves up one row', () => { + const screen = new MockScreen(80, 24); + screen.write('\n'); // row 1 + screen.write(cursorUp(1)); + expect(screen.cursorRow).toBe(0); + }); + + it('cursorTo moves cursor to specified column', () => { + const screen = new MockScreen(80, 24); + screen.write(cursorTo(10)); + expect(screen.cursorCol).toBe(10); + }); + + it('cursorTo column 0 moves to first column', () => { + const screen = new MockScreen(80, 24); + screen.write('hello'); + screen.write(cursorTo(0)); + expect(screen.cursorCol).toBe(0); + }); + + it('clearLine clears the current row', () => { + const screen = new MockScreen(80, 24); + screen.write('hello world'); + screen.write(clearLine); + expect(screen.getRow(0)).toBe(''); + }); + + it('clearLine does not affect other rows', () => { + const screen = new MockScreen(80, 24); + screen.write('row0'); + screen.write('\n'); + screen.write('row1'); + screen.write('\r'); // move to col 0 on row 1 + screen.write(cursorUp(1)); // back to row 0 + screen.write(clearLine); + expect(screen.getRow(0)).toBe(''); + expect(screen.getRow(1)).toBe('row1'); + }); + + it('\\n above bottom row moves cursor down, no violation', () => { + const screen = new MockScreen(80, 3); + screen.write('\n'); // row 1 + expect(screen.cursorRow).toBe(1); + expect(screen.scrollbackViolations).toBe(0); + }); + + it('\\n at bottom row increments scrollbackViolations', () => { + const screen = new MockScreen(80, 3); + screen.write('\n\n'); // row 2 (last row for 3-row screen) + expect(screen.cursorRow).toBe(2); + screen.write('\n'); + expect(screen.scrollbackViolations).toBe(1); + }); + + it('\\n at bottom row shifts rows up and clears new bottom row', () => { + const screen = new MockScreen(80, 3); + screen.write('A'); + screen.write('\n'); + screen.write('B'); + screen.write('\n'); + screen.write('C'); + // rows: A, B, C (cursorRow=2) + screen.write('\n'); + // scroll: row0(A) lost, row1(B)->row0, row2(C)->row1, new empty row2 + expect(screen.getRow(0)).toBe('B'); + expect(screen.getRow(1)).toBe('C'); + expect(screen.getRow(2)).toBe(''); + expect(screen.scrollbackViolations).toBe(1); + }); + + it('assertNoScrollbackViolations passes when violations = 0', () => { + const screen = new MockScreen(80, 24); + screen.write('hello'); + expect(() => screen.assertNoScrollbackViolations()).not.toThrow(); + }); + + it('assertNoScrollbackViolations throws when violations > 0', () => { + const screen = new MockScreen(80, 1); + screen.write('\n'); + expect(() => screen.assertNoScrollbackViolations()).toThrow(); + }); + + it('writing fills a row enters pending-wrap; next char wraps to next row', () => { + const screen = new MockScreen(5, 24); + screen.write('ABCDE'); // 5 chars fills 5-col row, enters pending-wrap at last col + expect(screen.cursorRow).toBe(0); + expect(screen.cursorCol).toBe(4); + expect(screen.getRow(0)).toBe('ABCDE'); + screen.write('F'); // next char triggers wrap to row 1 + expect(screen.cursorRow).toBe(1); + expect(screen.cursorCol).toBe(1); + expect(screen.getRow(1)).toBe('F'); + }); + + it('writing past end of last row causes scrollback violation', () => { + const screen = new MockScreen(5, 2); + screen.write('\n'); // row 1 (last row for 2-row screen) + screen.write('ABCDE'); // fills last row, enters pending-wrap, no violation yet + expect(screen.scrollbackViolations).toBe(0); + screen.write('F'); // triggers wrap past last row, scrollback violation + expect(screen.scrollbackViolations).toBe(1); + }); + + it('\\r moves cursor to column 0', () => { + const screen = new MockScreen(80, 24); + screen.write('hello'); + screen.write('\r'); + expect(screen.cursorCol).toBe(0); + expect(screen.cursorRow).toBe(0); + }); + + it('\\r followed by write overwrites from column 0', () => { + const screen = new MockScreen(80, 24); + screen.write('AAAAA'); + screen.write('\r'); + screen.write('BB'); + expect(screen.getRow(0)).toBe('BBAAA'); + }); + + it('getRow trims trailing empty cells', () => { + const screen = new MockScreen(80, 24); + screen.write('hi'); + expect(screen.getRow(0)).toBe('hi'); + expect(screen.getRow(0).length).toBe(2); + }); + + it('getRow returns empty string for blank row', () => { + const screen = new MockScreen(80, 24); + expect(screen.getRow(0)).toBe(''); + }); + + it('ESC[row;colH moves cursor to absolute position (1-based)', () => { + const screen = new MockScreen(80, 24); + screen.write('\x1B[5;10H'); // row 5, col 10 (1-based) + expect(screen.cursorRow).toBe(4); // 0-based + expect(screen.cursorCol).toBe(9); // 0-based + }); + + it('ESC[1;1H moves cursor to top-left', () => { + const screen = new MockScreen(80, 24); + screen.write('\n\n\n'); // row 3 + screen.write('\x1B[1;1H'); + expect(screen.cursorRow).toBe(0); + expect(screen.cursorCol).toBe(0); + }); + + it('ESC[row;colH clamps to screen boundaries', () => { + const screen = new MockScreen(10, 5); + screen.write('\x1B[99;99H'); + expect(screen.cursorRow).toBe(4); // clamped to rows - 1 + expect(screen.cursorCol).toBe(9); // clamped to columns - 1 + }); +}); diff --git a/test/prepareEditor.spec.ts b/test/prepareEditor.spec.ts new file mode 100644 index 0000000..06072ef --- /dev/null +++ b/test/prepareEditor.spec.ts @@ -0,0 +1,44 @@ +import { describe, expect, it } from 'vitest'; +import type { EditorState } from '../src/editor.js'; +import { prepareEditor } from '../src/renderer.js'; + +// In test (non-TTY) environments process.stdout.columns is undefined, +// so renderer falls back to 80 columns via `|| 80`. +const COLUMNS = 80; +const PROMPT = '> '; // visual width 2 + +function editor(text: string, col?: number): EditorState { + return { lines: [text], cursor: { row: 0, col: col ?? text.length } }; +} + +describe('prepareEditor', () => { + describe('cursorCol is always within-row (< columns), not the absolute offset', () => { + it('cursor at exact wrap boundary wraps cursorCol to 0', () => { + // prompt(2) + text(78) = 80 = COLUMNS: cursor is at the start of visual row 1 + const result = prepareEditor(editor('a'.repeat(78)), PROMPT); + expect(result.cursorRow).toBe(1); + expect(result.cursorCol).toBe(0); + }); + + it('cursor past wrap boundary gives correct within-row offset', () => { + // prompt(2) + text(83) = 85: visual row 1, column 5 + const result = prepareEditor(editor('a'.repeat(83)), PROMPT); + expect(result.cursorRow).toBe(1); + expect(result.cursorCol).toBe(5); + }); + + it('cursor before any wrap is unchanged', () => { + // prompt(2) + text(10) = 12, stays on row 0 + const result = prepareEditor(editor('a'.repeat(10)), PROMPT); + expect(result.cursorRow).toBe(0); + expect(result.cursorCol).toBe(12); + }); + + it('cursorCol is always less than terminal width for all wrap positions', () => { + for (const len of [77, 78, 79, 80, 81, 158, 159, 160, 161]) { + const result = prepareEditor(editor('a'.repeat(len)), PROMPT); + expect(result.cursorCol, `len=${len}`).toBeLessThan(COLUMNS); + } + }); + }); +}); diff --git a/test/sanitise.spec.ts b/test/sanitise.spec.ts index 456ecf0..ec89a54 100644 --- a/test/sanitise.spec.ts +++ b/test/sanitise.spec.ts @@ -1,5 +1,6 @@ +import stringWidth from 'string-width'; import { describe, expect, it } from 'vitest'; -import { sanitiseLoneSurrogates } from '../src/sanitise.js'; +import { sanitiseLoneSurrogates, sanitiseZwj } from '../src/sanitise.js'; describe('sanitiseLoneSurrogates', () => { it('replaces lone high surrogate', () => { @@ -26,3 +27,42 @@ describe('sanitiseLoneSurrogates', () => { expect(actual).toBe(expected); }); }); + +// Bug: string-width treats ZWJ sequences as composed (width 2), +// but terminals render them as individual emojis (4 x 2 = 8 cells). +// This mismatch causes wrong line height and cursor position. +describe('sanitiseZwj', () => { + it('raw ZWJ sequence: stringWidth returns 2 not 8, exposing the terminal mismatch', () => { + const familyEmoji = '\u{1F468}\u200D\u{1F469}\u200D\u{1F467}\u200D\u{1F466}'; + + const actual = stringWidth(familyEmoji); + + expect(actual).toBe(2); + }); + + it('strips ZWJ characters from a compound emoji sequence', () => { + const input = '\u{1F468}\u200D\u{1F469}\u200D\u{1F467}\u200D\u{1F466}'; + const expected = '\u{1F468}\u{1F469}\u{1F467}\u{1F466}'; + + const actual = sanitiseZwj(input); + + expect(actual).toBe(expected); + }); + + it('stringWidth of sanitised sequence equals sum of individual emoji widths', () => { + const input = '\u{1F468}\u200D\u{1F469}\u200D\u{1F467}\u200D\u{1F466}'; + const expected = 8; // 4 emojis x 2 cells each + + const actual = stringWidth(sanitiseZwj(input)); + + expect(actual).toBe(expected); + }); + + it('leaves text without ZWJ unchanged', () => { + const input = 'hello world'; + + const actual = sanitiseZwj(input); + + expect(actual).toBe(input); + }); +}); diff --git a/test/terminal-integration.spec.ts b/test/terminal-integration.spec.ts new file mode 100644 index 0000000..91ac36e --- /dev/null +++ b/test/terminal-integration.spec.ts @@ -0,0 +1,237 @@ +import { describe, expect, it } from 'vitest'; +import type { BuiltComponent, LayoutInput } from '../src/Layout.js'; +import { layout } from '../src/Layout.js'; +import type { EditorRender } from '../src/renderer.js'; +import { Renderer } from '../src/TerminalRenderer.js'; +import { Viewport } from '../src/Viewport.js'; +import { MockScreen } from './MockScreen.js'; + +function makeEditorRender(lineCount: number, cursorRow = 0, cursorCol = 0): EditorRender { + const lines = Array.from({ length: lineCount }, (_, i) => `line ${i}`); + return { lines, cursorRow, cursorCol }; +} + +function makeComponent(rows: string[]): BuiltComponent { + return { rows, height: rows.length }; +} + +function runPipeline(screen: MockScreen, viewport: Viewport, renderer: Renderer, input: LayoutInput): void { + const result = layout(input); + const frame = viewport.resolve(result.buffer, screen.rows, result.cursorRow, result.cursorCol); + renderer.render(frame); +} + +describe('Terminal integration', () => { + it('full cycle: no scrollback violations', () => { + const screen = new MockScreen(80, 10); + const viewport = new Viewport(); + const renderer = new Renderer(screen); + const input = { + editor: makeEditorRender(3, 1, 0), + status: makeComponent(['status']), + attachments: null, + preview: null, + question: null, + columns: 80, + } satisfies LayoutInput; + + runPipeline(screen, viewport, renderer, input); + + screen.assertNoScrollbackViolations(); + }); + + it('50-line editor on 10-row screen: no scrollback violations', () => { + const screen = new MockScreen(80, 10); + const viewport = new Viewport(); + const renderer = new Renderer(screen); + const input = { + editor: makeEditorRender(50, 25, 0), + status: null, + attachments: null, + preview: null, + question: null, + columns: 80, + } satisfies LayoutInput; + + runPipeline(screen, viewport, renderer, input); + + screen.assertNoScrollbackViolations(); + }); + + it('resize from 24 to 10 rows: viewport adapts, no violations, cursor visible', () => { + const viewport = new Viewport(); + const input = { + editor: makeEditorRender(20, 10, 0), + status: null, + attachments: null, + preview: null, + question: null, + columns: 80, + } satisfies LayoutInput; + + // First render at 24 rows. Viewport scrollOffset is established. + const bigScreen = new MockScreen(80, 24); + const bigRenderer = new Renderer(bigScreen); + runPipeline(bigScreen, viewport, bigRenderer, input); + + // Resize to 10 rows. Viewport state (scrollOffset) persists across the resize. + const smallScreen = new MockScreen(80, 10); + const smallRenderer = new Renderer(smallScreen); + const { buffer, cursorRow, cursorCol } = layout(input); + const frame = viewport.resolve(buffer, 10, cursorRow, cursorCol); + smallRenderer.render(frame); + + smallScreen.assertNoScrollbackViolations(); + expect(frame.visibleCursorRow).toBeGreaterThanOrEqual(0); + expect(frame.visibleCursorRow).toBeLessThan(10); + }); + + it('writeHistory during active editor: no violations', () => { + const screen = new MockScreen(80, 20); + const viewport = new Viewport(); + const renderer = new Renderer(screen); + const input = { + editor: makeEditorRender(3, 1, 0), + status: null, + attachments: null, + preview: null, + question: null, + columns: 80, + } satisfies LayoutInput; + + runPipeline(screen, viewport, renderer, input); + renderer.writeHistory('a history line'); + + screen.assertNoScrollbackViolations(); + }); + + it('question cleared before history write: zone re-renders without question', () => { + // Reproduces the bug where renderer.writeHistory re-rendered lastFrame + // (stale, containing the question) instead of the current layout state. + // Fix: writeHistoryLine + fresh runPipeline so zone reflects current state. + const screen = new MockScreen(80, 20); + const viewport = new Viewport(); + const renderer = new Renderer(screen); + + // Initial render: zone includes a question component. + const withQuestion = { + editor: makeEditorRender(2, 0, 0), + status: makeComponent(['status']), + attachments: null, + preview: null, + question: makeComponent(['Pick one:', '1) Yes', '2) No']), + columns: 80, + } satisfies LayoutInput; + runPipeline(screen, viewport, renderer, withQuestion); + + // Question is answered: clearQuestionLines() was called. Now history is + // written. The zone must re-render WITHOUT the question. + const withoutQuestion = { + editor: makeEditorRender(2, 0, 0), + status: makeComponent(['status']), + attachments: null, + preview: null, + question: null, + columns: 80, + } satisfies LayoutInput; + renderer.writeHistoryLine('answer: Yes'); + runPipeline(screen, viewport, renderer, withoutQuestion); + + screen.assertNoScrollbackViolations(); + // Question content must not appear in any visible row. + expect(screen.getRow(0)).toBe('answer: Yes'); + const visibleRows = Array.from({ length: 20 }, (_, i) => screen.getRow(i)); + expect(visibleRows.some((r) => r.includes('Pick one:'))).toBe(false); + }); + + it('cursor at first row of viewport: no violations', () => { + const screen = new MockScreen(80, 10); + const viewport = new Viewport(); + const renderer = new Renderer(screen); + const input = { + editor: makeEditorRender(5, 0, 0), + status: null, + attachments: null, + preview: null, + question: null, + columns: 80, + } satisfies LayoutInput; + + runPipeline(screen, viewport, renderer, input); + + screen.assertNoScrollbackViolations(); + }); + + it('first render with cursor mid-screen: no scrollback violations', () => { + // Simulates startup: previous output leaves cursor at row 7 of a 10-row screen. + // Content is 2 rows (status + 1 editor line). Viewport pads the frame to 10 rows. + // Without trimming, render() would write 10 rows from row 7, causing 7 scrollback + // violations (\n at row 9 fires 7 times). With trimming, only 2 rows are written + // (rows 7-8), leaving row 9 untouched. + const screen = new MockScreen(80, 10); + const viewport = new Viewport(); + const renderer = new Renderer(screen); + screen.cursorRow = 7; + const input = { + editor: makeEditorRender(1, 0, 0), + status: makeComponent(['status']), + attachments: null, + preview: null, + question: null, + columns: 80, + } satisfies LayoutInput; + + runPipeline(screen, viewport, renderer, input); + + screen.assertNoScrollbackViolations(); + }); + + it('cursor at last row of viewport: no violations', () => { + const screen = new MockScreen(80, 5); + const viewport = new Viewport(); + const renderer = new Renderer(screen); + const input = { + editor: makeEditorRender(5, 4, 0), + status: null, + attachments: null, + preview: null, + question: null, + columns: 80, + } satisfies LayoutInput; + + runPipeline(screen, viewport, renderer, input); + + screen.assertNoScrollbackViolations(); + }); + + it('short history line does not leave stale zone content on same row', () => { + // Reproduces the bug where writeHistoryLine lacked clearLine. + // A zone row (80-char wide) at zone top, then a shorter history line + // written there, left old zone content at cols history_len..79 visible. + const screen = new MockScreen(80, 10); + const viewport = new Viewport(); + const renderer = new Renderer(screen); + const wideRow = 'A'.repeat(80); + const input = { + editor: makeEditorRender(1, 0, 0), + status: makeComponent([wideRow]), + attachments: null, + preview: null, + question: null, + columns: 80, + } satisfies LayoutInput; + + runPipeline(screen, viewport, renderer, input); + + // Write a history line shorter than the 80-char zone row. + const shortHistory = 'short line'; + renderer.writeHistoryLine(shortHistory); + // Re-render zone so the history row is now in scrollback. + const { buffer, cursorRow, cursorCol } = layout(input); + const frame = viewport.resolve(buffer, screen.rows, cursorRow, cursorCol); + renderer.render(frame); + + // Row 0 must contain only the history text, no stale zone content. + expect(screen.getRow(0)).toBe(shortHistory); + }); +}); diff --git a/test/viewport.spec.ts b/test/viewport.spec.ts new file mode 100644 index 0000000..1250934 --- /dev/null +++ b/test/viewport.spec.ts @@ -0,0 +1,125 @@ +import { describe, expect, it } from 'vitest'; +import { Viewport } from '../src/Viewport.js'; + +describe('Viewport', () => { + it('buffer shorter than screen: returns screenRows entries (content + padding)', () => { + const vp = new Viewport(); + const buffer = ['a', 'b', 'c', 'd', 'e']; + const result = vp.resolve(buffer, 10, 0, 0); + expect(result.rows.length).toBe(10); + expect(result.rows[0]).toBe('a'); + expect(result.rows[4]).toBe('e'); + expect(result.rows[5]).toBe(''); + expect(result.rows[9]).toBe(''); + }); + + it('buffer equals screen: returns all rows, no padding', () => { + const vp = new Viewport(); + const buffer = Array.from({ length: 10 }, (_, i) => `row ${i}`); + const result = vp.resolve(buffer, 10, 0, 0); + expect(result.rows.length).toBe(10); + expect(result.rows[0]).toBe('row 0'); + expect(result.rows[9]).toBe('row 9'); + }); + + it('buffer taller than screen: returns exactly screenRows rows', () => { + const vp = new Viewport(); + const buffer = Array.from({ length: 50 }, (_, i) => `row ${i}`); + const result = vp.resolve(buffer, 10, 0, 0); + expect(result.rows.length).toBe(10); + }); + + it('cursor at row 0: scrollOffset = 0, visibleCursorRow = 0', () => { + const vp = new Viewport(); + const buffer = Array.from({ length: 5 }, (_, i) => `row ${i}`); + const result = vp.resolve(buffer, 10, 0, 3); + expect(result.visibleCursorRow).toBe(0); + expect(result.visibleCursorCol).toBe(3); + }); + + it('cursor at row 45 (buffer 50, screen 10): scrollOffset adjusts, visibleCursorRow within 0-9', () => { + const vp = new Viewport(); + const buffer = Array.from({ length: 50 }, (_, i) => `row ${i}`); + const result = vp.resolve(buffer, 10, 45, 0); + expect(result.visibleCursorRow).toBeGreaterThanOrEqual(0); + expect(result.visibleCursorRow).toBeLessThanOrEqual(9); + // cursor at 45, screen 10 => scrollOffset = 45 - 10 + 1 = 36, visibleCursorRow = 45 - 36 = 9 + expect(result.visibleCursorRow).toBe(9); + expect(result.rows[9]).toBe('row 45'); + }); + + it('cursor in stable range: scrollOffset does not change when cursor is already visible', () => { + const vp = new Viewport(); + const buffer = Array.from({ length: 50 }, (_, i) => `row ${i}`); + // First call places cursor at row 45 (scrollOffset = 36) + vp.resolve(buffer, 10, 45, 0); + // Second call: cursor moves to row 40 (still in 36..45 range) + const result = vp.resolve(buffer, 10, 40, 0); + // scrollOffset stays 36, visibleCursorRow = 40 - 36 = 4 + expect(result.visibleCursorRow).toBe(4); + expect(result.rows[4]).toBe('row 40'); + }); + + it('cursor chasing up: cursor moves above viewport, scrollOffset snaps to cursorRow', () => { + const vp = new Viewport(); + const buffer = Array.from({ length: 50 }, (_, i) => `row ${i}`); + // First call: cursor at 45, scrollOffset = 36 + vp.resolve(buffer, 10, 45, 0); + // Second call: cursor jumps to row 5 (below scrollOffset=36) + const result = vp.resolve(buffer, 10, 5, 0); + // scrollOffset snaps to 5, visibleCursorRow = 0 + expect(result.visibleCursorRow).toBe(0); + expect(result.rows[0]).toBe('row 5'); + }); + + it('cursor chasing down: scrollOffset = cursorRow - screenRows + 1', () => { + const vp = new Viewport(); + const buffer = Array.from({ length: 50 }, (_, i) => `row ${i}`); + // First call: cursor at 0, scrollOffset = 0 + vp.resolve(buffer, 10, 0, 0); + // Second call: cursor jumps to row 20 (beyond 0 + 10 - 1 = 9) + const result = vp.resolve(buffer, 10, 20, 0); + // scrollOffset = 20 - 10 + 1 = 11, visibleCursorRow = 20 - 11 = 9 + expect(result.visibleCursorRow).toBe(9); + expect(result.rows[9]).toBe('row 20'); + }); + + it('resize shrink (24 to 10): scrollOffset capped, cursor still visible', () => { + const vp = new Viewport(); + const buffer = Array.from({ length: 50 }, (_, i) => `row ${i}`); + // First call at screen height 24, cursor at 30 + // scrollOffset = 30 - 24 + 1 = 7 + vp.resolve(buffer, 24, 30, 0); + // Resize to 10: scrollOffset capped at max(0, 50 - 10) = 40, then cursor chasing + // cursor 30 < scrollOffset(after cap)... let's trace: + // After cap: scrollOffset = min(7, max(0, 50 - 10)) = min(7, 40) = 7 + // Then cursor chasing: cursorRow(30) >= 7 + 10 = 17 => scrollOffset = 30 - 10 + 1 = 21 + // visibleCursorRow = 30 - 21 = 9 + const result = vp.resolve(buffer, 10, 30, 0); + expect(result.rows.length).toBe(10); + expect(result.visibleCursorRow).toBeGreaterThanOrEqual(0); + expect(result.visibleCursorRow).toBeLessThanOrEqual(9); + }); + + it('resize grow (10 to 24): more content visible, scrollOffset does not increase', () => { + const vp = new Viewport(); + const buffer = Array.from({ length: 50 }, (_, i) => `row ${i}`); + // First call at screen 10, cursor at 45 => scrollOffset = 36 + vp.resolve(buffer, 10, 45, 0); + // Grow to 24, cursor still at 45 + // cap: scrollOffset = min(36, max(0, 50 - 24)) = min(36, 26) = 26 + // cursor chasing: 45 >= 26 + 24 = 50? No. 45 < 26? No. => stays 26 + // visibleCursorRow = 45 - 26 = 19 + const result = vp.resolve(buffer, 24, 45, 0); + expect(result.rows.length).toBe(24); + expect(result.visibleCursorRow).toBeGreaterThanOrEqual(0); + expect(result.visibleCursorRow).toBeLessThanOrEqual(23); + }); + + it('visibleCursorCol always equals input cursorCol', () => { + const vp = new Viewport(); + const buffer = ['a', 'b', 'c']; + const result = vp.resolve(buffer, 10, 1, 42); + expect(result.visibleCursorCol).toBe(42); + }); +});