nvim-lua / diagnostic-nvim

A wrapper for neovim built in LSP diagnosis config
Apache License 2.0
219 stars 15 forks source link

Support buffer-local show_sign and enable_virtual_text options #64

Open andrewferrier opened 4 years ago

andrewferrier commented 4 years ago

This pull request may not be merged exactly as-is. But I'm including it here to show a principle and hopefully start a debate.

One of the things I like to/want to do is enable and disable the LSP support on a per-buffer basis ("this virtual text is annoying, hide it!"). Currently that's not really practical. What this pull request does is introduce two-buffer local variables, vim.b.diagnostic_enable_virtual_text and vim.b.diagnostic_show_sign, which can be turned on or off per-buffer. This enables me to create a toggle function (not included in this pull request, as it sits outside the plugin) which turns them on or off.

Is this the kind of thing which you'd be open to see being included in diagnostic-nvim? Currently one big limitation I see with LSP support in Nvim is it's kinda 'all or nothing' on a per-filetype basis - you can't temporarily disable it for one buffer. This would allow for that (and other patterns).

mrossinek commented 4 years ago

I like the idea which this is going in as it appears to be (at least partially) a solution to #60. May I ask why you decided not to include the toggle command as part of this PR? I.e. why would you keep that outside of the plugin?

andrewferrier commented 4 years ago

In this case that was partly because I wanted a local configuration that handled this with minimal changes to diagnostic-nvim, so I could keep my fork of it up-to-date; I wasn't sure if you'd see that as within the scope of this plugin! I wasn't aware of https://github.com/nvim-lua/diagnostic-nvim/issues/60 either.

If you'd be interested I can work on a more complete pull request that includes a Toggle command like what #60 is asking for; I already have that working locally, more or less...

andrewferrier commented 4 years ago

FWIW, whilst we wait for a reply from one of the plugin maintainers, here's (roughly speaking) the toggle command I have elsewhere.... just add keybindings as appropriate.

function LspHide()
    if #vim.lsp.buf_get_clients() > 0 then
        vim.b.lsp_enable = 0
        vim.b.diagnostic_enable_virtual_text = 0
        vim.b.diagnostic_show_sign = 0
        vim.lsp.util.buf_clear_diagnostics(vim.api.nvim_win_get_buf(0))
        diagnostic.on_BufEnter()
        echomsg("LSP Disabled.")
    else
        echomsg('LSP not enabled in this buffer.')
    end
end

function LspShow()
    if #vim.lsp.buf_get_clients() > 0 then
        vim.b.lsp_enable = 1
        vim.b.diagnostic_enable_virtual_text = 1
        vim.b.diagnostic_show_sign = 1
        diagnostic.on_BufEnter()
        echomsg('LSP Enabled.')
    else
        echoerr('LSP not enabled in this buffer.')
    end
end

function M.LspSwap()
    if vim.b.lsp_enable == 0 then
        LspShow()
    else
        LspHide()
    end
end
mrossinek commented 4 years ago

I would definitely be interested in that, but before putting more work on it you should probably also wait for the input from one of the maintainers of this plugin. I also think the minimal example above is a good starting point :+1:

One question about the lsp_enable buffer variable: is that a thing elsewhere? I didn't find it in the docs. Or do you use this in your personal setup for some other logic like e.g. additional logic that depends on the toggled diagnostics state?

andrewferrier commented 4 years ago

@mrossinek no, it's something I invented for this change. It seemed like a logical name.

mrossinek commented 4 years ago

Okay thanks for clarifying! Because I was looking for something like that to use inside my CursorHold autocommand which triggers a popup for the diagnostics on the current line. With a variable like that, the toggle command will also toggle those popups :+1:

mrossinek commented 3 years ago

@andrewferrier A quick update: I checked with @tjdevries who is in the progress of merging diagnostics-nvim into Neovim (see #73 and neovim/neovim#12655). He also included an option to achieve what we have discussed here. He outlined it to some degree during his live stream at the time of writing. I might go back to the video later and add a link to the corresponding timestamp here.