Repository navigation
Conversation
82d1aa3 to
812a5f6
Compare
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
Walkthrough
Priority: Merge Risk:
|
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments: 1
tests/e2e/emacs-keybindings-file-editor.spec.ts: 1
111-113: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert Quick Open through the DOM in both routing checks.
activeModalonly checks store state and can miss a broken Quick Open render. Use the dialog exposed byCommandDialogfor both assertions.Suggested fix
const kb = orcaPage.keyboard + const quickOpenDialog = orcaPage.getByRole('dialog', { name: 'Go to file' }) // Prefix echo: a pending C-x shows in the status row and C-g cancels it without editing. @@ - expect(await orcaPage.evaluate(() => window.__store?.getState().activeModal)).not.toBe( - 'quick-open' - ) + await expect(quickOpenDialog).toBeHidden() @@ - await expect - .poll(() => orcaPage.evaluate(() => window.__store?.getState().activeModal), { - timeout: 5_000 - }) - .toBe('quick-open') + await expect(quickOpenDialog).toBeVisible({ timeout: 5_000 })
ℹ️ Review info
⚙️ Run configuration
- Source mode: Auto
- Repository: Unavailable
- Branch: Unavailable
- File: No configuration file from this repository applied
- Commit: Unavailable
- Layers: Repository UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
31829d4b-e0b8-4c0b-8449-cb80e4104eef
📥 Commits
Reviewing files that changed from the base of the PR and between fd8b2dc and 812a5f66c99796583b16d91fe219b6337fc626f6.
⛔ Files ignored due to path filters: 1
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing: 55
config/build-plugins/monaco-editor-esm-alias.tselectron.vite.config.tspackage.jsonsrc/main/window/createMainWindow-editor-keymap-focus-shortcuts.test.tssrc/main/window/main-window-focus-lifecycle.tssrc/main/window/main-window-shortcut-routing.tssrc/preload/api/ui-bridge-clipboard-and-window-controls.tssrc/preload/api/ui-window-api.tssrc/renderer/src/app-shell/app-command-handlers.tssrc/renderer/src/components/editor/MonacoEditor.tsxsrc/renderer/src/components/editor/VimCheatsheet.tsxsrc/renderer/src/components/editor/editor-shortcuts.tssrc/renderer/src/components/editor/emacs-window-commands.test.tssrc/renderer/src/components/editor/emacs-window-commands.tssrc/renderer/src/components/editor/monaco-emacs-mode.test.tssrc/renderer/src/components/editor/monaco-emacs-mode.tssrc/renderer/src/components/editor/monaco-vim-mode.tssrc/renderer/src/components/editor/use-editor-keymap-focus-mirror.test.tsxsrc/renderer/src/components/editor/use-editor-keymap-focus-mirror.tssrc/renderer/src/components/editor/use-monaco-emacs-mode.tssrc/renderer/src/components/settings/EditorKeybindingsSetting.test.tsxsrc/renderer/src/components/settings/EditorKeybindingsSetting.tsxsrc/renderer/src/components/settings/GeneralEditorSettingsSection.tsxsrc/renderer/src/components/settings/ShortcutCommandBlock.tsxsrc/renderer/src/components/settings/ShortcutFilterRail.tsxsrc/renderer/src/components/settings/ShortcutRowsList.tsxsrc/renderer/src/components/settings/ShortcutsPane.tsxsrc/renderer/src/components/settings/shortcut-editor-status.tssrc/renderer/src/components/settings/shortcut-row-visibility.test.tssrc/renderer/src/components/settings/shortcut-row-visibility.tssrc/renderer/src/components/terminal-workspace-model.tssrc/renderer/src/i18n/locales/en.jsonsrc/renderer/src/i18n/locales/es.jsonsrc/renderer/src/i18n/locales/ja.jsonsrc/renderer/src/i18n/locales/ko.jsonsrc/renderer/src/i18n/locales/zh.jsonsrc/renderer/src/lib/activate-workspace-tab.tssrc/renderer/src/lib/keybinding-context.test.tssrc/renderer/src/lib/keybinding-context.tssrc/renderer/src/lib/tab-number-shortcuts.tssrc/renderer/src/web/preload-api/web-ui-api.tssrc/shared/constants.test.tssrc/shared/default-global-settings.tssrc/shared/editor-keybindings-preset.tssrc/shared/global-settings-base.tssrc/shared/keybindings-editor-context.test.tssrc/shared/keybindings.tssrc/shared/keybindings/editor-keymap-chords.tssrc/shared/keybindings/matching.tssrc/shared/keybindings/types.tssrc/shared/window-shortcut-policy-editor-context.test.tssrc/shared/workspace-layout/tab-group-layout-tree.tstests/e2e/emacs-keybindings-file-editor.spec.tstests/e2e/vim-keybindings-file-editor.spec.tsvite.web.config.ts
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.
812a5f6 to
3585551
Compare
|
Re the nitpick on |
93a121a to
bf00874
Compare
Add an `editorKeybindings: 'default' | 'vim'` setting (default 'default') that enables monaco-vim modal editing in the built-in file editor, with a status bar and :w/:wq/:x wired to save. The toggle lives in Settings → Editor next to Word Wrap as a Default/Vim segmented control. monaco-vim loads lazily and is scoped to the editable, full-height editor (not read-only diffs or auto-height preview editors); the status bar is a real flex-column row below the editor rather than an overlay. The adapter disposes idempotently on toggle or unmount, and :w routes to the focused editor. It shares Orca's locally-bundled monaco instance via loader.config. monaco-vim is a keymap emulator, so LSP-backed :gd, :%s live preview, and macro replay @q are unsupported — documented as known limitations. Closes stablyai#9699.
…tsheet Build on the initial Vim keybindings preset with the app-integration and discoverability pieces the first pass left out: - Wire :q/:q! (close tab) and make :wq/:x save-and-close, routed through the focused editor's Orca save/close like :w already was. - Map editor-display :set options onto Monaco -- wrap/nowrap, number/nonumber, tabstop/shiftwidth/expandtab -- with :set paste/nopaste and unknown options as graceful no-ops (Monaco has no terminal paste-mangling to disable). - Add a command cheatsheet: a hint button in the Vim status bar opens a popover listing supported motions/operators/registers/visual/macro/Ex commands plus the documented limitations. - Correct the preset's limitation notes: registers, visual mode, macros, and :%s substitution are verified working (save/sync proven clean); the real gaps are gd, :%s live-preview, Backspace replay inside a macro, and monaco-vim's extra trailing blank line on linewise paste of a newline-terminated buffer.
Drive the real monaco-vim adapter in a headless Electron run: mode switching with i/Esc/o/dd, :w save-to-disk, yank/paste + :%s substitution + visual-mode delete, macro record/replay, and the :set line-number toggle + :q close. Also exercises the status-bar cheatsheet popover.
- Extend `editorKeybindings` to `'default' | 'vim' | 'emacs'` via an exported `EDITOR_KEYBINDINGS_PRESETS` tuple so the editor and settings share one list - Add the Emacs segment to Settings → General → Editor; unknown stored presets fall back to Default instead of being coerced - Reword the English description and retire the Vim-only copy from target locales so they fall back to English until retranslated
…s selected - Add `monaco-emacs-mode.ts`, which lazy-loads `monaco-emacs` 0.3.0, starts the extension on the mounted editor, and mirrors the pending prefix key / `C-u` argument and mark state into the status row below the editor - Patch monaco-emacs's process-global command table once per renderer: plain Tab goes back to Monaco instead of formatDocument, and `C-x h` (select all) and `S-C-z` (redo) are added - Generalise the Vim status bar row into a keymap status bar shared by both presets; the Vim cheatsheet still renders only for Vim - Move the preset tuple and `isEditorKeybindingsPreset` into `src/shared/editor-keybindings-preset.ts` so the editor and the settings control agree on which stored values are presets
… to its ESM entry
- Add `tests/e2e/emacs-keybindings-file-editor.spec.ts`: the lazy import
attaches (block caret), the `C-x` prefix echoes and `C-g` cancels it, `C-SPC`
/ `M-w` / `C-y` copy a region through the kill ring, `C-k` / `C-y` kill and
yank a line, and plain Tab indents; edits are read back from disk via autosave
- Add `config/build-plugins/monaco-editor-esm-alias.ts` and use it in the
desktop and web Vite configs: monaco-editor's `require` export condition is
its AMD bundle, so monaco-emacs's CommonJS `require('monaco-editor')` bundled
a second Monaco that threw `define is not defined` at runtime
bf00874 to
cb69d69
Compare
|
Rebased onto current main (conflicts in |
There was a problem hiding this comment.
Actionable comments posted: 1

Review info
-
Run configuration-
- Source mode: Auto
- Repository: Unavailable
- Branch: Unavailable
- File: No configuration file from this repository applied
- Commit: Unavailable
- Layers: Repository UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
a90c4e35-7ce3-4894-9890-6733c0394968
Files ignored due to path filters: 1-
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
Files selected for processing: 37-
config/build-plugins/monaco-editor-esm-alias.tselectron.vite.config.tspackage.jsonsrc/main/window/createMainWindow-editor-keymap-focus-shortcuts.test.tssrc/main/window/main-window-focus-lifecycle.tssrc/main/window/main-window-shortcut-routing.tssrc/preload/api/ui-bridge-clipboard-and-window-controls.tssrc/preload/api/ui-window-api.tssrc/renderer/src/app-shell/app-command-handlers.tssrc/renderer/src/components/editor/MonacoEditor.tsxsrc/renderer/src/components/editor/emacs-window-commands.tssrc/renderer/src/components/editor/monaco-emacs-mode.tssrc/renderer/src/components/editor/use-editor-keymap-focus-mirror.tssrc/renderer/src/components/editor/use-monaco-emacs-mode.tssrc/renderer/src/components/settings/EditorKeybindingsSetting.tsxsrc/renderer/src/components/settings/GeneralEditorSettingsSection.tsxsrc/renderer/src/components/settings/ShortcutCommandBlock.tsxsrc/renderer/src/components/settings/ShortcutsPane.tsxsrc/renderer/src/components/settings/shortcut-editor-status.tssrc/renderer/src/components/settings/shortcut-row-visibility.test.tssrc/renderer/src/i18n/locales/en.jsonsrc/renderer/src/i18n/locales/es.jsonsrc/renderer/src/i18n/locales/ja.jsonsrc/renderer/src/i18n/locales/ko.jsonsrc/renderer/src/i18n/locales/zh.jsonsrc/renderer/src/lib/keybinding-context.tssrc/renderer/src/lib/tab-number-shortcuts.tssrc/renderer/src/web/preload-api/web-ui-api.tssrc/shared/default-global-settings.tssrc/shared/editor-keybindings-preset.tssrc/shared/global-settings-base.tssrc/shared/keybindings.tssrc/shared/keybindings/editor-keymap-chords.tssrc/shared/keybindings/matching.tssrc/shared/workspace-layout/tab-group-layout-tree.tstests/e2e/emacs-keybindings-file-editor.spec.tsvite.web.config.ts
Files skipped from review as they are similar to previous changes: 5-
- src/renderer/src/i18n/locales/en.json
- src/renderer/src/i18n/locales/zh.json
- src/renderer/src/i18n/locales/ja.json
- src/renderer/src/i18n/locales/ko.json
- src/renderer/src/i18n/locales/es.json
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.
-
- Add an `editor` keybinding context: while a Monaco editor running the Emacs preset has focus, `keybindingMatchesAction` ignores the chords monaco-emacs binds (Ctrl+P/N/F/B/A/E/W/K/Y/S/…, Alt+F/B/W/…), listed in `src/shared/keybindings/editor-keymap-chords.ts`; every other chord keeps working, and Default/Vim users see no change - Renderer resolves the context from the `data-orca-editor-keymap` marker MonacoEditor sets; the editor-local save/find handlers honour it too - Mirror editor-keymap focus to the main process (`ui:setEditorKeymapFocused`) so the before-input-event allowlist yields the same chords instead of stealing them first - Settings → Shortcuts badges the rows the Emacs preset takes over - E2E drives Ctrl+P natively through before-input-event: line-up inside the editor, quick open outside it
- Register `C-x C-s` in the monaco-emacs table and route it to the focused editor's save hook, the way `:w` reaches it for Vim; with the Emacs preset Ctrl+S is incremental search, so the file needs this path - Make `C-g` close Monaco's find widget after keyboard-quit, as it ends an isearch in Emacs; the command's precondition keeps it a no-op otherwise - Add Monaco keybinding rules scoped to Emacs editors (`orcaEmacsKeymap` context key) so `C-s`/`C-r`/`C-g` step through matches and close the widget while its input has focus, where monaco-emacs never sees keys - Bind redo to Emacs 28's `undo-redo` keys (`C-?`, `C-M-_`) alongside `S-C-z`, and yield those chords to the editor like the rest - E2E: autosave off, every write goes through `C-x C-s`; cover the find widget flow and focus returning to the editor
- `C-x 2` / `C-x 3` open the same file in a new group below / right and move point there; `C-x o` cycles the layout leaves and focuses the group's visible tab (editor, terminal, browser) through the activation path the tab-number shortcuts already use; `C-x 0` closes this view only and refuses the sole window, as Emacs does - Extract `activateWorkspaceTab` from the tab-number shortcut so both callers share one activation path, and move the Emacs attach effect into `useMonacoEmacsMode` to keep MonacoEditor under the line budget - Register the digit chords under Monaco's key names (`C-x Digit2`): monaco-emacs only strips the `Key` prefix, so `C-x 2` never matches - Hold focus on the new view until the layout settles or the next window command runs: inserting a split re-parents the source group, whose editor remounts and would take the keyboard back - Re-read the echo state after each key so a finished `C-x …` clears; upstream fires before running the command and never after the reset - Note that `C-u` numeric arguments are broken upstream for the same digit-naming reason
- Check the caller's cancellation after the lazy import, before any state is created - A stale install's dispose no longer removes the hooks and context key of a newer install on the same editor
… signal - Track which editor container last reported focus to main - Unmounting another split view, or turning the preset off there, no longer hands Emacs chords such as Ctrl+P back to Orca while an Emacs editor still has focus
…sewhere - A pointerdown while the split settles cancels the focus hold - Clicking another surface right after C-x 2 or C-x 3 keeps the focus the user chose
cb69d69 to
28579fb
Compare
ELI5
Orca's file editor gets an Emacs option next to Default and Vim in Settings → General → Editor. Turn it on and the editor answers to the Emacs keys people know from IntelliJ/Android Studio's Emacs keymap:
C-f/C-b/C-n/C-pto move,C-SPCto set a mark,C-k/C-w/M-w/C-yfor the kill ring,C-sto search,C-x C-sto save. On Linux and Windows those Ctrl keys used to belong to Orca (Ctrl+P opened Go to File, Ctrl+W closed the tab); while an Emacs editor has focus they now reach the editor, and everywhere else they keep doing what they always did.Stacked on #9755 (Vim preset) as its author suggested in #30055, so the first three commits here are xxjjvxb's. The Emacs work is the six commits on top.
What Changed
Before: the
editorKeybindingssetting from #9755 knowsdefaultandvim. On macOS, Monaco's built-in Ctrl keys cover basic movement but there is no Meta (Option+Ftypesƒ), no mark, no kill ring. On Linux/Windows the Emacs Ctrl keys are Orca app shortcuts and are consumed before the editor sees them.After:
editorKeybindingsaccepts'emacs'; the Settings segment shows Default / Vim / Emacs. Unknown stored values fall back to Default, so a profile written by a newer build does not break an older one.monaco-emacsadapter is imported lazily and attached to writable, full-height editors only, with the same attach/dispose lifecycle as the Vim adapter. A status row under the editor echoes the pending prefix (C-x) and "Mark set", and clears once the chord has run (upstream leaves the last prefix on screen). Orca adds what the library lacks:C-x h(select all), redo on Emacs 28'sC-?/C-M-_plusS-C-z,C-x C-s(save through Orca's normal save path, like:w),C-galso closes the find widget, and inside the find widgetC-s/C-rstep to the next/previous match whileC-gcloses it. Emacs window commands map onto Orca's split groups:C-x 2/C-x 3show the same file in a new group below / to the right and move point there,C-x ocycles through the groups in layout order (a terminal or browser in the next group gets focus the way its tab click would), andC-x 0closes this view while the file stays open in its other windows (refused for the sole window, as in Emacs). Plain Tab is handed back to Monaco (upstream maps it to format-document).editorkeybinding context. While a Monaco editor running the Emacs preset has focus (its find widget included), both shortcut layers yield the chords monaco-emacs binds — the main-processbefore-input-eventallowlist and the renderer's window-capture handlers, plus the editor-local save/find handlers. Everything the preset does not bind (Ctrl+Shift+P, Ctrl+Tab, Ctrl+1…) keeps working inside the editor, and nothing changes for Default/Vim users or outside the editor. Settings → Shortcuts shows an "Emacs editor" badge on the affected rows while the preset is on.monaco-editor'srequireexport condition points at its AMD bundle, so a CommonJS dependency doingrequire('monaco-editor')pulled a second, AMD copy of Monaco into the renderer and threwdefine is not definedat runtime. A Vite alias (desktop and web configs) pins the bare specifier to the ESM entry.Why
monaco.editor.addKeybindingRulesalone could remap keys, but mark, kill ring andM-ywould have to be reimplemented;monaco-emacsalready has them and works on the Monaco 0.55.1 Orca ships (verified in the live app by the e2e spec below). Same call xxjjvxb made for monaco-vim.~/.orca/keybindings.jsononly moves the collision to another default and cannot cover editor commands. Yielding by focus context mirrors the existingterminal-firstpolicy, is per-chord (an explicit list of what the preset binds, not "all Ctrl keys"), and leaves every other surface untouched.Mod. Emacs chords are Control chords on macOS too, so the yielded list is written withCtrl; macOS Cmd shortcuts are never affected.Linked Issue
Fixes #30055 (follow-up to #9699; based on #9755)
Visual Proof
Before (Default preset): Meta keys do nothing,
C-SPCopens suggestions instead of setting a mark,C-x 3types3, and Ctrl+P (bound to quick open as on Linux/Windows) opens Go to File from inside the editor.orca-emacs-before.webm
After (Emacs preset, Settings → General → Editor Keybindings): movement, mark +
M-w+C-y,C-k/C-y, Ctrl+P as previous-line inside the editor,C-s/C-g,C-x C-ssave,C-x 3/C-x o/C-x 0, undo / redo.orca-emacs-after.webm
Testing
Platforms: macOS (Apple Silicon) locally. Linux/Windows behaviour is covered by platform-parameterised unit tests on the shared matcher and the main-process router; the e2e spec rebinds quick open to
Ctrl+Pso the Linux/Windows collision is exercised on macOS too.pnpm testfor the touched areas: setting + Settings segment, Emacs adapter, shared keybinding matcher (editorcontext, per platform), main-process router (before-input-eventwith editor focus reported/withdrawn, foreign-sender IPC ignored), renderer context resolution and focus mirror, Settings badge, localization catalog.tests/e2e/emacs-keybindings-file-editor.spec.ts(@golden, headless Electron): preset attaches to the live editor,C-xprefix echo andC-g, mark +M-w+C-y,C-k/C-y, Tab indents,C-sopens find andC-s/C-r/C-gwork inside the widget,C-x C-swrites to disk with autosave off,C-x 3/C-x o/C-x 0split, cycle and close views, and — driven through Electron'sbefore-input-eventwith a native input event —Ctrl+Pis line-up inside the editor but opens quick open from app focus.tests/e2e/vim-keybindings-file-editor.spec.tsstill passes.pnpm tc:web,pnpm tc:node,pnpm run check:code-quality:changed(the one remaining finding,VimCheatsheet.tsx:235, is from feat: add a Vim keybindings preset to the Monaco file editor #9755),pnpm build:web.Note for anyone extending the e2e: Playwright's
keyboard.pressnever reaches Electron'sbefore-input-event; main-routed chords have to be sent withwebContents.sendInputEvent.AI Disclosure
Written with Claude Code (Claude Fable 5.1): design discussion, implementation, tests and this description, reviewed and run by me.
Review
Agent skill upstream boundary
Notes
C-unumeric arguments do not work with current Monaco, because monaco-emacs expects digit keys to be named4while Monaco 0.55 names themDigit4. The window commands are registered under Monaco's names for that reason. Not fixed here; it belongs upstream.monaco-emacs@0.3.0(devDependency, bundled by Vite likemonaco-vim; last release 2022, verified on Monaco 0.55.1).'emacs'is a new optional value of an existing optional setting; older clients fall back to Default.Checklist
N/Awith reasonpnpm lint,pnpm typecheck,pnpm test, andpnpm buildpass (or CI will cover; local preferred)