fix: Windows terminal copy/paste, right-click menu, and clickable links#81
fix: Windows terminal copy/paste, right-click menu, and clickable links#81jcanizalez merged 2 commits intomainfrom
Conversation
- Fix copy: Ctrl+C copies selection on Windows/Linux, falls through to SIGINT when nothing is selected. Ctrl+Shift+C as dedicated copy shortcut. - Fix paste duplication: add preventDefault() to stop browser from firing a native paste event alongside our manual clipboard read. - Add right-click context menu with Copy/Paste for all terminals. - Load WebLinksAddon so URLs are clickable via Cmd/Ctrl+click. - Add shell:openExternal IPC to open links in the default browser.
There was a problem hiding this comment.
Pull request overview
This PR addresses Windows terminal UX issues (copy/paste correctness and context menus) and adds link-clicking support in xterm output by routing external URL opens through a new IPC handler.
Changes:
- Add
WebLinksAddonsupport so terminal URLs can be opened via Cmd/Ctrl+click using a newshell:openExternalIPC. - Fix Windows/Linux keyboard copy/paste handling in xterm (Ctrl+C copies selection; Ctrl+V pastes once via manual clipboard read +
preventDefault()). - Add a custom right-click Copy/Paste context menu for both shell terminals and terminal instances.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/renderer/lib/terminal-registry.ts | Loads WebLinksAddon and adjusts Windows/Linux Ctrl+C/Ctrl+V handling; exports helper functions for selection/paste. |
| src/renderer/components/TerminalPanel.tsx | Adds right-click context menu support for shell terminals. |
| src/renderer/components/TerminalInstance.tsx | Adds right-click context menu support for terminal instances. |
| src/renderer/components/TerminalContextMenu.tsx | New portal-based Copy/Paste context menu component. |
| src/preload/index.ts | Exposes openExternal() to the renderer via IPC. |
| src/main/ipc-handlers.ts | Registers IPC.OPEN_EXTERNAL handler using Electron shell.openExternal. |
| packages/shared/src/types.ts | Adds OPEN_EXTERNAL IPC channel constant. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/main/ipc-handlers.ts
Outdated
| safeHandle(IPC.OPEN_EXTERNAL, (_, url: string) => { | ||
| if (typeof url === 'string' && /^https?:\/\//i.test(url)) { | ||
| return shell.openExternal(url) | ||
| } |
There was a problem hiding this comment.
URL validation for IPC.OPEN_EXTERNAL is currently a simple regex on the raw string. This can allow URLs with leading/trailing whitespace or control characters and doesn’t guard against other parsing edge cases. Consider parsing with new URL(url), verifying protocol is exactly http:/https:, and rejecting (throwing) invalid inputs so callers can detect failure instead of silently succeeding.
| safeHandle(IPC.OPEN_EXTERNAL, (_, url: string) => { | |
| if (typeof url === 'string' && /^https?:\/\//i.test(url)) { | |
| return shell.openExternal(url) | |
| } | |
| safeHandle(IPC.OPEN_EXTERNAL, (_, rawUrl: string) => { | |
| if (typeof rawUrl !== 'string') { | |
| throw new Error('Invalid URL: expected string') | |
| } | |
| let parsed: URL | |
| try { | |
| parsed = new URL(rawUrl.trim()) | |
| } catch { | |
| throw new Error('Invalid URL: parse failure') | |
| } | |
| if (parsed.protocol !== 'http:' && parsed.protocol !== 'https:') { | |
| throw new Error('Invalid URL protocol') | |
| } | |
| return shell.openExternal(parsed.toString()) |
| <motion.div | ||
| ref={menuRef} | ||
| initial={{ opacity: 0, y: -4, scale: 0.96 }} | ||
| animate={{ opacity: 1, y: 0, scale: 1 }} | ||
| exit={{ opacity: 0, y: -4, scale: 0.96 }} | ||
| transition={{ type: 'spring', stiffness: 500, damping: 30 }} | ||
| className="fixed z-[150] rounded-lg border border-white/[0.1] py-1 shadow-2xl" | ||
| style={{ top, left, background: '#1e1e22', minWidth: menuWidth }} | ||
| > | ||
| <button | ||
| onClick={handleCopy} | ||
| disabled={!selection} | ||
| className="w-full flex items-center gap-2.5 px-3 py-2.5 text-xs text-gray-300 |
There was a problem hiding this comment.
The custom context menu is rendered as a plain div with buttons but doesn’t expose menu semantics or manage focus. For accessibility, consider adding role="menu" on the container, role="menuitem" (or appropriate roles) on items, and moving focus into the menu when it opens (and back to the terminal on close).
| return createPortal( | ||
| <AnimatePresence> | ||
| <motion.div |
There was a problem hiding this comment.
AnimatePresence won’t be able to run the exit animation here because TerminalContextMenu unmounts as a whole when contextMenu becomes null, taking AnimatePresence with it. Either remove AnimatePresence/exit to simplify, or move AnimatePresence to the parent so it remains mounted while the menu animates out.
…atePresence - Parse URLs with `new URL()` instead of regex for openExternal IPC - Add role="menu" / role="menuitem" to terminal context menu - Remove AnimatePresence wrapper (exit animation can't fire when the component unmounts)
Summary
preventDefault()stops the browser from firing a duplicate native paste event — fixes the double-paste bug.WebLinksAddonso URLs in terminal output are clickable via Cmd+click (Mac) / Ctrl+click (Windows/Linux), opened in the default browser via a newshell:openExternalIPC.Closes #78