mirror of
https://github.com/stablyai/orca.git
synced 2026-10-18 08:03:22 +00:00
fix(shortcuts): preserve native input-source switching
This commit is contained in:
@@ -6,7 +6,11 @@ import type { IDisposable } from '@xterm/xterm'
|
||||
import type { ManagedPane, PaneManager } from '@/lib/pane-manager/pane-manager'
|
||||
import type { PtyTransport } from './pty-transport'
|
||||
import { safeFind } from '../terminal-search-safe-find'
|
||||
import { resolveTerminalShortcutAction } from './terminal-shortcut-policy'
|
||||
import {
|
||||
consumeNativeOnlyTerminalShortcutCompanion,
|
||||
getTerminalShortcutKeyIdentity,
|
||||
resolveTerminalShortcutAction
|
||||
} from './terminal-shortcut-policy'
|
||||
import type { MacOptionAsAlt } from './terminal-shortcut-policy'
|
||||
import {
|
||||
keybindingMatchesAction,
|
||||
@@ -260,6 +264,7 @@ export function useTerminalKeyboardShortcuts({
|
||||
// held. To distinguish left vs right Option, we record the Option key's
|
||||
// location from its own keydown event and clear it on keyup.
|
||||
let optionKeyLocation = 0
|
||||
const pendingNativeOnlyShortcutKeys = new Set<string>()
|
||||
const onModifierDown = (e: KeyboardEvent): void => {
|
||||
if (e.key === 'Alt') {
|
||||
optionKeyLocation = e.location
|
||||
@@ -300,6 +305,8 @@ export function useTerminalKeyboardShortcuts({
|
||||
}
|
||||
|
||||
const onKeyDown = (e: KeyboardEvent): void => {
|
||||
const shortcutKeyIdentity = getTerminalShortcutKeyIdentity(e)
|
||||
pendingNativeOnlyShortcutKeys.delete(shortcutKeyIdentity)
|
||||
const manager = managerRef.current
|
||||
if (!manager) {
|
||||
return
|
||||
@@ -398,6 +405,14 @@ export function useTerminalKeyboardShortcuts({
|
||||
return
|
||||
}
|
||||
|
||||
if (action.type === 'switchInputSource') {
|
||||
// Why: the OS must receive its default action, while xterm must receive
|
||||
// none of the keydown, keypress, or keyup sequence.
|
||||
pendingNativeOnlyShortcutKeys.add(shortcutKeyIdentity)
|
||||
e.stopImmediatePropagation()
|
||||
return
|
||||
}
|
||||
|
||||
if (action.type === 'sendInput') {
|
||||
e.preventDefault()
|
||||
e.stopImmediatePropagation()
|
||||
@@ -607,13 +622,28 @@ export function useTerminalKeyboardShortcuts({
|
||||
}
|
||||
}
|
||||
|
||||
const onNativeOnlyShortcutCompanion = (e: KeyboardEvent): void => {
|
||||
if (consumeNativeOnlyTerminalShortcutCompanion(e, pendingNativeOnlyShortcutKeys)) {
|
||||
// Why: canceling only the companion keypress prevents Chromium's text
|
||||
// insertion without canceling the keydown default used by the OS switch.
|
||||
if (e.type === 'keypress') {
|
||||
e.preventDefault()
|
||||
}
|
||||
e.stopImmediatePropagation()
|
||||
}
|
||||
}
|
||||
|
||||
window.addEventListener('keydown', onModifierDown, { capture: true })
|
||||
window.addEventListener('keyup', onModifierUp, { capture: true })
|
||||
window.addEventListener('keydown', onKeyDown, { capture: true })
|
||||
window.addEventListener('keypress', onNativeOnlyShortcutCompanion, { capture: true })
|
||||
window.addEventListener('keyup', onNativeOnlyShortcutCompanion, { capture: true })
|
||||
return () => {
|
||||
window.removeEventListener('keydown', onModifierDown, { capture: true })
|
||||
window.removeEventListener('keyup', onModifierUp, { capture: true })
|
||||
window.removeEventListener('keydown', onKeyDown, { capture: true })
|
||||
window.removeEventListener('keypress', onNativeOnlyShortcutCompanion, { capture: true })
|
||||
window.removeEventListener('keyup', onNativeOnlyShortcutCompanion, { capture: true })
|
||||
}
|
||||
}, [
|
||||
isActive,
|
||||
|
||||
@@ -1,5 +1,7 @@
|
||||
import { describe, expect, it, vi } from 'vitest'
|
||||
import {
|
||||
consumeNativeOnlyTerminalShortcutCompanion,
|
||||
getTerminalShortcutKeyIdentity,
|
||||
resolveTerminalShortcutAction,
|
||||
type TerminalShortcutEvent
|
||||
} from './terminal-shortcut-policy'
|
||||
@@ -674,6 +676,60 @@ describe('resolveTerminalShortcutAction', () => {
|
||||
)
|
||||
).toEqual({ type: 'splitActivePane', direction: 'horizontal' })
|
||||
})
|
||||
|
||||
it('resolves terminal.switchInputSource via explicit override (for OS input-source chords)', () => {
|
||||
// Why: the configured chord must route to the native-only handler rather
|
||||
// than the terminal shortcut paths that cancel the browser default.
|
||||
const overrides = { 'terminal.switchInputSource': ['Shift+Space'] }
|
||||
expect(
|
||||
resolveTerminalShortcutAction(
|
||||
event({ key: ' ', code: 'Space', shiftKey: true }),
|
||||
true,
|
||||
'false',
|
||||
0,
|
||||
false,
|
||||
overrides
|
||||
)
|
||||
).toEqual({ type: 'switchInputSource' })
|
||||
|
||||
const otherChord = { 'terminal.switchInputSource': ['Ctrl+Space'] }
|
||||
expect(
|
||||
resolveTerminalShortcutAction(
|
||||
event({ key: ' ', code: 'Space', ctrlKey: true }),
|
||||
false,
|
||||
'false',
|
||||
0,
|
||||
false,
|
||||
otherChord
|
||||
)
|
||||
).toEqual({ type: 'switchInputSource' })
|
||||
})
|
||||
|
||||
it('does not resolve switchInputSource for ordinary chords without override', () => {
|
||||
expect(
|
||||
resolveTerminalShortcutAction(event({ key: ' ', code: 'Space', shiftKey: true }), true)
|
||||
).toBeNull()
|
||||
})
|
||||
|
||||
it('suppresses the full companion event sequence for a native-only shortcut', () => {
|
||||
const pendingKeys = new Set([
|
||||
getTerminalShortcutKeyIdentity(event({ key: ' ', code: 'Space', shiftKey: true }))
|
||||
])
|
||||
|
||||
expect(
|
||||
consumeNativeOnlyTerminalShortcutCompanion(
|
||||
{ type: 'keypress', key: ' ', code: 'Space' },
|
||||
pendingKeys
|
||||
)
|
||||
).toBe(true)
|
||||
expect(
|
||||
consumeNativeOnlyTerminalShortcutCompanion(
|
||||
{ type: 'keyup', key: ' ', code: 'Space' },
|
||||
pendingKeys
|
||||
)
|
||||
).toBe(true)
|
||||
expect(pendingKeys.size).toBe(0)
|
||||
})
|
||||
})
|
||||
|
||||
describe('kitty keyboard protocol panes', () => {
|
||||
|
||||
@@ -11,6 +11,10 @@ export type TerminalShortcutEvent = {
|
||||
repeat?: boolean
|
||||
}
|
||||
|
||||
type TerminalShortcutCompanionEvent = Pick<TerminalShortcutEvent, 'key' | 'code'> & {
|
||||
type: string
|
||||
}
|
||||
|
||||
export type MacOptionAsAlt = 'true' | 'false' | 'left' | 'right'
|
||||
|
||||
// Why: macOS composition replaces event.key for punctuation, so we map
|
||||
@@ -42,6 +46,30 @@ export type TerminalShortcutAction =
|
||||
| { type: 'splitActivePane'; direction: 'vertical' | 'horizontal' }
|
||||
| { type: 'scrollViewport'; position: 'top' | 'bottom' }
|
||||
| { type: 'sendInput'; data: string }
|
||||
| { type: 'switchInputSource' }
|
||||
|
||||
export function getTerminalShortcutKeyIdentity(
|
||||
event: Pick<TerminalShortcutEvent, 'key' | 'code'>
|
||||
): string {
|
||||
return event.code || event.key
|
||||
}
|
||||
|
||||
export function consumeNativeOnlyTerminalShortcutCompanion(
|
||||
event: TerminalShortcutCompanionEvent,
|
||||
pendingKeys: Set<string>
|
||||
): boolean {
|
||||
if (event.type !== 'keypress' && event.type !== 'keyup') {
|
||||
return false
|
||||
}
|
||||
const identity = getTerminalShortcutKeyIdentity(event)
|
||||
if (!pendingKeys.has(identity)) {
|
||||
return false
|
||||
}
|
||||
if (event.type === 'keyup') {
|
||||
pendingKeys.delete(identity)
|
||||
}
|
||||
return true
|
||||
}
|
||||
|
||||
/** Kitty keyboard protocol modifier field: 1 + shift(1) + alt(2). */
|
||||
function kittyAltModifiers(shiftKey: boolean): number {
|
||||
@@ -96,6 +124,13 @@ export function resolveTerminalShortcutAction(
|
||||
isWindowsTerminalHost: () => boolean = () => isWindows
|
||||
): TerminalShortcutAction | null {
|
||||
const platform: NodeJS.Platform = isMac ? 'darwin' : isWindows ? 'win32' : 'linux'
|
||||
|
||||
// Why: native-only chords must be captured even on repeat without blocking
|
||||
// the OS default that performs the input-source switch.
|
||||
if (keybindingMatchesAction('terminal.switchInputSource', event, platform, keybindings)) {
|
||||
return { type: 'switchInputSource' }
|
||||
}
|
||||
|
||||
if (!event.repeat) {
|
||||
if (keybindingMatchesAction('terminal.copySelection', event, platform, keybindings)) {
|
||||
return { type: 'copySelection' }
|
||||
|
||||
@@ -145,6 +145,12 @@ describe('shouldBypassXtermKeyboardEvent — Windows/Linux', () => {
|
||||
shouldBypassXtermKeyboardEvent(event({ key: 'c', code: 'KeyC', metaKey: true }), noSel)
|
||||
).toBe(false)
|
||||
})
|
||||
|
||||
it('leaves ordinary Shift+Space available to the terminal', () => {
|
||||
expect(
|
||||
shouldBypassXtermKeyboardEvent(event({ key: ' ', code: 'Space', shiftKey: true }), noSel)
|
||||
).toBe(false)
|
||||
})
|
||||
})
|
||||
|
||||
describe('shouldSuppressTerminalImeKeyboardEvent — Windows/Linux', () => {
|
||||
|
||||
@@ -148,6 +148,12 @@ describe('shouldBypassXtermKeyboardEvent — macOS', () => {
|
||||
shouldBypassXtermKeyboardEvent(event({ key: 'A', code: 'KeyA', shiftKey: true }), opts)
|
||||
).toBe(false)
|
||||
})
|
||||
|
||||
it('leaves ordinary Shift+Space available to the terminal', () => {
|
||||
expect(
|
||||
shouldBypassXtermKeyboardEvent(event({ key: ' ', code: 'Space', shiftKey: true }), opts)
|
||||
).toBe(false)
|
||||
})
|
||||
})
|
||||
|
||||
describe('shouldSuppressTerminalImeKeyboardEvent — macOS', () => {
|
||||
|
||||
@@ -78,6 +78,22 @@ describe('keybindings', () => {
|
||||
})
|
||||
})
|
||||
|
||||
it('allows Shift-only chords only for native input-source switching', () => {
|
||||
const shiftSpace = {
|
||||
key: ' ',
|
||||
code: 'Space',
|
||||
control: false,
|
||||
meta: false,
|
||||
alt: false,
|
||||
shift: true
|
||||
}
|
||||
|
||||
expect(keybindingFromInput(shiftSpace, 'darwin')).toMatchObject({ ok: false })
|
||||
expect(
|
||||
keybindingFromInputForAction('terminal.switchInputSource', shiftSpace, 'darwin')
|
||||
).toEqual({ ok: true, value: 'Shift+Space' })
|
||||
})
|
||||
|
||||
it('captures key events into canonical editable shortcuts', () => {
|
||||
expect(
|
||||
keybindingFromInput(
|
||||
|
||||
@@ -111,6 +111,7 @@ export type KeybindingActionId =
|
||||
| 'terminal.closePane'
|
||||
| 'terminal.splitRight'
|
||||
| 'terminal.splitDown'
|
||||
| 'terminal.switchInputSource'
|
||||
|
||||
export type KeybindingOverrides = Partial<Record<KeybindingActionId, string[]>>
|
||||
|
||||
@@ -146,6 +147,7 @@ export type KeybindingDefinition = {
|
||||
defaultBindings: PlatformBindings
|
||||
allowInTerminal?: boolean
|
||||
allowBareKeybindings?: boolean
|
||||
allowShiftOnlyKeybindings?: boolean
|
||||
conflictGroup?: string
|
||||
}
|
||||
|
||||
@@ -179,6 +181,7 @@ type ParsedKeybinding = {
|
||||
|
||||
type NormalizeKeybindingOptions = {
|
||||
allowBareKeybindings?: boolean
|
||||
allowShiftOnlyKeybindings?: boolean
|
||||
}
|
||||
|
||||
export type KeybindingValidationResult = { ok: true; value: string } | { ok: false; error: string }
|
||||
@@ -1029,6 +1032,32 @@ export const KEYBINDING_DEFINITIONS: readonly KeybindingDefinition[] = [
|
||||
win32: ['Alt+Shift+D']
|
||||
}
|
||||
},
|
||||
{
|
||||
id: 'terminal.switchInputSource',
|
||||
title: 'Switch input source / language (native)',
|
||||
group: 'Terminal Panes',
|
||||
scope: 'terminal',
|
||||
searchKeywords: [
|
||||
'shortcut',
|
||||
'input',
|
||||
'source',
|
||||
'language',
|
||||
'korean',
|
||||
'english',
|
||||
'ime',
|
||||
'switch',
|
||||
'hangul',
|
||||
'layout'
|
||||
],
|
||||
defaultBindings: {
|
||||
darwin: [],
|
||||
linux: [],
|
||||
win32: []
|
||||
},
|
||||
// Why: macOS permits Shift+Space as an input-source shortcut, while normal
|
||||
// Orca actions reject Shift-only bindings to avoid stealing typed text.
|
||||
allowShiftOnlyKeybindings: true
|
||||
},
|
||||
...buildAgentTabKeybindingDefinitions()
|
||||
]
|
||||
|
||||
@@ -1380,13 +1409,21 @@ function normalizeKeybindingWithOptions(
|
||||
}
|
||||
const isShiftInsert = parsed.shift && parsed.key === 'Insert'
|
||||
const isBareAllowed = options.allowBareKeybindings === true && isSafeBareKey(parsed)
|
||||
const isShiftOnlyAllowed =
|
||||
options.allowShiftOnlyKeybindings === true &&
|
||||
parsed.shift &&
|
||||
!parsed.mod &&
|
||||
!parsed.meta &&
|
||||
!parsed.control &&
|
||||
!parsed.alt
|
||||
if (
|
||||
!parsed.mod &&
|
||||
!parsed.meta &&
|
||||
!parsed.control &&
|
||||
!parsed.alt &&
|
||||
!isShiftInsert &&
|
||||
!isBareAllowed
|
||||
!isBareAllowed &&
|
||||
!isShiftOnlyAllowed
|
||||
) {
|
||||
return { ok: false, error: 'Include at least one modifier key.' }
|
||||
}
|
||||
@@ -1446,8 +1483,10 @@ function normalizeKeybindingArrayWithOptions(
|
||||
}
|
||||
|
||||
function normalizeOptionsForAction(actionId: KeybindingActionId): NormalizeKeybindingOptions {
|
||||
const definition = DEFINITIONS_BY_ID.get(actionId)
|
||||
return {
|
||||
allowBareKeybindings: DEFINITIONS_BY_ID.get(actionId)?.allowBareKeybindings === true
|
||||
allowBareKeybindings: definition?.allowBareKeybindings === true,
|
||||
allowShiftOnlyKeybindings: definition?.allowShiftOnlyKeybindings === true
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1756,7 +1795,8 @@ export function keybindingFromInputForAction(
|
||||
function getDefaultBindings(definition: KeybindingDefinition, platform: NodeJS.Platform): string[] {
|
||||
return definition.defaultBindings[getKeybindingPlatform(platform)].map((binding) => {
|
||||
const normalized = normalizeKeybindingWithOptions(binding, {
|
||||
allowBareKeybindings: definition.allowBareKeybindings === true
|
||||
allowBareKeybindings: definition.allowBareKeybindings === true,
|
||||
allowShiftOnlyKeybindings: definition.allowShiftOnlyKeybindings === true
|
||||
})
|
||||
return normalized.ok ? normalized.value : binding
|
||||
})
|
||||
|
||||
Reference in New Issue
Block a user