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

Multi workspace support for sumneko_lua #2302

Closed
musjj opened this issue Dec 9, 2022 · 16 comments · Fixed by #2303
Closed

Multi workspace support for sumneko_lua #2302

musjj opened this issue Dec 9, 2022 · 16 comments · Fixed by #2303
Labels
enhancement New feature or request

Comments

@musjj
Copy link
Contributor

musjj commented Dec 9, 2022

Language server

sumneko_lua

Requested feature

https://github.com/sumneko/lua-language-server/wiki/Developing#multiple-workspace-support
Is it possible to have a multi-workspace environment for sumneko_lua? AKA, a single LSP instance for multiple projects?
I've also noticed this PR: #2287, does it have anything to do with this feature?
I'm on the latest lspconfig commit with the latest nvim nightly, but multiple Lua projects still spawn multiple LSP instances.
A few other related issues I've found:
#842
prabirshrestha/vim-lsp#1069

Other clients which have this feature

vim-lsp

@musjj musjj added the enhancement New feature or request label Dec 9, 2022
@musjj
Copy link
Contributor Author

musjj commented Dec 9, 2022

Did a quick look through the code, and it seems that this is a bug caused by treating clients as a number-keyed table instead of a string-keyed table:

if clients[1] then
local client = lsp.get_client_by_id(clients[1])
if client.name == new_config.name then
local params = lsp.util.make_workspace_params(
{ { uri = vim.uri_from_fname(root_dir), name = root_dir } },
{ {} }
)
table.insert(client.workspace_folders, params.event.added[1])
return clients[1]
end
end

When it is properly treated, multi workspace now works (the list of workspace folders also have to be checked and initialized):

local _, id = next(clients)
if id then
  local client = lsp.get_client_by_id(id)
  if client.name == new_config.name then
    local params = lsp.util.make_workspace_params(
      { { uri = vim.uri_from_fname(root_dir), name = root_dir } },
      { {} }
    )
    client.workspace_folders = client.workspace_folders or {}
    table.insert(client.workspace_folders, params.event.added[1])
    return id
  end
end

@glepnir
Copy link
Member

glepnir commented Dec 10, 2022

yes that's a key-value table. thanks for point it out. now should works fine.

@ehaynes99
Copy link

Is this the intended behavior? Previously, if you opened a directory such as:

.
├── project1
│   ├── example.lua
│   └── .git
├── project2
│   ├── example.lua
│   └── .git
└── project3
    ├── example.lua
    └── .git

You would get a server for each project:

 Language client log: /home/erich/.local/state/nvim/lsp.log
 Detected filetype:   lua
 
 2 client(s) attached to this buffer: 
 
 Client: null-ls (id: 1, pid: 5001, bufnr: [3, 4])
...<SNIP>
 
 Client: sumneko_lua (id: 2, pid: nil, bufnr: [4])
 	filetypes:       lua
 	autostart:       true
 	root directory:  /home/erich/tmp/project-example/project2
 	cmd:             lua-language-server
 
 1 active client(s) not attached to this buffer: 
 
 Client: sumneko_lua (id: 3, pid: nil, bufnr: [3])
 	filetypes:       lua
 	autostart:       true
 	root directory:  /home/erich/tmp/project-example/project1
 	cmd:             lua-language-server
 
 Configured servers list: html, yamlls, tsserver, sumneko_lua, cssls, pyright, bashls, jsonls, rust_analyzer

Now, opening the same project, you get:

 Language client log: /home/erich/.local/state/nvim/lsp.log
 Detected filetype:   lua
 
 2 client(s) attached to this buffer: 
 
 Client: null-ls (id: 1, pid: 5001, bufnr: [4])
...<SNIP>
 
 Client: sumneko_lua (id: 2, pid: nil, bufnr: [4])
 	filetypes:       lua
 	autostart:       true
 	root directory:  /home/erich/tmp/project-example/project2
 	cmd:             lua-language-server
 
 Configured servers list: pyright, rust_analyzer, jsonls, bashls, html, cssls, sumneko_lua, tsserver, yamlls

Even when you're in a file in project1 or project3. It's just arbitrarily picking the root based on whichever file happened to be opened first. Isn't that going to cause issues?

Ironically, I stumbled into this because I wanted to use the sumneko_lua behavior as a reference point to fix the problem in null-ls:

@glepnir
Copy link
Member

glepnir commented Dec 10, 2022

seems like the clients store the null-ls not sumneko_lua so there still spawn the new lua server.

@ehaynes99
Copy link

Sorry, I don't follow. Previously (I was on f11fdff7e8b5b415e5ef1837bdcdd37ea6764dda), it would spawn a new lua server. Now, it does not. There's just one instance with a now arbitrary root.

@glepnir
Copy link
Member

glepnir commented Dec 10, 2022

I don't think spawn multiple servers is correct. it should use one instance with multiple workspacefolders. in your situation it should have one lua instance. when you open file1 in project 1 the proejct 1 will insert to worskpaceFolders . same as other projects.

@musjj
Copy link
Contributor Author

musjj commented Dec 10, 2022

@glepnir
You also need to check if client.workspace_folders is nil and initialize a new table if so.

table.insert(client.workspace_folders, params.event.added[1])

@ehaynes99
Yes, it does look weird, but it works. Doing it this way saves a lot of RAM for heavier LSPs.
I think in the future the workspace folders should also be listed in :LspInfo to make it clearer.

@glepnir
Copy link
Member

glepnir commented Dec 10, 2022

I will fix the root dir of lspinfo . it should show the correct path from workspace folders.

@glepnir
Copy link
Member

glepnir commented Dec 10, 2022

@musj when it's nil it works as single_file_mode and it store in single_file_clients, I don't think there is need to check is nil or not. but I found another problem. open a single lua file then open a lua file in project there still spawn a lua server . also need fix this..

@musjj
Copy link
Contributor Author

musjj commented Dec 10, 2022

If client.workspace_folders is nil, then it will crash, because table.insert only accepts tables.
I had it crash because of that, even though the client wasn't in single file mode.
Also, another possible edge case is a client that doesn't support multi workspaces. I think most clients supports it now, but there might be some that don't.

@glepnir
Copy link
Member

glepnir commented Dec 10, 2022

@musjj @ehaynes99 check #2304

@ehaynes99
Copy link

ehaynes99 commented Dec 11, 2022

Do I need to explicitly do something to specify workspace_folders? Now, for the first file opened, I get:

 Detected filetype:   lua
 
 2 client(s) attached to this buffer: 
 
 Client: null-ls (id: 1, bufnr: [4, 3])
 	filetypes:       handlebars, json, markdown, javascript, javascriptreact, typescript, typescriptreact, vue, css, scss, less, html, jsonc, yaml, markdown.mdx, graphql, python, lua, luau
 	autostart:       false
 	root directory:  /home/erich/workspace/tmp/lspconfig-test/project1
 	cmd:             <function>
 
 Client: sumneko_lua (id: 2, bufnr: [4, 3])
 	filetypes:       lua
 	autostart:       true
 	root directory:  /home/erich/workspace/tmp/lspconfig-test/project1
 	cmd:             lua-language-server
 
 Configured servers list: tsserver, yamlls, cssls, pyright, rust_analyzer, sumneko_lua, html, jsonls, bashls

For the second file in project2, it's:

 Detected filetype:   lua
 
 2 client(s) attached to this buffer: 
 
 Client: null-ls (id: 1, bufnr: [4, 3])
 	filetypes:       handlebars, json, markdown, javascript, javascriptreact, typescript, typescriptreact, vue, css, scss, less, html, jsonc, yaml, markdown.mdx, graphql, python, lua, luau
 	autostart:       false
 	root directory:  Running in single file mode.
 	cmd:             <function>
 
 Client: sumneko_lua (id: 2, bufnr: [4, 3])
 	filetypes:       lua
 	autostart:       true
 	root directory:  Running in single file mode.
 	cmd:             lua-language-server
 
 Configured servers list: tsserver, yamlls, cssls, pyright, rust_analyzer, sumneko_lua, html, jsonls, bashls

I created the project above like below, then simply started nvim from the parent dir:

#!/usr/bin/env bash

for i in 1 2 3
do
  DIR="project${i}"
  mkdir $DIR
  pushd $DIR
  git init .
  touch example.lua
  popd
done

@glepnir
Copy link
Member

glepnir commented Dec 11, 2022

#2304 not merge now. if you want test .please use #2304 branch for test not master.

@ehaynes99
Copy link

I used this, correct?

  { 'glepnir/nvim-lspconfig', branch = 'fixing' },

@glepnir
Copy link
Member

glepnir commented Dec 11, 2022

@ehaynes99 please update should be works fine now.

@ehaynes99
Copy link

Cool, looks good now!

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

Successfully merging a pull request may close this issue.

3 participants