diff --git a/src/tui/tui-app.ts b/src/tui/tui-app.ts index 94123bfc..4054619f 100644 --- a/src/tui/tui-app.ts +++ b/src/tui/tui-app.ts @@ -260,9 +260,22 @@ export function formatPrefixKey(prefix: string | undefined): string { return raw; } -/** The whole chord: prefix, then `d`. */ -export function detachChord(prefix?: string): string { - return `${formatPrefixKey(prefix)} D`; +/** tmux's stock `detach-client` binding, used when `list-keys` cannot be read. */ +export const DEFAULT_DETACH_KEY = 'd'; + +/** + * The chord that ends an attach: the prefix, then the key bound to + * `detach-client`. + * + * ⚠️ Case is load-bearing here and this shipped wrong once. tmux binds + * LOWERCASE `d` to `detach-client` and CAPITAL `D` to `choose-client`, so a bar + * advertising `Ctrl+B D` opened a client chooser and nothing detached (reported + * from the beta: the way out was on screen and still did not work). The key is + * therefore READ from tmux exactly like the prefix already is, and never passed + * through `formatPrefixKey`, which uppercases. + */ +export function detachChord(prefix?: string, key?: string): string { + return `${formatPrefixKey(prefix)} then ${key || DEFAULT_DETACH_KEY}`; } /** `#` opens `#[…]`/`#{…}` in a tmux format, so a name carrying one must double it. */ @@ -280,12 +293,20 @@ function escapeTmuxFormat(value: string): string { * pane behind on the first try). The bar exists for the length of the attach * and is put back exactly as it was on detach. * - * `reverse` rather than a palette: the TUI paints its own selected row with the - * same SGR 7, so the bar inherits whatever theme the terminal has instead of - * guessing at light or dark. + * ⚠️ `status-style` is set EXPLICITLY and is not optional. Styling only + * `status-format[0]` leaves tmux's stock `status-style` (`bg=green,fg=black`) + * underneath it, which paints a full-width bright green slab across the bottom + * of the pane, `#[reverse]` on top of it included (reported from the beta as + * "a big green line"). `bg=default,fg=default` lets the bar sit on the + * terminal's own background so it reads as a hint line rather than a banner, + * and the chord alone carries emphasis. */ -export function buildAttachBanner(options: { prefix?: string; label?: string }): Record { - const chord = escapeTmuxFormat(detachChord(options.prefix)); +export function buildAttachBanner(options: { + prefix?: string; + label?: string; + detachKey?: string; +}): Record { + const chord = escapeTmuxFormat(detachChord(options.prefix, options.detachKey)); const label = escapeTmuxFormat(truncateLabel((options.label ?? '').trim(), ATTACH_BANNER_LABEL_MAX)); // ONE option, not `status-left`/`status-right`/`status-style`: `status-format[0]` // owns the whole line, which is what removes tmux's window list (`0:bash*`) @@ -294,7 +315,8 @@ export function buildAttachBanner(options: { prefix?: string; label?: string }): const right = label ? `#[align=right] ${label} ` : ''; return { status: 'on', - 'status-format[0]': `#[reverse] #[bold]${chord}#[nobold] detach, back to the codeman dashboard${right}#[default]`, + 'status-style': 'bg=default,fg=default', + 'status-format[0]': `#[align=left] press #[bold]${chord}#[nobold] to detach, back to the codeman dashboard${right}#[default]`, }; } @@ -361,6 +383,8 @@ export interface TuiAttachHandoff { */ export async function beginAttachHandoff(client: TuiClient, muxName: string, label: string): Promise { const prefix = (await client.readPrefixKey(muxName)) ?? undefined; + // Read, not assumed: see detachChord() for the `d` vs `D` mix-up this closes. + const detachKey = (await client.readDetachKey()) ?? undefined; // Codeman pins its windows to the size the BROWSER dictates (`window-size // manual` + `resize-window`, tmux-manager.ts), so a terminal of any other // shape attaches to a window that does not fill it and tmux pads the gap with @@ -369,11 +393,15 @@ export async function beginAttachHandoff(client: TuiClient, muxName: string, lab // and the caller is blocked in `spawnSync`. const sizing = await client.readWindowSizing(muxName); await client.followAttachingClient(muxName); - const banner = buildAttachBanner({ ...(prefix ? { prefix } : {}), label }); + const banner = buildAttachBanner({ + ...(prefix ? { prefix } : {}), + ...(detachKey ? { detachKey } : {}), + label, + }); const options = await client.readSessionOptions(muxName, Object.keys(banner)); await client.applySessionOptions(muxName, banner); return { - chord: detachChord(prefix), + chord: detachChord(prefix, detachKey), async restore(): Promise { // Options first, then the size: dropping the status bar gives its row // back to the pane, and the resize is what re-pins the browser's diff --git a/src/tui/tui-client.ts b/src/tui/tui-client.ts index 81051105..fe3dde26 100644 --- a/src/tui/tui-client.ts +++ b/src/tui/tui-client.ts @@ -467,6 +467,28 @@ export function parseSessionOptions(stdout: string, keys: readonly string[]): Tu return options; } +/** + * The key bound to a bare `detach-client` in `list-keys -T prefix` output, or + * null when nothing there detaches. + * + * ⚠️ Read rather than assumed because the two candidates differ only by case: + * tmux ships `d` as `detach-client` and `D` as `choose-client`, and advertising + * the wrong one leaves a tester attached with the way out on screen. Bindings + * that pass ARGUMENTS to `detach-client` (`-a`, `-P`) are skipped: those act on + * other clients or kill the pane's process, which is not what the bar promises. + * A single-character binding wins over a named key, since that is what a status + * line can print literally. + */ +export function parseDetachKey(stdout: string): string | null { + const candidates: string[] = []; + for (const line of stdout.split('\n')) { + const match = /^bind-key\s+(?:-\S+\s+)*?-T\s+prefix\s+(\S+)\s+detach-client\s*$/.exec(line.trim()); + const key = match?.[1]; + if (key) candidates.push(unquoteTmuxValue(key)); + } + return candidates.find((key) => key.length === 1) ?? candidates[0] ?? null; +} + /** `status-format[0]` → `status-format`; a plain option name → null. */ export function arrayOptionBase(key: string): string | null { const match = /^([^[\]]+)\[\d+\]$/.exec(key); @@ -853,6 +875,20 @@ export class TuiClient { return null; } + /** + * The key that detaches, straight from tmux's own key table. Key tables are + * server-global, so unlike the prefix this takes no session. A failure is + * null and the caller falls back to tmux's stock `d`. + */ + async readDetachKey(): Promise { + try { + const { stdout } = await this.exec('tmux', ['-L', this.socket, 'list-keys', '-T', 'prefix']); + return parseDetachKey(stdout); + } catch { + return null; + } + } + /** Snapshot the session-level options an attach is about to overwrite. */ async readSessionOptions(muxName: string, keys: readonly string[]): Promise { if (!MUX_NAME_PATTERN.test(muxName)) return null; diff --git a/test/tui/tui-app.test.ts b/test/tui/tui-app.test.ts index 3f450f5b..28b1d751 100644 --- a/test/tui/tui-app.test.ts +++ b/test/tui/tui-app.test.ts @@ -471,20 +471,45 @@ describe('the way out of an attach', () => { }); it('names the chord, not just the prefix', () => { - expect(detachChord('C-a')).toBe('Ctrl+A D'); - expect(detachChord()).toBe('Ctrl+B D'); + expect(detachChord('C-a', 'd')).toBe('Ctrl+A then d'); + expect(detachChord()).toBe('Ctrl+B then d'); + }); + + it('names the LOWERCASE detach key, because capital D is choose-client', () => { + // Regression: the bar shipped reading `Ctrl+B D`, and a beta tester pressing + // exactly that landed in tmux's client chooser while staying attached. tmux + // key tables are case-sensitive and `D` is bound to a different command. + expect(detachChord()).not.toContain(' D'); + expect(detachChord()).toMatch(/ then d$/); + expect(buildAttachBanner({})['status-format[0]']).not.toContain('#[bold]Ctrl+B D#[nobold]'); + }); + + it('prints a rebound detach key verbatim, never uppercased like the prefix', () => { + // formatPrefixKey() uppercases (`C-a` → `Ctrl+A`); running the detach key + // through it would reintroduce the same class of bug on a rebound tmux. + expect(detachChord('C-a', 'q')).toBe('Ctrl+A then q'); + expect(buildAttachBanner({ prefix: 'C-a', detachKey: 'q' })['status-format[0]']).toContain('Ctrl+A then q'); }); it('builds ONE status-format option, so tmux draws no window list beside it', () => { const banner = buildAttachBanner({ prefix: 'C-b', label: 'w3-codeman' }); - expect(Object.keys(banner).sort()).toEqual(['status', 'status-format[0]']); + expect(Object.keys(banner).sort()).toEqual(['status', 'status-format[0]', 'status-style']); expect(banner.status).toBe('on'); - expect(banner['status-format[0]']).toContain('#[bold]Ctrl+B D#[nobold]'); + expect(banner['status-format[0]']).toContain('#[bold]Ctrl+B then d#[nobold]'); expect(banner['status-format[0]']).toContain('#[align=right] w3-codeman '); }); + it('sets status-style, or tmux paints its stock green bar under the bar', () => { + // Regression: styling only status-format[0] left tmux's default + // `bg=green,fg=black` status-style underneath, which a beta tester saw as a + // full-width bright green slab across the bottom of the pane. + const banner = buildAttachBanner({ prefix: 'C-b' }); + expect(banner['status-style']).toBe('bg=default,fg=default'); + expect(banner['status-format[0]']).not.toContain('#[reverse]'); + }); + it('carries the remapped prefix into the bar', () => { - expect(buildAttachBanner({ prefix: 'C-a' })['status-format[0]']).toContain('Ctrl+A D'); + expect(buildAttachBanner({ prefix: 'C-a' })['status-format[0]']).toContain('Ctrl+A then d'); }); it('escapes a label that would otherwise open a tmux format', () => { @@ -506,11 +531,11 @@ describe('the way out of an attach', () => { }); it("tells the help overlay how to get back, in the socket's own prefix", () => { - const keys = helpKeysFor(GLYPHS, { server: true, detach: 'Ctrl+A D' }); - const detach = keys.find(([key]) => key === 'Ctrl+A D'); + const keys = helpKeysFor(GLYPHS, { server: true, detach: 'Ctrl+A then d' }); + const detach = keys.find(([key]) => key === 'Ctrl+A then d'); expect(detach?.[1]).toContain('detach'); // Degraded mode still attaches, so it still needs the way out. - expect(helpKeysFor(GLYPHS, { server: false }).map(([key]) => key)).toContain('Ctrl+B D'); + expect(helpKeysFor(GLYPHS, { server: false }).map(([key]) => key)).toContain('Ctrl+B then d'); }); }); diff --git a/test/tui/tui-client.test.ts b/test/tui/tui-client.test.ts index 4fbc9667..b5d695e5 100644 --- a/test/tui/tui-client.test.ts +++ b/test/tui/tui-client.test.ts @@ -22,6 +22,7 @@ import { parseEnvFile, arrayOptionBase, parseSessionOptions, + parseDetachKey, parseTmuxSessionList, parseWindowSizing, readCodemanCredentials, @@ -599,6 +600,47 @@ describe('attach window sizing', () => { }); }); +describe('parseDetachKey', () => { + // Verbatim from `tmux -L codeman list-keys -T prefix` on tmux 3.4, trimmed to + // the two lines that matter. They differ only by case, which is the whole + // point: `D` is choose-client and advertising it leaves the tester attached. + const REAL_TMUX_34 = [ + 'bind-key -T prefix C-b send-prefix', + 'bind-key -T prefix d detach-client', + 'bind-key -T prefix D choose-client -Z', + 'bind-key -T prefix x confirm-before -p "kill-pane #P? (y/n)" kill-pane', + ].join('\n'); + + it('picks the lowercase detach-client binding out of real tmux output', () => { + expect(parseDetachKey(REAL_TMUX_34)).toBe('d'); + }); + + it('follows a rebound detach key rather than assuming d', () => { + expect(parseDetachKey('bind-key -T prefix Q detach-client')).toBe('Q'); + }); + + it('tolerates the -r repeat flag ahead of -T', () => { + expect(parseDetachKey('bind-key -r -T prefix d detach-client')).toBe('d'); + }); + + it('ignores detach-client bindings that carry arguments', () => { + // `-a` detaches OTHER clients and `-P` kills the pane's process; neither is + // what the bar promises, so a socket with only those reports nothing. + expect(parseDetachKey('bind-key -T prefix X detach-client -a')).toBeNull(); + expect(parseDetachKey('bind-key -T prefix Y detach-client -P')).toBeNull(); + }); + + it('prefers a single-character binding over a named key', () => { + const both = 'bind-key -T prefix F1 detach-client\nbind-key -T prefix d detach-client'; + expect(parseDetachKey(both)).toBe('d'); + }); + + it('is null when nothing detaches, so the caller can fall back', () => { + expect(parseDetachKey('')).toBeNull(); + expect(parseDetachKey('bind-key -T prefix D choose-client -Z')).toBeNull(); + }); +}); + describe('attach status bar options', () => { function optionsClient(exec: TuiExecFile): TuiClient { return new TuiClient({ baseUrl: BASE_URL, socket: 'codeman-beta', exec });