diff --git a/docs/reference/keyboard-layout-shortcut-dispatch.md b/docs/reference/keyboard-layout-shortcut-dispatch.md new file mode 100644 index 000000000..33a2a4d86 --- /dev/null +++ b/docs/reference/keyboard-layout-shortcut-dispatch.md @@ -0,0 +1,66 @@ +# Keyboard Layout Shortcut Dispatch + +## Problem + +Keyboard shortcuts must follow the user's active keyboard layout. A shortcut like `Cmd+W` +means "Command plus the key that produces `w`", not "Command plus the physical key labeled +W on a US keyboard." Physical-position matching breaks Dvorak, Colemak, AZERTY, JIS, and +other non-US layouts, and it also makes user keybinding overrides impossible to reason about. + +## Decision + +Orca app shortcuts dispatch by logical key by default. + +The shared keybinding registry in `src/shared/keybindings.ts` is the source of truth for +app commands, configurable commands, shortcut recording, labels, conflict detection, browser +guest forwarding, and terminal pane commands. Code handling a user-facing app command must +call `keybindingMatchesAction`, `keybindingMatchesInput`, or a policy function built on those +helpers. + +Physical `KeyboardEvent.code` may only decide a shortcut when the key is layout-invariant or +the platform cannot provide a real logical key. + +Allowed physical-code uses: + +- Modifier key release tracking, such as left/right Control release for held `Ctrl+Tab`. +- Layout-invariant keys, such as arrows, Tab, Enter, Escape, Backspace, Delete, Insert, + PageUp, PageDown, and explicit numpad bindings. +- Dead, unidentified, or missing logical keys where `KeyboardEvent.key` cannot describe the + produced key. +- Terminal byte encoding where the intent is a physical terminal escape sequence rather than + an Orca command. + +Disallowed physical-code uses: + +- Letter shortcuts for app actions. +- Punctuation shortcuts for app actions when `KeyboardEvent.key` reports the produced + punctuation. +- Clipboard shortcuts that are exposed as app or terminal UI commands. +- Hardcoded undo/redo/new/close/copy/paste handling outside the shared registry. + +## Terminal Boundary + +Terminal handling has two different jobs: + +1. Orca commands that act on terminal UI, such as copy selection, paste, search, clear, + pane focus, split, and close. These are app shortcuts and must be layout-aware. +2. Bytes sent to the shell, such as readline escapes and Option-as-Alt sequences. These may + use physical key positions when terminal compatibility requires it. + +This boundary is intentional. It lets non-US layouts use Orca commands naturally while +preserving shell behavior where users expect physical terminal-control sequences. + +## Regression Requirements + +Shortcut tests must cover both directions of a non-QWERTY swap: + +- The key that produces the configured logical character must match, even if its physical code + differs. +- The physical US key must not match when it produces a different logical character. + +Tests must also cover intentional exceptions: + +- Dead or missing key fallback. +- Shifted punctuation aliases. +- Numpad-specific bindings. +- Terminal byte-encoding paths that intentionally use physical codes. diff --git a/mobile/src/hooks/use-mobile-dictation.ts b/mobile/src/hooks/use-mobile-dictation.ts index 2a8c16170..29963ecec 100644 --- a/mobile/src/hooks/use-mobile-dictation.ts +++ b/mobile/src/hooks/use-mobile-dictation.ts @@ -1,3 +1,6 @@ +/* oxlint-disable max-lines -- Why: mobile dictation keeps permission, recording, + * chunk upload, completion, and cancellation in one hook so native audio state + * cannot drift from the runtime RPC lifecycle. */ import { useCallback, useEffect, useRef, useState } from 'react' import { Buffer } from 'buffer' import { diff --git a/src/renderer/src/App.tsx b/src/renderer/src/App.tsx index 86a712779..f3d25f048 100644 --- a/src/renderer/src/App.tsx +++ b/src/renderer/src/App.tsx @@ -1136,7 +1136,12 @@ function App(): React.JSX.Element { // counterpart, so suppressing them here would silently no-op when // focus lives inside the floating panel. if (isFloatingWorkspacePanelFocused()) { - if (isFloatingWorkspacePanelShortcut(e, isMac)) { + if ( + isFloatingWorkspacePanelShortcut(e, shortcutPlatform, null, keybindings, { + context, + terminalShortcutPolicy: settings?.terminalShortcutPolicy + }) + ) { return } } diff --git a/src/renderer/src/components/floating-terminal/FloatingTerminalPanel.tsx b/src/renderer/src/components/floating-terminal/FloatingTerminalPanel.tsx index 49723d5d6..b97fdb2f4 100644 --- a/src/renderer/src/components/floating-terminal/FloatingTerminalPanel.tsx +++ b/src/renderer/src/components/floating-terminal/FloatingTerminalPanel.tsx @@ -55,7 +55,8 @@ import { FLOATING_TERMINAL_WORKTREE_ID } from '../../../../shared/constants' import { keybindingMatchesAction, type KeybindingActionId, - type KeybindingContext + type KeybindingContext, + type KeybindingMatchOptions } from '../../../../shared/keybindings' import type { BrowserTab as BrowserTabState, @@ -719,38 +720,57 @@ export function FloatingTerminalPanel({ return } + const state = useAppStore.getState() + const platform = getShortcutPlatform() + const context: KeybindingContext = isFloatingWorkspaceTerminalInputTarget(event.target) + ? 'terminal' + : 'app' + const matchOptions: KeybindingMatchOptions = { + context, + terminalShortcutPolicy: state.settings?.terminalShortcutPolicy + } + const matches = (actionId: KeybindingActionId): boolean => + keybindingMatchesAction( + actionId, + event.nativeEvent, + platform, + state.keybindings, + matchOptions + ) + if ( !isFloatingWorkspacePanelShortcut( - event, - navigator.userAgent.includes('Mac'), - panelRef.current + event.nativeEvent, + platform, + panelRef.current, + state.keybindings, + matchOptions ) ) { return } - const key = event.key.toLowerCase() - if (!event.shiftKey && key === 't') { + if (matches('tab.newTerminal')) { event.preventDefault() createFloatingTerminalTab() return } - if (event.shiftKey && key === 'b') { + if (matches('tab.newBrowser')) { event.preventDefault() createFloatingBrowserTab() return } - if (event.shiftKey && key === 'm') { + if (matches('tab.newMarkdown')) { event.preventDefault() createFloatingMarkdownTab() return } - if (event.shiftKey && key === 'o') { + if (matches('tab.openMarkdown')) { event.preventDefault() openFloatingMarkdownTab() return } - if (!event.shiftKey && key === 'w') { + if (matches('tab.close')) { event.preventDefault() if (activeClosableTab) { closeFloatingItem(activeClosableTab.id) diff --git a/src/renderer/src/components/right-sidebar/useFileExplorerKeys.ts b/src/renderer/src/components/right-sidebar/useFileExplorerKeys.ts index 80e07d2ab..03ef1f970 100644 --- a/src/renderer/src/components/right-sidebar/useFileExplorerKeys.ts +++ b/src/renderer/src/components/right-sidebar/useFileExplorerKeys.ts @@ -14,38 +14,6 @@ import { } from './fileExplorerUndoRedo' import { keybindingMatchesAction } from '../../../../shared/keybindings' -function isCmdZRedo(e: KeyboardEvent): boolean { - const isMac = navigator.userAgent.includes('Mac') - const mod = isMac ? e.metaKey : e.ctrlKey - if (!mod || e.altKey) { - return false - } - if (isMac) { - return e.code === 'KeyZ' && e.shiftKey - } - // Windows/Linux: Ctrl+Shift+Z or Ctrl+Y - return (e.code === 'KeyZ' && e.shiftKey) || (e.code === 'KeyY' && !e.shiftKey) -} - -function isCmdZUndo(e: KeyboardEvent): boolean { - const isMac = navigator.userAgent.includes('Mac') - const mod = isMac ? e.metaKey : e.ctrlKey - if (!mod || e.altKey || e.shiftKey) { - return false - } - // Prefer code (layout-independent); fall back to key for edge IME/layout cases. - return e.code === 'KeyZ' || e.key.toLowerCase() === 'z' -} - -function matchesLegacyFileDeleteShortcut(e: KeyboardEvent): boolean { - const isMac = navigator.userAgent.includes('Mac') - return ( - (isMac && e.key === 'Backspace' && e.metaKey) || - (isMac && e.key === 'Delete' && !e.metaKey && !e.ctrlKey && !e.altKey && !e.shiftKey) || - (!isMac && e.key === 'Delete' && !e.metaKey && !e.ctrlKey) - ) -} - /** * Keyboard shortcuts for the file explorer. * @@ -127,8 +95,13 @@ export function useFileExplorerKeys(opts: { // Why: require focus inside the explorer shell (includes the scrollbar, not just // the viewport — Radix renders the scrollbar as a sibling of the viewport). const inExplorer = focusInExplorer() - const wantUndo = isCmdZUndo(e) && fileExplorerHasUndo() - const wantRedo = isCmdZRedo(e) && fileExplorerHasRedo() + const platform = getShortcutPlatform() + const wantUndo = + keybindingMatchesAction('fileExplorer.undo', e, platform, keybindings) && + fileExplorerHasUndo() + const wantRedo = + keybindingMatchesAction('fileExplorer.redo', e, platform, keybindings) && + fileExplorerHasRedo() if (inExplorer && (wantUndo || wantRedo)) { e.preventDefault() const run = wantRedo ? redoFileExplorer() : undoFileExplorer() @@ -147,14 +120,12 @@ export function useFileExplorerKeys(opts: { startRenameRef.current(node) return } - const platform = getShortcutPlatform() - const hasDeleteOverride = Object.prototype.hasOwnProperty.call( - keybindings, - 'fileExplorer.delete' + const wantsDelete = keybindingMatchesAction( + 'fileExplorer.delete', + e, + platform, + keybindings ) - const wantsDelete = hasDeleteOverride - ? keybindingMatchesAction('fileExplorer.delete', e, platform, keybindings) - : matchesLegacyFileDeleteShortcut(e) if (wantsDelete) { e.preventDefault() requestDeleteAllRef.current( @@ -170,7 +141,6 @@ export function useFileExplorerKeys(opts: { if (!focusInExplorer()) { return } - const platform = getShortcutPlatform() const wantsCopyRelativePath = keybindingMatchesAction( 'fileExplorer.copyRelativePath', e, diff --git a/src/renderer/src/components/terminal-pane/xterm-bypass-policy.test.ts b/src/renderer/src/components/terminal-pane/xterm-bypass-policy.test.ts index f7ccaa057..889861aed 100644 --- a/src/renderer/src/components/terminal-pane/xterm-bypass-policy.test.ts +++ b/src/renderer/src/components/terminal-pane/xterm-bypass-policy.test.ts @@ -34,6 +34,15 @@ describe('shouldBypassXtermKeyboardEvent — macOS', () => { ).toBe(true) }) + it('matches Cmd+C by produced logical key rather than physical key', () => { + expect( + shouldBypassXtermKeyboardEvent(event({ key: 'c', code: 'KeyJ', metaKey: true }), opts) + ).toBe(true) + expect( + shouldBypassXtermKeyboardEvent(event({ key: 'j', code: 'KeyC', metaKey: true }), opts) + ).toBe(false) + }) + it('does NOT bubble other Cmd chords — Orca window handlers intercept them before xterm', () => { // Why: this policy is narrowly scoped to Cmd+C, the one clipboard chord // Orca does not intercept at the window level. Cmd+V, Cmd+F, Cmd+D, Cmd+K, @@ -54,8 +63,7 @@ describe('shouldBypassXtermKeyboardEvent — macOS', () => { it('bubbles already-handled Cmd app shortcuts so kitty does not also write to shell', () => { // Why: some window-level shortcuts call preventDefault without stopping - // propagation. VS Code returns false for resolved Meta keybindings for the - // same kitty reason: app shortcuts must not also become terminal input. + // propagation. App shortcuts must not also become terminal input. expect( shouldBypassXtermKeyboardEvent( event({ key: 'b', code: 'KeyB', defaultPrevented: true, metaKey: true }), @@ -161,6 +169,21 @@ describe('shouldBypassXtermKeyboardEvent — Windows/Linux', () => { ).toBe(true) }) + it('matches Ctrl+Shift+C by produced logical key rather than physical key', () => { + expect( + shouldBypassXtermKeyboardEvent( + event({ key: 'C', code: 'KeyJ', ctrlKey: true, shiftKey: true }), + noSel + ) + ).toBe(true) + expect( + shouldBypassXtermKeyboardEvent( + event({ key: 'J', code: 'KeyC', ctrlKey: true, shiftKey: true }), + noSel + ) + ).toBe(false) + }) + it('bubbles Ctrl+C only when there is a selection (otherwise SIGINT)', () => { // Why: bare Ctrl+C without a selection must reach the shell as SIGINT. // With a selection, terminals like Windows Terminal copy instead. @@ -172,6 +195,18 @@ describe('shouldBypassXtermKeyboardEvent — Windows/Linux', () => { ).toBe(false) }) + it('matches Ctrl+C with selection by produced logical key rather than physical key', () => { + expect( + shouldBypassXtermKeyboardEvent(event({ key: 'c', code: 'KeyJ', ctrlKey: true }), withSel) + ).toBe(true) + expect( + shouldBypassXtermKeyboardEvent(event({ key: 'j', code: 'KeyC', ctrlKey: true }), withSel) + ).toBe(false) + expect( + shouldBypassXtermKeyboardEvent(event({ key: 'c', code: 'KeyJ', ctrlKey: true }), noSel) + ).toBe(false) + }) + it('bubbles Ctrl+V and Ctrl+Shift+V for paste', () => { expect( shouldBypassXtermKeyboardEvent(event({ key: 'v', code: 'KeyV', ctrlKey: true }), noSel) @@ -184,6 +219,15 @@ describe('shouldBypassXtermKeyboardEvent — Windows/Linux', () => { ).toBe(true) }) + it('matches paste by produced logical key rather than physical key', () => { + expect( + shouldBypassXtermKeyboardEvent(event({ key: 'v', code: 'KeyK', ctrlKey: true }), noSel) + ).toBe(true) + expect( + shouldBypassXtermKeyboardEvent(event({ key: 'k', code: 'KeyV', ctrlKey: true }), noSel) + ).toBe(false) + }) + it('bubbles Shift+Insert (X11/Linux paste convention)', () => { expect( shouldBypassXtermKeyboardEvent( diff --git a/src/renderer/src/components/terminal-pane/xterm-bypass-policy.ts b/src/renderer/src/components/terminal-pane/xterm-bypass-policy.ts index dde53fb27..1e04b3147 100644 --- a/src/renderer/src/components/terminal-pane/xterm-bypass-policy.ts +++ b/src/renderer/src/components/terminal-pane/xterm-bypass-policy.ts @@ -1,3 +1,5 @@ +import { keybindingMatchesInput } from '../../../../shared/keybindings' + // Why: when a CLI activates kitty progressive enhancement (CSI > N u), xterm's // KittyKeyboard encoder turns every modifier chord — including plain Cmd+C — // into a CSI-u sequence with `cancel: true`, which calls preventDefault() on @@ -9,13 +11,6 @@ // that should bubble to the browser / host (clipboard, native menu). Returning // `false` makes xterm bail *before* the kitty encoder runs, so the browser's // copy pipeline and the OS-level keybinding both fire normally. -// -// Rule source — Ghostty (src/input/key_encode.zig:543-545): -// "on macOS, command+keys do not encode text ... They don't in native text -// inputs (TextEdit) and they also don't in other native terminals -// (Terminal.app, iTerm2)." -// VS Code (terminalInstance.ts:1115-1171) and Superset's terminal (which hit -// this exact bug) both converge on the same pattern. export type XtermBypassEvent = { type: string @@ -49,6 +44,14 @@ function isXtermHandledKeyEvent(type: string): boolean { return type === 'keydown' || type === 'keyup' } +function matchesClipboardBinding( + binding: string, + event: XtermBypassEvent, + platform: NodeJS.Platform +): boolean { + return keybindingMatchesInput(binding, event, platform) +} + /** * Decide whether a chord should bypass xterm's key handlers so the native * browser pipeline (Chromium `copy` event, Electron menu accelerators) or @@ -69,8 +72,8 @@ export function shouldBypassXtermKeyboardEvent( if (event.defaultPrevented && platformModifierHeld) { // Why: window-level Orca shortcuts may have already handled the chord but - // not stopped propagation. Match VS Code by preventing xterm's kitty - // encoder from also sending that app shortcut to the shell. + // not stopped propagation. Do not let xterm also send that shortcut to + // the shell. return true } @@ -88,35 +91,26 @@ export function shouldBypassXtermKeyboardEvent( } if (isMac) { - // Narrow Ghostty rule to Cmd+C only: Ghostty bubbles every Cmd chord on - // macOS, but Orca's window-level handlers (keyboard-handlers.ts, - // TerminalPane.tsx Cmd+V interception) already swallow every Cmd chord - // that does something meaningful before xterm sees it. Cmd+C is the one - // chord that was never intercepted, so it's the only real-world breakage. - // Limiting the bypass to Cmd+C avoids accidentally regressing xterm's - // native Cmd+A select-all path, which goes through a different evaluator - // branch than the kitty encoder. - return ( - event.code === 'KeyC' && event.metaKey && !event.ctrlKey && !event.altKey && !event.shiftKey - ) + // Why: window-level handlers already consume other Cmd chords before xterm + // sees them; this path covers native copy, which must bubble to Chromium. + return matchesClipboardBinding('Mod+C', event, 'darwin') } // Windows/Linux: standard clipboard bindings bubble; Ctrl+C only bubbles // with a selection (otherwise it's SIGINT and must reach the shell). - const onlyCtrl = event.ctrlKey && !event.metaKey && !event.altKey && !event.shiftKey - const ctrlShiftOnly = event.ctrlKey && event.shiftKey && !event.metaKey && !event.altKey - const onlyShift = event.shiftKey && !event.ctrlKey && !event.metaKey && !event.altKey - - if (event.code === 'KeyC' && ctrlShiftOnly) { + if (matchesClipboardBinding('Ctrl+Shift+C', event, 'linux')) { return true } - if (event.code === 'KeyC' && onlyCtrl && hasSelection) { + if (matchesClipboardBinding('Ctrl+C', event, 'linux') && hasSelection) { return true } - if (event.code === 'KeyV' && (onlyCtrl || ctrlShiftOnly)) { + if ( + matchesClipboardBinding('Ctrl+V', event, 'linux') || + matchesClipboardBinding('Ctrl+Shift+V', event, 'linux') + ) { return true } - if (event.code === 'Insert' && onlyShift) { + if (matchesClipboardBinding('Shift+Insert', event, 'linux')) { return true } diff --git a/src/renderer/src/lib/floating-workspace-shortcut-policy.ts b/src/renderer/src/lib/floating-workspace-shortcut-policy.ts new file mode 100644 index 000000000..6e52b6c62 --- /dev/null +++ b/src/renderer/src/lib/floating-workspace-shortcut-policy.ts @@ -0,0 +1,55 @@ +import { + keybindingMatchesAction, + type KeybindingActionId, + type KeybindingMatchOptions, + type KeybindingOverrides +} from '../../../shared/keybindings' + +type FloatingWorkspaceShortcutEvent = Pick< + KeyboardEvent, + 'altKey' | 'code' | 'ctrlKey' | 'key' | 'metaKey' | 'shiftKey' | 'target' +> + +const FLOATING_WORKSPACE_SHORTCUT_SURFACE_SELECTOR = '[data-floating-terminal-shortcut-surface]' +const FLOATING_WORKSPACE_PANEL_SHORTCUT_ACTIONS: readonly KeybindingActionId[] = [ + 'tab.newTerminal', + 'tab.newBrowser', + 'tab.newMarkdown', + 'tab.openMarkdown', + 'tab.close' +] + +function defaultIsMacPlatform(): boolean { + return typeof navigator !== 'undefined' && navigator.userAgent.includes('Mac') +} + +export function isFloatingWorkspacePanelShortcutTarget( + target: EventTarget | null, + panelRoot: HTMLElement | null = null +): boolean { + if (!(target instanceof HTMLElement)) { + return false + } + return ( + target === panelRoot || + target.getAttribute('data-floating-terminal-panel') !== null || + target.closest(FLOATING_WORKSPACE_SHORTCUT_SURFACE_SELECTOR) !== null + ) +} + +export function isFloatingWorkspacePanelShortcut( + event: FloatingWorkspaceShortcutEvent, + platformOrIsMac: NodeJS.Platform | boolean = defaultIsMacPlatform(), + panelRoot: HTMLElement | null = null, + keybindings?: KeybindingOverrides, + options: KeybindingMatchOptions = {} +): boolean { + if (!isFloatingWorkspacePanelShortcutTarget(event.target, panelRoot)) { + return false + } + const platform: NodeJS.Platform = + typeof platformOrIsMac === 'boolean' ? (platformOrIsMac ? 'darwin' : 'linux') : platformOrIsMac + return FLOATING_WORKSPACE_PANEL_SHORTCUT_ACTIONS.some((actionId) => + keybindingMatchesAction(actionId, event, platform, keybindings, options) + ) +} diff --git a/src/renderer/src/lib/floating-workspace-terminal-actions.test.ts b/src/renderer/src/lib/floating-workspace-terminal-actions.test.ts index ec8693eda..5954a4082 100644 --- a/src/renderer/src/lib/floating-workspace-terminal-actions.test.ts +++ b/src/renderer/src/lib/floating-workspace-terminal-actions.test.ts @@ -213,6 +213,40 @@ describe('isFloatingWorkspacePanelShortcut', () => { ) }) + it('claims shortcuts by produced logical key rather than physical key', () => { + expect( + isFloatingWorkspacePanelShortcut( + shortcutSurfaceEvent({ key: 'w', code: 'Comma', metaKey: true }), + 'darwin' + ) + ).toBe(true) + expect( + isFloatingWorkspacePanelShortcut( + shortcutSurfaceEvent({ key: ',', code: 'KeyW', metaKey: true }), + 'darwin' + ) + ).toBe(false) + }) + + it('honors customized tab shortcuts for the floating panel surface', () => { + expect( + isFloatingWorkspacePanelShortcut( + shortcutSurfaceEvent({ key: 'n', code: 'KeyN', ctrlKey: true }), + 'linux', + null, + { 'tab.newTerminal': ['Ctrl+N'] } + ) + ).toBe(true) + expect( + isFloatingWorkspacePanelShortcut( + shortcutSurfaceEvent({ key: 't', code: 'KeyT', ctrlKey: true }), + 'linux', + null, + { 'tab.newTerminal': ['Ctrl+N'] } + ) + ).toBe(false) + }) + it('does not claim shortcuts with Alt or the wrong platform modifier', () => { expect( isFloatingWorkspacePanelShortcut(shortcutSurfaceEvent({ key: 't', metaKey: true }), false) diff --git a/src/renderer/src/lib/floating-workspace-terminal-actions.ts b/src/renderer/src/lib/floating-workspace-terminal-actions.ts index 6b5323f7f..9d5be096c 100644 --- a/src/renderer/src/lib/floating-workspace-terminal-actions.ts +++ b/src/renderer/src/lib/floating-workspace-terminal-actions.ts @@ -14,6 +14,10 @@ import { isWebRuntimeSessionActive } from '@/runtime/web-runtime-session' import { focusTerminalTabSurface } from './focus-terminal-tab-surface' +export { + isFloatingWorkspacePanelShortcut, + isFloatingWorkspacePanelShortcutTarget +} from './floating-workspace-shortcut-policy' type FloatingWorkspaceTerminalStore = Pick< AppState, @@ -35,13 +39,7 @@ type FloatingWorkspaceTabSwitchStore = Pick< | 'unifiedTabsByWorktree' > -type FloatingWorkspaceShortcutEvent = Pick< - KeyboardEvent, - 'altKey' | 'ctrlKey' | 'key' | 'metaKey' | 'shiftKey' | 'target' -> - const FLOATING_WORKSPACE_PANEL_SELECTOR = '[data-floating-terminal-panel]' -const FLOATING_WORKSPACE_SHORTCUT_SURFACE_SELECTOR = '[data-floating-terminal-shortcut-surface]' function getActiveFloatingWorkspaceGroup(store: FloatingWorkspaceTabSwitchStore): TabGroup | null { const groups = store.groupsByWorktree[FLOATING_TERMINAL_WORKTREE_ID] ?? [] @@ -170,10 +168,6 @@ function getNextFloatingWorkspaceTerminalTab( ] } -function defaultIsMacPlatform(): boolean { - return typeof navigator !== 'undefined' && navigator.userAgent.includes('Mac') -} - export function isFloatingWorkspacePanelVisible( doc: Pick = document ): boolean { @@ -197,37 +191,6 @@ export function isFloatingWorkspaceTerminalInputTarget(target: EventTarget | nul return target.classList.contains('xterm-helper-textarea') || target.closest('.xterm') !== null } -export function isFloatingWorkspacePanelShortcutTarget( - target: EventTarget | null, - panelRoot: HTMLElement | null = null -): boolean { - if (!(target instanceof HTMLElement)) { - return false - } - return ( - target === panelRoot || - target.getAttribute('data-floating-terminal-panel') !== null || - target.closest(FLOATING_WORKSPACE_SHORTCUT_SURFACE_SELECTOR) !== null - ) -} - -export function isFloatingWorkspacePanelShortcut( - event: FloatingWorkspaceShortcutEvent, - isMacPlatform = defaultIsMacPlatform(), - panelRoot: HTMLElement | null = null -): boolean { - const mod = isMacPlatform ? event.metaKey && !event.ctrlKey : event.ctrlKey && !event.metaKey - if (!mod || event.altKey) { - return false - } - - const key = event.key.toLowerCase() - const claimedChord = event.shiftKey - ? key === 'b' || key === 'm' || key === 'o' - : key === 't' || key === 'w' - return claimedChord && isFloatingWorkspacePanelShortcutTarget(event.target, panelRoot) -} - export function shouldMinimizeFloatingWorkspacePanelOnCloseShortcut({ activeView, activeWorktreeId, diff --git a/src/shared/keybindings.test.ts b/src/shared/keybindings.test.ts index e082f13a2..bf8a709b0 100644 --- a/src/shared/keybindings.test.ts +++ b/src/shared/keybindings.test.ts @@ -9,6 +9,7 @@ import { keybindingFromInput, keybindingFromInputForAction, keybindingMatchesAction, + keybindingMatchesInput, normalizeKeybinding, normalizeKeybindingListForAction, normalizeKeybindingList @@ -81,6 +82,34 @@ describe('keybindings', () => { expect(formatKeybindingList([], 'win32')).toBe('Unassigned') }) + it('preserves explicit numpad shortcut tokens', () => { + const numpadAdd = { + key: '+', + code: 'NumpadAdd', + control: false, + meta: true, + alt: false, + shift: false + } + + expect(keybindingFromInput(numpadAdd, 'darwin')).toEqual({ + ok: true, + value: 'Mod+NumpadAdd' + }) + expect(keybindingMatchesAction('zoom.in', numpadAdd, 'darwin')).toBe(true) + expect( + keybindingMatchesAction( + 'zoom.out', + { + ...numpadAdd, + key: '-', + code: 'NumpadSubtract' + }, + 'darwin' + ) + ).toBe(true) + }) + it('defines a default shortcut for opening markdown notes', () => { expect(getEffectiveKeybindingsForAction('tab.openMarkdown', 'darwin')).toEqual(['Mod+Shift+O']) expect(formatKeybindingList(['Mod+Shift+O'], 'darwin')).toBe('⌘⇧O') @@ -260,6 +289,107 @@ describe('keybindings', () => { ).toBe(true) }) + it('matches file explorer undo and redo by produced logical key', () => { + expect(getEffectiveKeybindingsForAction('fileExplorer.undo', 'darwin')).toEqual(['Mod+Z']) + expect(getEffectiveKeybindingsForAction('fileExplorer.redo', 'darwin')).toEqual(['Mod+Shift+Z']) + expect(getEffectiveKeybindingsForAction('fileExplorer.redo', 'linux')).toEqual([ + 'Mod+Shift+Z', + 'Ctrl+Y' + ]) + + expect( + keybindingMatchesAction( + 'fileExplorer.undo', + { key: 'z', code: 'Semicolon', control: false, meta: true, alt: false, shift: false }, + 'darwin' + ) + ).toBe(true) + expect( + keybindingMatchesAction( + 'fileExplorer.undo', + { key: ';', code: 'KeyZ', control: false, meta: true, alt: false, shift: false }, + 'darwin' + ) + ).toBe(false) + expect( + keybindingMatchesAction( + 'fileExplorer.redo', + { key: 'Z', code: 'Semicolon', control: false, meta: true, alt: false, shift: true }, + 'darwin' + ) + ).toBe(true) + expect( + keybindingMatchesAction( + 'fileExplorer.redo', + { key: 'y', code: 'KeyF', control: true, meta: false, alt: false, shift: false }, + 'linux' + ) + ).toBe(true) + expect( + keybindingMatchesAction( + 'fileExplorer.redo', + { key: 'f', code: 'KeyY', control: true, meta: false, alt: false, shift: false }, + 'linux' + ) + ).toBe(false) + }) + + it('matches non-QWERTY shortcuts by the produced logical key', () => { + const dvorakPhysicalW = { + key: ',', + code: 'KeyW', + control: false, + meta: true, + alt: false, + shift: false + } + const dvorakPhysicalComma = { + key: 'w', + code: 'Comma', + control: false, + meta: true, + alt: false, + shift: false + } + + expect(keybindingMatchesAction('app.settings', dvorakPhysicalW, 'darwin')).toBe(true) + expect(keybindingMatchesAction('tab.close', dvorakPhysicalW, 'darwin')).toBe(false) + expect(keybindingMatchesAction('tab.close', dvorakPhysicalComma, 'darwin')).toBe(true) + expect(keybindingMatchesAction('app.settings', dvorakPhysicalComma, 'darwin')).toBe(false) + expect(keybindingFromInput(dvorakPhysicalW, 'darwin')).toEqual({ + ok: true, + value: 'Mod+Comma' + }) + expect(keybindingFromInput(dvorakPhysicalComma, 'darwin')).toEqual({ + ok: true, + value: 'Mod+W' + }) + }) + + it('uses shifted punctuation aliases only while Shift is pressed', () => { + const shiftedComma = { + key: '<', + code: 'Comma', + control: false, + meta: true, + alt: false, + shift: true + } + + expect(keybindingMatchesInput('Mod+Shift+Comma', shiftedComma, 'darwin')).toBe(true) + expect(keybindingFromInput(shiftedComma, 'darwin')).toEqual({ + ok: true, + value: 'Mod+Shift+Comma' + }) + expect( + keybindingMatchesInput( + 'Mod+Comma', + { ...shiftedComma, code: 'IntlBackslash', shift: false }, + 'darwin' + ) + ).toBe(false) + }) + it('matches logical bracket shortcuts on JIS keyboards without changing code fallback', () => { const jisLeftBracket = { key: '[', diff --git a/src/shared/keybindings.ts b/src/shared/keybindings.ts index 5a444d445..5053f67c2 100644 --- a/src/shared/keybindings.ts +++ b/src/shared/keybindings.ts @@ -69,6 +69,8 @@ export type KeybindingActionId = | 'editor.save' | 'editor.markdownPreview' | 'editor.copyContext' + | 'fileExplorer.undo' + | 'fileExplorer.redo' | 'fileExplorer.copyPath' | 'fileExplorer.copyRelativePath' | 'fileExplorer.delete' @@ -541,6 +543,26 @@ export const KEYBINDING_DEFINITIONS: readonly KeybindingDefinition[] = [ searchKeywords: ['shortcut', 'editor', 'copy', 'context'], defaultBindings: platformBindings(['Mod+Alt+C']) }, + { + id: 'fileExplorer.undo', + title: 'Undo file operation', + group: 'File Explorer', + scope: 'fileExplorer', + searchKeywords: ['shortcut', 'file explorer', 'undo'], + defaultBindings: platformBindings(['Mod+Z']) + }, + { + id: 'fileExplorer.redo', + title: 'Redo file operation', + group: 'File Explorer', + scope: 'fileExplorer', + searchKeywords: ['shortcut', 'file explorer', 'redo'], + defaultBindings: { + darwin: ['Mod+Shift+Z'], + linux: ['Mod+Shift+Z', 'Ctrl+Y'], + win32: ['Mod+Shift+Z', 'Ctrl+Y'] + } + }, { id: 'fileExplorer.copyPath', title: 'Copy file path', @@ -725,6 +747,9 @@ function hasModifier( } function normalizeKeyToken(token: string): string | null { + if (token === ' ') { + return 'Space' + } const trimmed = token.trim() if (!trimmed) { return null @@ -785,6 +810,8 @@ function normalizeKeyToken(token: string): string | null { BRACKETRIGHT: 'BracketRight', NUMPADADD: 'NumpadAdd', NUMPADSUBTRACT: 'NumpadSubtract', + ADD: 'NumpadAdd', + SUBTRACT: 'NumpadSubtract', COMMA: 'Comma', PERIOD: 'Period', SLASH: 'Slash', @@ -1005,13 +1032,57 @@ const MODIFIER_KEYS = new Set([ 'SymbolLock' ]) -function keyTokenFromInput(input: KeybindingInput): string | null { - const code = input.code ?? '' - const key = input.key ?? '' +const PUNCTUATION_KEY_TOKENS = new Set([ + 'BracketLeft', + 'BracketRight', + 'Minus', + 'Underscore', + 'Equal', + 'Plus', + 'Comma', + 'Period', + 'Slash', + 'Backslash', + 'Semicolon', + 'Quote', + 'Backquote' +]) +const PHYSICAL_CODE_FALLBACK_KEYS = new Set(['', 'Dead', 'Unidentified']) + +const SHIFTED_PUNCTUATION_KEY_TOKENS: Record = { + '<': 'Comma', + '>': 'Period', + '?': 'Slash', + '|': 'Backslash', + ':': 'Semicolon', + '"': 'Quote', + '~': 'Backquote' +} + +function logicalKeyTokenFromInput(input: KeybindingInput): string | null { + const key = input.key ?? '' if (MODIFIER_KEYS.has(key)) { return null } + const normalizedKey = normalizeKeyToken(key) + if (normalizedKey) { + return normalizedKey + } + if (hasModifier(input, 'shift')) { + return SHIFTED_PUNCTUATION_KEY_TOKENS[key] ?? null + } + return null +} + +function canUsePhysicalCodeFallback(input: KeybindingInput): boolean { + // Why: layout-aware shortcuts must trust real logical keys; physical code is + // only a fallback when the platform cannot report the produced key. + return PHYSICAL_CODE_FALLBACK_KEYS.has(input.key ?? '') +} + +function physicalCodeKeyTokenFromInput(input: KeybindingInput): string | null { + const code = input.code ?? '' if (code.startsWith('Key') && code.length === 4) { return code.slice(3).toUpperCase() } @@ -1019,7 +1090,27 @@ function keyTokenFromInput(input: KeybindingInput): string | null { return code.slice(5) } - return normalizeKeyToken(key) ?? normalizeKeyToken(code) + return normalizeKeyToken(code) +} + +function numpadCodeKeyTokenFromInput(input: KeybindingInput): string | null { + const code = input.code ?? '' + return code === 'NumpadAdd' || code === 'NumpadSubtract' ? normalizeKeyToken(code) : null +} + +function keyTokenFromInput(input: KeybindingInput): string | null { + const numpadKey = numpadCodeKeyTokenFromInput(input) + if (numpadKey) { + return numpadKey + } + const logicalKey = logicalKeyTokenFromInput(input) + if (logicalKey) { + return logicalKey + } + if (!canUsePhysicalCodeFallback(input)) { + return null + } + return physicalCodeKeyTokenFromInput(input) } function keybindingFromInputWithOptions( @@ -1162,38 +1253,33 @@ function modifierStateMatches( } function letterKeyMatches(input: KeybindingInput, letter: string): boolean { - const key = (input.key ?? '').toLowerCase() - if (key.length === 1 && key >= 'a' && key <= 'z') { - return key === letter.toLowerCase() + const logicalKey = logicalKeyTokenFromInput(input) + if (logicalKey && logicalKey.length === 1 && logicalKey >= 'A' && logicalKey <= 'Z') { + return logicalKey === letter.toUpperCase() } - return input.code === `Key${letter.toUpperCase()}` + return canUsePhysicalCodeFallback(input) && input.code === `Key${letter.toUpperCase()}` +} + +function digitKeyMatches(input: KeybindingInput, digit: string): boolean { + const logicalKey = logicalKeyTokenFromInput(input) + if (logicalKey && logicalKey.length === 1 && logicalKey >= '0' && logicalKey <= '9') { + return logicalKey === digit + } + return canUsePhysicalCodeFallback(input) && input.code === `Digit${digit}` +} + +function isPunctuationKeyToken(token: string | null): token is string { + return token !== null && PUNCTUATION_KEY_TOKENS.has(token) } function semanticPunctuationKey(input: KeybindingInput): string | null { - switch (input.key) { - case '[': - case '{': - return 'BracketLeft' - case ']': - case '}': - return 'BracketRight' - case '\\': - case '|': - return 'Backslash' - default: - return null - } + const logicalKey = logicalKeyTokenFromInput(input) + return isPunctuationKeyToken(logicalKey) ? logicalKey : null } function physicalPunctuationKey(input: KeybindingInput): string | null { - switch (input.code) { - case 'BracketLeft': - case 'BracketRight': - case 'Backslash': - return input.code - default: - return null - } + const physicalKey = physicalCodeKeyTokenFromInput(input) + return isPunctuationKeyToken(physicalKey) ? physicalKey : null } function shouldUseSemanticPunctuation( @@ -1227,42 +1313,34 @@ function keyMatches( return letterKeyMatches(input, parsedKey) } if (parsedKey.length === 1 && parsedKey >= '0' && parsedKey <= '9') { - return input.key === parsedKey || input.code === `Digit${parsedKey}` + return digitKeyMatches(input, parsedKey) } - const key = input.key ?? '' - const code = input.code ?? '' - switch (parsedKey) { - case 'BracketLeft': - case 'BracketRight': - case 'Backslash': { - // Why: shortcut labels name logical punctuation, but international - // layouts can report the same character from different physical codes. - const semanticKey = semanticPunctuationKey(input) - if (semanticKey !== null && shouldUseSemanticPunctuation(parsed, input, platform)) { - return semanticKey === parsedKey - } - return code === parsedKey - } - case 'Minus': - // Why: shifted "_" is terminal undo/readline input. Users who want it - // as zoom-out can bind it explicitly instead of having the default steal it. - return key === '-' || key === 'Minus' || code === 'Minus' - case 'Underscore': - return key === '_' || key === 'Underscore' - case 'Equal': - return key === '=' || key === 'Equal' || code === 'Equal' - case 'Plus': - return key === '+' || key === 'Plus' - case 'NumpadAdd': - return code === 'NumpadAdd' || key === 'Add' - case 'NumpadSubtract': - return code === 'NumpadSubtract' || key === 'Subtract' - case 'Enter': - return key === 'Enter' && (code === 'Enter' || code === 'NumpadEnter' || code === '') - default: - return key === parsedKey || code === parsedKey + if (parsedKey === 'NumpadAdd' || parsedKey === 'NumpadSubtract') { + return ( + numpadCodeKeyTokenFromInput(input) === parsedKey || + logicalKeyTokenFromInput(input) === parsedKey + ) } + + if (isPunctuationKeyToken(parsedKey)) { + // Why: shortcut labels name logical punctuation, but international + // layouts can report the same character from different physical codes. + const semanticKey = semanticPunctuationKey(input) + if (semanticKey !== null) { + if (!shouldUseSemanticPunctuation(parsed, input, platform)) { + return false + } + return semanticKey === parsedKey + } + return canUsePhysicalCodeFallback(input) && physicalPunctuationKey(input) === parsedKey + } + + const logicalKey = logicalKeyTokenFromInput(input) + if (logicalKey !== null) { + return logicalKey === parsedKey + } + return canUsePhysicalCodeFallback(input) && physicalCodeKeyTokenFromInput(input) === parsedKey } export function keybindingMatchesInput(