fix(tui): copilot review on #16707 — naming, label consistency, esc priority
- Rename `removeAt` → `removeAtInPlace` and document the mutation contract; the old name read like a non-mutating helper. - Hotkey table + queue header: use `Ctrl+X` / `Esc` to match the rest of the UI (was `⌃X` / `esc`). - Render the queued header as a single template literal so JSX text-node whitespace can't sneak into the rendered line. - Make `Esc` while editing beat the `terminal.hasSelection` clear: the header promises 'Esc cancel', so an active selection shouldn't silently consume the keystroke.
This commit is contained in:
@@ -1,26 +1,26 @@
|
||||
import { describe, expect, it } from 'vitest'
|
||||
|
||||
import { removeAt } from '../hooks/useQueue.js'
|
||||
import { removeAtInPlace } from '../hooks/useQueue.js'
|
||||
|
||||
describe('removeAt', () => {
|
||||
describe('removeAtInPlace', () => {
|
||||
it('removes the item at the given index in place', () => {
|
||||
const arr = ['a', 'b', 'c']
|
||||
|
||||
removeAt(arr, 1)
|
||||
removeAtInPlace(arr, 1)
|
||||
expect(arr).toEqual(['a', 'c'])
|
||||
})
|
||||
|
||||
it('is a no-op when the index is out of bounds', () => {
|
||||
const arr = ['a', 'b']
|
||||
|
||||
removeAt(arr, -1)
|
||||
removeAt(arr, 5)
|
||||
removeAtInPlace(arr, -1)
|
||||
removeAtInPlace(arr, 5)
|
||||
expect(arr).toEqual(['a', 'b'])
|
||||
})
|
||||
|
||||
it('returns the same reference (mutates in place)', () => {
|
||||
const arr = ['x']
|
||||
const same = removeAt(arr, 0)
|
||||
const same = removeAtInPlace(arr, 0)
|
||||
|
||||
expect(same).toBe(arr)
|
||||
expect(arr).toEqual([])
|
||||
|
||||
@@ -307,14 +307,17 @@ export function useInputHandlers(ctx: InputHandlerContext): InputHandlerResult {
|
||||
return scrollTranscript(key.pageUp ? -step : step)
|
||||
}
|
||||
|
||||
if (key.escape && terminal.hasSelection) {
|
||||
return clearSelection()
|
||||
}
|
||||
|
||||
// Queue-edit cancel beats selection-clear: the queue header explicitly
|
||||
// promises "Esc cancel", so honoring it takes priority over the implicit
|
||||
// selection-dismissal convention. Without an active edit, fall through.
|
||||
if (key.escape && cState.queueEditIdx !== null) {
|
||||
return cActions.clearIn()
|
||||
}
|
||||
|
||||
if (key.escape && terminal.hasSelection) {
|
||||
return clearSelection()
|
||||
}
|
||||
|
||||
if (key.upArrow && !cState.inputBuf.length) {
|
||||
const inputSel = getInputSelection()
|
||||
const cursor = inputSel && inputSel.start === inputSel.end ? inputSel.start : null
|
||||
|
||||
@@ -24,8 +24,9 @@ export function QueuedMessages({ cols, queueEditIdx, queued, t }: QueuedMessages
|
||||
return (
|
||||
<Box flexDirection="column" marginTop={1}>
|
||||
<Text color={t.color.dim} dimColor>
|
||||
queued ({queued.length})
|
||||
{queueEditIdx !== null ? ` · editing ${queueEditIdx + 1} · ⌃X delete · esc cancel` : ''}
|
||||
{`queued (${queued.length})${
|
||||
queueEditIdx !== null ? ` · editing ${queueEditIdx + 1} · Ctrl+X delete · Esc cancel` : ''
|
||||
}`}
|
||||
</Text>
|
||||
|
||||
{q.showLead && (
|
||||
|
||||
@@ -23,7 +23,7 @@ export const HOTKEYS: [string, string][] = [
|
||||
[paste + '+V / /paste', 'paste text; /paste attaches clipboard image'],
|
||||
['Tab', 'apply completion'],
|
||||
['↑/↓', 'completions / queue edit / history'],
|
||||
['Ctrl+X', 'delete the queued message you’re editing (esc cancels edit)'],
|
||||
['Ctrl+X', 'delete the queued message you’re editing (Esc cancels edit)'],
|
||||
[action + '+A/E', 'home / end of line'],
|
||||
[action + '+Z / ' + action + '+Y', 'undo / redo input edits'],
|
||||
[action + '+W', 'delete word'],
|
||||
|
||||
@@ -1,6 +1,8 @@
|
||||
import { useCallback, useRef, useState } from 'react'
|
||||
|
||||
export function removeAt<T>(arr: T[], i: number): T[] {
|
||||
// Mutates `arr` in place; returned reference is the same input array, kept
|
||||
// so callers can chain. Use `Array.prototype.toSpliced` if you need a copy.
|
||||
export function removeAtInPlace<T>(arr: T[], i: number): T[] {
|
||||
if (i < 0 || i >= arr.length) {
|
||||
return arr
|
||||
}
|
||||
@@ -50,7 +52,7 @@ export function useQueue() {
|
||||
(i: number) => {
|
||||
const before = queueRef.current.length
|
||||
|
||||
removeAt(queueRef.current, i)
|
||||
removeAtInPlace(queueRef.current, i)
|
||||
|
||||
if (queueRef.current.length !== before) {
|
||||
syncQueue()
|
||||
|
||||
Reference in New Issue
Block a user