neovim / node-client

Nvim Node.js client and plugin host
https://neovim.io/node-client/
MIT License
490 stars 53 forks source link

findNvim: support finding nvim in Windows WSL #430

Open xiyaowong opened 1 month ago

justinmk commented 1 month ago

Ok, I see that this blocks https://github.com/vscode-neovim/vscode-neovim/pull/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 commented 4 weeks ago

Should we search wsl's $PATH?

justinmk commented 4 weeks ago

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 commented 4 weeks ago

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

justinmk commented 4 weeks ago

The missing piece is that consumers like https://github.com/vscode-neovim/vscode-neovim/pull/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 commented 4 weeks ago

I have a PR that I'll post in a minute. https://github.com/neovim/node-client/pull/432

gjf7 commented 4 weeks ago

Ah, I see.