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: set buftype before filetype #67

Merged
merged 1 commit into from Jul 22, 2021
Merged

fix: set buftype before filetype #67

merged 1 commit into from Jul 22, 2021

Conversation

jose-elias-alvarez
Copy link
Contributor

@jose-elias-alvarez jose-elias-alvarez commented Jul 22, 2021

Hey! This is fundamentally an issue with null-ls, since (as far as I know) we are the only LSP that will attempt to attach to any buffer, regardless of filetype, but it should (hopefully) be a simple fix.

Steps to reproduce:

  1. Enable a null-ls source that has filetypes set to { "*" }, such as the new misspell source.
  2. Open a file and create a spelling error that misspell will catch, e.g. "langauge".
  3. Open a Trouble buffer and watch it go into an infinite loop of linting itself.

I was surprised by this, since :set buftype? in the Trouble buffer correctly shows nofile, and that should mean that nvim-lspconfig's try_add won't attach.

I think the issue is that Trouble sets filetype before buftype, so when the FileType event is triggered and null-ls calls try_add on the Trouble buffer, it goes through. This PR simply switches the order of the two lines, which aligns with lspconfig's expectations and fixes the issue.

Let me know if there was a reason for the original order or if there's something I'm overlooking by attempting to fix it in this way. (I think we also have to be a bit more aggressive about filtering out non-file buffers on the null-ls side, but either way we'll have to check buftype, so the issue would remain.)

@folke
Copy link
Owner

folke commented Jul 22, 2021

I actually noticed that too and have fixed it on my local machine. Apparently I didnt push it yet 🙂

Will merge yours since I'm leaving on a vacation right now

Thanks!

@folke folke merged commit 169b2ec into folke:main Jul 22, 2021
@jose-elias-alvarez
Copy link
Contributor Author

jose-elias-alvarez commented Jul 22, 2021 via email

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.

None yet

2 participants