Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 24 additions & 10 deletions packages/core/src/ui/editorView.tsx
Original file line numberDiff line numberDiff line change
Expand Up@@ -73,14 +73,25 @@
* result) when the service reports a change with no other prop change —
* see that hook's own TSDoc.
*
* **Scope note on `viewportHeight`**: this task measures the available rows
* via an explicit, caller-supplied `viewportHeight` prop rather than
* observing the rendered container's actual height at runtime (which would
* need an OpenTUI resize-event listener wired to a `useState`) — the caller
* (`shell.tsx`'s `EditorArea`, tests) passes a fixed value. Auto-measurement
* from the live layout is left to a later task; nothing here would need to
* change shape to add it (`viewportHeight` would just come from `useState`
* instead of a prop default).
* **Scope note on `viewportHeight`** (Issue #92 — "Only the first 20 lines
* are displayed" regardless of terminal size): this component itself still
* takes the available rows as an explicit, caller-supplied `viewportHeight`
* prop rather than observing its own rendered container's height at
* runtime — `EditorView` has no OpenTUI resize-event listener of its own,
* and does not need one. The auto-measurement lives one level up instead:
* `shell.tsx`'s `EditorArea` reads the LIVE terminal height
* (`useLiveTerminalHeight`, wrapping `@opentui/react`'s resize event) and
* subtracts exactly the chrome it itself renders (tab bar, find widget,
* `Shell`'s sibling `Panel`, `StatusBar` —
* `viewport.ts`'s `computeEditorViewportHeight`), then passes the result
* down as this prop — so a real terminal resize reactively resizes the
* text plane's virtualization window, even though `EditorView` never reads
* the terminal itself. Every other caller (every test in this file,
* `editorView.snapshot.test.tsx`) keeps passing a fixed value exactly as
* before — `viewportHeight` still just is a number this component trusts,
* whatever supplies it. Omitting the prop entirely (no live terminal
* available, e.g. a caller/test outside a real `CliRenderer`) falls back
* to `DEFAULT_VIEWPORT_HEIGHT` below, unchanged.
*/

import { memo, useCallback, useMemo, useRef, useState, type ReactNode } from "react";
Expand DownExpand Up@@ -498,8 +509,11 @@ export interface EditorViewProps {
* the primary cursor that drives reveal scrolling. */
state: EditorState;
/** Rows available to the text plane (Req 13.1's virtualization). See this
* module's TSDoc for why this is a prop rather than a live measurement.
* Defaults to {@link DEFAULT_VIEWPORT_HEIGHT}. */
* module's TSDoc's "Scope note on `viewportHeight`" for why this
* component takes it as a prop rather than measuring its own container:
* `shell.tsx`'s `EditorArea` is what actually derives it from the live
* terminal size (Issue #92). Defaults to {@link DEFAULT_VIEWPORT_HEIGHT}
* when omitted. */
viewportHeight?: number;
/** Reads `editor.lineNumbers` (Req 9.5, design.md §8.3's gutter). Omitted
* in isolated tests, where line numbers default to shown (`true`) — the
Expand Down
55 changes: 55 additions & 0 deletions packages/core/src/ui/shell.snapshot.test.tsx
Original file line numberDiff line numberDiff line change
Expand Up@@ -358,6 +358,61 @@ describe("Shell — EditorArea wired to a DocumentManager (Req 6.5, 6.6, design.
expect(frame).not.toContain("No editor open.");
});

test("a terminal taller than the old hardcoded 20-row default shows more than 20 lines (Issue #92 regression)", async () => {
// Before this fix, `EditorView`'s `viewportHeight` was never threaded
// from a live measurement at all (`editorView.tsx`'s pre-fix "Scope
// note on `viewportHeight`"): `EditorArea` rendered `<EditorView>` with
// no `viewportHeight` prop, so it always fell back to its own
// `DEFAULT_VIEWPORT_HEIGHT` constant (20) no matter how tall the real
// terminal was. This is the test that would have caught it: a document
// with 60 lines, rendered into a terminal comfortably taller than 20
// rows, must show lines well past index 20.
const { slotRegistry, layoutState, context } = createHarness();
await layoutState.ready;
const bigFile = Array.from({ length: 60 }, (_, i) => `line${i}`).join("\n");
const documents = createDocumentManager({
log: createHostLog(),
sink: createRecordingSink(),
fs: createInMemoryFs({ "/workspace/big.ts": bigFile }),
});

const { renderOnce, captureCharFrame } = await testRender(
<ThemeProvider>
<ContextFocusTracker context={context}>
<Shell slotRegistry={slotRegistry} layoutState={layoutState} documents={documents} />
</ContextFocusTracker>
</ThemeProvider>,
// 50 rows total; one open tab (3-row tab bar) + the always-on 1-row
// status bar is the only chrome in this fixture (no find widget, no
// panel — `panelVisible` defaults `false`), leaving 46 rows for the
// text plane (`viewport.test.ts`'s `computeEditorViewportHeight`
// covers that arithmetic in isolation; this test proves it actually
// reaches `EditorView` through `EditorArea`/`Shell`'s wiring).
{ width: 60, height: 50 },
);
await act(async () => {
await renderOnce();
});

await act(async () => {
await documents.openDocument(pathToUri("/workspace/big.ts"));
});
await act(async () => {
await renderOnce();
});

const frame = captureCharFrame();
expect(frame).toContain("line0");
// Line index 20 (the 21st line) is exactly one past the old hardcoded
// cap — the pre-fix render could never show it no matter how tall the
// terminal was.
expect(frame).toContain("line20");
// Line index 40 is well within the ~46-row viewport this fixture's
// chrome leaves available, and nowhere near reachable under the old
// fixed 20-row viewport.
expect(frame).toContain("line40");
});

test("the tab bar shows the dirty marker the instant a document is edited, and drops it once saved (Task 3.5, Req 6.5)", async () => {
const { slotRegistry, layoutState, context } = createHarness();
await layoutState.ready;
Expand Down
125 changes: 124 additions & 1 deletion packages/core/src/ui/shell.tsx
Original file line numberDiff line numberDiff line change
Expand Up@@ -49,6 +49,8 @@

import { basename } from "node:path";
import { useCallback, useEffect, useMemo, useReducer, useRef, useState, type ReactNode } from "react";
import { CliRenderEvents } from "@opentui/core";
import { useAppContext } from "@opentui/react";
import type { Disposable, SlotId, Uri } from "@tecode/api";
import type { CoreDocument } from "../buffer/document";
import type { DocumentManager } from "../buffer/documentManager";
Expand All@@ -68,6 +70,7 @@ import type { LayoutState, LayoutStateService } from "./layoutState";
import { INPUT_BOX_FOCUS_CONTEXT_KEY, QUICK_PICK_FOCUS_CONTEXT_KEY } from "./modalCommands";
import type { SidebarPair, SlotRegistry, SlotViewEntry } from "./slotRegistry";
import { toColorInput, useTheme } from "./theme";
import { computeEditorViewportHeight, type EditorAreaChrome } from "./viewport";

/* ------------------------------------------------------------------ */
/* Shared reactive-subscription hooks */
Expand DownExpand Up@@ -317,6 +320,78 @@ export function Sidebar(props: SidebarProps): ReactNode {
/* EditorArea (TabBar + EditorView) */
/* ------------------------------------------------------------------ */

/** Rows `Tabs`' `<tab-select>` (`components.tsx`, over `@opentui/core`'s
* `TabSelectRenderable`) occupies once tabs are shown (Issue #92, this
* module's `EditorArea` — its `tabs.length > 0` condition below decides
* WHETHER this constant applies, never a made-up literal that could drift
* from the real render). `TabSelectRenderable`'s own
* `calculateDynamicHeight(showUnderline, showDescription)` — both left at
* their `@opentui/core@0.1.107` library defaults, `true`/`true`, since
* `Tabs` never overrides either — computes `1` (tab row) + `1` (underline)
* + `1` (description row) = `3`; confirmed directly against the vendored
* headless renderer (a mounted `<tab-select>` with no explicit `height`
* measures exactly `3` rows), not merely read off the library's source. */
const TAB_BAR_HEIGHT = 3;

/** Rows `FindWidget`'s own outer box occupies while open (`findWidget.tsx`'s
* `style={{ height: 1 }}`) — Req 11.1. */
const FIND_WIDGET_HEIGHT = 1;

/** Rows `StatusBar` occupies (`shell.tsx`'s `StatusBar`, `style={{ height: 1
* }}`) — always rendered, so always reserved. */
const STATUS_BAR_HEIGHT = 1;

/**
* The live terminal's current row count (Issue #92; Req 6.5, 6.6, 13.1;
* design.md §8.3's `EditorView` `viewportHeight` scope note), reactively —
* the "optional dependency, `undefined` when unavailable" shape `theme.tsx`'s
* {@link useLiveTheme}/`focus.tsx`'s `useFocusTracking` already use, applied
* to `@opentui/react`'s live `CliRenderer` instead of a `ThemeService`/
* `ContextService`.
*
* **Why this reads `@opentui/react`'s `AppContext` directly, rather than
* calling that package's own `useTerminalDimensions`/`useOnResize`**:
* verified against the installed `@opentui/react@0.1.107`'s
* `src/hooks/use-renderer.ts` — EVERY ONE of `useRenderer`,
* `useTerminalDimensions`, and `useOnResize` calls `useRenderer()`
* internally, which THROWS (`"Renderer not found."`) the instant no live
* `CliRenderer` is mounted above them (`modalOverlay.tsx`'s `useRenderer`
* TSDoc already relies on that renderer always being present for ITS
* caller — `ModalOverlay` is unconditionally mounted at the composition
* root). `EditorArea` has no such guarantee: a caller/test that constructs
* it directly (outside `Shell`, outside a `testRender`/`renderShellToTerminal`
* tree) would crash outright. `useAppContext()` itself never throws — its
* `AppContext`'s own default value is `{ renderer: null }`
* (`@opentui/react`'s `src/components/app.tsx`) — so reading `renderer`
* through it and replicating `useTerminalDimensions`'s own
* seed-then-subscribe-to-`"resize"` logic by hand gets the exact same live
* behavior with a real fallback path instead of a crash.
*
* Returns `undefined` when no renderer is mounted — `EditorArea` below
* falls back to `EditorView`'s own `DEFAULT_VIEWPORT_HEIGHT` constant in
* that case (this task's "fall back to the existing constant"), exactly
* mirroring `useLiveTheme`'s `themeService === undefined` fallback.
*/
function useLiveTerminalHeight(): number | undefined {
const { renderer } = useAppContext();
const [height, setHeight] = useState<number | undefined>(renderer?.height);
useEffect(() => {
if (!renderer) return undefined;
// Re-syncs to whatever the renderer's height is RIGHT NOW before
// subscribing — closes the same subscribe-after-render race
// `useLiveTheme`'s TSDoc documents (a resize landing in the gap
// between this render and this effect running would otherwise be
// missed until some later, unrelated re-render).
setHeight(renderer.height);
const onResize = (_width: number, newHeight: number) => setHeight(newHeight);
renderer.on(CliRenderEvents.RESIZE, onResize);
return () => {
renderer.off(CliRenderEvents.RESIZE, onResize);
};
}, [renderer]);
return renderer ? height : undefined;
}

/** Props for {@link EditorArea}. */
export interface EditorAreaProps {
/** Open editor tabs — one editor group, N tabs (Req 6.5). Empty by
Expand DownExpand Up@@ -349,6 +424,19 @@ export interface EditorAreaProps {
* `findService`/`config` above: a caller/test that omits it gets
* `EditorView`'s current (unhighlighted) rendering unchanged. */
highlightService?: Pick<HighlightService, "getSpansForLine" | "onDidChange">;
/** Whether `Shell`'s bottom `Panel` is currently visible, and its height
* when it is (`layoutState.ts`'s `LayoutState.panelVisible`/
* `panelHeight`) — Issue #92. `Panel` is `EditorArea`'s SIBLING at the
* `Shell` level (design.md §8.1's component tree), not a descendant, but
* both sit in the same flex column above `StatusBar`, so `Panel`'s
* height still eats into the space left for `EditorArea` (and therefore
* `EditorView`'s text plane) to stretch into — the live-`viewportHeight`
* computation below needs both to size the text plane correctly. Omitted
* (a caller/test that constructs `EditorArea` directly, without `Shell`):
* treated as "no panel", matching every other optional-dependency
* fallback in this module. */
panelVisible?: boolean;
panelHeight?: number;
}

/** The editor area (Req 6.1, 6.5, 6.6, 11.1): a `TabBar` over the real
Expand DownExpand Up@@ -442,13 +530,45 @@ export interface EditorAreaProps {
* retried no matter WHICH of the four guards clears first — the command
* palette, an input box, the find widget, or the explorer sidebar — with
* no separate per-key wiring needed for each.
*
* **Sizing `EditorView`'s `viewportHeight` to the real terminal** (Issue
* #92 — "Only the first 20 lines are displayed" no matter how tall the
* terminal actually is): {@link useLiveTerminalHeight} reads the live
* terminal row count, {@link computeEditorViewportHeight} (`viewport.ts`)
* subtracts exactly the chrome THIS render actually draws — the tab bar
* (`tabs.length > 0`), the find widget (the same `find && isFindOpen &&
* props.findService` condition the JSX below uses to decide whether
* `<FindWidget>` renders at all), `Shell`'s sibling `Panel`
* (`props.panelVisible`/`panelHeight`), and `StatusBar` — and the result is
* threaded straight into `<EditorView>`'s `viewportHeight` prop. When no
* live terminal is available (a caller/test that constructs `EditorArea`
* outside a real/headless `CliRenderer`), `viewportHeight` is left
* `undefined` and `EditorView` falls back to its own
* `DEFAULT_VIEWPORT_HEIGHT` constant, unchanged from before this fix.
*/
export function EditorArea(props: EditorAreaProps): ReactNode {
const theme = useTheme();
const focusRef = useFocusTracking("editorFocus");
const tabs = props.tabs ?? [];
const find = props.activeEditorState?.find;
const isFindOpen = find?.isOpen ?? false;
// The exact same condition the JSX below uses to decide whether
// `<FindWidget>` renders at all (this component's TSDoc's "Sizing
// `EditorView`'s `viewportHeight`") — computed once and reused for both,
// so the chrome height calculation can never drift from what's actually
// drawn.
const findWidgetVisible = Boolean(find && isFindOpen && props.findService);

// Issue #92 — see this component's own TSDoc.
const terminalHeight = useLiveTerminalHeight();
const chrome: EditorAreaChrome = {
tabBar: tabs.length > 0 ? TAB_BAR_HEIGHT : 0,
findWidget: findWidgetVisible ? FIND_WIDGET_HEIGHT : 0,
panel: props.panelVisible ? (props.panelHeight ?? 0) : 0,
statusBar: STATUS_BAR_HEIGHT,
};
const viewportHeight =
terminalHeight !== undefined ? computeEditorViewportHeight(terminalHeight, chrome) : undefined;

const textPlaneNodeRef = useRef<FocusableNode | null>(null);
const wasFindOpenRef = useRef(false);
Expand DownExpand Up@@ -568,7 +688,7 @@ export function EditorArea(props: EditorAreaProps): ReactNode {
{tabs.length > 0 ? (
<Tabs tabs={tabs} activeId={props.activeTabId} onSelect={props.onSelectTab} />
) : null}
{find && isFindOpen && props.findService ? (
{findWidgetVisible && find && props.findService ? (
<FindWidget find={find} findService={props.findService} />
) : null}
<box style={{ flexDirection: "column", flexGrow: 1 }}>
Expand All@@ -581,6 +701,7 @@ export function EditorArea(props: EditorAreaProps): ReactNode {
key={props.activeDocument.uri}
document={props.activeDocument}
state={props.activeEditorState}
viewportHeight={viewportHeight}
config={props.config}
highlightService={props.highlightService}
onTextPlaneNode={handleTextPlaneNode}
Expand DownExpand Up@@ -935,6 +1056,8 @@ export function Shell(props: ShellProps): ReactNode {
config={props.config}
findService={props.findService}
highlightService={props.highlightService}
panelVisible={layout.panelVisible}
panelHeight={layout.panelHeight}
/>
</box>
<Panel slotRegistry={props.slotRegistry} visible={layout.panelVisible} height={layout.panelHeight} />
Expand Down
62 changes: 61 additions & 1 deletion packages/core/src/ui/viewport.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -5,7 +5,15 @@
*/

import { describe, expect, test } from "bun:test";
import { computeVisibleLineRange, gutterDigitWidth, revealLine } from "./viewport";
import {
computeEditorViewportHeight,
computeVisibleLineRange,
gutterDigitWidth,
revealLine,
type EditorAreaChrome,
} from "./viewport";

const NO_CHROME: EditorAreaChrome = { tabBar: 0, findWidget: 0, panel: 0, statusBar: 0 };

describe("computeVisibleLineRange (design.md §8.3's virtualized text layer)", () => {
test("a full window in the middle of a long document", () => {
Expand DownExpand Up@@ -102,3 +110,55 @@ describe("gutterDigitWidth (design.md §8.3's gutter width, 9/10/100 boundaries)
expect(gutterDigitWidth(-5)).toBe(1);
});
});

describe("computeEditorViewportHeight (Issue #92; design.md §8.1-§8.3)", () => {
test("no chrome at all: every terminal row goes to the text plane", () => {
expect(computeEditorViewportHeight(24, NO_CHROME)).toBe(24);
});

test("a tall terminal with typical chrome (tab bar + status bar) leaves well over 20 rows", () => {
// The exact scenario Issue #92 reports: a terminal much taller than the
// old hardcoded 20-row default, with just a tab bar (3) and status bar
// (1) drawn — the fix's whole point is that this stops being clamped
// to 20.
const chrome: EditorAreaChrome = { tabBar: 3, findWidget: 0, panel: 0, statusBar: 1 };
expect(computeEditorViewportHeight(50, chrome)).toBe(46);
expect(computeEditorViewportHeight(50, chrome)).toBeGreaterThan(20);
});

test("every chrome region present at once subtracts all four", () => {
const chrome: EditorAreaChrome = { tabBar: 3, findWidget: 1, panel: 10, statusBar: 1 };
expect(computeEditorViewportHeight(40, chrome)).toBe(25);
});

test("no tabs, no find, no panel: only the always-on status bar is reserved", () => {
const chrome: EditorAreaChrome = { ...NO_CHROME, statusBar: 1 };
expect(computeEditorViewportHeight(30, chrome)).toBe(29);
});

test("clamp: chrome consuming the entire terminal still yields at least 1 row, never 0 or negative", () => {
const chrome: EditorAreaChrome = { tabBar: 3, findWidget: 1, panel: 10, statusBar: 1 };
// Exactly as much chrome as terminal height (15 == 3+1+10+1): a naive
// `terminalHeight - consumed` would be 0.
expect(computeEditorViewportHeight(15, chrome)).toBe(1);
// Chrome taller than the terminal itself: a naive subtraction would go
// negative, which `computeVisibleLineRange` would treat as "empty
// range" (this module's own TSDoc) rather than a degenerate-but-usable
// 1-row viewport.
expect(computeEditorViewportHeight(5, chrome)).toBe(1);
expect(computeEditorViewportHeight(0, chrome)).toBe(1);
expect(computeEditorViewportHeight(-10, chrome)).toBe(1);
});

test("clamp at the very short terminals a real user's window could plausibly be", () => {
const chrome: EditorAreaChrome = { tabBar: 3, findWidget: 0, panel: 0, statusBar: 1 };
// A 4-row terminal with just tab bar + status bar chrome (4 rows) has
// nothing left over — still clamps to 1, not 0.
expect(computeEditorViewportHeight(4, chrome)).toBe(1);
expect(computeEditorViewportHeight(3, chrome)).toBe(1);
});

test("non-integer terminal heights are truncated, not rounded or left fractional", () => {
expect(computeEditorViewportHeight(24.9, NO_CHROME)).toBe(24);
});
});
Loading
Loading