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 |
frontend/app/view/term/termwrap.ts
Outdated
| 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).
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).