Skip to content

feat(fs): add file and directory management functions (rename, mkdir, rmdir) to IPC and local/remote file systems#1348

Merged
arnestrickmann merged 2 commits intogeneralaction:mainfrom
yashdev9274:feat-yd-2
Mar 11, 2026
Merged

feat(fs): add file and directory management functions (rename, mkdir, rmdir) to IPC and local/remote file systems#1348
arnestrickmann merged 2 commits intogeneralaction:mainfrom
yashdev9274:feat-yd-2

Conversation

@yashdev9274
Copy link
Contributor

@yashdev9274 yashdev9274 commented Mar 7, 2026

Summary

Add a right-click context menu to files and folders in the file explorer, matching the behavior users expect from VS Code and other traditional IDEs.

Fixes

Fixes #1228

Snapshot

changes video-

Emdash2.mp4

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • Chore (refactoring code, technical debt, workflow improvements)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Refactor (does not change functionality, e.g. code style improvements, linting)
  • This change requires a documentation update

Mandatory Tasks

  • I have self-reviewed the code
  • A decent size PR with self-review

Checklist

  • I have read the contributing guide
  • My code follows the style guidelines of this project (pnpm run format)
  • I have commented my code, particularly in hard-to-understand areas
  • I have checked if my PR needs changes to the documentation
  • I have checked if my changes generate no new warnings (pnpm run lint)
  • I have added tests that prove my fix is effective or that my feature works
  • I have checked if new and existing unit tests pass locally with my changes

@vercel
Copy link

vercel bot commented Mar 7, 2026

@yashdev9274 is attempting to deploy a commit to the General Action Team on Vercel.

A member of the Team first needs to authorize it.

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Mar 7, 2026

Greptile Summary

This PR adds right-click context menu functionality to the file explorer, backed by three new IPC channels (fs:rename, fs:mkdir, fs:rmdir) that expose rename, create directory, and delete directory operations for both local and remote (SFTP) file systems.

Verified Issues:

  1. Windows Incompatibility (Critical): Remote path construction in all three new IPC handlers uses path.join() instead of path.posix.join(). On Windows, this produces backslash-separated paths that are invalid on remote POSIX servers, breaking all remote operations on that platform.

  2. Incomplete Interface Design: The PR adds rename and mkdir as required methods to the IFileSystem interface, but omits rmdir. The fs:rmdir handler delegates to the optional remove() method instead, creating an inconsistency in the interface contract.

  3. Event Loop Blocking: Local-path handlers use synchronous fs operations (fs.renameSync, fs.mkdirSync, fs.rmSync) inside async IPC handlers, blocking the main process event loop. The existing LocalFileSystem class already provides async equivalents that could be reused.

  4. UX Inconsistency: "Open in Terminal" and "Reveal in Finder" context menu items are only shown for files, not directories, despite both handlers supporting directories and the PR's stated goal of matching VS Code's behavior.

  5. Unused Variable: The LocalFileSystem.rename() method assigns an oldStat variable that is never used.

The renderer-side changes are well-structured and the IPC type definitions are correct, but the server-side implementation has functional and design gaps that should be addressed before merge.

Confidence Score: 1/5

  • Not safe to merge — the path.join() bug will silently break all remote rename/mkdir/rmdir operations on Windows.
  • The Windows-incompatible path.join() usage for remote paths is a critical functional bug affecting an entire platform. The incomplete interface design (missing rmdir while adding rename and mkdir) is an architectural gap. The synchronous fs calls in async handlers block the main process event loop, degrading responsiveness. The UX inconsistency with VS Code is a secondary concern but worth addressing before merge.
  • src/main/services/fsIpc.ts needs the most attention — it contains the Windows incompatibility bug, the sync-in-async calls, and the optional remove() delegation. src/main/services/fs/types.ts should define rmdir as a required interface method to complete the abstraction.

Last reviewed commit: b2b25f1

Comment on lines 248 to 258
{node.type === 'file' && (
<span className="mr-1.5">
<FileIcon filename={node.name} isDirectory={false} isExpanded={false} />
</span>
<>
<ContextMenuSeparator />
<ContextMenuItem onSelect={() => onContextMenuOpenTerminal?.(node)}>
Open in Terminal
</ContextMenuItem>
<ContextMenuItem onSelect={() => onContextMenuReveal?.(node)}>
Reveal in Finder
</ContextMenuItem>
</>
)}
Copy link
Contributor

Choose a reason for hiding this comment

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

"Open in Terminal" and "Reveal in Finder" are shown only for files (gated on node.type === 'file'), but both operations are arguably more useful on directories. The PR aims to match VS Code's behavior, which shows "Open in Integrated Terminal" on directories, not individual files. The handlers already support directories correctly:

const dirPath =
  node.type === 'directory'
    ? pathUtils.join(rootPath, node.path)
    : pathUtils.join(rootPath, pathUtils.dirname(node.path));

Consider moving "Open in Terminal" to show for both files and directories (or just directories). Similarly, review "Reveal in Finder" to ensure it works as expected for directories.

@yashdev9274
Copy link
Contributor Author

hey @arnestrickmann do review this !

@yashdev9274
Copy link
Contributor Author

anything wrong with my PRs @arnestrickmann ??

@arnestrickmann
Copy link
Contributor

Thanks! @yashdev9274
CleanShot 2026-03-10 at 17 01 40@2x

Could you make sure that the Delete button has appropriate and consistent padding?

… rmdir) to IPC and local/remote file systems
- Fixed Windows path separator bug in remote fs:rename, fs:mkdir, fs:rmdir handlers
- On Windows, path.join produces backslashes which break POSIX remote servers
- Minor cleanup: unused variable removal in LocalFileSystem
- UI: added padding to delete confirmation button in FileTree
@yashdev9274
Copy link
Contributor Author

fixed it

@arnestrickmann
Copy link
Contributor

Thanks for addressing the points! I will test and lyk. @yashdev9274

@arnestrickmann arnestrickmann merged commit 1efa442 into generalaction:main Mar 11, 2026
2 of 3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Feature: Right-click context menu in file explorer (rename, delete, etc.)

2 participants