mirror of
https://github.com/Ark0N/Codeman.git
synced 2026-10-04 22:49:41 +02:00
fix(terminal): stitch a wrapped line through the indent its continuation carries
An agent's numbered list wraps its URL, and the link opened a PREFIX of it:
1. https://github.com/users/someone/packages/container/p
ackage/thing
opened `…/container/p`. The provider already stitched hard wraps — Ink emits a real
newline, so nothing is flagged `isWrapped` and a row that fills the last column is
taken as continuing — but it joined the row texts VERBATIM, and the continuation
carries the list's own three-space indent. That whitespace lands in the middle of
the token, which is exactly where the URL pattern stops. Flush-left wrapped URLs
(Claude Code's own `/login`) worked, which is why this survived.
The touch-selection helpers had the shallower version of the same bug: they walked
`isWrapped` only, so `Line` grabbed the single row on screen rather than the
logical line, and a long-press on a wrapped token selected only its visible half.
So the reconstruction now lives in ONE place, `terminalLogicalLine` in
constants.js, and both consumers use it — the link provider matching patterns over
its text and the selection helpers measuring words and lines with it. A link that
spans a wrap and a `Line` that stops at the screen edge were the same bug twice.
The helper drops the leading whitespace of a HARD continuation (the program's
indent) and keeps that of a SOFT one (the emulator inserts nothing, so it is real
content), records the dropped width per segment so the offset↔cell mapping stays
exact in both directions, trims only the final row so earlier offsets stay aligned
to cells, and keeps the 12-row bound that stops a screenful of full-width output
from being re-scanned on every hover.
⚠️ Selection spans are computed in CELLS, not text offsets: an xterm selection is
one contiguous run, so a token spanning a hard wrap also covers the indent cells
between its halves. A run that skipped them cannot be expressed, and would not
match what is highlighted.
Tests: `test/terminal-logical-line.test.ts` (8 cases: the indent drop, resolving
from either row, both mapping directions, soft continuations kept verbatim, no
over-reach past a short row, the row bound, final-row trimming, a missing row) and
5 in `terminal-touch-tap.test.ts` (the whole URL from either row, a token selected
across the wrap, `Line` spanning both rows, no reach into the next line). Removing
either half of the fix reds 5 and 8 of them respectively.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
2e58da7479
commit
aae90599e5
@@ -0,0 +1,137 @@
|
||||
// Port: none (a pure function in a vm context — no browser, no server).
|
||||
//
|
||||
// `terminalLogicalLine` (constants.js) reconstructs the logical line a terminal row
|
||||
// belongs to. Two consumers depend on it and must not disagree: the link provider
|
||||
// matches its patterns over this text, and touch selection measures words and whole
|
||||
// lines with it.
|
||||
//
|
||||
// The bug it exists to fix, reported from a phone: an agent printing a numbered list
|
||||
//
|
||||
// 1. https://github.com/users/someone/packages/container/p
|
||||
// ackage/thing
|
||||
//
|
||||
// opened only `…/container/p`. Ink wraps its own output and emits a real newline, so
|
||||
// nothing is flagged `isWrapped`, and the continuation carries the list's indent —
|
||||
// joining the rows verbatim put whitespace in the middle of the URL, where the
|
||||
// pattern stops. "Line" was broken by the same shape: it walked `isWrapped` only, so
|
||||
// it grabbed the single row on screen.
|
||||
import { readFileSync } from 'node:fs';
|
||||
import { resolve } from 'node:path';
|
||||
import vm from 'node:vm';
|
||||
import { describe, expect, it } from 'vitest';
|
||||
|
||||
const SOURCE = readFileSync(resolve(import.meta.dirname, '../src/web/public/constants.js'), 'utf8');
|
||||
|
||||
type Row = { text: string; wrapped?: boolean };
|
||||
type Logical = {
|
||||
startRow: number;
|
||||
endRow: number;
|
||||
text: string;
|
||||
offsetToCell: (offset: number) => { row: number; col: number };
|
||||
cellToOffset: (row: number, col: number) => number;
|
||||
} | null;
|
||||
|
||||
const COLS = 60;
|
||||
|
||||
function load(): (buffer: unknown, row: number, cols: number, maxRows?: number) => Logical {
|
||||
const context = vm.createContext({ console, window: undefined });
|
||||
vm.runInContext(`${SOURCE}\nglobalThis.__fn = terminalLogicalLine;`, context, { filename: 'constants.js' });
|
||||
return (context as { __fn: never })['__fn'] as never;
|
||||
}
|
||||
|
||||
/** An xterm-shaped buffer: untrimmed rows pad to the full width, as xterm's do. */
|
||||
function buffer(rows: Row[]) {
|
||||
return {
|
||||
length: rows.length,
|
||||
getLine: (r: number) =>
|
||||
r >= 0 && r < rows.length
|
||||
? {
|
||||
isWrapped: !!rows[r].wrapped,
|
||||
translateToString: (trim?: boolean) => (trim === false ? rows[r].text.padEnd(COLS) : rows[r].text),
|
||||
}
|
||||
: undefined,
|
||||
};
|
||||
}
|
||||
|
||||
const terminalLogicalLine = load();
|
||||
const URL_RE = /https?:\/\/(?:[^\s"'<>|;&)\]\x00-\x1f]|&(?!&))+/;
|
||||
|
||||
describe('terminalLogicalLine', () => {
|
||||
// Row 0 runs to the last column — the only trace a hard wrap leaves — and row 1
|
||||
// carries the three spaces the list indent put there.
|
||||
const HEAD = '1. https://example.com/';
|
||||
const ROW0 = HEAD + 'a'.repeat(COLS - HEAD.length);
|
||||
const ROW1 = ' ackage/thing';
|
||||
const HARD = [{ text: ROW0 }, { text: ROW1 }];
|
||||
|
||||
it('stitches a hard wrap and drops the indent the continuation carries', () => {
|
||||
const line = terminalLogicalLine(buffer(HARD), 0, COLS)!;
|
||||
|
||||
expect(line.startRow).toBe(0);
|
||||
expect(line.endRow).toBe(1);
|
||||
expect(line.text).toBe(ROW0 + 'ackage/thing');
|
||||
// The whole URL now matches, which is the entire point.
|
||||
expect(line.text.match(URL_RE)![0]).toBe('https://example.com/' + 'a'.repeat(COLS - HEAD.length) + 'ackage/thing');
|
||||
});
|
||||
|
||||
it('finds the same line from the continuation row', () => {
|
||||
// A tap or hover lands on either row; both must resolve the whole thing.
|
||||
const line = terminalLogicalLine(buffer(HARD), 1, COLS)!;
|
||||
|
||||
expect([line.startRow, line.endRow]).toEqual([0, 1]);
|
||||
expect(line.text).toBe(ROW0 + 'ackage/thing');
|
||||
});
|
||||
|
||||
it('maps offsets back to the right cell across the dropped indent', () => {
|
||||
const line = terminalLogicalLine(buffer(HARD), 0, COLS)!;
|
||||
const at = line.text.indexOf('ackage/thing');
|
||||
|
||||
// 'a' of 'ackage' is the 4th cell of row 1 (0-based col 3), after the indent.
|
||||
expect(line.offsetToCell(at)).toEqual({ row: 1, col: 3 });
|
||||
// …and the reverse direction agrees.
|
||||
expect(line.cellToOffset(1, 3)).toBe(at);
|
||||
// Row 0 is unshifted.
|
||||
expect(line.offsetToCell(3)).toEqual({ row: 0, col: 3 });
|
||||
expect(line.cellToOffset(0, 3)).toBe(3);
|
||||
});
|
||||
|
||||
it('keeps a SOFT continuation verbatim, indent and all', () => {
|
||||
// The emulator inserts nothing when it wraps, so leading spaces there are real
|
||||
// content and dropping them would corrupt the text.
|
||||
const soft = [{ text: 'x'.repeat(COLS) }, { text: ' tail', wrapped: true }];
|
||||
const line = terminalLogicalLine(buffer(soft), 1, COLS)!;
|
||||
|
||||
expect(line.text).toBe('x'.repeat(COLS) + ' tail');
|
||||
expect(line.offsetToCell(COLS)).toEqual({ row: 1, col: 0 });
|
||||
});
|
||||
|
||||
it('does not stitch a row that stops short of the last column', () => {
|
||||
// A line that genuinely ended is not a wrap, and over-reaching would glue
|
||||
// unrelated output into one link.
|
||||
const rows = [{ text: 'short line' }, { text: 'next line' }];
|
||||
const line = terminalLogicalLine(buffer(rows), 0, COLS)!;
|
||||
|
||||
expect([line.startRow, line.endRow]).toEqual([0, 0]);
|
||||
expect(line.text).toBe('short line');
|
||||
});
|
||||
|
||||
it('bounds the span so a screenful of full-width output cannot be re-scanned per hover', () => {
|
||||
const rows = Array.from({ length: 40 }, () => ({ text: 'y'.repeat(COLS) }));
|
||||
const line = terminalLogicalLine(buffer(rows), 30, COLS, 4)!;
|
||||
|
||||
expect(line.endRow - line.startRow).toBeLessThanOrEqual(4);
|
||||
});
|
||||
|
||||
it('trims only the final row, so every earlier offset stays aligned to a cell', () => {
|
||||
const rows = [{ text: 'z'.repeat(COLS) }, { text: 'tail' }];
|
||||
const line = terminalLogicalLine(buffer(rows), 0, COLS)!;
|
||||
|
||||
// Row 0 contributes exactly COLS characters; the last row is trimmed.
|
||||
expect(line.text).toBe('z'.repeat(COLS) + 'tail');
|
||||
expect(line.offsetToCell(COLS)).toEqual({ row: 1, col: 0 });
|
||||
});
|
||||
|
||||
it('answers null for a row that does not exist', () => {
|
||||
expect(terminalLogicalLine(buffer([{ text: 'a' }]), 5, COLS)).toBeNull();
|
||||
});
|
||||
});
|
||||
@@ -1094,3 +1094,91 @@ describe('terminal touch selection', () => {
|
||||
expect(app._ensureTouchSelectionBar()).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
describe('terminal wrapped-line handling', () => {
|
||||
// Reported from a phone against the shipped fix: an agent's numbered list wraps its
|
||||
// URL, and tapping it opened only the part on screen. Ink emits a real newline (so
|
||||
// nothing is flagged isWrapped) and indents the continuation under the list marker,
|
||||
// so joining the rows verbatim put whitespace inside the URL. "Line" broke the same
|
||||
// way, grabbing the one visible row.
|
||||
//
|
||||
// Grid: 80 cols, 8×16 cells, screen rect at (0,0), viewportY 0.
|
||||
const COLS = 80;
|
||||
const at = (index: number, row = 0) => ({ clientX: index * 8 + 4, clientY: row * 16 + 8 });
|
||||
const press = (app: any, index: number, row = 0) =>
|
||||
app._beginTouchSelection(at(index, row).clientX, at(index, row).clientY);
|
||||
const HEAD = '1. https://example.com/';
|
||||
// Row 0 runs to the last column, which is the only trace a hard wrap leaves.
|
||||
const ROW0 = HEAD + 'a'.repeat(COLS - HEAD.length);
|
||||
const ROW1 = ' ackage/thing';
|
||||
const FULL_URL = 'https://example.com/' + 'a'.repeat(COLS - HEAD.length) + 'ackage/thing';
|
||||
|
||||
function wrappedHarness() {
|
||||
const { app, windowRef } = loadTerminalUiHarness();
|
||||
app.activeSessionId = 'sess-1';
|
||||
app.sessions = new Map([['sess-1', { mode: 'claude' }]]);
|
||||
app._sendInputAsync = vi.fn();
|
||||
app.terminal = createTerminalGrid([ROW0, ROW1, '❯ '], 2);
|
||||
app.terminal.registerLinkProvider = vi.fn();
|
||||
app.openFilePreview = vi.fn();
|
||||
app.openLogViewerWindow = vi.fn();
|
||||
app._isExternalPreviewPath = () => false;
|
||||
windowRef.open = vi.fn();
|
||||
app.registerFilePathLinkProvider();
|
||||
return { app, windowRef, select: app.terminal.select as ReturnType<typeof vi.fn> };
|
||||
}
|
||||
|
||||
it('opens the WHOLE wrapped URL, not the part on screen', () => {
|
||||
const { app, windowRef } = wrappedHarness();
|
||||
|
||||
expect(app._handleMobileTerminalTap(at(HEAD.length + 5), false, 'content')).toBe('link');
|
||||
expect(windowRef.open).toHaveBeenCalledWith(FULL_URL, '_blank', 'noopener,noreferrer');
|
||||
});
|
||||
|
||||
it('opens the whole URL from the continuation row too', () => {
|
||||
// Tapping the second half is the natural gesture when that is what you can see.
|
||||
const { app, windowRef } = wrappedHarness();
|
||||
|
||||
expect(app._handleMobileTerminalTap(at(5, 1), false, 'content')).toBe('link');
|
||||
expect(windowRef.open).toHaveBeenCalledWith(FULL_URL, '_blank', 'noopener,noreferrer');
|
||||
});
|
||||
|
||||
it('selects a token that spans the wrap, across both rows', () => {
|
||||
const { app, select } = wrappedHarness();
|
||||
|
||||
press(app, 5, 1); // inside 'ackage/thing' on the continuation row
|
||||
// From the URL's first cell (row 0, col 3) through the token's last cell
|
||||
// (row 1, col 14). The run covers the indent cells between the halves, because
|
||||
// an xterm selection is one contiguous run and a gap cannot be expressed.
|
||||
const index = 3;
|
||||
const end = COLS + ROW1.length - 1;
|
||||
expect(select).toHaveBeenCalledWith(3, 0, end - index + 1);
|
||||
});
|
||||
|
||||
it('Line takes every row of a HARD-wrapped line, not just the visible one', () => {
|
||||
const { app, select } = wrappedHarness();
|
||||
press(app, HEAD.length + 5);
|
||||
select.mockClear();
|
||||
|
||||
app._selectTouchSelectionLine();
|
||||
|
||||
// Row 0 col 0 through row 1's last non-blank cell.
|
||||
expect(select).toHaveBeenCalledWith(0, 0, COLS + ROW1.length);
|
||||
});
|
||||
|
||||
it('does not reach into the next line when a row stops short of the edge', () => {
|
||||
// Over-reaching would glue unrelated output into one link or one "Line".
|
||||
const { app, select } = loadTerminalUiHarness();
|
||||
void select;
|
||||
app.activeSessionId = 'sess-1';
|
||||
app.sessions = new Map([['sess-1', { mode: 'claude' }]]);
|
||||
app.terminal = createTerminalGrid(['short output', 'https://example.com/next', '❯ '], 2);
|
||||
app.terminal.registerLinkProvider = vi.fn();
|
||||
app.registerFilePathLinkProvider();
|
||||
|
||||
press(app, 2); // inside 'short'
|
||||
app._selectTouchSelectionLine();
|
||||
|
||||
expect(app.terminal.select).toHaveBeenCalledWith(0, 0, 'short output'.length);
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user