Changed Server Capabilities are not preserved upon `VenvSelect`

[chrisgrieser]

Thanks for this nice plugin, just stumbled upon it by chance!

So I am using pyright and pylsp at the same time, and I use this snippet to disable pyright's hover capabilities, since I prefer pylsp's:

require("lspconfig").pyright.setup {
  on_attach = function(client, _) client.server_capabilities.hoverProvider = false end
}

However, it seems that the way this plugin sets up the LSPs, only the settings are carried over, but not the disabled server capabilities, meaning that as soon as I use VenvSelect, the hover capability is re-enabled again.

(I tried various versions of updating the lsp config via hooks or simply using local on_attach = vim.deepcopy(pyright.config.on_attach) to preserve the disabled hover, but could not make it work.)

[linux-cultist]

You're welcome. :)

Interesting scenario for sure. I added some more debug output just now that prints how the pyright config looks like when you select a virtual env.

It should look something like this (at the end of output) if you type :messages after selecting a virtual env:

VenvSelect: Pyright settings:
VenvSelect: 
{
  python = {
    analysis = {
      autoSearchPaths = true,
      diagnosticMode = "workspace",
      useLibraryCodeForTypes = true
    },
    pythonPath = "/home/cado/Code/sampleprogram/venv/bin/python"
  }
}

Can you print how your pyright lsp settings look like before VenvSelect overwrites them?

[chrisgrieser]

Do I have to do something to get the output?

I tried :mess before and after using :VenvSelect, if both cases nothing gets added from this plugin.

I just run the snippet above without passing anything to settings, which is maybe why no settings are shown? As far as I can tell, server_capabilities are not part of lsp settings? 🤔

[linux-cultist]

Sorry, you need to add enable_debug_output = true to the plugin settings also. :)

I havent looked into the pyright LSP details either, I just did what was necessary to get the plugin to work when I first started writing it. But I was curious how your pyright settings look like after you run your snippet there, to maybe get some clues how it should look like to not overwrite the client.server_capabilities.hoverProvider setting.

Maybe its not the right way and I just need to read up more about the lsp and figure out how to do this. :)

[chrisgrieser]

ah, got it. Okay, so here is the output then:

VenvSelect: 
{
  anaconda_base_path = "",
  anaconda_envs_path = "/Users/chrisgrieser/.conda/envs",
  auto_refresh = true,
  cache_dir = "/Users/chrisgrieser/.cache/venv-selector/",
  cache_file = "/Users/chrisgrieser/.cache/venv-selector/venvs.json",
  changed_venv_hooks = { <function 1>, <function 2>, <function 3> },
  dap_enabled = true,
  enable_debug_output = true,
  hatch_path = "~/Library/Application/Support/hatch/env/virtual",
  name = { ".venv" },
  notify_user_on_activate = false,
  parents = 0,
  pipenv_path = "~/.local/share/virtualenvs",
  poetry_path = "~/Library/Caches/pypoetry/virtualenvs",
  pyenv_path = "~/.pyenv/versions",
  search = true,
  search_venv_managers = true,
  search_workspace = true,
  venvwrapper_path = "~/.virtualenvs"
}
VenvSelect: Setting fd_binary_name to 'fd' since it was found on system.
VenvSelect: Telescope path: /Users/chrisgrieser/Repos/axelrod-prisoner-dilemma
VenvSelect: Looking for parent venvs in '/Users/chrisgrieser/Repos/axelrod-prisoner-dilemma' using the following parameters:
VenvSelect: 
{ "--absolute-path", "--color", "never", "-E", "/proc", "-HItd", "(^.venv$)", "/Users/chrisgrieser/Repos/axelrod-prisoner-dilemma" }
VenvSelect: No virtual env selected in telescope.
VenvSelect: Found venv in parent search: /Users/chrisgrieser/Repos/axelrod-prisoner-dilemma/.venv/
VenvSelect: Found workspace folder: /Users/chrisgrieser/Repos/axelrod-prisoner-dilemma
VenvSelect: Running search for workspace venvs with: fd -HItd --absolute-path --color never '(^.venv$)' /Users/chrisgrieser/Repos/axelrod-prisoner-dilemma 
VenvSelect: Found venv in Workspace search: /Users/chrisgrieser/Repos/axelrod-prisoner-dilemma/.venv/
VenvSelect: Found no venv manager directories to search for venvs.
VenvSelect: There are 1 results to show:

[linux-cultist]

I think you need to have the pyright LSP activated also for the important part to show up. So open a python file also, activate the venv with VenvSelect and then do the :messages command. :)

[chrisgrieser]

• enable_debug_output = true
• in a python file
• pyright active
• no venv enabled
• then run :VenvSelect

gets me exactly the output I posted above

[cmetz]

yeah it's a "bug" in the plugin, i also have trouble there. the options @chrisgrieser is mentioning are not under settings instead they are config options like: .capabilities, .handlers, ... https://github.com/neovim/nvim-lspconfig/blob/a27356f1ef9c11e1f459cc96a3fcac5c265e72d6/doc/lspconfig.txt#L132

venv-select is only taking a deep copy of settings, but not the other configuration parameters, if that is even possible and then starting a new lsp server with it's own then reduced configuration.

i see there are only three/four solutions:

1. maybe the on_attach can be modified by the hook without a new server setup and then call a server restart/reset if there is one
2. copy other (or complete) configure options as well, not only settings, if that is possible
3. allow the user to define their own config options in the lsp hook
4. or the user just replaces the complete hook with there own one, this might already be possible

[linux-cultist]

Ok thank you @cmetz and @chrisgrieser , sounds like a bit more digging is required here.

venv-select is only taking a deep copy of settings, but not the other configuration parameters, if that is even possible and then starting a new lsp server with it's own then reduced configuration.

Yes this is correct. The plugin calls the pyright hook which makes a copy of settings here:

https://github.com/linux-cultist/venv-selector.nvim/blob/6841d78e4f88efbceca9b2546e6080a9914109c4/lua/venv-selector/hooks.lua#L22-L36

There are instructions in the README how to write your own hook replacing this one, which can be a start for exploring how to add support for multiple LSP's, if you want to give it a shot. I havent ventured very deep into that particular jungle myself at this point. :)

[cmetz]

i replaced the hook with that code below and it works, but i don't know how that whole setup can handle different buffers with different venv. maybe there is a way to have multiple lspserver running for different workspaces, or just start new vim session.

--- @alias VenvChangedHook fun(venv_path: string, venv_python: string): nil
--- @type VenvChangedHook
function M.pyright_hook(_, venv_python)
  M.execute_for_client("pyright", function(pyright)
    local utils = require("venv-selector.utils")
    pyright.config.before_init = function(_, c)
      c.settings.python.pythonPath = venv_python
      utils.dbg("Pyright settings:")
      utils.dbg(c.settings)
    end
    vim.cmd(string.format("LspRestart %d", pyright.id))
  end)
end

maybe there is a better way to reload the server, but that seems not that easy https://github.com/neovim/nvim-lspconfig/blob/a27356f1ef9c11e1f459cc96a3fcac5c265e72d6/plugin/lspconfig.lua#L89

this works also:

--- @alias VenvChangedHook fun(venv_path: string, venv_python: string): nil
--- @type VenvChangedHook
function M.pyright_hook(_, venv_python)
  M.execute_for_client("pyright", function(pyright)
    pyright.config.settings.python.pythonPath = venv_python
    vim.cmd(string.format("LspRestart %d", pyright.id))
  end)
end

i don't know why it was originally done by before_init.

[linux-cultist]

Using the before_init function is almost instant and avoids restarting the entire LSP server. I also think LspRestart restarts all languages LSP servers in the editor. Some servers are slow in the beginning before they have cached the project.

It would be nice to keep using the before_init to not restart other lsp servers. Do both solutions above work for your usecases?

[cmetz]

@linux-cultist LspRestart with an client id only restarts specific server. the current use of .setup(..) is also restarting the whole server only evaluating a before init handler to manipulate the config. this might be useful for setting some dynamic stuff e.g. when calling an restart and evaluate some dynamic code. but the code is setting only a fix venv value, so i'm pretty sure this can be done completely in the config.settings not using the init function.

or if you want to set some value depending on a config value like they did it here:

https://github.com/neovim/nvim-lspconfig/blob/a27356f1ef9c11e1f459cc96a3fcac5c265e72d6/lua/lspconfig/server_configurations/codeqlls.lua#L11

from the documentation: https://neovim.io/doc/user/lsp.html

before_init: Callback with parameters (initialize_params, config) invoked before the LSP "initialize" phase, where params contains the parameters being sent to the server and config is the config that was passed to [vim.lsp.start_client()](https://neovim.io/doc/user/lsp.html#vim.lsp.start_client()). You can use this to modify parameters before they are sent.

[linux-cultist]

I remember reading this quickly when I started writing this plugin a long time ago. It was my first plugin and I was learning lua at the same time, so I was just happy to get something working. :)

[cmetz]

yeah you did really a great job. Lazvim uses it as default :)

but i just found out that for pyright this can even be done quicker. there is a command which allows you to change the pythonpath and then evokes an workspace update in the lsp server, so no restart of the server needed :)

--- @alias VenvChangedHook fun(venv_path: string, venv_python: string): nil
--- @type VenvChangedHook
function M.pyright_hook(_, venv_python)
  if vim.fn.exists(":PyrightSetPythonPath") > 0 then
    vim.cmd(string.format("PyrightSetPythonPath %s", venv_python))
  end
end

https://github.com/neovim/nvim-lspconfig/blob/a27356f1ef9c11e1f459cc96a3fcac5c265e72d6/lua/lspconfig/server_configurations/pyright.lua#L21C16-L21C31

maybe it's better to not rely on a user command and invoke that lsp command directly

--- @alias VenvChangedHook fun(venv_path: string, venv_python: string): nil
--- @type VenvChangedHook
function M.pyright_hook(_, venv_python)
  M.execute_for_client("pyright", function(pyright)
    pyright.config.settings =
      vim.tbl_deep_extend("force", pyright.config.settings, { python = { pythonPath = venv_python } })
    pyright.notify("workspace/didChangeConfiguration", { settings = nil })
  end)
end

Some more technical background notes for nerds:

• with the method above pyright will also restart the service (maybe only internally): https://github.com/microsoft/pyright/blob/0474001001fdd63f0134e425837a32bb833d1749/packages/pyright-internal/src/languageServerBase.ts#L522
• why { settings = nil } => because pyright will fetch the now changed plugin default configuration from the lsp client (neovim) https://github.com/microsoft/pyright/blob/0474001001fdd63f0134e425837a32bb833d1749/packages/pyright-internal/src/languageServerBase.ts#L513C14-L513C14

[linux-cultist]

Thats super cool, I will try it out tomorrow right away. :) Thanks so much for helping out!

[linux-cultist]

I really like the last solution here:

--- @alias VenvChangedHook fun(venv_path: string, venv_python: string): nil
--- @type VenvChangedHook
function M.pyright_hook(_, venv_python)
  M.execute_for_client("pyright", function(pyright)
    pyright.config.settings =
      vim.tbl_deep_extend("force", pyright.config.settings, { python = { pythonPath = venv_python } })
    pyright.notify("workspace/didChangeConfiguration", { settings = nil })
  end)
end

Seems to work equally well for me, and this also solves the issue for both of you?

[cmetz]

@linux-cultist yeah this solves the issue for me, but maybe it would make sense to iterate over all matching pyright clients if there are more than one.

meanwhile i switched to configuring my pythonPath through an neoconf config file, as it is installed with lazyvim by default and also respects some other configs options like that from vscode and others.

maybe i'll try to write a venvselect hook that will configuring the .neoconf.json file for a specific workspace.

this is an example .neoconf.json

{
  "lspconfig": {
    "pyright": {
      "python.pythonPath": "venv/bin/python"
    }
  }
}

i also configured my dap resolve_path via an function which resolves the pythonpath from the pyright config

  {
    "mfussenegger/nvim-dap-python",
    init = function()
      require("dap-python").resolve_python = function()
        return vim.lsp.get_active_clients({ name = "pyright" })[1].config.settings.python.pythonPath
      end
    end,
  },

i just switched to nvim again couple of days ago and it's quite interesting to see different packages solving things a different way. but none of them is having a venvselect :)

[chrisgrieser]

okay, I am not sure, I can follow everything in the discussion, but the last snippet here:

function M.pyright_hook(_, venv_python)
  M.execute_for_client("pyright", function(pyright)
    pyright.config.settings =
      vim.tbl_deep_extend("force", pyright.config.settings, { python = { pythonPath = venv_python } })
    pyright.notify("workspace/didChangeConfiguration", { settings = nil })
  end)
end

seems to work for me!

Haven't done thorough testing, but haven't noticed any issues with it either. Can be merged in the plugin, I think?

[linux-cultist]

Yeah I think it seems to work fine and it would make sense to replace the default hook with this one. Just wanted to make sure it doesnt do anything weird. Im not too read-up on the specifics on the LSP. @cmetz knows more than me for sure. :)

[linux-cultist]

i just switched to nvim again couple of days ago and it's quite interesting to see different packages solving things a different way. but none of them is having a venvselect :)

Im still on it but after years of maintaining my own config, I actually switched to Lazyvim due to always having some weird small bugs in my own configs. Its fun to have my own config but its also kind of hard to understand why something is buggy now and then.

[cmetz]

@linux-cultist i don't understand your commit it seems redundant for me handlig some lsp servers.

in the venv.lua (M.set_venv_and_system_paths) the pythonpath is set twice, once via an autocommand i don't understand why this is used.:

 M.set_pythonpath(venv_python)

and a second time via hook

  for _, hook in ipairs(config.settings.changed_venv_hooks) do
    hook(venv_path, venv_python)
  end

maybe the autocommand is used to override other plugins switching the pythonpath, but in my understanding this will create a new and add a new autocommand every time you switch the environment, not removing the old one.

[linux-cultist]

The original way (first thing you are showing above) is how VenvSelect was setting python interpreter before someone added hooks to the codebase.

But the hook doesn't modify the system path as I understand it, so it's still necessary to modify the path (putting the venv first in the path) for the venv to be activated in any terminal windows opened from inside neovim.

That was my original thinking anyway. Maybe it's time to have a look at that code and remove anything it's doing that is now redundant....

And the commit was just trying to improve the default hook that was in the code base, but maybe I misunderstood what we discussed above?

[linux-cultist]

I removed the changed code now, so we can understand better what needs to be done.

[cmetz]

@linux-cultist the hook is fine, but i think the code got a little bit redundant on different places. may we could do a life coding session at some point to test and debug the code a little bit. i'm not very deep into lua and nvim. just startet a few days ago using neovim again.

[linux-cultist]

Yeah i think it probably is. I cant really do a live coding session since I work full time and have family etc, so its hard to plan in advance when i can spend time on this, and also I try to be a bit anonymous... but you seem pretty good at understanding the LSP, specially for someone who just came back to neovim.. :)

[cmetz]

ah that is fine, I'm in a same situation, maybe i can invest some time and create a pull request.

[linux-cultist]

I would love if you do. I think you already know more than me about the LSP and having another person go through the code will be very helpful I think. Just be aware that some parts may look a bit out of place - there has been many people contributing (which I actually like, but can lead to one person not knowing the entire code base). :)

[mennohofste]

I created a PR that essentially reimplements the commit shown above. The only difference is that it loops over all pyright clients instead of just changing the first one.

the hook is fine, but i think the code got a little bit redundant on different places.

As @cmetz said, the commit was good and solves the issue. The fact that a new autocommand is created every time you set a different python environment is a different issue. I think a new issue should be opened for that one.

The code change solves this issue, and does not make the other problem better or worse, they are not related. So I would argue to solve this bug first, and then to look at the duplication after.

[mennohofste]

I can also make this issue if you want. 👍

[linux-cultist]

I love any support you can offer. :)