Byron / open-rs

Open a path or URL with the system-defined program
http://byron.github.io/open-rs
MIT License
319 stars 49 forks source link

Use `wslview` on WSL even when `gio` is installed #71

Closed rgwood closed 1 year ago

rgwood commented 1 year ago

This PR fixes a bug I've been experiencing on WSL.

I have gio installed in my WSL environment because it comes bundled with glib. Because of this, open-rs attempts to use gio and fails:

thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: Custom { kind: Other, error: "Launcher \"gio\" \"open\" \"https://google.com\" failed with ExitStatus(unix_wait_status(512))" }', examples/website.rs:4:38
stack backtrace:
   0: rust_begin_unwind
             at /rustc/90743e7298aca107ddaa0c202a4d3604e29bfeb6/library/std/src/panicking.rs:575:5
   1: core::panicking::panic_fmt
             at /rustc/90743e7298aca107ddaa0c202a4d3604e29bfeb6/library/core/src/panicking.rs:65:14
   2: core::result::unwrap_failed
             at /rustc/90743e7298aca107ddaa0c202a4d3604e29bfeb6/library/core/src/result.rs:1791:5
   3: core::result::Result<T,E>::unwrap
             at /rustc/90743e7298aca107ddaa0c202a4d3604e29bfeb6/library/core/src/result.rs:1113:23
   4: website::main
             at ./examples/website.rs:4:5
   5: core::ops::function::FnOnce::call_once
             at /rustc/90743e7298aca107ddaa0c202a4d3604e29bfeb6/library/core/src/ops/function.rs:251:5
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

To solve this, this PR just reorders the launchers used on unix so that wslview is attempted first. I think this is desirable behaviour because if wslview is present we're almost certainly in WSL.

If you foresee any problems with this approach, I could rewrite this to check whether we're in WSL before attempting to use wslview.

rgwood commented 1 year ago

Sure thing, I've updated the PR so it will only attempt to run wslview when running in WSL.

I've looked into this space before and I believe the is-wsl crate is the best way to do this check. It's maintained by Sean Larkinn at Microsoft; I have reviewed the crate's code (and contributed to it). The author is actively maintaining it (unlike the older wsl crate). It caches the result with once_cell so we'll only pay the cost once.

I ran cargo bloat before+after this PR and it seems like it adds about 13KB to the open executable on Linux:

image

image

Byron commented 1 year ago

Thanks a lot, especially for the due diligence!

I have additionally limited the is-wsl dependency to be only for unix.

And now there is a new release with the fix as well.