fix(desktop): match VS Code terminal clipboard handling (originally by @AytuncYildizli)#3415
Merged
Conversation
Contributor
|
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 platform- and selection-aware clipboard shortcut detection and select-all handling for the terminal. New helpers determine when clipboard shortcuts should bubble to the host; the keyboard handler uses them to short-circuit xterm consumption. Tests cover macOS, Windows, and Linux variants. Changes
Sequence Diagram(s)sequenceDiagram
participant Keyboard as KeyboardEvent
participant Handler as setupKeyboardHandler
participant Shortcuts as shouldBubbleClipboardShortcut / shouldSelectAllShortcut
participant XTerm as xterm
participant Host as Host/Window
Keyboard->>Handler: keydown with modifiers
Handler->>Shortcuts: evaluate event + platform + selection
alt select-all matched
Shortcuts-->>Handler: select-all
Handler->>XTerm: call xterm.selectAll() and preventDefault
Handler-->>Host: stop further bubbling
else clipboard shortcut matched
Shortcuts-->>Handler: bubble=true
Handler-->>Host: allow bubble (return false to stop xterm)
Host->>Host: handle copy/paste
else no match
Shortcuts-->>Handler: bubble=false
Handler->>XTerm: continue terminal-reserved processing
XTerm->>XTerm: handle event
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
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 docstrings
🧪 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 |
Greptile SummaryThis PR extracts clipboard shortcut forwarding logic into a dedicated Key changes:
Issue found:
Confidence Score: 3/5Safe to merge on Windows/Linux; macOS has a platform-guard bug that could silently drop keystrokes intended for TUI apps. The core feature is well-designed and the integration point in helpers.ts is correct. However, the missing !isMac guard on the Linux fallback is a real logic error: on macOS, Ctrl+Shift+V, Shift+Insert, and Ctrl+Shift+C (with a selection) will be bubbled to Electron instead of reaching the PTY. Any TUI application on macOS that uses those key sequences as internal shortcuts will silently break. The fix is a one-liner, so the score reflects a targeted fix remaining before merge. clipboardShortcuts.ts (the fallback return) and clipboardShortcuts.test.ts (missing macOS boundary assertions) Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[KeyboardEvent in xterm handler] --> B{shouldBubbleClipboardShortcut?}
B -->|true - clipboard shortcut| C[return false\nhost handles copy/paste]
B -->|false - not clipboard| D{isTerminalReservedEvent?\nCtrl+C/D/Z/S/Q}
D -->|true| E[return true\nxterm sends to PTY]
D -->|false| F{Registered app hotkey?}
F -->|yes| G[return false\napp handles hotkey]
F -->|no| E
subgraph shouldBubbleClipboardShortcut logic
H{isMac and onlyMeta?} -->|yes| I{KeyV or KeyC+selection?}
I -->|yes| J[return true]
I -->|no| K[return false]
H -->|no| L{isWindows?}
L -->|yes| M{KeyV+Ctrl or CtrlShift or KeyC+selection+Ctrl/CtrlShift?}
M -->|yes| J
M -->|no| K
L -->|no - Linux fallback also runs on macOS| N{KeyV+CtrlShift or Insert+Shift or KeyC+CtrlShift+selection?}
N -->|yes| J
N -->|no| K
end
Reviews (1): Last reviewed commit: "Handle more keyboard shortcuts" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Contributor
🚀 Preview Deployment🔗 Preview Links
Preview updates automatically with new commits |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
1 issue found across 4 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/clipboardShortcuts.ts">
<violation number="1" location="apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/clipboardShortcuts.ts:50">
P2: The final return block is reachable on macOS when the key event doesn't include `Cmd` (e.g. `Ctrl+Shift+V`, `Shift+Insert`). The `isMac && onlyMeta` guard on line 35 only early-returns for `Cmd`-based shortcuts; any other modifier combination on macOS falls through to this Linux-specific block. This causes Linux clipboard bindings to incorrectly fire on macOS, intercepting shortcuts that should reach the PTY.
Wrap this block in `if (!isMac)` and add a `return false` fallback:</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Kitenite
changed the title
Kitenite
changed the title
MocA-Love
pushed a commit
to MocA-Love/superset
that referenced
this pull request
Apr 14, 2026
…3415) * Fix code TUI copy * Handle more keyboard shortcuts * fix(desktop): match VS Code terminal clipboard handling
4 tasks
MocA-Love
pushed a commit
to MocA-Love/superset
that referenced
this pull request
Apr 14, 2026
All 9 upstream commits have been individually cherry-picked via PR#159~#163: | Upstream | Our PR | Description | |---|---|---| | d656b7e (superset-sh#3415) | #159 (PR#1) | terminal clipboard handling | | 31fcf19 (superset-sh#3416) | #162 (PR#4) | v1 split pane startup sizing fix | | 039edf2 (superset-sh#3403) | #161 (PR#3) | Cmd+Alt+Arrow spatial pane focus | | b18a00c (superset-sh#3421) | #159 (PR#1) | v2 right sidebar toggle reactive | | 3dd1de2 (superset-sh#3420) | #161 (PR#3) | v2 diff viewer + tab title resolution | | b42a114 (superset-sh#3418) | #159 (PR#1) | CodeMirror hotkey enablement | | c925f4d (superset-sh#3422) | #160 (PR#2) | unbound defaults + restore prev/next tab/workspace | | bb12c09 (superset-sh#3419) | #163 (PR#5) | version bump 1.5.3 | | 47efa73 (superset-sh#3432) | #159 (PR#1) | pending/update-required error selectable | Fork-specific features preserved: - auto-updater (IS_FORK, GitHub Releases API) - QuitMode/cleanupMainWindowResources lifecycle - GitHubSyncService, SpreadsheetViewer - BROWSER_RELOAD / BROWSER_HARD_RELOAD / SEARCH_IN_FILES hotkeys - HotkeyCategory "Browser" - v1 deep-link navigation (useSearch/WorkspaceSearchParams) - v1 tRPC-based PREV/NEXT_WORKSPACE handlers - v1 CLOSE_TERMINAL/CLOSE_TAB hotkey handlers - v2 extra state (rightSidebarOpenViewWidth, showPresetsBar)
5 tasks
9 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Reported and fixed from: #3407
Summary
Cmd+A) to match VS Code's terminal action behaviorRoot Cause
#3390changed the v1 terminal so unregistered chords fall through to the PTY. That fixed TUI shortcut forwarding in general, but it also let xterm consume host clipboard chords as CSI-u input before Electron/browser copy-paste handling could run. On macOS this brokeCmd+CandCmd+V; the follow-up implementation also needed an explicit guard so Linux fallback bindings likeCtrl+Shift+VandShift+Insertwould not be intercepted on macOS.Behavior
Cmd+Ccopies only when there is a terminal selection,Cmd+Vpastes,Cmd+Aselects allCtrl+V,Ctrl+Shift+V, and copy chords with a selection bubble to the hostCtrl+Shift+C,Ctrl+Shift+V, andShift+Insertfollow VS Code's terminal bindingsValidation
bun test apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/clipboardShortcuts.test.tsbun run typecheck --filter=@superset/desktopbunx biome check apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.ts apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/clipboardShortcuts.ts apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/clipboardShortcuts.test.tsSummary by CodeRabbit
Tests
New Features