mirror of
https://github.com/Ark0N/Codeman.git
synced 2026-10-03 05:59:43 +02:00
fix(tui): advertise the key that actually detaches, and stop tmux painting it green
Two things the attach status bar got wrong, both found in a beta test. The bar read `Ctrl+B D`. tmux key tables are case-sensitive: lowercase `d` is `detach-client`, capital `D` is `choose-client`. Pressing what the bar said opened a client chooser and left the tester attached, with the way out on screen and inert. The key is now READ from `list-keys -T prefix` the same way the prefix already was, rather than hardcoded, so a rebound tmux is followed too and the label cannot drift from the binding again. It never goes through formatPrefixKey(), which uppercases. The bar also rendered as a full-width bright green slab. Only `status-format[0]` was styled, so tmux's stock `status-style` (`bg=green,fg=black`) stayed underneath it and won; `#[reverse]` on top could not undo it. `status-style` is now set explicitly to `bg=default,fg=default` and snapshotted/restored with the rest, so the bar sits on the terminal's own background and reads as a hint line. Tests pin both: that the chord ends in lowercase `d` and never ` D`, that a rebound key prints verbatim, that `status-style` is part of the banner, and that parseDetachKey() picks `d` out of verbatim tmux 3.4 `list-keys` output while ignoring `detach-client -a`/`-P`, which act on other clients.
This commit is contained in:
+39
-11
@@ -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<string, string> {
|
||||
const chord = escapeTmuxFormat(detachChord(options.prefix));
|
||||
export function buildAttachBanner(options: {
|
||||
prefix?: string;
|
||||
label?: string;
|
||||
detachKey?: string;
|
||||
}): Record<string, string> {
|
||||
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<TuiAttachHandoff> {
|
||||
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<void> {
|
||||
// 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
|
||||
|
||||
@@ -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<string | null> {
|
||||
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<TuiSessionOptions | null> {
|
||||
if (!MUX_NAME_PATTERN.test(muxName)) return null;
|
||||
|
||||
@@ -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');
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
@@ -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 });
|
||||
|
||||
Reference in New Issue
Block a user