mirror of
https://github.com/Ark0N/Codeman.git
synced 2026-10-20 09:09:35 +02:00
fix(tiles): Redraw on a tile sends the forced resize and reports only what it sent
Redraw (Ctrl+Shift+R and the header button, restoreTerminalSize) on a
focused tile or the split's Pane B called tile.fit({ force: true }) and
always toasted "Terminal restored to CxR". In TerminalTile._sendResize,
force only skipped the client-side dedupe: the frame carried no `f`, so
Session.resize skipped a size equal to the one it last applied and the
server did nothing. And _sendResize returned silently with the socket down
or the session popped out to its own window, while the toast still
claimed success.
Both halves are fixed, the first as parity with the primary pane:
- a forced fit now sends `f: true`, the flag the primary's sendResize sets,
which the server honours (ws-routes reads msg.f, Session.resize then runs
tmux resize-window and the PTY resize at the same size);
- fit() and _sendResize() return whether a frame went out, and
restoreTerminalSize toasts success only then. Otherwise it says why, as
the primary branch does: "sized by its own window" for a detached
session, "not connected" while the tile's socket is down (it announces
its size again on reopen), and the primary's "Could not determine
terminal size" for a pane that measured nothing.
What this does not claim: a forced resize to the size the PTY already has
changes no geometry, so it is not a cure for a garbled tile whose PTY
already matches; the primary pane's forced resize has the same limit. It
matters when the server's recorded size has drifted from the tmux window.
Tests: the forced frame carries f:true (and plain ones do not), fit()'s
return value on send, dedupe, detached and closed-socket paths, and Redraw
end to end on a real tile (sent, socket down, popped out), plus the three
no-success toasts in focused-pane-shortcuts.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This commit is contained in:
1 parent
eb5d982c38
commit
cdb3c34ae0
5 files changed
+138
-16
No files matched your search
@@ -801,7 +801,7 @@ Further detail: with many sessions the horizontal strip stops being scannable, w
|
||||
|
||||
### Split-pane sessions
|
||||
|
||||
**Split-pane sessions** (`showSplitButton`, header button, default OFF, per-device like `showFileViewerButton` — not in `SettingsUpdateSchema`, `displayKeys` in settings-ui.js): shows two live sessions side-by-side in one Codeman window. Pane A is the untouched, existing singleton terminal (`this.terminal`/`this._ws` in terminal-ui.js); Pane B is an independent `TerminalTile` (terminal-tile.js, constructed by the orchestration in terminal-split.js) with its own xterm instance and its own `/ws/sessions/:id/terminal` WebSocket, whose `cid` is the tab's identity with a `:tile` suffix, so it can never supersede the primary pane's socket (the registry supersedes by cid per session). ⚠️ **Pane B is deliberately plainer than Pane A** — no local-echo overlay, no CJK IME, no touch/mobile handlers, no keyboard accessory bar (it does carry the primary pane's hollow-buffer wheel paging and desktop click report, through terminal-ui.js's own gates aimed at the tile, and its own keyCode-229 soft-keyboard controller from terminal-keycode229-recovery.js, the #441 next-keydown drain and #541's edit-based diff for Android autocorrect, since the 1180px width gate is reachable by a wide Android tablet; see terminal-tile.js's fileoverview) — since this is a desktop-only feature (a split view needs a wide viewport) and those features exist for mobile/touch input; `.btn-split` is hard-hidden below 1180px regardless of the setting by the `@media (max-width: 1179px)` rule in styles.css (mobile.css only carries a comment pointing at it: that file loads up to 1023px, so it cannot cover the 1024-1179px tablet range the feature also needs to stay off), and the per-device setting means turning it on at a desk can never sync it onto a phone in the first place. No persistence: closing the browser tab or reloading always returns to the normal single-pane view; there is no localStorage key for split state. ⚠️ Splitting a session against itself is disallowed (the picker excludes the active session), **as is splitting against a popped-out (detached) session** — `buildSplitPickerSessions()` excludes `detachedSessions` because a detached session's own window is already claiming its PTY size, and `MAX_WS_PER_SESSION` needs no change since Pane A/B are always two different sessions. ⚠️ Either pane's session ending (deleted locally or from another client) auto-collapses the split — Pane A's session ending promotes Pane B to the new single pane via `selectSession(id, { auto: true })` (an app-driven selection, so it must not spend the session's idle alert — see the Approvals Inbox note above), never by trying to hot-swap the lightweight `TerminalTile` object into the primary singleton state. ⚠️ Pane B refits on every window/sidebar/tab-rail resize via the SAME trailing-edge `ResizeObserver` callback that resizes Pane A (`throttledResize` in terminal-ui.js) — it only ever measured Pane A's own container, so without the explicit `this._forEachTile?.((tile) => tile.fit(), { grid: false })` call there (grid tiles are left out: the grid's own observer refits them on the same resize) Pane B silently kept its stale PTY size through every resize that did not happen to be a divider drag. ⚠️ A dropped WebSocket writes a `[disconnected, reconnecting…]` marker into Pane B and reconnects on the primary pane's backoff ladder (`CodemanWsReconnect` plus jitter; the attempt count resets only on a successful open), then refreshes the buffer to close the output gap. ⚠️ The open handler clears `_wsClosed`/`_markerOwed` BEFORE that refresh, or the refresh re-owes the marker and stamps it under a healthy pane. ⚠️ 4003/4004/4009/4010 stop the pane for good, report once through `onExit(code)` and write a marker saying why (`CodemanWsReconnect` alone would retry 4003). ⚠️ A replacement socket detaches the old one first and every handler ignores a socket that is no longer `this.ws`, so a late 4010 from a superseded socket cannot stop its successor; `destroy()` cancels a pending reconnect. Related but distinct: `detachSession()` already opens one session in a separate OS-level browser window (`isSoloWindow`) — that is prior art for "two sessions visible at once" but not for one window with a draggable in-page divider, which is what this feature adds. ⚠️ Pane B installs its own `attachCustomKeyEventHandler` gating the same app-level chords Pane A's own handler gates (command palette, Alt+1-9/[/] tab nav, Alt+B sidebar toggle, Ctrl+Z suspend, Shift/Ctrl+Enter newline, smart-copy Ctrl+C) — without it the document capture-phase handler's `preventDefault()` (which never stops xterm) let each chord ALSO write its raw byte/escape sequence into Pane B's live PTY on top of whatever the app action did to Pane A (COD-153). Ctrl+Z is swallowed unless Pane B's own session is `mode === 'shell'`, mirroring terminal-ui.js's reasoning: in a plain shell it is the user's own job-control tool, everywhere else it silently suspends an unattenLine truncated
|
||||
**Split-pane sessions** (`showSplitButton`, header button, default OFF, per-device like `showFileViewerButton` — not in `SettingsUpdateSchema`, `displayKeys` in settings-ui.js): shows two live sessions side-by-side in one Codeman window. Pane A is the untouched, existing singleton terminal (`this.terminal`/`this._ws` in terminal-ui.js); Pane B is an independent `TerminalTile` (terminal-tile.js, constructed by the orchestration in terminal-split.js) with its own xterm instance and its own `/ws/sessions/:id/terminal` WebSocket, whose `cid` is the tab's identity with a `:tile` suffix, so it can never supersede the primary pane's socket (the registry supersedes by cid per session). ⚠️ **Pane B is deliberately plainer than Pane A** — no local-echo overlay, no CJK IME, no touch/mobile handlers, no keyboard accessory bar (it does carry the primary pane's hollow-buffer wheel paging and desktop click report, through terminal-ui.js's own gates aimed at the tile, and its own keyCode-229 soft-keyboard controller from terminal-keycode229-recovery.js, the #441 next-keydown drain and #541's edit-based diff for Android autocorrect, since the 1180px width gate is reachable by a wide Android tablet; see terminal-tile.js's fileoverview) — since this is a desktop-only feature (a split view needs a wide viewport) and those features exist for mobile/touch input; `.btn-split` is hard-hidden below 1180px regardless of the setting by the `@media (max-width: 1179px)` rule in styles.css (mobile.css only carries a comment pointing at it: that file loads up to 1023px, so it cannot cover the 1024-1179px tablet range the feature also needs to stay off), and the per-device setting means turning it on at a desk can never sync it onto a phone in the first place. No persistence: closing the browser tab or reloading always returns to the normal single-pane view; there is no localStorage key for split state. ⚠️ Splitting a session against itself is disallowed (the picker excludes the active session), **as is splitting against a popped-out (detached) session** — `buildSplitPickerSessions()` excludes `detachedSessions` because a detached session's own window is already claiming its PTY size, and `MAX_WS_PER_SESSION` needs no change since Pane A/B are always two different sessions. ⚠️ Either pane's session ending (deleted locally or from another client) auto-collapses the split — Pane A's session ending promotes Pane B to the new single pane via `selectSession(id, { auto: true })` (an app-driven selection, so it must not spend the session's idle alert — see the Approvals Inbox note above), never by trying to hot-swap the lightweight `TerminalTile` object into the primary singleton state. ⚠️ Pane B refits on every window/sidebar/tab-rail resize via the SAME trailing-edge `ResizeObserver` callback that resizes Pane A (`throttledResize` in terminal-ui.js) — it only ever measured Pane A's own container, so without the explicit `this._forEachTile?.((tile) => tile.fit(), { grid: false })` call there (grid tiles are left out: the grid's own observer refits them on the same resize) Pane B silently kept its stale PTY size through every resize that did not happen to be a divider drag. ⚠️ A dropped WebSocket writes a `[disconnected, reconnecting…]` marker into Pane B and reconnects on the primary pane's backoff ladder (`CodemanWsReconnect` plus jitter; the attempt count resets only on a successful open), then refreshes the buffer to close the output gap. ⚠️ The open handler clears `_wsClosed`/`_markerOwed` BEFORE that refresh, or the refresh re-owes the marker and stamps it under a healthy pane. ⚠️ 4003/4004/4009/4010 stop the pane for good, report once through `onExit(code)` and write a marker saying why (`CodemanWsReconnect` alone would retry 4003). ⚠️ A replacement socket detaches the old one first and every handler ignores a socket that is no longer `this.ws`, so a late 4010 from a superseded socket cannot stop its successor; `destroy()` cancels a pending reconnect. Related but distinct: `detachSession()` already opens one session in a separate OS-level browser window (`isSoloWindow`) — that is prior art for "two sessions visible at once" but not for one window with a draggable in-page divider, which is what this feature adds. ⚠️ Pane B installs its own `attachCustomKeyEventHandler` gating the same app-level chords Pane A's own handler gates (command palette, Alt+1-9/[/] tab nav, Alt+B sidebar toggle, Ctrl+Z suspend, Shift/Ctrl+Enter newline, smart-copy Ctrl+C) — without it the document capture-phase handler's `preventDefault()` (which never stops xterm) let each chord ALSO write its raw byte/escape sequence into Pane B's live PTY on top of whatever the app action did to Pane A (COD-153). Ctrl+Z is swallowed unless Pane B's own session is `mode === 'shell'`, mirroring terminal-ui.js's reasoning: in a plain shell it is the user's own job-control tool, everywhere else it silently suspends an unattenLine truncated
|
||||
|
||||
### Tile grid
|
||||
|
||||
|
||||
@@ -1205,23 +1205,26 @@
|
||||
// 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.
|
||||
// `force` resends an unchanged size and asks the server to apply it anyway
|
||||
// (Redraw, restoreTerminalSize). Returns whether a resize went out.
|
||||
fit({ force = false } = {}) {
|
||||
this.localFit();
|
||||
this._sendResize({ force });
|
||||
return this._sendResize({ force });
|
||||
}
|
||||
|
||||
// Returns true only once a `{t:'z'}` frame was sent, false at every early
|
||||
// exit, so Redraw can say when nothing reached the PTY.
|
||||
_sendResize({ force = false } = {}) {
|
||||
if (!this._wsReady || !this.fitAddon || !this.terminal) return;
|
||||
if (!this._wsReady || !this.fitAddon || !this.terminal) return false;
|
||||
// One PTY cannot hold two sizes (mirrors sendResize's own
|
||||
// detachedElsewhere yield in terminal-ui.js): the session got detached
|
||||
// to its own window AFTER this pane was opened, so its own window now
|
||||
// owns the PTY's size and this pane must stand aside.
|
||||
if (this.detachedSessions?.has(this.sessionId)) return;
|
||||
if (this.detachedSessions?.has(this.sessionId)) return false;
|
||||
// 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();
|
||||
if (!dims || !Number.isFinite(dims.cols) || !Number.isFinite(dims.rows)) return;
|
||||
if (!dims || !Number.isFinite(dims.cols) || !Number.isFinite(dims.rows)) return false;
|
||||
// Report what the xterm actually holds, so the PTY gets exactly the size
|
||||
// the pane renders at. Unclamped, unlike the primary pane's 40x10 floor:
|
||||
// a floor would misreport the split's Pane B at its divider's reachable
|
||||
@@ -1231,9 +1234,20 @@
|
||||
const cols = this.terminal.cols;
|
||||
const rows = this.terminal.rows;
|
||||
const last = this._lastSentDims;
|
||||
if (!force && last && last.cols === cols && last.rows === rows) return;
|
||||
if (!force && last && last.cols === cols && last.rows === rows) return false;
|
||||
// `f` is the primary pane's forced resize (sendResize, terminal-ui.js):
|
||||
// Session.resize (session.ts) otherwise skips a size equal to the one it
|
||||
// last applied, so without it a forced resend reached the server and did
|
||||
// nothing there (no tmux resize-window, no PTY resize).
|
||||
const msg = { t: 'z', c: cols, r: rows, v: 'desktop' };
|
||||
if (force) msg.f = true;
|
||||
try {
|
||||
this.ws.send(JSON.stringify(msg));
|
||||
} catch {
|
||||
return false; // nothing went out, so nothing is recorded as sent
|
||||
}
|
||||
this._lastSentDims = { cols, rows };
|
||||
this.ws.send(JSON.stringify({ t: 'z', c: cols, r: rows, v: 'desktop' }));
|
||||
return true;
|
||||
}
|
||||
|
||||
// The session's PTY is new: a tile can connect before its session has a
|
||||
|
||||
@@ -4664,11 +4664,24 @@ Object.assign(CodemanApp.prototype, {
|
||||
*/
|
||||
async restoreTerminalSize() {
|
||||
// A second pane owns its own geometry: refit it and force its PTY to the
|
||||
// size it renders at (TerminalTile.fit), whatever another device set.
|
||||
// size it renders at (TerminalTile.fit sends the same forced `f` resize
|
||||
// sendResize sends below), whatever another device set. fit() says whether
|
||||
// the resize went out; when it did not, say why rather than report a size
|
||||
// that was never sent, as the primary branch does below.
|
||||
const pane = this._focusedPane();
|
||||
if (!pane.isPrimary) {
|
||||
pane.tile.fit({ force: true });
|
||||
this.showToast(`Terminal restored to ${pane.terminal.cols}x${pane.terminal.rows}`, 'success');
|
||||
const sent = pane.tile.fit({ force: true });
|
||||
if (sent !== false) {
|
||||
this.showToast(`Terminal restored to ${pane.terminal.cols}x${pane.terminal.rows}`, 'success');
|
||||
} else if (this.detachedSessions?.has(pane.sessionId)) {
|
||||
// Its own window owns the PTY's size (TerminalTile._sendResize yields).
|
||||
this.showToast('This session is sized by its own window', 'warning');
|
||||
} else if (!pane.tile._wsReady) {
|
||||
// The tile announces its size again as soon as its socket reopens.
|
||||
this.showToast('Terminal not connected: its size is sent when it reconnects', 'warning');
|
||||
} else {
|
||||
this.showToast('Could not determine terminal size', 'error');
|
||||
}
|
||||
return;
|
||||
}
|
||||
if (!this.activeSessionId) {
|
||||
|
||||
@@ -127,6 +127,36 @@ describe('terminal shortcuts follow the focused pane', () => {
|
||||
expect(app.showToast).toHaveBeenCalledWith('Terminal restored to 60x30', 'success');
|
||||
});
|
||||
|
||||
// TerminalTile.fit() returns whether its resize went out. When it did not,
|
||||
// Redraw used to report a size that was never sent.
|
||||
it.each([
|
||||
[
|
||||
'popped out to its own window',
|
||||
{ detached: true, wsReady: true },
|
||||
'This session is sized by its own window',
|
||||
'warning',
|
||||
],
|
||||
[
|
||||
'whose socket is down',
|
||||
{ detached: false, wsReady: false },
|
||||
'Terminal not connected: its size is sent when it reconnects',
|
||||
'warning',
|
||||
],
|
||||
['that could not measure itself', { detached: false, wsReady: true }, 'Could not determine terminal size', 'error'],
|
||||
])('Ctrl+Shift+R on a second pane %s reports no success', async (_label, state, message, level) => {
|
||||
const app = loadApp();
|
||||
app.detachedSessions = new Set(state.detached ? ['session-b'] : []);
|
||||
const tile = paneB({ fit: vi.fn(() => false), _wsReady: state.wsReady });
|
||||
app._noteFocusedTile(tile);
|
||||
|
||||
await app.restoreTerminalSize();
|
||||
|
||||
expect(tile.fit).toHaveBeenCalledWith({ force: true });
|
||||
expect(app.showToast).toHaveBeenCalledTimes(1);
|
||||
expect(app.showToast).toHaveBeenCalledWith(message, level);
|
||||
expect(app.sendResize).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('Ctrl+Shift+R keeps restoring the primary when it holds the keyboard', async () => {
|
||||
const app = loadApp();
|
||||
|
||||
|
||||
@@ -118,7 +118,7 @@ type Tile = {
|
||||
connect(): Promise<void>;
|
||||
destroy(): void;
|
||||
reconnectNow(): void;
|
||||
fit(opts?: { force?: boolean }): void;
|
||||
fit(opts?: { force?: boolean }): boolean;
|
||||
detachedSessions?: Set<string>;
|
||||
ws: FakeSocket | null;
|
||||
_reconnectAttempts: number;
|
||||
@@ -514,15 +514,80 @@ describe('TerminalTile geometry (#464: the pane and its PTY never disagree)', ()
|
||||
const { tile, ws } = await connectTile(makeApp());
|
||||
ws.open();
|
||||
|
||||
tile.fit();
|
||||
expect(tile.fit()).toBe(false); // unchanged: nothing sent
|
||||
expect(resizeFrames(ws)).toHaveLength(1);
|
||||
|
||||
FakeFit.proposed = { cols: 100, rows: 30 };
|
||||
tile.fit();
|
||||
expect(tile.fit()).toBe(true);
|
||||
expect(resizeFrames(ws).at(-1)).toEqual({ t: 'z', c: 100, r: 30, v: 'desktop' });
|
||||
|
||||
tile.fit({ force: true });
|
||||
expect(tile.fit({ force: true })).toBe(true);
|
||||
expect(resizeFrames(ws)).toHaveLength(3);
|
||||
// The primary pane's forced resize flag: without it Session.resize skips a
|
||||
// size equal to the one it last applied, and the forced resend did nothing.
|
||||
expect(resizeFrames(ws).at(-1)).toEqual({ t: 'z', c: 100, r: 30, v: 'desktop', f: true });
|
||||
});
|
||||
|
||||
it('force on a closed socket sends nothing and says so', async () => {
|
||||
const { tile, ws } = await connectTile(makeApp());
|
||||
ws.open();
|
||||
ws.drop(1006);
|
||||
|
||||
expect(tile.fit({ force: true })).toBe(false);
|
||||
expect(resizeFrames(ws)).toHaveLength(1); // only the open's own announcement
|
||||
});
|
||||
|
||||
describe('Redraw (Ctrl+Shift+R, the header button) on a focused tile', () => {
|
||||
type RedrawApp = App & {
|
||||
restoreTerminalSize(): Promise<void>;
|
||||
_noteFocusedTile(tile: unknown): void;
|
||||
detachedSessions?: Set<string>;
|
||||
};
|
||||
|
||||
it('forces the resize through to the server and reports the size it sent', async () => {
|
||||
const app = makeApp() as RedrawApp;
|
||||
const { tile, ws } = await connectTile(app);
|
||||
ws.open();
|
||||
app._noteFocusedTile(tile);
|
||||
|
||||
await app.restoreTerminalSize();
|
||||
|
||||
expect(resizeFrames(ws).at(-1)).toEqual({ t: 'z', c: 80, r: 24, v: 'desktop', f: true });
|
||||
expect(app.showToast).toHaveBeenCalledWith('Terminal restored to 80x24', 'success');
|
||||
});
|
||||
|
||||
it('with the socket down, sends nothing and does not report a success', async () => {
|
||||
const app = makeApp() as RedrawApp;
|
||||
const { tile, ws } = await connectTile(app);
|
||||
ws.open();
|
||||
app._noteFocusedTile(tile);
|
||||
ws.drop(1006);
|
||||
|
||||
await app.restoreTerminalSize();
|
||||
|
||||
expect(resizeFrames(ws)).toHaveLength(1);
|
||||
expect(app.showToast).not.toHaveBeenCalledWith(expect.stringContaining('restored'), 'success');
|
||||
expect(app.showToast).toHaveBeenCalledWith(
|
||||
'Terminal not connected: its size is sent when it reconnects',
|
||||
'warning'
|
||||
);
|
||||
});
|
||||
|
||||
it('for a session popped out to its own window, says that window sizes it', async () => {
|
||||
const app = makeApp() as RedrawApp;
|
||||
const detached = new Set<string>();
|
||||
app.detachedSessions = detached;
|
||||
const { tile, ws } = await connectTile(app, { detachedSessions: detached });
|
||||
ws.open();
|
||||
app._noteFocusedTile(tile);
|
||||
detached.add('s-tile'); // popped out after the tile opened
|
||||
|
||||
await app.restoreTerminalSize();
|
||||
|
||||
expect(resizeFrames(ws)).toHaveLength(1);
|
||||
expect(app.showToast).not.toHaveBeenCalledWith(expect.stringContaining('restored'), 'success');
|
||||
expect(app.showToast).toHaveBeenCalledWith('This session is sized by its own window', 'warning');
|
||||
});
|
||||
});
|
||||
|
||||
it('re-announces an unchanged size on a reconnected socket', async () => {
|
||||
@@ -598,7 +663,7 @@ describe('TerminalTile geometry (#464: the pane and its PTY never disagree)', ()
|
||||
ws.open();
|
||||
FakeFit.proposed = { cols: 120, rows: 40 };
|
||||
|
||||
tile.fit();
|
||||
expect(tile.fit()).toBe(false);
|
||||
|
||||
expect(resizeFrames(ws)).toEqual([]);
|
||||
});
|
||||
|
||||
Reference in new issue
Block a user