Skip to content

feat: drag and drop file paths into terminal#2857

Merged
sawka merged 3 commits intowavetermdev:mainfrom
JustHereToHelp:feat/terminal-drag-drop
Mar 30, 2026
Merged

feat: drag and drop file paths into terminal#2857
sawka merged 3 commits intowavetermdev:mainfrom
JustHereToHelp:feat/terminal-drag-drop

Conversation

@JustHereToHelp
Copy link
Copy Markdown
Contributor

@JustHereToHelp JustHereToHelp commented Feb 11, 2026

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 killed File.path. Handles spaces in filenames.

Needs app restart after install (preload change).

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
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Feb 11, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: a576ce06-b0d0-45f0-8c2d-f7e381434eb7

📥 Commits

Reviewing files that changed from the base of the PR and between 1efa5ad and 6e6c89d.

📒 Files selected for processing (2)
  • frontend/app/view/term/termutil.ts
  • frontend/app/view/term/termwrap.ts
✅ Files skipped from review due to trivial changes (1)
  • frontend/app/view/term/termwrap.ts

Walkthrough

Adds drag-and-drop file support to the terminal. The preload exposes api.getPathForFile(file: File): string delegating to webUtils. The global ElectronApi type is extended with getPathForFile. The terminal registers dragover/drop handlers that resolve dropped File objects to paths, quote them for POSIX shells, paste the joined quoted paths, and dispose handlers on teardown.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

��� Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately and concisely describes the main feature being added: drag and drop file path insertion into the terminal.
Description check ✅ Passed The description is directly related to the changeset, explaining the drag-and-drop functionality, the use of webUtils.getPathForFile(), shell quoting for spaces, and linked issues.
Linked Issues check ✅ Passed The pull request fully addresses both linked issues (#746, #2813) by implementing drag-and-drop support for files from Finder and other file managers, inserting quoted file paths into terminal input.
Out of Scope Changes check ✅ Passed All changes are directly scoped to implementing drag-and-drop file path insertion: preload bridge, terminal event handlers, shell quoting utility, and type definitions. No unrelated modifications detected.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@JustHereToHelp JustHereToHelp marked this pull request as ready for review February 26, 2026 18:53
const file = e.dataTransfer.files[i];
const filePath = getApi().getPathForFile(file);
if (filePath) {
const quoted = "'" + filePath.replace(/'/g, "'\\''") + "'";
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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);
}
@kilo-code-bot
Copy link
Copy Markdown
Contributor

kilo-code-bot bot commented Feb 26, 2026

Code Review Summary

Status: 1 Issue Found | Recommendation: Address before merge

Overview

Severity Count
CRITICAL 0
WARNING 1
SUGGESTION 0
Issue Details (click to expand)

WARNING

File Line Issue
frontend/app/view/term/termutil.ts 393 Shell quoting uses POSIX-style which won't work on Windows (cmd.exe/PowerShell)
Files Reviewed (4 files)
  • emain/preload.ts - No issues
  • frontend/app/view/term/termutil.ts - 1 issue (new file, issue moved from termwrap.ts)
  • frontend/app/view/term/termwrap.ts - No issues (previous issue at line 218 is now FIXED)
  • frontend/types/custom.d.ts - No issues

Fix these issues in Kilo Cloud


Reviewed by minimax-m2.5-20260211 · 2,287,214 tokens

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

ℹ️ Review info

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 343d009 and 4b3e314.

📒 Files selected for processing (3)
  • emain/preload.ts
  • frontend/app/view/term/termwrap.ts
  • frontend/types/custom.d.ts
return lines;
}

export function quoteForPosixShell(filePath: string): string {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@sawka sawka merged commit 02b9e5f into wavetermdev:main Mar 30, 2026
4 checks passed
sgeraldes pushed a commit to sgeraldes/waveterm that referenced this pull request Mar 31, 2026
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).
suruoxi pushed a commit to suruoxi/waveterm that referenced this pull request Apr 1, 2026
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).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants