Add drag-and-drop file support to terminal #2742
Conversation
Implements drag-and-drop functionality for terminal, matching macOS Terminal behavior. Users can now drag files or folders from Finder directly into the terminal to insert their paths. Features: - Automatically inserts file paths when files are dropped - Handles multiple files (space-separated) - Auto-quotes paths containing spaces - Uses full file paths from Electron File API - Works with files, folders, and multiple selections Usage: Simply drag a file from Finder and drop it into any terminal block. The file path will be inserted at the cursor position. 🤖 Generated with Claude Code Co-Authored-By: Claude <noreply@anthropic.com>
Use Electron's webUtils.getPathForFile() API to get the actual full file path instead of just the file name. This matches macOS Terminal behavior where dragging a file inserts its complete path. Before: CV_Document.pdf After: /Users/steven/Downloads/CV_Document.pdf 🤖 Generated with Claude Code Co-Authored-By: Claude <noreply@anthropic.com>
Expose webUtils.getPathForFile through the Electron preload API so the renderer process can get full file system paths from File objects during drag-and-drop operations. Changes: - Added getPathForFile method to preload script using webUtils - Updated ElectronApi TypeScript interface - Simplified terminal drag-drop to use new API via getApi() This properly implements macOS Terminal-style drag-and-drop with full file paths instead of just filenames. 🤖 Generated with Claude Code Co-Authored-By: Claude <noreply@anthropic.com>
Added missing import of getApi from @/store/global to fix 'getApi is not defined' error when dropping files. Also added debug logging to help troubleshoot file path retrieval. 🤖 Generated with Claude Code Co-Authored-By: Claude <noreply@anthropic.com>
|
Warning Rate limit exceeded@stevenvo has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 13 minutes and 48 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (4)
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughThis change introduces file drag-and-drop support for the terminal view by adding a new Electron API method. The implementation spans three layers: an Electron preload API method that resolves dropped files to filesystem paths, a TypeScript type definition for the new API, and terminal UI event handlers that collect dropped files, resolve their paths via the API, and send them to the terminal controller. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Pre-merge checks and finishing touches✅ Passed checks (3 passed)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
frontend/types/custom.d.ts (1)
137-137: Clarify the API comment.The comment
// get-path-for-filefollows the pattern used for IPC channel names, but this method useswebUtils.getPathForFiledirectly rather than IPC. Consider updating the comment to clarify the implementation approach.🔎 Suggested comment clarification
- getPathForFile: (file: File) => string; // get-path-for-file + getPathForFile: (file: File) => string; // webUtils.getPathForFile
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
emain/preload.tsfrontend/app/view/term/term.tsxfrontend/types/custom.d.ts
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-21T05:09:26.916Z
Learnt from: sawka
Repo: wavetermdev/waveterm PR: 2465
File: frontend/app/onboarding/onboarding-upgrade.tsx:13-21
Timestamp: 2025-10-21T05:09:26.916Z
Learning: In the waveterm codebase, clientData is loaded and awaited in wave.ts before React runs, ensuring it is always available when components mount. This means atoms.client will have data on first render.
Applied to files:
frontend/app/view/term/term.tsx
🧬 Code graph analysis (1)
frontend/app/view/term/term.tsx (1)
frontend/app/store/global.ts (1)
getApi(826-826)
🔇 Additional comments (4)
frontend/app/view/term/term.tsx (3)
11-11: LGTM!The import of
getApiis correctly added to access the Electron API for file path resolution.
357-362: LGTM!The drag event handlers are correctly implemented with proper event handling (preventDefault, stopPropagation) and appropriate drop effects.
Also applies to: 409-417
420-427: LGTM!The drag-and-drop handlers are correctly attached to the terminal container with proper event handler bindings.
emain/preload.ts (1)
4-4: LGTM. The implementation correctly useswebUtils.getPathForFileto obtain filesystem paths for dropped files. This is the documented Electron API for this purpose and is properly exposed via contextBridge in the preload script.
frontend/app/view/term/term.tsx
Outdated
| const pathString = paths.map(path => { | ||
| // Quote paths that contain spaces | ||
| if (path.includes(" ")) { | ||
| return `"${path}"`; | ||
| } | ||
| return path; | ||
| }).join(" "); |
There was a problem hiding this comment.
Path quoting is incomplete and may cause command injection risks.
The current implementation only quotes paths containing spaces, but shell commands require escaping many other special characters such as:
- Single quotes (
') - Double quotes already in the path (
") - Dollar signs (
$) - Backticks (
`) - Backslashes (
\) - Ampersands (
&) - Semicolons (
;) - Pipes (
|)
Without proper escaping, a malicious filename could inject shell commands. Consider implementing proper shell escaping or using a library for this purpose.
🔎 Example of improved shell escaping
For POSIX shells, you could use single-quote escaping where internal single quotes are replaced with '\'':
// Insert the path(s) into the terminal
// If multiple files, separate with spaces and quote if necessary
const pathString = paths.map(path => {
- // Quote paths that contain spaces
- if (path.includes(" ")) {
- return `"${path}"`;
- }
- return path;
+ // Escape for POSIX shells: wrap in single quotes and escape any single quotes
+ return "'" + path.replace(/'/g, "'\\''") + "'";
}).join(" ");Note: This assumes a POSIX-compatible shell. For Windows Command Prompt or PowerShell, different escaping rules apply. You may need to detect the shell type or use a cross-platform escaping library.
📝 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.
| const pathString = paths.map(path => { | |
| // Quote paths that contain spaces | |
| if (path.includes(" ")) { | |
| return `"${path}"`; | |
| } | |
| return path; | |
| }).join(" "); | |
| const pathString = paths.map(path => { | |
| // Escape for POSIX shells: wrap in single quotes and escape any single quotes | |
| return "'" + path.replace(/'/g, "'\\''") + "'"; | |
| }).join(" "); |
🤖 Prompt for AI Agents
In frontend/app/view/term/term.tsx around lines 393 to 399, the current logic
only quotes paths with spaces which leaves many shell-special characters
unescaped and risks command injection; replace this ad-hoc quoting with a safe
approach: either (A) stop concatenating into a shell string and pass paths as an
argv array to the child process API (preferred), or (B) if you must build a
shell string for a POSIX shell, wrap every path in single quotes and escape
internal single quotes by replacing ' with '\'' before joining, and for Windows
choose a platform-appropriate escaping library; ideally use a well-tested
cross-platform escaping library instead of hand-rolled logic.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 12b35b4623
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
frontend/app/view/term/term.tsx
Outdated
| const paths = files.map((file: File) => { | ||
| try { | ||
| // Use the exposed Electron API to get the full path | ||
| const fullPath = getApi().getPathForFile(file); | ||
| console.log("File:", file.name, "-> Full path:", fullPath); |
There was a problem hiding this comment.
Electron’s webUtils.getPathForFile returns a Promise, but the drop handler treats it as a synchronous string (getApi().getPathForFile(file) and later path.includes(...)). When a user drops a file, paths becomes an array of Promises, so the subsequent .includes/join logic throws and no path is sent to the terminal. Resolve the promise (e.g., with await/Promise.all) before quoting and sending the path string.
Useful? React with 👍 / 👎.
- Only intercept file drops with isFileDrop() check - Better shell character quoting with /[\s'"]/ regex - Cleaner error handling and path filtering
1 participant
Summary
webUtils.getPathForFile()to get full file paths from dropped filesChanges
webUtilsand exposegetPathForFileAPITesting
Drag files from Finder onto the terminal - the full file path(s) should be inserted into the terminal input.