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

feat(vim.ui.open): try PowerShell before Explorer #29443

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

sybereal
Copy link

This comes from a similar motivation as #28424: explorer.exe does not work particularly well for opening arbitrary paths and URLs. However, in addition, due to an outstanding issue with WSL,1 explorer.exe always exits with a status of 1, resulting in an a superfluous error message being displayed.
PowerShell, on the other hand, does not suffer from this issue and preserves exit codes. Additionally, it handles URLs correctly that explorer.exe fails on.

Footnotes

  1. explorer.exe always return 1 regardless the result microsoft/WSL#6565

This comes from a similar motivation as neovim#28424: explorer.exe does not
work particularly well for opening arbitrary paths and URLs. However, in
addition, due to an outstanding issue with WSL [1], explorer.exe always
exits with a status of 1, resulting in an a superfluous error message
being displayed.
PowerShell, on the other hand, does not suffer from this issue and
preserves exit codes. Additionally, it handles URLs correctly that
explorer.exe fails on.

[1]: microsoft/WSL#6565
@justinmk
Copy link
Member

justinmk commented Jun 24, 2024

Thanks. What is the difference between cmd.exe start and powershell start? I would guess they invoke the same program. But cmd.exe is presumably faster and more reliable (not sure if it returns the same exit codes though). So, why use powershell?

The only reason we used explore.exe in the first place was because of some purported problem with start (I don't remember what). Does start handle all URLs and filepaths (and directory paths!) that explorer.exe handles? Does it open pdfs, images, directories, and URLs, in the same (or better) way?

@justinmk justinmk added the needs:response waiting for reply from the author label Jun 24, 2024
@justinmk
Copy link
Member

justinmk commented Jun 24, 2024

Also in #23401 (comment) I mentioned that netrw uses rundll32 ... which was presumably chosen because it is the most reliable and effective. So please test that and confirm whether it works as good or better.

@sybereal
Copy link
Author

I chose PowerShell because that is what wslview, which is currently already at a higher priority than explorer.exe due to the issue linked in my initial comment, uses internally. Therefore, what this change would achieve is that WSL users who don't have wslview installed would receive (mostly) the same functionality, avoiding the fallback to explorer.exe.

Alternatively, since you mentioned rundll32, that could also be used from WSL by removing the has('win32') check and adding an explicit .exe, so that it can be found from WSL's PATH. Since you seem to be partial to rundll32, would that solution be acceptable to you?

@github-actions github-actions bot removed the needs:response waiting for reply from the author label Jun 24, 2024
@justinmk
Copy link
Member

I chose PowerShell because that is what wslview ... uses internally

seems like a detail worth mentioning in a one-line code comment, and the commit message.

Since you seem to be partial to rundll32, would that solution be acceptable to you?

I'm partial to whatever works best. Up to now, no one has really confirmed/disconfirmed whether rundll32 works best. And it seems worth confirming that since this is the 5th PR that has tweaked this feature :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants