kickstart.nvim icon indicating copy to clipboard operation
kickstart.nvim copied to clipboard

Consider adding diagnostics display toggle

Open aedificatio opened this issue 10 months ago • 8 comments

Suggestion

Consider adding a toggle for displaying diagnostics. Based on how many diagnostics there are you might want to toggle between:

  • no diagnostics
  • only signcolumn
  • virtual text and signcolumn

In my lspconfig I made a toggle that does just that! <leader>dt toggles between the various settings. The only 'clunky' bit is the function refresh_diagnostics_screen(). To trigger a screen refresh from the lsp without restarting the lsp, I trigger a InsertLeave event in the refresh_diagnostics_screen() function. Any suggestions how this could be done in a better way?

vim.g.diagnostics_state = 1

local function refresh_diagnostics_screen()
  -- to refresh lsp diagnostics on screen trigger an InsertLeave event
  -- else use: vim.cmd([[ LspRestart ]]) which is slower
  vim.api.nvim_buf_set_lines(0, 0, 0, false, { " " })
  vim.api.nvim_buf_set_lines(0, 0, 1, false, {})
  vim.diagnostic.show()
end

local function toggle_diagnostics()
  -- toggle vim.g.diagnostics_state between:
  -- 0 no diagnostics
  -- 1 only in signcolumn
  -- 2 virtualtext and signcolumn

  vim.g.diagnostics_state = (vim.g.diagnostics_state + 1) % 3

  if vim.g.diagnostics_state == 0 then
    vim.diagnostic.hide()
    print("Diagnostics: Hidden")
  elseif vim.g.diagnostics_state == 1 then
    vim.lsp.handlers["textDocument/publishDiagnostics"] =
      vim.lsp.with(vim.lsp.diagnostic.on_publish_diagnostics, {
        virtual_text = false,
      })
    refresh_diagnostics_screen()
    print("Diagnostics: Signs only")
  elseif vim.g.diagnostics_state == 2 then
    vim.lsp.handlers["textDocument/publishDiagnostics"] =
      vim.lsp.with(vim.lsp.diagnostic.on_publish_diagnostics, {
        virtual_text = {
	  source = "always",
	  prefix = "●",
	},
      })
    refresh_diagnostics_screen()
    print("Diagnostics: Virtual text and Signs")
  end
end

vim.keymap.set("n", "<leader>dt", toggle_diagnostics, { desc = "Diagnostics [T]oggle" })

aedificatio avatar Apr 05 '24 12:04 aedificatio

I would recommend using <leader>td. From my observation of various plugins <leader>t seems to be a well established leading key combo for [T]oggle of anything. It's also what gitsigns uses for toggles (the gitsigns keymaps were part of kickstart before but unfortunately they were removed in the latest rewrite). Since you have a solution already prepared you might as well just open a PR to let the maintainers decide if they want to accept it.

dam9000 avatar Apr 05 '24 13:04 dam9000

I would recommend using <leader>td. From my observation of various plugins <leader>t seems to be a well established leading key combo for [T]oggle of anything. It's also what gitsigns uses for toggles (the gitsigns keymaps were part of kickstart before but unfortunately they were removed in the latest rewrite). Since you have a solution already prepared you might as well just open a PR to let the maintainers decide if they want to accept it.

Thank you for your response. I hesitate to create a PR because of the 'clunky' refresh_diagnostics_screen() function. Any suggestions on how this could be improved?

aedificatio avatar Apr 05 '24 15:04 aedificatio

Hmm it is indeed a bit 'clunky' :) I don't know what would be the proper way to do that, perhaps someone else will come forward.

dam9000 avatar Apr 05 '24 16:04 dam9000

I'm not sure how to handle this either. @tjdevries - this might be a good one to swoop in and drop some advanced Neovim science :)

feoh avatar Apr 18 '24 01:04 feoh

Actually, would this toggle be better included in Telescope as it already offers various diagnostic display opitons?

feoh avatar Apr 18 '24 01:04 feoh

Hmm no idea how this would be integrated with telescope...

btw I tested this out and a side effect of the use of nvim_buf_set_lines() is that it marks the buffer as modified, so definitely not ready to be used as is.

dam9000 avatar Apr 18 '24 11:04 dam9000

How about starting with something simpler, just a toggle of diagnostics on/off:

vim.keymap.set('n', '<leader>td', function()
  if vim.diagnostic.is_disabled() then
    vim.diagnostic.enable()
  else
    vim.diagnostic.disable()
  end
end, { desc = '[T]oggle [d]iagnostics' })

dam9000 avatar Apr 18 '24 11:04 dam9000

btw I tested this out and a side effect of the use of nvim_buf_set_lines() is that it marks the buffer as modified, so definitely not ready to be used as is.

The nvim_buf_set_lines() is only used as a trigger to make nvim redraw the diagnostics screen immediately. If the nvim_buf_set_lines() are removed from the refresh_diagnostics_screen() function the code works fine. The screen would represent the new setting only after the next InsertLeave event, so it's not instant.

If the nvim_buf_set_lines() are replaced with vim.cmd([[ LspRestart ]]) there is no modified buffer, but it takes much longer to process because the LSP needs to restart. I think for now this would be the best solution. It would be nice to have something like :redrawstatusline but for the diagnostics.

local function refresh_diagnostics_screen()
  vim.cmd('LspRestart')
  vim.diagnostic.show()
end

aedificatio avatar Apr 18 '24 13:04 aedificatio

Please feel free to make this change in your personal fork. I'm personally happy to use the existing keymaps to show/hide the various types of diagnostics. I don't think we need to add more code to the main project for this.

Please feel free to re-open, or even better file a pull request if you disagree.

feoh avatar Apr 23 '24 15:04 feoh