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

fix(fennel-ls): use cwd when git repo parent is not found #3389

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

justiceenunciate
Copy link
Contributor

Context

For the fennel-ls lspconfig, I recently changed how the fennel-ls configuration file is found, i.e. nearest config -> git root.

However, when there is no fennel-ls configuration available, and all parent directories of the current buffer do contain git repositories, then the LSP won't be loaded at all. This PR fixes that.

Changes

  • Use cwd for fennel-ls when git repository parent is unavailable

Questions

  1. Can I write a test so this doesn't fail again? If so, where should that live?
  2. If root_dir is nil, should the LSP fail to load? I'm not sure if there are legitimate cases to ever send nil to an LSP but I thought I'd ask if a wider, structural solution is appropriate.

Thanks for your help!

Previously when using `fennel-ls`, the LSP would fail to load if no parent was a
Git repository and if a relative `flsproject.fnl` was not found.

Now when using `fennel-ls`, the `root_dir` configuration option is set to the
current working directory as a fallback when all other possible directories
are not available.
@@ -8,7 +8,7 @@ return {
local has_fls_project_cfg = function(path)
return util.path.is_file(vim.fs.joinpath(path, 'flsproject.fnl'))
end
return util.search_ancestors(dir, has_fls_project_cfg) or vim.fs.root(0, '.git')
return util.search_ancestors(dir, has_fls_project_cfg) or vim.fs.root(0, '.git') or vim.fn.getcwd()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use vim.uv.cwd() would be better.

@justinmk
Copy link
Member

Isn't this something that could be done by default? But IIRC that was discussed and there were reasons against it. I can't find the discussion though. Why is this reasonable for e.g. fennel and pyright, but not others?

@mfussenegger
Copy link
Member

Isn't this something that could be done by default?

At least from core perspective nil is legitimate for servers who support single-file's without project setup.
Not every server does - I think that's why lspconfig introduced a dedicated flag to indicate if they do.

And afaik a cwd as fallback was mostly removed because it led to users accidentally having their $HOME indexed by a language server.

@justinmk
Copy link
Member

Thanks for those reminders!

cwd as fallback was mostly removed because it led to users accidentally having their $HOME indexed by a language server.

Right, and then this PR reintroduces that issue...

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

Successfully merging this pull request may close these issues.

4 participants