Seeker14491 / opener

Open a file or link in the system default program.
Apache License 2.0
49 stars 10 forks source link

Reveal file in file explorer #18

Closed bash closed 1 year ago

bash commented 1 year ago

I'm finally done 🥳

Resolves #17

Name

I chose reveal because that's the name used in macOS's open command, but I'm open to better suggestions :)

Linux

I found a good explanation of Chromium's solution and used that as an inspiration. If it fails, I'm using your suggested fallback of opening the containing directory with open.

Errors

I didn't want to expose dbus::Error, so I decided to wrap it in std::io::Error instead.

Dependencies

Tested on

Seeker14491 commented 1 year ago

Looks good, I just made some changes:

Any thoughts?

edit: I found one little issue which I also encountered in a related PR: On Windows, if you reveal a non-existent file, no error is returned; the explorer just opens the top-level "This PC" view.

bash commented 1 year ago

I removed the feature-gating on the binary. The binary only exists for testing and debugging purposes, so we don't need the feature-gating.

Makes sense - I wasn't sure about the binary's purpose :)

Regarding the non-existent file handling on Windows: Should I manually check that the file exists? Of course if the file is deleted between our check and the call to explorer.exe we'll still get the "This PC" view... I think on Linux and macOS it always returns an error (Although I'm not sure about the org.freedesktop.portal.Desktop portal).

Seeker14491 commented 1 year ago

I found there is a Windows API function we could use instead, which does return an error code if the file doesn't exist. I think that would be the best solution for Windows. I found this repo which demonstrates that approach (though I think for correctness with respect to COM, it should be using a separate thread). For WSL though, I don't know of a better approach, so maybe checking if the file exists beforehand is the best we can do.

bash commented 1 year ago

I have experimented with this function before, but I was worried about the CoInitialize call as the concurrency mode can't be changed after it's been set for a thread. Having a separate thread dedicated for the API call is a really creative solution :)

Should I spawn a thread for each call to reveal or should I keep a thread around that I can re-use?

I'll take a look at the example repostitory, maybe I can incorporate that.

Seeker14491 commented 1 year ago

Since you linked the Chromium repo above, I was curious how they implemented this on Windows so I looked at the source and that's where I learned about this Windows API function. I also noticed they were using a worker thread. Source link

I think spawning a thread for each call is fine; this is a function that would be called infrequently, so the overhead of spawning a thread is negligible.

bash commented 1 year ago

The latest version now calls SHOpenFolderAndSelectItems on a worker thread. For WSL I'm still using explorer.exe /select.