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

findNvim: support finding nvim in Windows WSL #430

Open
xiyaowong opened this issue Oct 19, 2024 · 7 comments · May be fixed by #432
Open

findNvim: support finding nvim in Windows WSL #430

xiyaowong opened this issue Oct 19, 2024 · 7 comments · May be fixed by #432
Labels

Comments

@xiyaowong
Copy link
Contributor

No description provided.

@justinmk justinmk added the enhancement feature label Oct 19, 2024
@justinmk justinmk changed the title Support finding nvim in the WSL findNvim: support finding nvim in Windows WSL Oct 19, 2024
@justinmk
Copy link
Member

Ok, I see that this blocks vscode-neovim/vscode-neovim#2287 .

https://github.com/vscode-neovim/vscode-neovim/blob/c3e8387a9e9f923ec39c28af6cb8985ca431e438/src/main_controller.ts#L181-L186

if (config.useWsl) {
    args.push("C:\\Windows\\system32\\wsl.exe");
    if (config.wslDistribution.length) {
        args.push("-d", config.wslDistribution);
    }
}

I think that findNvim can support this without much complexity, by accepting changing paths: string[] to paths: string[][], so that consumers can pass a command like:

findNvim(..., {
 paths: [
   ['C:\\Windows\\system32\\wsl.exe', '-d', ..., 'nvim'],
 ],
})

Perhaps in the future, findNvim could be more clever and do this internally. But meanwhile this is a simple change.

@gjf7
Copy link
Contributor

gjf7 commented Oct 21, 2024

Should we search wsl's $PATH?

@justinmk
Copy link
Member

Should we search wsl's $PATH?

We could try that in the future, but I think just changing the current paths: string[] to a cmds: string[][] solve the main problem for now.

@gjf7
Copy link
Contributor

gjf7 commented Oct 21, 2024

I'm not sure I follow. What should findNvim actually return in this case? The full WSL command, or just the nvim path?

@justinmk
Copy link
Member

The missing piece is that consumers like vscode-neovim/vscode-neovim#2287 can't send a full command which represents "nvim". It can only send a location (arg0) or directory.

node-client doesn't need to know about WSL, it just needs to run what the consumer gives it. Currently the consumer can only send arg0, but we can fix that by allowing consumers to send [arg0, arg1, ...].

@justinmk
Copy link
Member

justinmk commented Oct 21, 2024

I have a PR that I'll post in a minute. #432

@gjf7
Copy link
Contributor

gjf7 commented Oct 21, 2024

Ah, I see.

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

Successfully merging a pull request may close this issue.

3 participants