Skip to content

fix(desktop-gpui): reveal and open paths through GPUI - #2965

Open
Ectropy wants to merge 2 commits into
CapSoftware:mainfrom
Ectropy:fix/open-recording-bundle-windows
Open

Ectropy wants to merge 2 commits into
CapSoftware:mainfrom
Ectropy:fix/open-recording-bundle-windows

Conversation

@Ectropy

@Ectropy Ectropy commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor

Problem

On Windows, the folder icon in the recordings list and the editor's Open recording bundle button open the user's Documents folder instead of revealing the .cap bundle.

Two causes:

  1. reveal_in_folder ran explorer with /select,<path> as one argument. Bundle names contain spaces, so Rust quotes the whole argument ("/select,C:\…\My Recording.cap"). Explorer doesn't recognize a quoted /select, so it falls back to Documents.
  2. The editor (and screenshot editor) hold their bundle path from std::fs::canonicalize, which on Windows returns a \?\C:\… verbatim path. The shell rejects that with The parameter is incorrect. (0x80070057), so even a correctly quoted reveal does nothing.

Fix

The hand-written per-OS open / explorer / cmd /C start / xdg-open spawns in library.rs are replaced with GPUI's own cx.reveal_path and cx.open_with_system, which use the native APIs on each platform (SHOpenFolderAndSelectItems / ShellExecuteW on Windows, NSWorkspace / open on macOS, the desktop portal on Linux).

Two thin helpers, library::reveal_path and library::open_with_system, pass the path through dunce::simplified first so verbatim paths never reach the shell. dunce was already in the lockfile, and simplified is a no-op on macOS and Linux. Canonicalization itself is unchanged, because other code compares those canonical paths (for example the editor's delete check).

The tray's macOS-only open spawn now uses the same helpers. Windows already called GPUI there. On Linux that tray item previously did nothing, and now opens.

Related: #2309 ("Open File" after export flashes a terminal on Windows) came from the removed cmd /C start path, so this may also address it (not yet verified). #2316 edits the reveal_in_folder block this PR deletes.

How to test

Use recordings and screenshots whose names contain spaces (the default names do). On macOS every item should behave exactly as before. On Windows, items 1–7 previously opened Documents or did nothing.

Reveal: Explorer/Finder opens the folder with the item selected

  • 1. Main window, recordings list: folder icon on a studio recording card
  • 2. Main window, screenshots list: folder icon on a screenshot card
  • 3. Editor: the Open recording bundle folder button in the header
  • 4. Screenshot editor: … (More) menu → Open Folder (selects original.png)
  • 5. Settings → Recordings: folder icon on a studio recording row
  • 6. Settings → Screenshots: Open folder icon on a screenshot row
  • 7. Settings → Feedback → Diagnostic Report: run it, then Show File

Open: opens in the default app

  • 8. Main window, recordings list: folder icon on an instant recording opens its content folder
  • 9. Main window, recordings list: clicking an instant recording card with no share link opens its content folder
  • 10. Settings → Recordings: folder icon on an instant recording row opens its content folder
  • 11. Editor → Export to file → Open File: opens the exported file (on Windows, no terminal window should flash, Export complete | Clicking 'Open File' causes a visible terminal window #2309)
  • 12. Opening an instant .cap from outside the app (an open_editor deep link) opens output.mp4 in the system player
  • 13. Tray → Previous → an instant recording: opens the share link, or output.mp4 if not shared

On a dev build without the cap CLI sidecar, the diagnostic report (item 7) is partial and says so, but Show File still reveals it.

Tested

  • Windows 11: items 1–13 pass.
  • macOS: not tested.
  • cargo clippy --all-targets for apps/desktop-gpui shows no new warnings, and the library, tray and deeplink tests pass.

RetriggerView in GreptileConfidence Score: 5/5

The PR appears safe to merge; no actionable defects were found.

Summary

The PR replaces hand-written file-opening commands with GPUI calls.

  • Recording folder actions open instant content and reveal other bundles.
  • Reveal actions show recording and screenshot files in their folders.
  • Exported and instant-recording files open through the system’s default app.

No actionable issues were found. Review checks covered the diff, callers, tray callbacks, and repository rules. No build or native-platform runtime tests were run.

Reviews (1) · Last reviewed commit: "Merge remote-tracking branch 'upstream/m..." · Reviewed by Greptile

@vercel

vercel Bot commented Oct 23, 2026

Copy link
Copy Markdown
Contributor

@Ectropy is attempting to deploy a commit to the Cap Software Inc Team on Vercel.

A member of the Team first needs to authorize it.

@Ectropy
Ectropy force-pushed the fix/open-recording-bundle-windows branch from b2657f5 to 933ab22 Compare October 23, 2026 14:34
On Windows, "open folder" / "reveal" actions opened the Documents folder
instead of the recording bundle. `explorer /select,<path>` was passed as a
single argument, so Rust quoted it (bundle names contain spaces) and
Explorer ignored the switch. The editor's bundle path additionally comes
from `std::fs::canonicalize`, whose `\?\` verbatim form the shell rejects
with E_INVALIDARG.

Replace the hand-rolled per-OS `open` / `explorer` / `cmd start` /
`xdg-open` spawns with GPUI's `reveal_path` and `open_with_system`, behind
`library::reveal_path` / `library::open_with_system` helpers that pass the
path through `dunce::simplified` first. The tray's macOS-only `open` spawn
goes through the same helpers.
@Ectropy
Ectropy force-pushed the fix/open-recording-bundle-windows branch from 933ab22 to 203f103 Compare October 23, 2026 15:08
@richiemcilroy

Copy link
Copy Markdown
Member

Thanks, this looks good to us now. When you're happy with it, please mark it ready for review and merge main into the branch; we'll then run the Greptile review and CI, and it needs 5/5 plus green CI to merge.

@Ectropy

Ectropy commented Oct 24, 2026

Copy link
Copy Markdown
Contributor Author

I've done all 13 tests I wanted to do now.
Be aware this is untested on Mac. Ideally behavior will remain identical on Mac.

I can confirm thru testing via a build made with cargo run --release that it appears to fix #2309. In retail 0.6.1 there is a terminal that flashes if clicking the Open file button after an export to file, but in my release build the terminal wasn't visible. Given this, PR #2316 is likely no longer needed since I believe it was addressing the same issue that we incidentally solved here.

I'll merge in main's changes then open the PR. Should be good pending any Mac tests you wish to try.

This branch has not been deployed

No deployments
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.

2 participants