mirror of
https://github.com/Ark0N/Codeman.git
synced 2026-10-09 16:59:43 +02:00
fix(split): Pane B and its PTY never disagree about size (#464)
- Font size, family and weight changes refit Pane B AND tell its PTY. They used to reflow the xterm only, leaving the CLI wrapping at the old column count, the garbled-redraw class #464 fixed for the primary pane. - The resize frame reports the size the xterm actually holds, with no 40x10 floor (the divider's 20% clamp leaves about 28 columns), skips an unchanged size, and is always re-sent on a fresh socket so it re-registers as a desktop viewer. - The server's {t:'zc'} geometry report is handled: a different column count is adopted, rows stay local, using the primary pane's own reconcilePtyGeometry verdict. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -107,6 +107,11 @@
|
|||||||
this._stoppedCode = null;
|
this._stoppedCode = null;
|
||||||
this._markerText = TerminalTile.MARKER_RECONNECTING;
|
this._markerText = TerminalTile.MARKER_RECONNECTING;
|
||||||
this.onExit = typeof opts.onExit === 'function' ? opts.onExit : null;
|
this.onExit = typeof opts.onExit === 'function' ? opts.onExit : null;
|
||||||
|
// The `{ cols, rows }` last sent in a `{t:'z'}` frame, so an unchanged size
|
||||||
|
// is not resent (each one costs a `tmux resize-window` and a SIGWINCH).
|
||||||
|
// Cleared on every open: a fresh socket must announce its size, which is
|
||||||
|
// also what registers it as a desktop viewer server-side.
|
||||||
|
this._lastSentDims = null;
|
||||||
}
|
}
|
||||||
|
|
||||||
async connect() {
|
async connect() {
|
||||||
@@ -325,6 +330,8 @@
|
|||||||
} else if (msg.t === 'ia') {
|
} else if (msg.t === 'ia') {
|
||||||
// Input ACK. The frame names no session, so it is this pane's.
|
// Input ACK. The frame names no session, so it is this pane's.
|
||||||
global.app?._onWsInputAck?.(msg.seq, msg, this.sessionId);
|
global.app?._onWsInputAck?.(msg.seq, msg, this.sessionId);
|
||||||
|
} else if (msg.t === 'zc') {
|
||||||
|
this._onPtyGeometryReport(msg.c, msg.r);
|
||||||
}
|
}
|
||||||
} catch {
|
} catch {
|
||||||
/* Malformed frame — ignore, matches primary pane's tolerance. */
|
/* Malformed frame — ignore, matches primary pane's tolerance. */
|
||||||
@@ -377,6 +384,7 @@
|
|||||||
this._wsClosed = false;
|
this._wsClosed = false;
|
||||||
this._markerOwed = false;
|
this._markerOwed = false;
|
||||||
this._reconnectAttempts = 0;
|
this._reconnectAttempts = 0;
|
||||||
|
this._lastSentDims = null;
|
||||||
this._registerInputSocket();
|
this._registerInputSocket();
|
||||||
this._sendResize();
|
this._sendResize();
|
||||||
if (reconnected) this._refreshBuffer();
|
if (reconnected) this._refreshBuffer();
|
||||||
@@ -757,27 +765,55 @@
|
|||||||
this.fitAddon.fit();
|
this.fitAddon.fit();
|
||||||
}
|
}
|
||||||
|
|
||||||
fit() {
|
// Reflow to the container and tell the PTY, as one step: the xterm and the
|
||||||
|
// PTY must never disagree about size (#464), and a font change is a size
|
||||||
|
// change too, so the font setters call this rather than localFit().
|
||||||
|
// `force` resends an unchanged size.
|
||||||
|
fit({ force = false } = {}) {
|
||||||
this.localFit();
|
this.localFit();
|
||||||
this._sendResize();
|
this._sendResize({ force });
|
||||||
}
|
}
|
||||||
|
|
||||||
_sendResize() {
|
_sendResize({ force = false } = {}) {
|
||||||
if (!this._wsReady || !this.fitAddon) return;
|
if (!this._wsReady || !this.fitAddon || !this.terminal) return;
|
||||||
// One PTY cannot hold two sizes (mirrors sendResize's own
|
// One PTY cannot hold two sizes (mirrors sendResize's own
|
||||||
// detachedElsewhere yield in terminal-ui.js): the session got detached
|
// detachedElsewhere yield in terminal-ui.js): the session got detached
|
||||||
// to its own window AFTER this split was opened, so its own window now
|
// to its own window AFTER this split was opened, so its own window now
|
||||||
// owns the PTY's size and Pane B must stand aside.
|
// owns the PTY's size and Pane B must stand aside.
|
||||||
if (this.detachedSessions?.has(this.sessionId)) return;
|
if (this.detachedSessions?.has(this.sessionId)) return;
|
||||||
|
// A hidden pane (a web tab over it, a zoomed neighbour) measures NaN, and
|
||||||
|
// fit() then leaves the xterm alone: there is no size worth reporting.
|
||||||
const dims = this.fitAddon.proposeDimensions();
|
const dims = this.fitAddon.proposeDimensions();
|
||||||
if (!dims) return;
|
if (!dims || !Number.isFinite(dims.cols) || !Number.isFinite(dims.rows)) return;
|
||||||
// Send the real proposed dimensions unclamped, matching the primary
|
// Report what the xterm actually holds, so the PTY gets exactly the size
|
||||||
// pane's convention (terminal-ui.js's getTerminalDimensions()) — the
|
// the pane renders at. Unclamped, unlike the primary pane's 40x10 floor:
|
||||||
// server enforces its own valid range ([1,500]/[1,200] in ws-routes.ts).
|
// a floor here misreported Pane B's width at the divider's reachable 20%
|
||||||
// A 40/10 floor here misreported Pane B's real width to the PTY at the
|
// position (about 28 columns), causing real output-wrapping bugs, and a
|
||||||
// divider's own reachable 20% floor position, causing real
|
// floored xterm would be wider than its container. The server enforces
|
||||||
// output-wrapping bugs.
|
// its own valid range ([1,500]/[1,200] in ws-routes.ts).
|
||||||
this.ws.send(JSON.stringify({ t: 'z', c: dims.cols, r: dims.rows, v: 'desktop' }));
|
const cols = this.terminal.cols;
|
||||||
|
const rows = this.terminal.rows;
|
||||||
|
const last = this._lastSentDims;
|
||||||
|
if (!force && last && last.cols === cols && last.rows === rows) return;
|
||||||
|
this._lastSentDims = { cols, rows };
|
||||||
|
this.ws.send(JSON.stringify({ t: 'z', c: cols, r: rows, v: 'desktop' }));
|
||||||
|
}
|
||||||
|
|
||||||
|
// The geometry the PTY actually holds (`{t:'zc'}`, the server's answer to
|
||||||
|
// every resize). A PTY and a terminal that disagree on WIDTH render
|
||||||
|
// garbled, so a different column count is adopted; rows stay local, as in
|
||||||
|
// the primary pane (_onPtyGeometryReport in terminal-ui.js, #464). The
|
||||||
|
// pure verdict is the primary's too (reconcilePtyGeometry, constants.js).
|
||||||
|
_onPtyGeometryReport(cols, rows) {
|
||||||
|
const terminal = this.terminal;
|
||||||
|
if (!terminal) return;
|
||||||
|
const verdict = global.CodemanTerminalGeometry?.reconcilePtyGeometry?.(
|
||||||
|
{ cols: terminal.cols, rows: terminal.rows },
|
||||||
|
{ cols, rows }
|
||||||
|
);
|
||||||
|
if (!verdict?.adopt) return;
|
||||||
|
terminal.resize(verdict.cols, terminal.rows);
|
||||||
|
this._lastSentDims = { cols: verdict.cols, rows: terminal.rows };
|
||||||
}
|
}
|
||||||
|
|
||||||
destroy() {
|
destroy() {
|
||||||
|
|||||||
@@ -5735,7 +5735,7 @@ Object.assign(CodemanApp.prototype, {
|
|||||||
this._predictiveEcho?.refreshFont();
|
this._predictiveEcho?.refreshFont();
|
||||||
this._forEachTile?.((tile) => {
|
this._forEachTile?.((tile) => {
|
||||||
tile.terminal.options.fontSize = size;
|
tile.terminal.options.fontSize = size;
|
||||||
tile.localFit();
|
tile.fit(); // a font change is a size change: tell its PTY too (#464)
|
||||||
});
|
});
|
||||||
},
|
},
|
||||||
|
|
||||||
@@ -5764,7 +5764,7 @@ Object.assign(CodemanApp.prototype, {
|
|||||||
this._predictiveEcho?.refreshFont();
|
this._predictiveEcho?.refreshFont();
|
||||||
this._forEachTile?.((tile) => {
|
this._forEachTile?.((tile) => {
|
||||||
tile.terminal.options.fontFamily = resolved;
|
tile.terminal.options.fontFamily = resolved;
|
||||||
tile.localFit();
|
tile.fit(); // a font change is a size change: tell its PTY too (#464)
|
||||||
});
|
});
|
||||||
},
|
},
|
||||||
|
|
||||||
@@ -5820,7 +5820,7 @@ Object.assign(CodemanApp.prototype, {
|
|||||||
this._forEachTile?.((tile) => {
|
this._forEachTile?.((tile) => {
|
||||||
tile.terminal.options.fontWeight = fontWeight;
|
tile.terminal.options.fontWeight = fontWeight;
|
||||||
tile.terminal.options.fontWeightBold = fontWeightBold;
|
tile.terminal.options.fontWeightBold = fontWeightBold;
|
||||||
tile.localFit();
|
tile.fit(); // a font change is a size change: tell its PTY too (#464)
|
||||||
});
|
});
|
||||||
},
|
},
|
||||||
|
|
||||||
|
|||||||
@@ -67,7 +67,7 @@ function makeApp(
|
|||||||
opts: {
|
opts: {
|
||||||
teammates?: number;
|
teammates?: number;
|
||||||
terminal?: ReturnType<typeof fakeTerminal> | null;
|
terminal?: ReturnType<typeof fakeTerminal> | null;
|
||||||
splitPane?: { terminal: ReturnType<typeof fakeTerminal>; localFit: () => void } | null;
|
splitPane?: { terminal: ReturnType<typeof fakeTerminal>; fit: () => void } | null;
|
||||||
} = {}
|
} = {}
|
||||||
) {
|
) {
|
||||||
const fit = vi.fn();
|
const fit = vi.fn();
|
||||||
@@ -108,10 +108,12 @@ function makeApp(
|
|||||||
}
|
}
|
||||||
|
|
||||||
describe('applyTerminalFontWeights', () => {
|
describe('applyTerminalFontWeights', () => {
|
||||||
it('reaches an open split pane: same weights, refit in place', () => {
|
it('reaches an open split pane: same weights, refit AND reported to its PTY', () => {
|
||||||
const splitTerminal = fakeTerminal();
|
const splitTerminal = fakeTerminal();
|
||||||
const localFit = vi.fn();
|
// fit(), not localFit(): a weight change can move the cell size, and the
|
||||||
const { app } = makeApp({ splitPane: { terminal: splitTerminal, localFit } });
|
// split pane's PTY must hear about the new geometry like the primary's does.
|
||||||
|
const fit = vi.fn();
|
||||||
|
const { app } = makeApp({ splitPane: { terminal: splitTerminal, fit } });
|
||||||
|
|
||||||
(app as unknown as { applyTerminalFontWeights: (s: unknown) => void }).applyTerminalFontWeights({
|
(app as unknown as { applyTerminalFontWeights: (s: unknown) => void }).applyTerminalFontWeights({
|
||||||
terminalFontWeight: '300',
|
terminalFontWeight: '300',
|
||||||
@@ -120,7 +122,7 @@ describe('applyTerminalFontWeights', () => {
|
|||||||
|
|
||||||
expect(splitTerminal.options.fontWeight).toBe(300);
|
expect(splitTerminal.options.fontWeight).toBe(300);
|
||||||
expect(splitTerminal.options.fontWeightBold).toBe(700);
|
expect(splitTerminal.options.fontWeightBold).toBe(700);
|
||||||
expect(localFit).toHaveBeenCalledTimes(1);
|
expect(fit).toHaveBeenCalledTimes(1);
|
||||||
});
|
});
|
||||||
|
|
||||||
it('writes both slots to the live terminal', () => {
|
it('writes both slots to the live terminal', () => {
|
||||||
|
|||||||
@@ -58,6 +58,20 @@ class FakeSocket {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/** The fit addon: proposes `FakeFit.proposed` and, like the real one, resizes to it (NaN = hidden pane). */
|
||||||
|
class FakeFit {
|
||||||
|
static proposed = { cols: 80, rows: 24 };
|
||||||
|
term: FakeTerminal | null = null;
|
||||||
|
fit() {
|
||||||
|
const { cols, rows } = FakeFit.proposed;
|
||||||
|
if (!Number.isFinite(cols) || !Number.isFinite(rows)) return;
|
||||||
|
this.term?.resize(cols, rows);
|
||||||
|
}
|
||||||
|
proposeDimensions() {
|
||||||
|
return { ...FakeFit.proposed };
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
class FakeTerminal {
|
class FakeTerminal {
|
||||||
static last: FakeTerminal | null = null;
|
static last: FakeTerminal | null = null;
|
||||||
options: Record<string, unknown>;
|
options: Record<string, unknown>;
|
||||||
@@ -69,7 +83,9 @@ class FakeTerminal {
|
|||||||
this.options = { ...options };
|
this.options = { ...options };
|
||||||
FakeTerminal.last = this;
|
FakeTerminal.last = this;
|
||||||
}
|
}
|
||||||
loadAddon() {}
|
loadAddon(addon: FakeFit) {
|
||||||
|
addon.term = this;
|
||||||
|
}
|
||||||
open() {}
|
open() {}
|
||||||
onData(cb: (data: string) => void) {
|
onData(cb: (data: string) => void) {
|
||||||
this.dataCb = cb;
|
this.dataCb = cb;
|
||||||
@@ -84,7 +100,9 @@ class FakeTerminal {
|
|||||||
clear() {
|
clear() {
|
||||||
this.writes.push('<CLEAR>');
|
this.writes.push('<CLEAR>');
|
||||||
}
|
}
|
||||||
|
resizes: Array<[number, number]> = [];
|
||||||
resize(cols: number, rows: number) {
|
resize(cols: number, rows: number) {
|
||||||
|
this.resizes.push([cols, rows]);
|
||||||
this.cols = cols;
|
this.cols = cols;
|
||||||
this.rows = rows;
|
this.rows = rows;
|
||||||
}
|
}
|
||||||
@@ -116,14 +134,7 @@ function loadContext() {
|
|||||||
HTMLCanvasElement: class HTMLCanvasElement {},
|
HTMLCanvasElement: class HTMLCanvasElement {},
|
||||||
WebSocket: FakeSocket,
|
WebSocket: FakeSocket,
|
||||||
Terminal: FakeTerminal,
|
Terminal: FakeTerminal,
|
||||||
FitAddon: {
|
FitAddon: { FitAddon: FakeFit },
|
||||||
FitAddon: class {
|
|
||||||
fit() {}
|
|
||||||
proposeDimensions() {
|
|
||||||
return { cols: 80, rows: 24 };
|
|
||||||
}
|
|
||||||
},
|
|
||||||
},
|
|
||||||
fetch: (...args: unknown[]) => fetchMock(...args),
|
fetch: (...args: unknown[]) => fetchMock(...args),
|
||||||
location: { protocol: 'http:', host: 'codeman.test' },
|
location: { protocol: 'http:', host: 'codeman.test' },
|
||||||
document: { addEventListener: vi.fn(), documentElement: { dataset: {} } },
|
document: { addEventListener: vi.fn(), documentElement: { dataset: {} } },
|
||||||
@@ -176,6 +187,8 @@ type Tile = {
|
|||||||
connect(): Promise<void>;
|
connect(): Promise<void>;
|
||||||
destroy(): void;
|
destroy(): void;
|
||||||
reconnectNow(): void;
|
reconnectNow(): void;
|
||||||
|
fit(opts?: { force?: boolean }): void;
|
||||||
|
detachedSessions?: Set<string>;
|
||||||
ws: FakeSocket | null;
|
ws: FakeSocket | null;
|
||||||
_reconnectAttempts: number;
|
_reconnectAttempts: number;
|
||||||
};
|
};
|
||||||
@@ -198,6 +211,7 @@ afterEach(() => {
|
|||||||
});
|
});
|
||||||
|
|
||||||
beforeEach(() => {
|
beforeEach(() => {
|
||||||
|
FakeFit.proposed = { cols: 80, rows: 24 };
|
||||||
FakeSocket.instances = [];
|
FakeSocket.instances = [];
|
||||||
fetchMock.mockReset();
|
fetchMock.mockReset();
|
||||||
fetchMock.mockImplementation(async () => ({
|
fetchMock.mockImplementation(async () => ({
|
||||||
@@ -493,3 +507,93 @@ describe('TerminalTile destroy()', () => {
|
|||||||
expect(onExit).not.toHaveBeenCalled();
|
expect(onExit).not.toHaveBeenCalled();
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
describe('TerminalTile geometry (#464: the pane and its PTY never disagree)', () => {
|
||||||
|
const resizeFrames = (ws: FakeSocket) => ws.sent.filter((f) => f.t === 'z');
|
||||||
|
|
||||||
|
it('announces its size as a desktop viewer when the socket opens', async () => {
|
||||||
|
const { ws } = await connectTile(makeApp());
|
||||||
|
ws.open();
|
||||||
|
|
||||||
|
expect(resizeFrames(ws)).toEqual([{ t: 'z', c: 80, r: 24, v: 'desktop' }]);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('does not resend an unchanged size, sends a changed one, and force resends', async () => {
|
||||||
|
const { tile, ws } = await connectTile(makeApp());
|
||||||
|
ws.open();
|
||||||
|
|
||||||
|
tile.fit();
|
||||||
|
expect(resizeFrames(ws)).toHaveLength(1);
|
||||||
|
|
||||||
|
FakeFit.proposed = { cols: 100, rows: 30 };
|
||||||
|
tile.fit();
|
||||||
|
expect(resizeFrames(ws).at(-1)).toEqual({ t: 'z', c: 100, r: 30, v: 'desktop' });
|
||||||
|
|
||||||
|
tile.fit({ force: true });
|
||||||
|
expect(resizeFrames(ws)).toHaveLength(3);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('re-announces an unchanged size on a reconnected socket', async () => {
|
||||||
|
vi.useFakeTimers();
|
||||||
|
const { ws } = await connectTile(makeApp());
|
||||||
|
ws.open();
|
||||||
|
ws.onclose?.({ code: 1006 });
|
||||||
|
await vi.advanceTimersByTimeAsync(300);
|
||||||
|
const ws2 = FakeSocket.instances[1];
|
||||||
|
|
||||||
|
ws2.open();
|
||||||
|
|
||||||
|
expect(resizeFrames(ws2)).toEqual([{ t: 'z', c: 80, r: 24, v: 'desktop' }]);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('applies no 40-column floor: a pane at the divider clamp gets its real width on both sides', async () => {
|
||||||
|
const { tile, ws, term } = await connectTile(makeApp());
|
||||||
|
ws.open();
|
||||||
|
|
||||||
|
FakeFit.proposed = { cols: 28, rows: 30 };
|
||||||
|
tile.fit();
|
||||||
|
|
||||||
|
expect(term.cols).toBe(28);
|
||||||
|
expect(resizeFrames(ws).at(-1)).toEqual({ t: 'z', c: 28, r: 30, v: 'desktop' });
|
||||||
|
});
|
||||||
|
|
||||||
|
it('reports nothing while hidden (the fit addon measures NaN)', async () => {
|
||||||
|
const { tile, ws, term } = await connectTile(makeApp());
|
||||||
|
ws.open();
|
||||||
|
|
||||||
|
FakeFit.proposed = { cols: NaN, rows: NaN };
|
||||||
|
tile.fit();
|
||||||
|
|
||||||
|
expect(resizeFrames(ws)).toHaveLength(1);
|
||||||
|
expect([term.cols, term.rows]).toEqual([80, 24]);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('stands aside for a session detached into its own window', async () => {
|
||||||
|
const { tile, ws } = await connectTile(makeApp(), { detachedSessions: new Set(['s-tile']) });
|
||||||
|
ws.open();
|
||||||
|
FakeFit.proposed = { cols: 120, rows: 40 };
|
||||||
|
|
||||||
|
tile.fit();
|
||||||
|
|
||||||
|
expect(resizeFrames(ws)).toEqual([]);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('adopts the column count the PTY reports, keeping its own rows', async () => {
|
||||||
|
const { ws, term } = await connectTile(makeApp());
|
||||||
|
ws.open();
|
||||||
|
|
||||||
|
ws.receive({ t: 'zc', c: 132, r: 50 });
|
||||||
|
|
||||||
|
expect([term.cols, term.rows]).toEqual([132, 24]);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('leaves the pane alone when the PTY agrees on width', async () => {
|
||||||
|
const { ws, term } = await connectTile(makeApp());
|
||||||
|
ws.open();
|
||||||
|
const before = term.resizes.length;
|
||||||
|
|
||||||
|
ws.receive({ t: 'zc', c: 80, r: 60 });
|
||||||
|
|
||||||
|
expect(term.resizes.length).toBe(before);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|||||||
Reference in New Issue
Block a user