folke / neoconf.nvim

💼 Neovim plugin to manage global and project-local settings
Apache License 2.0
728 stars 30 forks source link

bug: `root_dir` broken, always gets set when `.neoconf.json` is present #74

Closed mehalter closed 3 months ago

mehalter commented 3 months ago

Did you check docs and existing issues?

Neovim version (nvim -v)

NVIM v0.10.0 (github release)

Operating system/version

Arch Linux

Describe the bug

I noticed recently some of my language servers that shouldn't be starting are starting and not respecting the root_dir that they have defined either the defaults provided by lspconfig nor the ones I specify in my own configuration. This lead me down one crazy rabbit hole of source code digging until I found that Neoconf is actually overriding root_dir function in a way that makes it always set the root directory if a neoconf.json file is set. This really messes up language servers that require configuration files to function and so the root_dir is quite strict to only enable when those configuration files are present.

Steps To Reproduce

Requires: lua language server in path, I picked one I figured you probably have, feel free to adjust the repro.lua accordingly

  1. rm -rf .repro, make sure it's a clean environment
  2. nvim -u repro.lua repro.lua, run neovim and open repro.lua
  3. :LspInfo, see that 0 clients are attached to the buffer
  4. quit neovim
  5. touch .neoconf.json, make an empty Neoconf configuration file
  6. nvim -u repro.lua repro.lua, run neovim and open repro.lua
  7. :LspInfo, see that 1 client is attached to the buffer

Expected Behavior

I would expect that my language servers respect the root_dir that are set for them. A lot of language servers have blanket root directories such as .git, but this is not all of them. Many language servers require the presence of configuration files and therefore having a blanket root directory is bad. Also, even if there is a default blanket type of root directory, if I manually set root directory configuration, it should be respected fully. It seems like with the current implementation in Neoconf, there is no way to remove behavior at all.

Repro

vim.env.LAZY_STDPATH = ".repro"
load(vim.fn.system("curl -s https://raw.githubusercontent.com/folke/lazy.nvim/main/bootstrap.lua"))()

require("lazy.minit").repro({
  spec = {
    "neovim/nvim-lspconfig",
    "folke/neoconf.nvim",
  },
})

require("neoconf").setup({})

-- set up a language server with no single file support
-- and a `root_dir` function that would never activate it
require("lspconfig").lua_ls.setup({
  root_dir = function()
    return nil -- effectively _never_ set a root directory
  end,
  single_file_support = false,
})
folke commented 3 months ago

I personally don't use neoconf anymore, but the only recent change I made related to what you describe should now be reverted in that new commit.

Can you confirm this is now working as expected again?

mehalter commented 3 months ago

Yup that seems to work again! Thanks so much @folke !