Right-click file operations, and an icon in the taskbar - #12
Merged
Conversation
Refresh, New folder, New file, Rename and Delete, on a context menu, in the local pane and the remote one alike — the FileZilla operations the panes looked like they already had. F2 renames and Del deletes, because a file manager that answers only the mouse is half a file manager. The remote half already had mkdir/rename/delete over SFTP and nothing called them; the local half had no mutations at all. Both sides now go through one bridge where the connection id is what selects local or remote, so a caller cannot act on the wrong pane by picking the wrong method name. Every mutation takes a directory and a bare entry name and joins them in the main process. EntryNameSchema rejects a separator, `..`, a NUL and surrounding space, so "New folder" in a listing cannot write outside the folder being listed — on the remote side, anywhere the SSH user can reach. Delete re-stats its target and refuses when what is on disk disagrees with what the renderer claimed, so a mislabelled request cannot turn one unlink into a recursive delete; a symlink is unlinked, never followed. Two things SFTP does not give you: creating a file (open 'wx', which fails rather than truncating an existing one) and removing a populated directory (rmdir only unlinks an empty one, so removeRecursive walks it depth first). Verified in a headless render of the real export under the app's real CSP, both themes: the menu opens on a row with all five items live, the row it opened on becomes the selection, and the New folder dialog focuses its field with its footer inside the panel. That render caught the first draft's bug — the container's handler ran after the row's as the event bubbled and cleared the target, leaving Rename and Delete greyed out on every row. 20 new schema tests cover the traversal cases. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GTQ3RzTAey9nT6r1kbGBCd
The window was created with no `icon`, so it carried no _NET_WM_ICON and a Linux taskbar had nothing to draw but a placeholder. That is invisible when the app is started from its .desktop file, because the launcher supplies the icon — and it is exactly what you see when the app is started any other way, which is how anyone running the AppImage or a checkout starts it. The window now takes resources/icon.png: from `process.resourcesPath` when packaged, from the repo when run out of a checkout, and undefined rather than a path to nothing, since BrowserWindow given a missing icon logs nothing and shows no icon. The second half is grouping. A taskbar ties a window to its launcher by matching WM_CLASS against the .desktop file's StartupWMClass, and neither desktop entry had one. Chromium derives WM_CLASS from the executable name, which differs between the deb, the AppImage and a dev run, so the app now pins it to `DiskPush` before the window is created and both entries — the installer's and electron-builder's — declare that same value. A test reads both files and fails if either drifts from the constant, because a rename that touched one of them would un-group the window with nothing else failing. Verified: the checkout icon path resolves against the real dist-electron layout, and the 5 new tests cover candidate ordering, the dev fallback, the missing-icon case and both desktop entries. Not verified here: the WM_CLASS the window actually reports. This box has no X server and no xprop, so the pinning is argued from Chromium's behaviour rather than measured. Its failure mode is the icon we now set being used without grouping, which is no worse than today. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GTQ3RzTAey9nT6r1kbGBCd
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Right-click file operations
Refresh, New folder, New file, Rename and Delete, on a context menu, in the local pane and the remote one alike — the FileZilla operations the panes looked like they already had.
F2renames andDeldeletes.The remote half already had
mkdir/rename/deleteover SFTP and nothing called them; the local half had no mutations at all. Both now go through one bridge where the connection id selects local or remote, so a caller cannot act on the wrong pane by reaching for the wrong method name.Not letting the renderer name a path
Every mutation takes a directory plus a bare entry name and joins them in the main process.
EntryNameSchemarejects a separator,.., a NUL and surrounding space — so "New folder" in a listing of/home/youcannot write outside it, and on the remote side cannot write anywhere the SSH user can reach.Delete re-stats its target and refuses when what is on disk disagrees with what the renderer claimed, so a mislabelled request cannot turn one unlink into a recursive delete. A symlink is unlinked, never followed into.
Two things SFTP does not hand you: creating a file (
openwithwx, which fails rather than truncating something already there) and removing a populated directory (rmdironly unlinks an empty one, soremoveRecursivewalks it depth first).What the render caught
Verified in a headless render of the real export under the app's real CSP, both themes: the menu opens on a row with all five items live, the row becomes the selection, and the New folder dialog focuses its field with its footer inside the panel.
That render found the first draft's bug — the container's
onContextMenuran after the row's as the event bubbled and cleared the target, leaving Rename and Delete greyed out on every row. It reads correctly in the source; it is obvious in a screenshot.An icon in the taskbar
The window was created with no
icon, so it carried no_NET_WM_ICONand a Linux taskbar had nothing to draw. That is invisible when the app starts from its.desktopfile, because the launcher supplies the icon — and it is exactly what you get starting it any other way, which is how anyone on the AppImage or a checkout starts it.The window now takes
resources/icon.png:process.resourcesPathwhen packaged, the repo copy in a checkout, andundefinedrather than a path to nothing.The other half is grouping. A taskbar ties a window to its launcher by matching
WM_CLASSagainst the entry'sStartupWMClass, and neither desktop entry had one. Chromium derivesWM_CLASSfrom the executable name, which differs between the deb, the AppImage and a dev run, so the app pins it toDiskPushbefore the window exists and both entries declare that value. A test reads both files and fails if either drifts from the constant.Honest limit: the
WM_CLASSthe window actually reports is not verified — this machine has no X server and noxprop, so the pinning is argued from Chromium's behaviour rather than measured. Its failure mode is the icon we now set being used without grouping, which is no worse than today.Checks
pnpm test— 296 passed, 27 files (25 new)pnpm typecheck— clean across every package and apppnpm smoke:desktop— assets resolve, served over its own scheme, CSP admits all 7 inline scripts (its Electron-launch step skips locally for missing system libs; it runs in CI)🤖 Generated with Claude Code
https://claude.ai/code/session_01GTQ3RzTAey9nT6r1kbGBCd