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(lsp): add tcp support #19916

Merged
merged 4 commits into from Aug 29, 2022
Merged

feat(lsp): add tcp support #19916

merged 4 commits into from Aug 29, 2022

Conversation

mfussenegger
Copy link
Member

@mfussenegger mfussenegger commented Aug 23, 2022

The plan is to make it possible to have a fully decoupled transport - in addition to TCP it should also be possible to have a "same process" transport (e.g. that null-ls could utilize)

Language servers like Godot only support TCP and currently require workarounds
like socat.

Native TCP support will also allow us to use a dummy server in the tests. Opposed to the fake-lsp-server process it would have the advantage that the server behavior could be part of a test case, as a fake tcp server can run in the same process. Currently the expected notifications and responses are split off in a separate file.

Used like:

vim.lsp.start({ name = 'godot', cmd = vim.lsp.rpc.connect('127.0.0.1', 6008) })

closes #11911

@github-actions github-actions bot added lsp lua stdlib labels Aug 23, 2022
@mfussenegger mfussenegger changed the title feature(lsp): add tcp support feat(lsp): add tcp support Aug 23, 2022
@mfussenegger
Copy link
Member Author

mfussenegger commented Aug 24, 2022

@jose-elias-alvarez Could you have a look at this to check if this would allow null-ls to stop monkey-patching rpc.start?

This PR would allow cmd to be a function that will receive the dispatchers and must return a table with notify, request, is_closing and terminate. (The handle is no longer exposed and is_closing/terminate is used by the lsp module instead)


For everyone else: This still needs tests, but otherwise I think it's ready for some feedback.

@mfussenegger mfussenegger marked this pull request as ready for review August 24, 2022 19:04
@jose-elias-alvarez
Copy link
Contributor

From a null-ls perspective this looks great! This should be more or less a drop-in replacement for us (I’m away from a PC for the next week or so but will test this as soon as I can).

To prepare for different transports like TCP where the handle won't have
a kill method.
Makes the previously inner functions re-usable for a TCP client
@eschmidscs
Copy link

eschmidscs commented Sep 2, 2022

Hi

I guess I just need some help here. I'm all newbie, lua, neovim, lsp...
I'm trying to use this for a language server that just uses some port available.

So instead of

vim.lsp.start({name="lsp", cmd=vim.lsp.rpc.connect(host, port)})

I try to do the following (simplified):

vim.lsp.start({
  name="lsp",
  cmd=function()
    start_lsp()
    port=get_lsp_port()
    return vim.lsp.rpc.connect(host, port)
  end
})

This fails with

Error executing lua callback: /usr/share/nvim/runtime/filetype.lua:20: Vim(append):Error executing lua callback: /usr/share/nvim/runtime/lua/vim/lsp.lua:1283: attempt to index upvalue 'rpc' (a function value)

It looks like something local should not be so, but even removing all this did not help.
If I just remove the function and keep the vim.lsp.rpc.connect it works.
Can you give me some hint what is wrong?

Or: Is this the completely wrong attempt? How should I get the port after server start? When should the server be started?

@clason
Copy link
Member

clason commented Sep 2, 2022

Sorry, but this is not a support forum -- usage questions should be asked on https://app.element.io/#/room/#neovim:matrix.org or https://neovim.discourse.group.

@eschmidscs
Copy link

I'll ask on discourse. I started here because it seems directly related to this PR (at least on how to use the tcp stuff).

@neovim-discourse
Copy link

This pull request has been mentioned on Neovim Discourse. There might be relevant details there:

https://neovim.discourse.group/t/starting-order-for-tcp-based-lsp-lua-error-attempt-to-index-upvalue/3108/1

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 this pull request may close these issues.

LSP with servers using TCP
5 participants