mirror of
https://github.com/Ark0N/Codeman.git
synced 2026-10-04 14:39:42 +02:00
fix(tui): confirm a kill with y, and make the dialog say what it would kill
Killing demanded the session's NAME typed out in full. That is the right
ceremony for dropping a production database and the wrong one for closing a
pane you are looking at; the beta tester's verdict was "thats stupid, just make
me type Y to confirm". `x` then `y` is already two deliberate keystrokes on a
row the user selected, and the conversation lives in its transcript, which a
kill does not touch.
Everything that is not `y` CANCELS rather than being ignored, so a stray key
closes the dialog instead of leaving a destructive prompt armed and waiting for
whatever gets typed next. Enter cancels too: it is the key most likely to be
hit by reflex, and this is the one dialog that destroys something.
⚠️ Found while verifying the new dialog: it did not name the session. The label
was computed as `row.session.name ?? id.slice(0, 8)`, and `??` falls back only
on null or undefined, so every session the server left with an EMPTY name — all
of them, until the TUI started naming its own — sailed through and the box read
"Kill ?". A destructive prompt that cannot say what it will destroy is worse
than no prompt, and it is now a single keystroke. The caller passes the same
label the LIST shows, so the dialog names the row in front of the user.
The typed-name machinery goes with it: TuiConfirmState.typed, setConfirmInput(),
confirmAccepts() and the 'typing'/'reject' steps are all removed rather than
left as unreachable branches.
This commit is contained in:
+15
-28
@@ -15,7 +15,6 @@ import {
|
||||
applyMuxNames,
|
||||
buildAttachBanner,
|
||||
buildListLines,
|
||||
confirmAccepts,
|
||||
confirmKillStep,
|
||||
detachChord,
|
||||
heldCtrlAlias,
|
||||
@@ -135,37 +134,25 @@ describe('isSelfSession', () => {
|
||||
describe('the kill confirmation', () => {
|
||||
const state: TuiConfirmState = { sessionId: 'abcdef01-2345', name: 'w4-api', typed: '' };
|
||||
|
||||
it('accepts the shown name or the id prefix a mux name carries, and nothing else', () => {
|
||||
expect(confirmAccepts(state, 'w4-api')).toBe(true);
|
||||
expect(confirmAccepts(state, ' w4-api ')).toBe(true);
|
||||
expect(confirmAccepts(state, 'abcdef01')).toBe(true);
|
||||
expect(confirmAccepts(state, 'w4')).toBe(false);
|
||||
expect(confirmAccepts(state, 'W4-API')).toBe(false);
|
||||
expect(confirmAccepts(state, '')).toBe(false);
|
||||
expect(confirmAccepts(state, ' ')).toBe(false);
|
||||
it('kills on y, upper or lower', () => {
|
||||
// Was: type the session's full name. The tester's verdict on that was
|
||||
// "thats stupid, just make me type Y to confirm", and they were right —
|
||||
// `x` then `y` is already two deliberate keystrokes on a selected row.
|
||||
expect(confirmKillStep(state, { type: 'char', value: 'y' })).toEqual({ kind: 'confirm' });
|
||||
expect(confirmKillStep(state, { type: 'char', value: 'Y' })).toEqual({ kind: 'confirm' });
|
||||
});
|
||||
|
||||
it('types, backspaces and cancels', () => {
|
||||
expect(confirmKillStep({ ...state, typed: 'w4' }, { type: 'char', value: '-' })).toEqual({
|
||||
kind: 'typing',
|
||||
typed: 'w4-',
|
||||
});
|
||||
expect(confirmKillStep({ ...state, typed: 'w4-' }, { type: 'backspace' })).toEqual({ kind: 'typing', typed: 'w4' });
|
||||
expect(confirmKillStep({ ...state, typed: '' }, { type: 'backspace' })).toEqual({ kind: 'typing', typed: '' });
|
||||
it('cancels on every other key, rather than leaving the prompt armed', () => {
|
||||
// A dialog that ignores unknown keys sits there consuming whatever the
|
||||
// user types next, which for a destructive prompt is the wrong default.
|
||||
expect(confirmKillStep(state, { type: 'char', value: 'n' })).toEqual({ kind: 'cancel' });
|
||||
expect(confirmKillStep(state, { type: 'char', value: 'x' })).toEqual({ kind: 'cancel' });
|
||||
expect(confirmKillStep(state, { type: 'escape' })).toEqual({ kind: 'cancel' });
|
||||
expect(confirmKillStep(state, { type: 'ctrl', key: 'c' })).toEqual({ kind: 'cancel' });
|
||||
expect(confirmKillStep(state, { type: 'ctrl', key: 'a' })).toEqual({ kind: 'ignore' });
|
||||
expect(confirmKillStep(state, { type: 'tab' })).toEqual({ kind: 'ignore' });
|
||||
});
|
||||
|
||||
it('confirms only on a match, and says so rather than doing nothing otherwise', () => {
|
||||
expect(confirmKillStep({ ...state, typed: 'w4-api' }, { type: 'enter' })).toEqual({ kind: 'confirm' });
|
||||
expect(confirmKillStep({ ...state, typed: 'w4' }, { type: 'enter' })).toEqual({ kind: 'reject' });
|
||||
expect(confirmKillStep({ ...state, typed: '' }, { type: 'enter' })).toEqual({ kind: 'reject' });
|
||||
});
|
||||
|
||||
it('backspaces one whole character, not one code unit', () => {
|
||||
expect(confirmKillStep({ ...state, typed: 'a🙂' }, { type: 'backspace' })).toEqual({ kind: 'typing', typed: 'a' });
|
||||
it('does NOT kill on Enter, the key most likely to be hit by reflex', () => {
|
||||
expect(confirmKillStep(state, { type: 'enter' })).toEqual({ kind: 'cancel' });
|
||||
});
|
||||
});
|
||||
|
||||
@@ -215,7 +202,7 @@ describe('footerKeysFor', () => {
|
||||
expect.arrayContaining([
|
||||
'attach — on a RECENT row, resume that conversation',
|
||||
'new session',
|
||||
'kill (typed confirmation)',
|
||||
'kill (y to confirm)',
|
||||
'quit',
|
||||
])
|
||||
);
|
||||
@@ -227,7 +214,7 @@ describe('footerKeysFor', () => {
|
||||
|
||||
it('follows the overlay that owns the keyboard', () => {
|
||||
expect(footerKeysFor('help', GLYPHS, { server: true })).toEqual(['esc close']);
|
||||
expect(footerKeysFor('confirm-kill', GLYPHS, { server: true }).join(' ')).toContain('type the name');
|
||||
expect(footerKeysFor('confirm-kill', GLYPHS, { server: true }).join(' ')).toContain('y kill');
|
||||
expect(footerKeysFor('message', GLYPHS, { server: true })).toEqual(['esc dismiss']);
|
||||
expect(footerKeysFor('new-session', GLYPHS, { server: true }).join(' ')).toContain('type to filter');
|
||||
expect(footerKeysFor('prompt', GLYPHS, { server: true }).join(' ')).toContain('send');
|
||||
|
||||
@@ -556,23 +556,32 @@ describe('codeman tui (under a pty)', () => {
|
||||
it('opens and closes the help overlay', async () => {
|
||||
term.write('?');
|
||||
await waitFor(() => frameLines(output).some((line) => line.includes('Keys')), 'the help overlay');
|
||||
expect(frameLines(output).join('\n')).toContain('kill (typed confirmation)');
|
||||
expect(frameLines(output).join('\n')).toContain('kill (y to confirm)');
|
||||
|
||||
term.write('\u001b');
|
||||
await waitFor(() => !frameLines(output).some((line) => line.includes('Keys')), 'escape to close the overlay');
|
||||
});
|
||||
|
||||
it('asks for the session name before killing anything', async () => {
|
||||
it('asks for a y before killing anything, and names what it would kill', async () => {
|
||||
term.write('x');
|
||||
await waitFor(() => frameLines(output).some((line) => line.includes('Kill session')), 'the kill confirmation');
|
||||
const overlay = frameLines(output).join('\n');
|
||||
expect(overlay).toContain('Type the name to confirm');
|
||||
expect(overlay).toContain('press y to kill');
|
||||
expect(overlay).toContain('w2-beta');
|
||||
|
||||
term.write('\u001b');
|
||||
await waitFor(() => !frameLines(output).some((line) => line.includes('Kill session')), 'escape to cancel the kill');
|
||||
});
|
||||
|
||||
it('cancels the kill on any key that is not y, and kills nothing', async () => {
|
||||
term.write('x');
|
||||
await waitFor(() => frameLines(output).some((line) => line.includes('Kill session')), 'the kill confirmation');
|
||||
term.write('n');
|
||||
await waitFor(() => !frameLines(output).some((line) => line.includes('Kill session')), 'the dialog to close');
|
||||
// The session is still listed: `n` cancelled rather than killed.
|
||||
await waitFor(() => frameLines(output).some((line) => line.includes('w2-beta')), 'w2-beta to still be listed');
|
||||
});
|
||||
|
||||
it('sends a one-line prompt with p', async () => {
|
||||
term.write('p');
|
||||
await waitFor(() => frameLines(output)[ROWS - 1].startsWith(' >'), 'the composer to open');
|
||||
|
||||
@@ -267,11 +267,17 @@ describe('the store', () => {
|
||||
it('tracks the confirm-kill overlay, keyed to the name it showed', () => {
|
||||
const model = createTuiModel();
|
||||
model.replaceSessions([session({ sessionId: 'a', name: 'w4-api' })]);
|
||||
model.beginConfirmKill(model.rows()[0]);
|
||||
model.beginConfirmKill(model.rows()[0], 'w4-api');
|
||||
expect(model.mode).toBe('confirm-kill');
|
||||
expect(model.confirm).toEqual({ sessionId: 'a', name: 'w4-api', typed: '' });
|
||||
model.setConfirmInput('w4-ap');
|
||||
expect(model.confirm?.typed).toBe('w4-ap');
|
||||
// The name is the whole payload: it is what the dialog shows so the user
|
||||
// knows WHICH session a `y` is about to destroy.
|
||||
expect(model.confirm).toEqual({ sessionId: 'a', name: 'w4-api' });
|
||||
|
||||
// Regression: the label is supplied by the caller. Deriving it here with
|
||||
// `name ?? id` let an EMPTY name through, and the dialog read "Kill ?".
|
||||
model.replaceSessions([session({ sessionId: 'b', name: '' })]);
|
||||
model.beginConfirmKill(model.rows()[0], 'mirofish');
|
||||
expect(model.confirm?.name).toBe('mirofish');
|
||||
model.closeOverlay();
|
||||
expect(model.mode).toBe('list');
|
||||
expect(model.confirm).toBeNull();
|
||||
|
||||
@@ -300,15 +300,15 @@ describe('overlays', () => {
|
||||
expect(lines[lines.length - 1]).toContain('close');
|
||||
});
|
||||
|
||||
it('draws the typed confirmation for a kill', () => {
|
||||
it('names what a kill would destroy, and asks for one key', () => {
|
||||
const model = fixture();
|
||||
model.beginConfirmKill(model.rows()[0]);
|
||||
model.setConfirmInput('w6-d');
|
||||
model.beginConfirmKill(model.rows()[0], 'w6-docs');
|
||||
const text = frameLines(render(model, 100, 30)).join('\n');
|
||||
expect(text).toContain('Kill w6-docs?');
|
||||
expect(text).toContain('Type the name to confirm:');
|
||||
expect(text).toContain('w6-d_');
|
||||
expect(text).toContain('esc cancel');
|
||||
expect(text).toContain('press y to kill, any other key cancels');
|
||||
// The name is the point of the dialog: it is what tells the user WHICH
|
||||
// session a keystroke is about to destroy.
|
||||
expect(text).not.toContain('Type the name to confirm');
|
||||
});
|
||||
|
||||
it('draws a message box', () => {
|
||||
|
||||
Reference in New Issue
Block a user