feat(desktop): add global "Use Option as Meta key" setting#2280
feat(desktop): add global "Use Option as Meta key" setting#2280emilbryggare wants to merge 2 commits into
Conversation
Replace per-key Option+letter mappings with a global toggle (like iTerm2, VS Code, Alacritty) that maps all Option+letter combos to ESC+letter on macOS. When enabled, Option+Left/Right send Alt-modified CSI sequences for TUI compatibility (Zellij, tmux). When disabled, behavior is unchanged. - Add optionAsMeta column to local-db settings schema with migration - Add get/set tRPC procedures following existing boolean setting pattern - Add generic Option+letter → ESC+letter keyboard handler using event.code - Add macOS-only toggle in Behavior settings with optimistic updates - Register setting in search index with relevant keywords - Add comprehensive tests covering enabled/disabled/arrow/layout scenarios
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds a persisted "Option as Meta" setting with DB migration, TRPC read/write endpoints, settings UI entry and toggle (macOS-only), terminal wiring to propagate the setting via a ref, and keyboard handler logic/tests to translate Option key events accordingly. Changes
Sequence DiagramsequenceDiagram
participant User as User (macOS)
participant UI as Behavior Settings UI
participant TRPC as TRPC Settings API
participant DB as Settings DB
participant Terminal as Terminal Component
participant KB as Keyboard Handler
participant XTerm as XTerm
User->>UI: toggle "Use Option as Meta"
UI->>TRPC: setOptionAsMeta({ enabled: true })
TRPC->>DB: upsert option_as_meta = 1
DB-->>TRPC: success
TRPC-->>UI: { success: true }
UI->>Terminal: invalidate / refresh
Terminal->>TRPC: getOptionAsMeta()
TRPC->>DB: read option_as_meta
DB-->>TRPC: 1
TRPC-->>Terminal: optionAsMeta = true
Terminal->>Terminal: optionAsMetaRef.current = true
User->>XTerm: press Option+B
XTerm->>KB: KeyboardEvent (Option+B)
KB->>KB: check optionAsMetaRef.current
alt optionAsMeta enabled
KB->>XTerm: write ESC + b
else disabled
KB->>XTerm: write ESC+b or CSI sequence
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 12 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.ts">
<violation number="1" location="apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.ts:648">
P2: Option+Shift+letter silently drops the Shift modifier. The handler always sends ESC+lowercase, so Option+Shift+P sends `ESC+p` instead of the expected `ESC+P`. These are distinct keybindings in tmux, vim, and other TUI apps. Honor `event.shiftKey` to send the correct case.</violation>
</file>
Since this is your first cubic review, here's how it works:
- cubic automatically reviews your code and comments on bugs and improvements
- Teach cubic by replying to its comments. cubic learns from your replies and gets better over time
- Add one-off context when rerunning by tagging
@cubic-dev-aiwith guidance or docs links (includingllms.txt) - Ask questions if you need clarification on any suggestion
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
apps/desktop/src/renderer/routes/_authenticated/settings/behavior/components/BehaviorSettings/BehaviorSettings.tsx (1)
166-169: Use the shared platform constant instead of querying it.
process.platformis static for the renderer, sowindow.getPlatformjust adds async state and an extra failure path for whether this control renders. There's already a shared platform constant inapps/desktop/src/shared/constants.ts; using that here would also let you gategetOptionAsMetawithenabled: isMac.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/routes/_authenticated/settings/behavior/components/BehaviorSettings/BehaviorSettings.tsx` around lines 166 - 169, Replace the async platform query with the shared compile-time platform constant from apps/desktop/src/shared/constants.ts (use the exported name there, e.g., SHARED_PLATFORM or IS_MAC) instead of calling electronTrpc.window.getPlatform.useQuery; compute isMac from that constant (isMac = SHARED_PLATFORM === "darwin" or use exported IS_MAC), remove the electronTrpc.window.getPlatform.useQuery call, and pass enabled: isMac to electronTrpc.settings.getOptionAsMeta.useQuery so getOptionAsMeta only runs on macOS.packages/local-db/src/schema/schema.ts (1)
211-211: Avoid persisting a tri-state for this boolean setting.This works today because the router coerces
NULLback tofalse, but it meanssettings.optionAsMetais not actually boolean at rest. If this migration is still pre-release, I'd make this.default(false)here and addDEFAULT 0inpackages/local-db/drizzle/0036_add_option_as_meta_setting.sqlso future callers don't have to remember the nullish fallback.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/local-db/src/schema/schema.ts` at line 211, The column declaration optionAsMeta currently allows NULL; change integer("option_as_meta", { mode: "boolean" }) to include a default false (e.g. integer("option_as_meta", { mode: "boolean", default: false })) so the setting is stored as a proper boolean at rest, and update the corresponding migration (0036_add_option_as_meta_setting.sql) to set DEFAULT 0 for the option_as_meta column so existing/new DB rows get a non-null default.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@apps/desktop/src/renderer/routes/_authenticated/settings/behavior/components/BehaviorSettings/BehaviorSettings.tsx`:
- Around line 166-169: Replace the async platform query with the shared
compile-time platform constant from apps/desktop/src/shared/constants.ts (use
the exported name there, e.g., SHARED_PLATFORM or IS_MAC) instead of calling
electronTrpc.window.getPlatform.useQuery; compute isMac from that constant
(isMac = SHARED_PLATFORM === "darwin" or use exported IS_MAC), remove the
electronTrpc.window.getPlatform.useQuery call, and pass enabled: isMac to
electronTrpc.settings.getOptionAsMeta.useQuery so getOptionAsMeta only runs on
macOS.
In `@packages/local-db/src/schema/schema.ts`:
- Line 211: The column declaration optionAsMeta currently allows NULL; change
integer("option_as_meta", { mode: "boolean" }) to include a default false (e.g.
integer("option_as_meta", { mode: "boolean", default: false })) so the setting
is stored as a proper boolean at rest, and update the corresponding migration
(0036_add_option_as_meta_setting.sql) to set DEFAULT 0 for the option_as_meta
column so existing/new DB rows get a non-null default.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e9405c5e-0a66-4f56-9be9-8d0882b0d4e9
📒 Files selected for processing (12)
apps/desktop/src/lib/trpc/routers/settings/index.tsapps/desktop/src/renderer/routes/_authenticated/settings/behavior/components/BehaviorSettings/BehaviorSettings.tsxapps/desktop/src/renderer/routes/_authenticated/settings/utils/settings-search/settings-search.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/Terminal.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.test.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/hooks/useTerminalLifecycle.tsapps/desktop/src/shared/constants.tspackages/local-db/drizzle/0036_add_option_as_meta_setting.sqlpackages/local-db/drizzle/meta/0036_snapshot.jsonpackages/local-db/drizzle/meta/_journal.jsonpackages/local-db/src/schema/schema.ts
- Honor Shift modifier: Option+Shift+P now sends ESC+P (uppercase), distinct from ESC+p, for correct tmux/vim keybindings - Use PLATFORM.IS_MAC constant instead of tRPC query for platform check - Gate getOptionAsMeta query with enabled: isMac to skip on non-macOS - Add test for Shift modifier preservation
|
Bump from #1359 reporter — sharing some additional context and a design question. Worth flagging: when this bug fires, Claude Code itself shows an in-terminal hint:
Since Superset claims A bit of historical context for reviewers seeing this for the first time — this PR is the natural follow-up to #899, which (rightly) made Option+letter pass through to native character composition so users on PL / DE / IT / FR / ES / Nordic layouts can type
Design question / alternative worth considering: both Kitty ( Are there other alternatives already on the table I might've missed? |
2 participants
Summary
Changes
optionAsMetaboolean column to local-db settings with migrationgetOptionAsMeta/setOptionAsMetaproceduresevent.codefor layout independence; Option+arrows send CSI sequences when enabledTest plan
bun test helpers.test.tspasses (122 tests)bun run typecheckpassesSummary by cubic
Adds a macOS-only “Use Option as Meta key” setting that maps Option+letter to ESC+letter in the terminal and enables Alt‑modified arrow sequences for TUI apps. When off, behavior is unchanged.
PLATFORM.IS_MAC.optionAsMetain local DB (defaultDEFAULT_OPTION_AS_META=false) and exposesgetOptionAsMeta/setOptionAsMetaviatRPC.event.codefor layout independence, maps Option+letter to ESC+letter, preserves Shift (Option+Shift+P → ESC+P), and sends Alt‑modified CSI for Option+Left/Right when enabled (falls back to ESC+b/f when disabled).Written for commit bd26967. Summary will update on new commits.
Summary by CodeRabbit
New Features
Settings / Search
Tests