linux-cultist / venv-selector.nvim

Allows selection of python virtual environment from within neovim
MIT License
379 stars 40 forks source link

Explore duplicate Python path change #81

Closed mennohofste closed 6 months ago

mennohofste commented 7 months ago

The Python path is changed twice, once by venv.set_pythonpath, and once with the hooks. The set_pythonpath creates an autocommand that changes the config every time a file is loaded. This is spin-off issue of #58.

Hooks: https://github.com/linux-cultist/venv-selector.nvim/blob/main/lua/venv-selector/hooks.lua venv.set_pythonpath: https://github.com/linux-cultist/venv-selector.nvim/blob/main/lua/venv-selector/venv.lua#L186C1-L213C1

Hooks:

function M.pyright_hook(_, venv_python)
  local clients = vim.lsp.get_active_clients({ name = "pyright" })
  for _, client in ipairs(clients) do
    client.config.settings = vim.tbl_deep_extend("force", client.config.settings, { python = { pythonPath = venv_python } })
    client.notify("workspace/didChangeConfiguration", { settings = nil })
  end
end

venv.set_pythonpath:

-- Hook into lspconfig so we can set the python to use.
function M.set_pythonpath(python_path)
    vim.api.nvim_create_autocmd({ "BufReadPost" }, {
        pattern = { "*.py" },
        callback = function()
            local active_clients = {}
            for _, client in
                ipairs(vim.lsp.get_active_clients({ name = "pyright", bufnr = vim.api.nvim_get_current_buf() }))
            do
                table.insert(active_clients, client)
            end

            for _, client in
                ipairs(vim.lsp.get_active_clients({ name = "pylance", bufnr = vim.api.nvim_get_current_buf() }))
            do
                table.insert(active_clients, client)
            end

            for _, client in ipairs(active_clients) do
                client.config.settings =
                    vim.tbl_deep_extend("force", client.config.settings, { python = { pythonPath = python_path } })

                client.notify("workspace/didChangeConfiguration", { settings = nil })
            end
        end,
    })
end

An autocommand runs every time the event is triggered. BufReadPost is ran when starting to edit a new buffer, after reading the file into the buffer, processing modelines. I do not understand why this is here, as pyright should stay active with the last set settings, so resetting every time a new buffer opens seems redundant. It could be overriding other plugins changing the python path as @cmetz wrote.

Even then, only one autocommand is necessary creating a new one every time the venv changes. Maybe there is a way to change an existing autocommand, if it is necessary.

I will experiment a bit to try and understand this better. Any hints or feedback is welcome.

mennohofste commented 7 months ago

I think #58 can be closed now. 😄

mennohofste commented 7 months ago

I removed theM.set_pythonpath function locally and could not find any difference in behaviour. From reading the function that would make sense, as its purpose is superceded by hooks.

Could someone else test this as well? Maybe there is some edge case I did not think of. Otherwise, I can submit a PR to remove the duplicate code.

neolooong commented 7 months ago

Found the problem a few months ago. And, I also thought M.set_pythonpath is redundant. So I attempted to remove M.set_pythonpath at that time, no issue occurred too.

linux-cultist commented 7 months ago

Thanks guys. Sounds good to make a PR to remove it.

I guess it's just a leftover and I admit I haven't put time into looking over all the code in a while now, so it's great that you guys are doing it. :)