Skip to content

fix: current buffer for lsp.start #506

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

powerman
Copy link
Contributor

@powerman powerman commented Jul 4, 2025

Quote from vim.lsp.start() docs:

To ensure a language server is only started for languages it can handle,
make sure to call |vim.lsp.start()| within a |FileType| autocmd.

Using vim.schedule_wrap() in FileType autocmd result in calling buf_attach() (and thus should_attach() and vim.lsp.start()) always on current buffer instead of buffer triggered FileType autocmd.

I've also removed unconditional call to buf_attach() at the end of setup() because it looks like an incomplete support for lazy loading (a complete support should take in account there may be more than one buffer at this moment).

This can be fixed in a different way (by remembering bufnr in FileType autocmd and then using vim.schedule to run buf_attach() with given bufnr), but this is much more intrusive change because it will need the whole chain of involved functions to accept and use extra param bufnr instead of working with a current buffer. Also adding support for bufnr param might result in possibility to re-enable lazy loading support. It may worth to implement later - please consider this PR a hotfix for security issue (prevent attaching to buffers with sensitive files).

Fixes #504

@AntoineGS
Copy link
Collaborator

Thanks for the other PRs!

While I see the issues with the existing code, the proposed change would be a breaking change and require configuration changes.
We should avoid this unless absolutely necessary, which leaves your second option, to properly implement lazy loading.

@powerman
Copy link
Contributor Author

powerman commented Jul 8, 2025

I agree. But at a glance such change looks too invasive and probably not suitable for an outside contributor. So, we probably have no choice than wait for you to implement this.

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.

should_attach is often called with wrong buffer id/name
2 participants