Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve support for wsl. #33

Closed
wants to merge 3 commits into from
Closed

Improve support for wsl. #33

wants to merge 3 commits into from

Conversation

apogeeoak
Copy link
Contributor

The default Ubuntu installation for wsl contains the gio command but no applications are registered to handle files. With the current implementation wslview is never called if another command exists but fails to open the file. This pull request modifies that() to loop over every open handler until one reports a successful exit status.

To work around a bug in wslview the wsl_path() function is introduced to convert an absolute path into a relative path. If wslview fails to find the path, which occurs with absolute paths or non-existent files it erroneously returns a successful exit status.

The which dependency was removed but the pathdiff dependency was added.

Copy link
Owner

@Byron Byron left a comment

Choose a reason for hiding this comment

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

Thanks a lot for this improvement! To me it looks like without it, open was never really up to the task.

Opening things properly is definitely not as easy as it seemed when I originally wrote this crate.

I have made one suggestion, and will merge this PR once its applied or rejected.

src/lib.rs Show resolved Hide resolved
let path = path.as_ref();
let open_handlers = [
("xdg-open", &[path] as &[_]),
("gio", &[OsStr::new("open"), path]),
Copy link
Owner

Choose a reason for hiding this comment

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

Seeing this implies that the previous invocation never worked, which is reflected in its man page.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

gio open works in the old code. The problem is that wsl Ubuntu is not setup with file handlers by default. Ubuntu not under wsl works as expected.

src/lib.rs Outdated Show resolved Hide resolved
src/lib.rs Outdated Show resolved Hide resolved
Two devs think this case should never be encountered, so let's put it to the test with `expect()`
@Byron
Copy link
Owner

Byron commented Jul 17, 2021

Unfortunately I made the mistake of fast-forwarding the patch provided here so GitHub doesn't see the merge is completed.
Therefore I am closing this PR by hand while referring to the latest release.

I am looking forward to releasing a 2.0 version soon for when correct error handling is made easy. Thanks a lot for all your work thus far!

@Byron Byron closed this Jul 17, 2021
@apogeeoak apogeeoak deleted the wsl branch July 18, 2021 15:52
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