smolck / uivonim

Fork of the Veonim Neovim GUI
GNU Affero General Public License v3.0
621 stars 12 forks source link

WSL support #99

Closed kalvinpearce closed 3 years ago

kalvinpearce commented 3 years ago

Would it be possible/how hard would it be to hook this up to Neovim running in WSL. I have my whole config in the WSL side of things and would love to have this app running on the windows side. Not sure if this covered by #2 or not so I thought I'd check. If its possibly a good first issue I would be willing to have a crack at it too ๐Ÿ˜Š

smolck commented 3 years ago

Hello! So Iโ€™m not familiar with WSL as I donโ€™t use Windows, but if itโ€™s just a simple binary that can be run somehow it should be fairly straightforward. #2 would, in that case, fix the issue, as you could just specify the nvim path; however, if a specific command is required to run nvim (I.e. wsl run nvim or something along those lines), then it would require a bit more tweaking. I see no reason it couldnโ€™t work though.

kalvinpearce commented 3 years ago

Ahh right yeah its run with wsl nvim. Might see if I can find some time to look into this a little over the weekend and try get a pr up

smolck commented 3 years ago

@kalvinpearce I think #109 might fix this, but I'm not convinced; would you mind testing it? You don't even need to use the PR, simply change this line: https://github.com/smolck/uivonim/blob/7188dd77b115b0268cedf4c3314ea2dc351a46cb/src/core/master-control.ts#L59 to spawn('wsl nvim', [, and do an npm run start and see if it works. If not, then I can add a --wsl flag like Neovide does in a different PR specifically for this usecase. (I have a feeling spawn('wsl nvim', [ won't work, but no harm in trying.)

kalvinpearce commented 3 years ago

Just tried it now and looks like it didn't work sorry. Double checked that the build is working and seems fine when spawning nvim but fails to spawn wsl nvim. I have added the console logs too if helps

electron console log & npm run start output ![image](https://user-images.githubusercontent.com/17244425/106243896-dc23ad80-6255-11eb-9ecc-a1e0e3b8df40.png) ``` $> npm run start > uivonim@0.27.0 start D:\Repos\uivonim > node tools/start.js cleaning build folder copying index html copying process-explorer html copying assets copying runtime files running babel stuff babel --extensions .ts,.tsx src -d build Successfully compiled 91 files with Babel (4003ms). electron build/bootstrap/main.js scrdir: D:\Repos\uivonim\build uivonim started in develop mode. you're welcome (node:2028) electron: The default of contextIsolation is deprecated and will be changing from false to true in a future release of Electron. See https://github.com/electron/electron/issues/23506 for more information (node:2028) ExtensionLoadWarning: Warnings loading extension at C:\Users\Kev\AppData\Roaming\Electron\extensions\fmkadmapgofadopljbjfkapdkoienihi: Unrecognized manifest key 'browser_action'. Unrecognized manifest key 'minimum_chrome_version '. Unrecognized manifest key 'update_url'. Cannot load extension with file or directory name _metadata. Filenames starting with "_" are reserved for use by the system. loaded ext: React Developer Tools (node:2028) ExtensionLoadWarning: Warnings loading extension at C:\Users\Kev\AppData\Roaming\Electron\extensions\lmhkpmbekcpmknklioeibfkpmmfibljd: Unrecognized manifest key 'commands'. Unrecognized manifest key 'homepage_url'. Unrecognized manifest key 'page_action'. Unrecognized manifest key 'short_name'. Unrecognized manifest key 'update_url'. Permission 'notifications' is unknown or URL pattern is malformed. Permission 'contextMenus' is unknown or URL pattern is malformed. Permission 'tabs' is unknown or URL pattern is malformed. Cannot load extension with file or directory name _metadata. Filenames starting with "_" are reserved for use by the system. loaded ext: Redux DevTools reloading changes... [2028:0129/170932.994:ERROR:CONSOLE(0)] "TypeError: Cannot read property 'isAnonymousInlineStyleSheet' of null", source: (0) reloading changes... [2028:0129/170933.190:ERROR:CONSOLE(1)] "Cannot find context with specified id", source: devtools://devtools/bundled/sdk/sdk.js (1) [2028:0129/170933.190:ERROR:CONSOLE(1)] "Extension server error: Inspector protocol error: Cannot find context with specified id", source: devtools://devtools/bundled/extensions/extensions.js (1) [2028:0129/170933.190:ERROR:CONSOLE(1)] "Cannot find context with specified id", source: devtools://devtools/bundled/sdk/sdk.js (1) [2028:0129/170933.190:ERROR:CONSOLE(1)] "Extension server error: Inspector protocol error: Cannot find context with specified id", source: devtools://devtools/bundled/extensions/extensions.js (1) [2028:0129/170933.215:ERROR:CONSOLE(1)] "Connection is closed, can't dispatch pending call", source: devtools://devtools/bundled/sdk/sdk.js (1) reloading changes... Attempting to call a function in a renderer window that has been closed or released. Function provided here: Object. (D:\Repos\uivonim\build\core\input.js:233:37 Attempting to call a function in a renderer window that has been closed or released. Function provided here: Object. (D:\Repos\uivonim\build\core\input.js:233:37 Attempting to call a function in a renderer window that has been closed or released. Function provided here: Object. (D:\Repos\uivonim\build\core\input.js:233:37 ```
smolck commented 3 years ago

Just tried it now and looks like it didn't work sorry. Double checked that the build is working and seems fine when spawning nvim but fails to spawn wsl nvim. I have added the console logs too if helps

Okay, no worries, I didn't think it would work. On the other hand, I believe this diff should work:

diff --git a/src/core/master-control.ts b/src/core/master-control.ts
index c878047c..3df50f73 100644
--- a/src/core/master-control.ts
+++ b/src/core/master-control.ts
@@ -56,7 +56,8 @@ const msgpackDecoder = new MsgpackStreamDecoder()
 const msgpackEncoder = new MsgpackStreamEncoder()

 const spawnVimInstance = (pipeName: string) =>
-  spawn('nvim', [
+  spawn('wsl', [
+    'nvim',
     '--cmd',
     `com! -nargs=+ -range -complete=custom,UivonimCmdCompletions Uivonim call Uivonim(<f-args>)`,
     '--embed',

That does, however, assume that any arguments passed after wsl nvim go to neovim and not to wsl. If that's not the case it would need more tweaking to get those other arguments passed to neovim. But if that works, then making it work on passing a --wsl param should be straightforward.

kalvinpearce commented 3 years ago

Sorry for the late reply, but yep that dif seems to have done the trick. I haven't tested too thoroughly but its defenitely loading and reading my wsl neovim instance ๐Ÿ™ thanks a bunch!

smolck commented 3 years ago

Great! Just to make sure, do :Uivonim commands work and tab-complete, e.g. :Uivonim nc? I believe they should, but I canโ€™t test that myself.

Iโ€™ll try to get a PR adding the --wsl option done and merged today (unless youโ€™d like to implement it yourself, modeled after #109?), and also get a new release up so you can use it.

kalvinpearce commented 3 years ago

:Uivonim nc didn't seem to work, no nyan cat unfortunately ๐Ÿ˜ž any others you'd want me to try out? And it seems like completion stuff is working and tab complete is working for commands too so most of it seems to be going?

I am busy in a game-jam this weekend sorry and it feels like the issue might be a little out of scope for me considering my unfamiliarity with the projec but I am happy to keep helping you test things out on my end ๐Ÿ™‚

smolck commented 3 years ago

:Uivonim nc didn't seem to work, no nyan cat unfortunately ๐Ÿ˜ž

Hmm, it almost seems more like a Windows-specific problem than one related to WSL. Maybe the ../assets/nc.gif URL doesn't work properly on Windows?

https://github.com/smolck/uivonim/blob/7188dd77b115b0268cedf4c3314ea2dc351a46cb/src/components/memes/nc.tsx#L14

any others you'd want me to try out?

You can just play around with :Uivonim commands if you want, like :Uivonim pick-color (which has a bug when entering the color in text, but the UI should still work), :Uivonim version (which I seem to have broken on master ๐Ÿ˜…), :Uivonim explorer, etc.

And it seems like completion stuff is working and tab complete is working for commands too so most of it seems to be going?

That's good! And yes, it does seem like most things work; WSL doesn't complicate things much, which is great. Just requires an extra prefix to the nvim command when passing --wsl. Out of curiosity, can you pass paths to wsl for the nvim binary? E.g. $ wsl /usr/bin/nvim? (Asking to see if --nvim and --wsl could/should be able to be used together.)

I am busy in a game-jam this weekend sorry and it feels like the issue might be a little out of scope for me considering my unfamiliarity with the projec but I am happy to keep helping you test things out on my end ๐Ÿ™‚

No worries, I just wanted to offer in case you wanted to try your hand at implementing it ;) Thanks for the help testing!

smolck commented 3 years ago

@kalvinpearce Would you be able to give #125 a try? The following commands should be enough to test it once you clone the branch:

# Assumes you've already run `npm install` or `npm ci`

$ npm run build
$ npx electron build/bootstrap/main.js --wsl

# Should also be able to specify path to neovim
$ npx electron build/bootstrap/main.js --wsl --nvim /path/to/nvim
kalvinpearce commented 3 years ago

Can confirm thats working nicely and feels good. Just after real quick testing, it seems like my config is working as expected too. Still no nyancat tho ๐Ÿ˜› (I didn't see any errors when trying the :Uivonim nc command tho)

smolck commented 3 years ago

Great! Iโ€™ll merge that soon, and hopefully get a release up by the end of the day.

Would you mind opening an issue about nyancat not working? (Note that if you check the dev tools, you can see if there are any errors after calling :Uivonim nc which may help debug this.)

smolck commented 3 years ago

@kalvinpearce I released 0.28.0 which includes the fix for this issue. If you run into any other problems, please don't hesitate to open an issue or ask about it on the Uivonim Gitter!