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

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

Closed
3 tasks done
mehalter opened this issue Jul 9, 2024 · 2 comments · Fixed by #75
Closed
3 tasks done

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

mehalter opened this issue Jul 9, 2024 · 2 comments · Fixed by #75
Labels
bug Something isn't working

Comments

@mehalter
Copy link
Contributor

mehalter commented Jul 9, 2024

Did you check docs and existing issues?

  • I have read all the neoconf.nvim docs
  • I have searched the existing issues of neoconf.nvim
  • I have searched the existing issues of plugins related to this issue

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,
})
@mehalter mehalter added the bug Something isn't working label Jul 9, 2024
@folke folke closed this as completed in 7137fde Jul 9, 2024
@folke
Copy link
Owner

folke commented Jul 9, 2024

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
Copy link
Contributor Author

mehalter commented Jul 9, 2024

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

folke pushed a commit that referenced this issue Jul 13, 2024
🤖 I have created a release *beep* *boop*
---


##
[1.3.1](v1.3.0...v1.3.1)
(2024-07-09)


### Bug Fixes

* **util:** if lcponfig doesnt return a root dir, then also dont use the
fallback. Fixes [#74](#74)
([7137fde](7137fde))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants