Repository navigation
Fix Cmd+K to clear terminal - #230
Conversation
Add CLEAR_TERMINAL hotkey (meta+k) and implement the keyboard handler to clear the terminal buffer when pressed. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughThis PR introduces a clear-terminal hotkey feature. It adds an Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes The changes follow established patterns across the codebase (similar to existing hotkey definitions and callback handlers), involve straightforward additions without complex business logic, and minimal scope across three files. Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/Terminal.tsx(1 hunks)apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.ts(2 hunks)apps/desktop/src/shared/hotkeys.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
apps/desktop/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (apps/desktop/AGENTS.md)
For Electron interprocess communication, ALWAYS use tRPC as defined in
src/lib/trpc
Files:
apps/desktop/src/shared/hotkeys.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/Terminal.tsx
apps/desktop/**/*.{ts,tsx}
📄 CodeRabbit inference engine (apps/desktop/AGENTS.md)
apps/desktop/**/*.{ts,tsx}: Please use alias as defined intsconfig.jsonwhen possible
Prefer zustand for state management if it makes sense. Do not use effect unless absolutely necessary
Files:
apps/desktop/src/shared/hotkeys.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/Terminal.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Avoid usinganytype - use explicit types instead for type safety
Use camelCase for variable and function names following existing codebase patterns
Keep diffs minimal with targeted edits only - avoid unnecessary changes when making modifications
Follow existing patterns and match the codebase style when writing new code
Files:
apps/desktop/src/shared/hotkeys.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/Terminal.tsx
apps/desktop/src/renderer/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Never import Node.js modules (fs, path, os, net, etc.) in renderer process code - browser environment only
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/Terminal.tsx
**/components/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
**/components/**/*.tsx: Create one folder per component with structure:ComponentName/ComponentName.tsx+index.tsfor barrel export
Co-locate tests next to the component file they test (e.g.,ComponentName.test.tsx)
Co-locate dependencies (utils, hooks, constants, config, stories) next to the file using them
Use nestedcomponents/subdirectory within a parent component only if a sub-component is used 2+ times within that parent; otherwise keep it in the parent'scomponents/folder
One component per file - avoid multi-component files
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/Terminal.tsx
apps/desktop/src/renderer/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
Call IPC methods using
window.ipcRenderer.invoke()with object parameters - TypeScript will infer the exact response type automatically
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/Terminal.tsx
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build
🔇 Additional comments (3)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/Terminal.tsx (1)
188-190: LGTM!The
onClearhandler correctly invokesxterm.clear()to clear the terminal buffer when the hotkey is triggered.apps/desktop/src/shared/hotkeys.ts (1)
116-120: LGTM!The hotkey definition follows the established pattern and is correctly categorized under "Terminal".
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.ts (1)
133-134: LGTM!The
onClearcallback is properly typed and documented, following the existing pattern established byonShiftEnter.
| // Handle Cmd+K to clear terminal (handle directly since it needs xterm access) | ||
| const isClearShortcut = | ||
| event.key.toLowerCase() === "k" && | ||
| event.metaKey && | ||
| !event.shiftKey && | ||
| !event.ctrlKey && | ||
| !event.altKey; | ||
|
|
||
| if (isClearShortcut) { | ||
| if (event.type === "keydown" && options.onClear) { | ||
| options.onClear(); | ||
| } | ||
| return false; | ||
| } |
There was a problem hiding this comment.
Critical: Cross-platform compatibility issue - Ctrl+K won't work on Windows/Linux.
The clear shortcut detection only checks event.metaKey (Line 166), which is the Cmd key on Mac or the Windows key on Windows/Linux—not the Ctrl key. This means Ctrl+K won't trigger the clear functionality on Windows/Linux, even though the hotkey definition uses "meta+k" which per the comment in hotkeys.ts should map to "Cmd on Mac, Ctrl on Windows/Linux."
The check at Line 168 explicitly excludes event.ctrlKey, further preventing Ctrl+K from working.
Apply this diff to support both Cmd (Mac) and Ctrl (Windows/Linux):
// Handle Cmd+K to clear terminal (handle directly since it needs xterm access)
const isClearShortcut =
event.key.toLowerCase() === "k" &&
- event.metaKey &&
+ (event.metaKey || event.ctrlKey) &&
!event.shiftKey &&
- !event.ctrlKey &&
!event.altKey;This aligns with the pattern at Line 179 where both metaKey and ctrlKey are treated as alternatives for cross-platform modifier key support.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Handle Cmd+K to clear terminal (handle directly since it needs xterm access) | |
| const isClearShortcut = | |
| event.key.toLowerCase() === "k" && | |
| event.metaKey && | |
| !event.shiftKey && | |
| !event.ctrlKey && | |
| !event.altKey; | |
| if (isClearShortcut) { | |
| if (event.type === "keydown" && options.onClear) { | |
| options.onClear(); | |
| } | |
| return false; | |
| } | |
| // Handle Cmd+K to clear terminal (handle directly since it needs xterm access) | |
| const isClearShortcut = | |
| event.key.toLowerCase() === "k" && | |
| (event.metaKey || event.ctrlKey) && | |
| !event.shiftKey && | |
| !event.altKey; | |
| if (isClearShortcut) { | |
| if (event.type === "keydown" && options.onClear) { | |
| options.onClear(); | |
| } | |
| return false; | |
| } |
🤖 Prompt for AI Agents
In
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.ts
around lines 163 to 176, the clear-shortcut check currently only recognizes
event.metaKey and explicitly excludes event.ctrlKey, preventing Ctrl+K from
working on Windows/Linux; change the detection so the modifier is (event.metaKey
|| event.ctrlKey) and remove the exclusion of ctrlKey (keep excluding
shift/alt/other modifiers), so that isClearShortcut is true for either Cmd+K on
macOS or Ctrl+K on Windows/Linux, and preserve the existing behavior of calling
options.onClear() on keydown and returning false.
Add CLEAR_TERMINAL hotkey (meta+k) and implement the keyboard handler to clear the terminal buffer when pressed.
🤖 Generated with Claude Code
Description
Related Issues
Type of Change
Testing
Screenshots (if applicable)
Additional Notes
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.