jbyuki / one-small-step-for-vimkind

Debug adapter for Neovim plugins
MIT License
386 stars 10 forks source link

Launching osv via cli args #43

Closed tmillr closed 1 month ago

tmillr commented 7 months ago

Screenshot 2024-02-03 at 11 21 35 AM

idk if this is possible or easy to fix but I thought I'd go ahead and create this issue anyway for reference/help/feedback. It's not a huge deal, but it does keep me from (or makes it harder at least) debugging tests (which are typically run in headless mode).

I haven't investigated this a ton yet; any ideas? Perhaps it has something to do with how nvim operates internally while in headless mode (e.g. not allowed to start a job or rpc client/server until after the ui channel/client/server/job has started)? Maybe it would require changing the rpc/server/client functions that are used for communication (e.g. to use libuv instead) in order to fix this? Or is this something that is supposed to work and is an issue on my end? Thanks

tmillr commented 7 months ago

Oh, it seems that nvim -u NONE -i NONE -n +'=require"osv".launch({port = 8086})' isn't working either. This might just be the same issue that I was experiencing last time in #37 (having to do with argv getting copied and reused in child processes recursively). I suppose that launching this plugin simply isn't supported via cli args. If that's the case, maybe we should document it?

Edit: just confirmed that this is indeed the issue as the following works just fine as a workaround:

VIMINIT='=require"osv".launch({env = {VIMINIT = ""}, port = 8086})' nvim --headless -i NONE -n

Other ideas:

...or maybe I'm just going about the whole thing wrong lol.

Also is there a way to make launch() wait for a connection before returning?

jbyuki commented 6 months ago

Good debugging of the issue. I haven't thought about it too much but at the moment the environment variable solution seem to be the most flexible. Maybe another way because I think this usecase is very specific and only people really knowing the inner working would use it, we should provide a solution so it doesn't affect the general usecase. I was thinking about adding a function to check if the server is already running so you can early exit in that case in the lua commands. How does it sound?

EDIT: Actually, this function is already implemented require"osv".is_running() but it is also true that an environment variable solution would avoid the hassle you had and provide an easier solution. On the other hand, like you say, it doesn't completetly solve the problem as it will still run other lua commands. In the end, the perfect solution would be no configuration from the user, and only spawn an embedded neovim instance without such recursive calls. But how to differentiate lua commands which are used to spawn the neovim instance itself and lua commands for the headless debugging instance?

jbyuki commented 1 month ago

I added a new API to override how the headless instance is spawned. It allows you to filter arguments and environment variables before it gets passed to the child instance like you proposed. The handle to the instance has to be returned by the callback.

For example:

require"osv".on["start_server"] = function(args, env)
      -- do something with args and env, args is a table and env is a dict
      return vim.fn.jobstart(args, {rpc = true, env = env})
end

I will close this for now, let me know if something else could be done.

tmillr commented 1 week ago

Thanks!

I was skimming over the source just now. I haven't tried it out yet, but I think 5417de4 fixes the recursive process spawn issue. But I'll let you know if I run into this again.