Skip to content
This repository has been archived by the owner on Jul 6, 2021. It is now read-only.

Support buffer-local show_sign and enable_virtual_text options #64

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

Support buffer-local show_sign and enable_virtual_text options #64

wants to merge 3 commits into from

Conversation

andrewferrier
Copy link

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
Copy link

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
Copy link
Author

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 #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
Copy link
Author

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
Copy link

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 👍

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
Copy link
Author

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

@mrossinek
Copy link

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 👍

@mrossinek
Copy link

@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.

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

Successfully merging this pull request may close these issues.

2 participants