Toggle source query

[lcrockett]

I've been trying to figure out how to toggle sources and tried to use buffer-scoped variables to achieve it in conjunction with a runtime_condition. See example below:

      runtime_condition = function()
        return vim.b.self_lsp_diagnostics_shellcheck_active == nil or vim.b.self_lsp_diagnostics_shellcheck_active == true
      end

This worked great, however I couldn't find a way to refresh diagnostics after changing the variable. Seemed to me I needed to send a textDocument/didChange to Neovim / LS somehow, but couldn't figure out how. So I was stuck making a change in the buffer for the runtime_condition to kick in. I could also solve it by using the command :edit, however that gave me a (harmless) error if the buffer is intentionally unsaved. Any tips or pointers on sending a textDocument/didChange or refreshing diagnostics on-demand are still welcome.

However, i've just stumbled upon the toggle function, just pushed a few hours ago. This means I can now simply toggle sources using lua require('null-ls').toggle('<sourcename>'), which is fantastic.

I've been trying to expand on this by using a query to also specify the method used. This way i'm able to distinguish between formatting and diagnostic sources that share the same name, for instance markdownlint. I've been trying to test the query first by using lua print(require('null-ls').is_registered({ name = 'lua-format', method = 'NULL_LS_FORMATTING' })) which returns false. However, when I use lua print(require('null-ls').is_registered('lua-format')) it returns true. Any pointers on using an appropriately formatted query ?

Cheers on your null-ls work, it superseded both efm-langserver and diagnosticls in my case and the on-demand functionality are USPs for my workflow.

[jose-elias-alvarez]

is_registered isn't supposed to take a query (though we can modify it to do so if you think it makes sense). You can use a query to target a specific source with enable, disable, or toggle or get access to the source object itself using get_source.

In the first implementation, disabling a source only hid its diagnostics (and regenerating them after re-enabling it would have required editing the buffer, as you noted). Sending synthetic notifications isn't that hard, but I stumbled on a pretty simple way to do this automatically and tried implementing it in 5ed9025. Since you are (to my knowledge) the first user of this API, let me know if this does what you expect and if you have any feedback.

[lcrockett]

Cheers on the is_registered change, that is much appreciated !

Regarding the error, I can reproduce it in the following way:

1. Open file with NVIM nvim filename
2. Once in NVIM, open an additional buffer (this is the critical step)
3. Go back to the buffer originally opened with NVIM
4. Introduce an error
5. Toggle the source (based on the state and these steps, the toggle would disable it)
6. Boom

It doesn't really matter which source I use, I can reproduce it with the above steps for sources shellcheck, vale, dotenv-linter.
I'm on the very latest master NVIM v0.6.0-dev+582-g87a053f12 @ Ubuntu 20.04 LTS, HEAD 7 commits prior to that produces the same results.

The weird thing is, the error is always referring to a buffer number that does not exist and it seems that the buffer number referenced is the first buffer number available. So if bufids 1,2,3,14 exist, it would reference bufid 4, and if bufids 1,2,3,4,5 exist, it would reference bufid 6.

[jose-elias-alvarez]

Still unable to replicate this, unfortunately (although I did see the error at least once or twice) but your description makes me pretty confident that this is an issue with the diagnostic API.

An easy solution would be to just wrap this line with pcall. If it's an upstream issue, I'd rather figure it out (or at least report it), but it would require us to put together a minimal reproduction with synthetic diagnostics and vim.diagnostic.set.

If this is a blocker for your workflow, let me know and I can just wrap it.

[lcrockett]

I agree, reproduction is essential. For now it might be practical to temporarily wrap aforementioned with pcall anyways, I might not be the only one trying out bleeding edge null-ls.

In the meantime i'll try to work out a few other ways to reproduce and for documentation sake i'll post a short video later depicting the issue.

[jose-elias-alvarez]

I followed the stack trace and found that the default signs handler checks that the buffer is loaded when showing diagnostics but not when hiding them. If this is the cause, I think the latest commits should help, since they'll prevent null-ls from setting diagnostics for unloaded buffers. Could you try updating and let me know?

[lcrockett]

Hmm, no luck yet, i'm on 82766cf52bd23988c0f7de093a058ac45b703069 and the error persists under the same circumstances. See video here below additionally. Also, i've found out that I don't have to switch back to the previous buffer to reproduce, I can simply start Neovim with a file, then open an additional buffer and use the toggle in that new additional buffer for the issue to pop up.

Screen.Recording.2021-11-16.at.18.22.38.mov

[jose-elias-alvarez]

Could you try a465bac just to rule out buffer validation completely?

If that's not it, I'm not sure what's going on or why I can't reproduce it locally even though I saw it at least once before. I'd need a true minimal reproduction (no other plugins, just null-ls and specific file(s)) to investigate this further, so feel free to open an issue if you're able to put that together.

[lcrockett]

Sorry, the issue still persists (with a465bac), this time however in a different function, nvim_buf_clear_namespace as opposed to sign_unplace. See trace below:

E5108: Error executing lua ...HEAD-87a053f_1/share/nvim/runtime/lua/vim/diagnostic.lua:968: Invalid buffer id: 3
stack traceback:
        [C]: in function 'nvim_buf_clear_namespace'
        ...HEAD-87a053f_1/share/nvim/runtime/lua/vim/diagnostic.lua:968: in function 'hide'
        ...HEAD-87a053f_1/share/nvim/runtime/lua/vim/diagnostic.lua:1054: in function 'hide'
        ...HEAD-87a053f_1/share/nvim/runtime/lua/vim/diagnostic.lua:1294: in function 'reset'
        ...ck/packer/start/null-ls.nvim/lua/null-ls/diagnostics.lua:30: in function 'hide_source_diagnostics'
        ...e/pack/packer/start/null-ls.nvim/lua/null-ls/sources.lua:107: in function 'cb'
        ...e/pack/packer/start/null-ls.nvim/lua/null-ls/sources.lua:28: in function 'for_each_matching'
        ...e/pack/packer/start/null-ls.nvim/lua/null-ls/sources.lua:104: in function 'toggle'
        [string ":lua"]:1: in main chunk
Press ENTER or type command to continue

It seems to me that the original problem persists; actions being triggered on a non-existent buffer.

In the meantime i'll see what I can do on trying to reproduce this with a minimal configuration.

[lcrockett]

I might have a simpler way to reproduce this:

1. Open a buffer directly with NVIM (only 1 is needed to trigger this issue)
2. Ensure it has a syntax error in order for diagnostics to be already shown
3. Run lua vim.diagnostic.open_float(0, {scope="buffer"})
4. Move a line up or down to exit the diagnostics float
5. Run lua require("null-ls").disable("markdownlint") (or whatever other source you're using, shellcheck works as well)
6. Boom

The reason i'm asking for 3 is that it looks like opening a float triggers the outcome of this issue. In my previous tests I was opening a second buffer via Telescope, which AFAIK opens it's picker in a float. In this case, you can simply trigger it by opening the NVIM LSP diagnostic float, which leads to the same behaviour. Let me know if that works for you.

[jose-elias-alvarez]

I hate to say it but I'm still not able to replicate the error. The fact that it's related to floating windows suggests that null-ls is setting diagnostics for these floating windows, which then triggers an error when they no longer exist – however, null-ls shouldn't attach to the buffers in these windows at all (and is not doing so on my end).

This could be a bad interaction with another plugin or a configuration option, so a full minimal reproduction case would be appreciated. Otherwise, I don't think there's more I can do.

[lcrockett]

Alright. Thank you for your time and patience so far. In other news, i've found the culprit; it's a bufferline plugin that renders LSP diagnostics based indicators (https://github.com/akinsho/bufferline.nvim). I'm using it in conjunction with the option diagnostics = 'nvim_lsp', this allows for LSP diagnostic indicators to appear in the bufferline.

I've been trying to replicate this behaviour without involving null-ls in order to prepare a reproducible issue:

• null-ls disable is triggered here
• null-ls hide_source_diagnostics is triggered here
  • If I understand correctly, it takes the null-ls LSP client ID to reset diagnostics based on the corresponding namespace ID number
• bufferline.nvim seems to be dealing with the LSP indicator here, however it's more difficult for me to follow this one

While writing this up i've actually found a way to replicate this solely in the bufferline.nvim plugin:

• lua vim.diagnostic.reset(vim.lsp.diagnostic.get_namespace(9)) where 9 is the LSP client ID of an LSP. It replicates with other LSPs, like sumneko_lua for instance, so this settles it from null-ls standpoint I guess.

Thanks again !

[jose-elias-alvarez]

Great, glad you were able to figure it out!