feat: drag and drop file paths into terminal#2857
Conversation
Drag files from Finder or Wave's file browser into a terminal block to insert the quoted path. Uses webUtils.getPathForFile() through a preload bridge since Electron 32 removed File.path. Paths with spaces are properly shell-quoted. Fixes wavetermdev#746, fixes wavetermdev#2813
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
WalkthroughAdds drag-and-drop file support to the terminal. The preload exposes Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
| const file = e.dataTransfer.files[i]; | ||
| const filePath = getApi().getPathForFile(file); | ||
| if (filePath) { | ||
| const quoted = "'" + filePath.replace(/'/g, "'\\''") + "'"; |
There was a problem hiding this comment.
WARNING: Shell quoting issue on Windows
The POSIX-style quoting '\'' won't work correctly on Windows shells (cmd.exe, PowerShell). Windows uses different quoting rules:
- cmd.exe: Uses
"for quoting and^for escaping - PowerShell: Uses
"for quoting and`for escaping
Consider adding platform-specific quoting:
import { PLATFORM, PlatformWindows } from "@/util/platformutil";
// In the drop handler:
if (filePath) {
let quoted: string;
if (PLATFORM === PlatformWindows) {
// Windows: use double quotes and escape internal quotes
quoted = '"' + filePath.replace(/"/g, '""') + '"';
} else {
// POSIX: use single quotes and escape internal quotes
quoted = "'" + filePath.replace(/'/g, "'\\''" + "'";
}
paths.push(quoted);
}
Code Review SummaryStatus: 1 Issue Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
Files Reviewed (4 files)
Fix these issues in Kilo Cloud Reviewed by minimax-m2.5-20260211 · 2,287,214 tokens |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/app/view/term/termwrap.ts`:
- Around line 202-225: The dragover/drop listeners attached to this.connectElem
are inline and never removed; refactor by extracting the callbacks into named
bound methods (e.g., onDragOver and onDrop) or stored function properties on the
class, replace the inline addEventListener calls with those references, and in
the class dispose() (or equivalent teardown) call
this.connectElem.removeEventListener("dragover", this.onDragOver) and
removeEventListener("drop", this.onDrop) so listeners are cleaned up and not
duplicated on re-init.
| return lines; | ||
| } | ||
|
|
||
| export function quoteForPosixShell(filePath: string): string { |
There was a problem hiding this comment.
WARNING: Shell quoting only handles POSIX shells - won't work on Windows (cmd.exe/PowerShell)
This function uses single-quote escaping which is POSIX-specific. Files dragged onto the terminal on Windows will not be properly quoted, causing issues with paths containing spaces or special characters.
Fixes wavetermdev#746, fixes wavetermdev#2813 Drag a file from Finder into a terminal and it pastes the quoted path. Uses `webUtils.getPathForFile()` through a preload bridge since Electron 32 killed `File.path`. Handles spaces in filenames. Needs app restart after install (preload change).
Fixes wavetermdev#746, fixes wavetermdev#2813 Drag a file from Finder into a terminal and it pastes the quoted path. Uses `webUtils.getPathForFile()` through a preload bridge since Electron 32 killed `File.path`. Handles spaces in filenames. Needs app restart after install (preload change).
Pulls in 55 upstream commits including the v0.14.4 release work, the xterm.js v6.0.0 upgrade, the official terminal file drag-and-drop implementation (wavetermdev#2857), the new ProcessViewer widget, the WebGL→DOM renderer fallback, and many other features and dependency bumps. Conflict reconciliation: * termwrap.ts — fully replaced by upstream's xterm v6 version. The xterm v6 commit (2b11043) explicitly removes "lots of special case patches for IME and scrolling that were put in to patch over 5.5.0 issues", including all of the isComposing/composingData/lastCompositionEnd state and the IME dedup window in handleTermData. Our earlier IME data-eating fix (b5d72f5, 296760d) is built on top of that removed infrastructure and is therefore redundant against v6 — v6 fixes the underlying problem in xterm.js itself. Dropping our IME patch in favor of upstream's resolution. * term.scss — kept the composition view visibility override (xterm v6 still ships the same near-invisible default composition-view styling with the same TODO comment). Dropped the local scrollbar customization that v6 made obsolete; took upstream's .xterm-decoration-overview-ruler hide and transparent .xterm-viewport. Our local .term-drag-over outline class is also gone since the new flow handles native file drops in termwrap.ts at the connectElem level (no React-rooted drag class needed). * term.tsx — auto-merged with no Git conflict, but the local drag-and-drop handler used (file as any).path which Electron 32+ removed. Removed the broken native-files branch entirely so termwrap.ts (which now uses webUtils.getPathForFile via the preload bridge) is the single owner of native file drops. Kept the react-dnd FILE_ITEM handler for the internal file browser and the text-drop handler — both are local features upstream lacks. * preload.ts / custom.d.ts / preview-electron-api.ts — combined our execCommand IPC channel (used by simpleeditor for git/svn) with upstream's getPathForFile webUtils bridge. * block.tsx / blockregistry.ts — upstream extracted BlockRegistry into a new blockregistry.ts module. Took upstream's block.tsx and moved our ClaudeSessionsViewModel and SimpleEditorModel registrations into the new blockregistry.ts. * blockutil.tsx — additive: kept both upstream's processviewer icon/name entries and our claudesessions/simpleeditor entries. * package-lock.json — took upstream entirely, then ran npm install to bring node_modules in sync (xterm v6, electron 41.1.0, etc.). * defaultconfig.ts / processviewer.preview.tsx — upstream's preview mocks were missing fields the merged FullConfigType / ProcessInfo now require (version, buildtime, numthreads). Added stub values so the post-merge tree passes tsc --noEmit. Verified: npx tsc --noEmit clean on the merged tree. A backup label is left at backup/pre-upstream-merge in case rollback is needed. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Fixes #746, fixes #2813
Drag a file from Finder into a terminal and it pastes the quoted path. Uses
webUtils.getPathForFile()through a preload bridge since Electron 32 killedFile.path. Handles spaces in filenames.Needs app restart after install (preload change).