mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-06-19 10:02:16 +00:00
fix(tui): support named-key tokens in voice.record_key (space, enter, …)
Reviewer caught that the round-1 parser in #18994 rejected every multi-character token, so a config value like ``ctrl+space`` (which the CLI happily binds via prompt_toolkit's ``c-space`` rewrite in ``cli.py``) silently fell back to the documented Ctrl+B default — re-introducing the same false-shortcut bug the PR was meant to fix, just at a different surface. Add explicit named-key support that mirrors what the CLI accepts: * ``space`` (alias: ``spc``) → matches ``ch === ' '`` * ``enter`` (alias: ``return``, ``ret``) → matches ``key.return`` * ``tab`` → matches ``key.tab`` * ``escape`` (alias: ``esc``) → matches ``key.escape`` * ``backspace`` (alias: ``bs``) → matches ``key.backspace`` * ``delete`` (alias: ``del``) → matches ``key.delete`` ``ParsedVoiceRecordKey`` gains an optional ``named`` field; ``ch`` holds either a single char (back-compat) or the canonical named token, and the runtime matcher dispatches on ``named`` before checking the modifier shape. Aliases collapse to one canonical name so ``ctrl+esc`` and ``ctrl+escape`` behave identically. Unrecognised multi-character tokens (e.g. ``ctrl+spcae`` typo, or unsupported keys like ``ctrl+f5``) still fall back to the Ctrl+B default rather than silently disabling the binding — keeps the "typo never silently kills the shortcut" guarantee. Tests: * ``parseVoiceRecordKey`` parametrised over every named token + each alias variant. * New ``isVoiceToggleKey`` cases for space (ch-based match), enter (``key.return``), tab, escape, backspace, delete, including modifier-mismatch negatives. * ``formatVoiceRecordKey`` renders named keys in title case (``Ctrl+Space``, ``Ctrl+Enter``). * Existing fall-back-to-Ctrl+B contract preserved for empty input AND unrecognised multi-char tokens. Full TUI suite: 559/559 pass; ``tsc --noEmit`` clean. Refs #18994 (round-1 review feedback) Co-authored-by: asheriif <ahmedsherif95@gmail.com>
This commit is contained in:
Tranquil-Flow
Brooklyn Nicholson
parent
5231815854
commit
2e7bbf2ee7
2 changed files with 170 additions and 17 deletions
|
|
@ -90,13 +90,10 @@ describe('isVoiceToggleKey', () => {
|
|||
})
|
||||
|
||||
describe('parseVoiceRecordKey (#18994)', () => {
|
||||
it('falls back to Ctrl+B for empty / malformed input', async () => {
|
||||
it('falls back to Ctrl+B for empty input', async () => {
|
||||
const { DEFAULT_VOICE_RECORD_KEY, parseVoiceRecordKey } = await importPlatform('linux')
|
||||
|
||||
expect(parseVoiceRecordKey('')).toEqual(DEFAULT_VOICE_RECORD_KEY)
|
||||
// Multi-character chunks are unsupported (CLI binds single keys), so a
|
||||
// typo like "ctrl+space" falls back to the doc default.
|
||||
expect(parseVoiceRecordKey('ctrl+space')).toEqual(DEFAULT_VOICE_RECORD_KEY)
|
||||
})
|
||||
|
||||
it('parses ctrl+<letter> bindings', async () => {
|
||||
|
|
@ -116,6 +113,37 @@ describe('parseVoiceRecordKey (#18994)', () => {
|
|||
expect(parseVoiceRecordKey('super+b').mod).toBe('super')
|
||||
expect(parseVoiceRecordKey('win+b').mod).toBe('super')
|
||||
})
|
||||
|
||||
it('parses named keys (space, enter, tab, escape, backspace, delete)', async () => {
|
||||
const { parseVoiceRecordKey } = await importPlatform('linux')
|
||||
|
||||
// Every named token from the CLI's prompt_toolkit ``c-<name>`` set is
|
||||
// accepted with both the canonical name and its common alias.
|
||||
expect(parseVoiceRecordKey('ctrl+space')).toEqual({
|
||||
ch: 'space',
|
||||
mod: 'ctrl',
|
||||
named: 'space',
|
||||
raw: 'ctrl+space'
|
||||
})
|
||||
expect(parseVoiceRecordKey('alt+enter').named).toBe('enter')
|
||||
expect(parseVoiceRecordKey('alt+return').named).toBe('enter') // ``return`` ↔ ``enter``
|
||||
expect(parseVoiceRecordKey('ctrl+tab').named).toBe('tab')
|
||||
expect(parseVoiceRecordKey('ctrl+escape').named).toBe('escape')
|
||||
expect(parseVoiceRecordKey('ctrl+esc').named).toBe('escape') // ``esc`` alias
|
||||
expect(parseVoiceRecordKey('ctrl+backspace').named).toBe('backspace')
|
||||
expect(parseVoiceRecordKey('ctrl+delete').named).toBe('delete')
|
||||
expect(parseVoiceRecordKey('ctrl+del').named).toBe('delete') // ``del`` alias
|
||||
})
|
||||
|
||||
it('falls back to Ctrl+B for unrecognised multi-character tokens', async () => {
|
||||
const { DEFAULT_VOICE_RECORD_KEY, parseVoiceRecordKey } = await importPlatform('linux')
|
||||
|
||||
// Typos / unsupported names (``ctrl+spcae``, ``ctrl+f5``, …) fall back
|
||||
// to the documented Ctrl+B default rather than silently disabling the
|
||||
// binding.
|
||||
expect(parseVoiceRecordKey('ctrl+spcae')).toEqual(DEFAULT_VOICE_RECORD_KEY)
|
||||
expect(parseVoiceRecordKey('ctrl+f5')).toEqual(DEFAULT_VOICE_RECORD_KEY)
|
||||
})
|
||||
})
|
||||
|
||||
describe('formatVoiceRecordKey (#18994)', () => {
|
||||
|
|
@ -127,6 +155,14 @@ describe('formatVoiceRecordKey (#18994)', () => {
|
|||
expect(formatVoiceRecordKey(parseVoiceRecordKey('alt+r'))).toBe('Alt+R')
|
||||
expect(formatVoiceRecordKey(parseVoiceRecordKey('cmd+b'))).toBe('Cmd+B')
|
||||
})
|
||||
|
||||
it('renders named keys in title case (Ctrl+Space, Ctrl+Enter)', async () => {
|
||||
const { formatVoiceRecordKey, parseVoiceRecordKey } = await importPlatform('linux')
|
||||
|
||||
expect(formatVoiceRecordKey(parseVoiceRecordKey('ctrl+space'))).toBe('Ctrl+Space')
|
||||
expect(formatVoiceRecordKey(parseVoiceRecordKey('alt+enter'))).toBe('Alt+Enter')
|
||||
expect(formatVoiceRecordKey(parseVoiceRecordKey('ctrl+esc'))).toBe('Ctrl+Escape')
|
||||
})
|
||||
})
|
||||
|
||||
describe('isVoiceToggleKey honours configured record key (#18994)', () => {
|
||||
|
|
@ -148,6 +184,35 @@ describe('isVoiceToggleKey honours configured record key (#18994)', () => {
|
|||
expect(isVoiceToggleKey({ ctrl: false, meta: false, super: false }, 'r', altR)).toBe(false)
|
||||
})
|
||||
|
||||
it('binds named keys via ink event flags (space → ch === " ", enter → key.return, …)', async () => {
|
||||
const { isVoiceToggleKey, parseVoiceRecordKey } = await importPlatform('linux')
|
||||
|
||||
const ctrlSpace = parseVoiceRecordKey('ctrl+space')
|
||||
expect(isVoiceToggleKey({ ctrl: true, meta: false, super: false }, ' ', ctrlSpace)).toBe(true)
|
||||
// Single-char ``b`` must NOT match a ``space``-configured binding.
|
||||
expect(isVoiceToggleKey({ ctrl: true, meta: false, super: false }, 'b', ctrlSpace)).toBe(false)
|
||||
// Space without the configured modifier must not fire either.
|
||||
expect(isVoiceToggleKey({ ctrl: false, meta: false, super: false }, ' ', ctrlSpace)).toBe(false)
|
||||
|
||||
const ctrlEnter = parseVoiceRecordKey('ctrl+enter')
|
||||
expect(isVoiceToggleKey({ ctrl: true, meta: false, return: true, super: false }, '', ctrlEnter)).toBe(true)
|
||||
expect(isVoiceToggleKey({ ctrl: true, meta: false, return: false, super: false }, '', ctrlEnter)).toBe(false)
|
||||
|
||||
const altTab = parseVoiceRecordKey('alt+tab')
|
||||
expect(isVoiceToggleKey({ alt: true, ctrl: false, meta: false, super: false, tab: true }, '', altTab)).toBe(true)
|
||||
expect(isVoiceToggleKey({ alt: false, ctrl: false, meta: false, super: false, tab: true }, '', altTab)).toBe(false)
|
||||
|
||||
const ctrlEscape = parseVoiceRecordKey('ctrl+escape')
|
||||
expect(isVoiceToggleKey({ ctrl: true, escape: true, meta: false, super: false }, '', ctrlEscape)).toBe(true)
|
||||
expect(isVoiceToggleKey({ ctrl: true, escape: false, meta: false, super: false }, '', ctrlEscape)).toBe(false)
|
||||
|
||||
const ctrlBackspace = parseVoiceRecordKey('ctrl+backspace')
|
||||
expect(isVoiceToggleKey({ backspace: true, ctrl: true, meta: false, super: false }, '', ctrlBackspace)).toBe(true)
|
||||
|
||||
const ctrlDelete = parseVoiceRecordKey('ctrl+delete')
|
||||
expect(isVoiceToggleKey({ ctrl: true, delete: true, meta: false, super: false }, '', ctrlDelete)).toBe(true)
|
||||
})
|
||||
|
||||
it('omitted configured key falls back to ctrl+b (back-compat)', async () => {
|
||||
const { isVoiceToggleKey } = await importPlatform('linux')
|
||||
|
||||
|
|
|
|||
|
|
@ -64,9 +64,18 @@ export const isCopyShortcut = (
|
|||
*/
|
||||
export type VoiceRecordKeyMod = 'alt' | 'ctrl' | 'meta' | 'super'
|
||||
|
||||
/** Named (multi-character) keys we support, matching the CLI's
|
||||
* prompt_toolkit binding shape (``c-space``, ``c-enter``, etc.) so a
|
||||
* config value like ``ctrl+space`` binds in both runtimes. */
|
||||
export type VoiceRecordKeyNamed = 'backspace' | 'delete' | 'enter' | 'escape' | 'space' | 'tab'
|
||||
|
||||
export interface ParsedVoiceRecordKey {
|
||||
/** Single character (``'b'``, ``'o'``) when ``named`` is undefined,
|
||||
* otherwise the named-key token (``'space'``, ``'enter'``…). Kept as
|
||||
* one field for back-compat with the v1 ``{ ch, mod, raw }`` shape. */
|
||||
ch: string
|
||||
mod: VoiceRecordKeyMod
|
||||
named?: VoiceRecordKeyNamed
|
||||
raw: string
|
||||
}
|
||||
|
||||
|
|
@ -90,10 +99,72 @@ const _MOD_ALIASES: Record<string, VoiceRecordKeyMod> = {
|
|||
windows: 'super'
|
||||
}
|
||||
|
||||
/** Map config-string named tokens to the canonical name used at match time.
|
||||
*
|
||||
* Aliases mirror what prompt_toolkit accepts (``return`` ↔ ``enter``,
|
||||
* ``esc`` ↔ ``escape``) so a config that round-trips through the CLI also
|
||||
* binds in the TUI. */
|
||||
const _NAMED_KEY_ALIASES: Record<string, VoiceRecordKeyNamed> = {
|
||||
backspace: 'backspace',
|
||||
bs: 'backspace',
|
||||
del: 'delete',
|
||||
delete: 'delete',
|
||||
enter: 'enter',
|
||||
esc: 'escape',
|
||||
escape: 'escape',
|
||||
ret: 'enter',
|
||||
return: 'enter',
|
||||
space: 'space',
|
||||
spc: 'space',
|
||||
tab: 'tab'
|
||||
}
|
||||
|
||||
interface RuntimeKeyEvent {
|
||||
alt?: boolean
|
||||
backspace?: boolean
|
||||
ctrl: boolean
|
||||
delete?: boolean
|
||||
escape?: boolean
|
||||
meta: boolean
|
||||
return?: boolean
|
||||
super?: boolean
|
||||
tab?: boolean
|
||||
}
|
||||
|
||||
/** Match an ink ``key`` event against a parsed named key. The ink runtime
|
||||
* sets one boolean per named key; ``space`` is a printable char so it
|
||||
* arrives as ``ch === ' '`` rather than a dedicated ``key.space`` flag. */
|
||||
const _matchesNamedKey = (
|
||||
named: VoiceRecordKeyNamed,
|
||||
key: RuntimeKeyEvent,
|
||||
ch: string
|
||||
): boolean => {
|
||||
switch (named) {
|
||||
case 'backspace':
|
||||
return key.backspace === true
|
||||
case 'delete':
|
||||
return key.delete === true
|
||||
case 'enter':
|
||||
return key.return === true
|
||||
case 'escape':
|
||||
return key.escape === true
|
||||
case 'space':
|
||||
return ch === ' '
|
||||
case 'tab':
|
||||
return key.tab === true
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Parse a config-string voice record key like ``ctrl+b`` / ``alt+r`` /
|
||||
* ``cmd+space`` into ``{mod, ch}``. Falls back to the documented Ctrl+B
|
||||
* default for empty / malformed input so a typo never silently disables
|
||||
* ``ctrl+space`` into ``{mod, ch, named?}``. Accepts single characters
|
||||
* AND the named tokens declared in ``_NAMED_KEY_ALIASES`` (``space``,
|
||||
* ``enter``/``return``, ``tab``, ``escape``/``esc``, ``backspace``,
|
||||
* ``delete``) — matching the keys prompt_toolkit accepts on the CLI
|
||||
* side via the ``c-<name>`` rewrite in ``cli.py``.
|
||||
*
|
||||
* Falls back to the documented Ctrl+B default for empty input or for
|
||||
* unrecognised multi-character tokens so a typo never silently disables
|
||||
* the shortcut.
|
||||
*/
|
||||
export const parseVoiceRecordKey = (raw: string): ParsedVoiceRecordKey => {
|
||||
|
|
@ -109,7 +180,7 @@ export const parseVoiceRecordKey = (raw: string): ParsedVoiceRecordKey => {
|
|||
return DEFAULT_VOICE_RECORD_KEY
|
||||
}
|
||||
|
||||
const ch = parts[parts.length - 1]
|
||||
const last = parts[parts.length - 1]
|
||||
const modCandidates = parts.slice(0, -1)
|
||||
|
||||
let mod: VoiceRecordKeyMod = 'ctrl'
|
||||
|
|
@ -123,29 +194,46 @@ export const parseVoiceRecordKey = (raw: string): ParsedVoiceRecordKey => {
|
|||
}
|
||||
}
|
||||
|
||||
// Reject multi-character chunks (e.g. "ctrl+space" → ch="space" — we
|
||||
// only support single-character bindings, matching the Python side's
|
||||
// prompt_toolkit binding shape).
|
||||
if (ch.length !== 1) {
|
||||
return DEFAULT_VOICE_RECORD_KEY
|
||||
if (last.length === 1) {
|
||||
return { ch: last, mod, raw: lower }
|
||||
}
|
||||
|
||||
return { ch, mod, raw: lower }
|
||||
const named = _NAMED_KEY_ALIASES[last]
|
||||
|
||||
if (named) {
|
||||
return { ch: named, mod, named, raw: lower }
|
||||
}
|
||||
|
||||
// Unknown multi-character token (e.g. typo'd ``ctrl+spcae``) — fall back
|
||||
// to the doc default rather than silently disabling the binding.
|
||||
return DEFAULT_VOICE_RECORD_KEY
|
||||
}
|
||||
|
||||
/** Render a parsed key back as ``Ctrl+B`` for status text. */
|
||||
/** Render a parsed key back as ``Ctrl+B`` / ``Ctrl+Space`` for status text. */
|
||||
export const formatVoiceRecordKey = (parsed: ParsedVoiceRecordKey): string => {
|
||||
const modLabel = parsed.mod === 'meta' ? 'Cmd' : parsed.mod[0].toUpperCase() + parsed.mod.slice(1)
|
||||
// Named tokens render in title case (Ctrl+Space, Ctrl+Enter); single
|
||||
// chars render upper-case to match the existing Ctrl+B convention.
|
||||
const keyLabel = parsed.named
|
||||
? parsed.named[0].toUpperCase() + parsed.named.slice(1)
|
||||
: parsed.ch.toUpperCase()
|
||||
|
||||
return `${modLabel}+${parsed.ch.toUpperCase()}`
|
||||
return `${modLabel}+${keyLabel}`
|
||||
}
|
||||
|
||||
export const isVoiceToggleKey = (
|
||||
key: { alt?: boolean; ctrl: boolean; meta: boolean; super?: boolean },
|
||||
key: RuntimeKeyEvent,
|
||||
ch: string,
|
||||
configured: ParsedVoiceRecordKey = DEFAULT_VOICE_RECORD_KEY
|
||||
): boolean => {
|
||||
if (ch.toLowerCase() !== configured.ch) {
|
||||
// Match the configured key first (single-char compare or named-key
|
||||
// event-property check). Bail out before evaluating modifier shape
|
||||
// so the wrong key never reaches the modifier guard.
|
||||
if (configured.named) {
|
||||
if (!_matchesNamedKey(configured.named, key, ch)) {
|
||||
return false
|
||||
}
|
||||
} else if (ch.toLowerCase() !== configured.ch) {
|
||||
return false
|
||||
}
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue