feat: vim.diagnostics

[Conni2461]

fix #1378
close #1581

After 0.6 is a requirement.

Still WIP, requires more cleanup and maybe we can do something with namespace. So if we have multiple namespaces it might be useful to actually display that, maybe. or optional

[Conni2461]

the whole severity limit | bound thing seems to work so this is done

[Conni2461]

Telescope diagnostic_document or Telescope diagnostics_document. I think the first one is better because of vim.diagnostic.

@fdschmidt93 @clason

[clason]

I think the module is called diagnostic, but you are still looking at "diagnostics", plural.

I'd say consistency across telescope is more important here than consistency with the underlying API.

[Conni2461]

Yeah makes sense thanks :)

[fdschmidt93]

Why not document_diagnostics? Seems like the most natural name to me. diagnostic(s)_{document, workspace} seems a bit wordy to me.

That said, document and workspace stem from LSP lingo I suppose, but vim.diagnostic.get is no more akin to (all) buffers and namespaces. But not sure whether that warrants changing the naming.

But I don't care that much.

[Conni2461]

I think its also not natural to say. but i dont like document_diagnostics I think the module should be in front.

I also just noticed that its now just the buffers so we can also just have a Telescope diagnostics with a option for all, default is false. If true diagnostics from all open buffers ?

[clason]

Yes, diagnostics are global (by default). For a document_diagnostics, the telescope picker would need to explicitly restrict to the current buffer. In this case, I'd just opt for diagnostics.

(Related: get takes a namespace; it would be neat if Telescope diagnostics <namespace> worked. Need not be this PR, of course.)

[Conni2461]

I've put in a namespace option: https://github.com/nvim-telescope/telescope.nvim/pull/1553/files#diff-79c55de55b65fbaa2b214c1f3d28277a87a5b6ac9e5e0c87168a3341a471e5d4R1487-R1488 so Telescope diagnostics namespace=7 should work.

[Conni2461]

maybe it would even be better with a bufnr option. Like bufnr = nil|0 means current buffer bufnr = 7 means from buffer 7 and bufnr == all means all ? Thats more flexible than all and almost as easy

[clason]

Should have actually looked at the PR before opinionating :D

maybe it would even be better with a bufnr option. Like bufnr = nil|0 means current buffer bufnr = 7 means from buffer 7 and bufnr == all means all ? Thats more flexible than all and almost as easy

It's probably best to stick with the diagnostic convention here:

Use 0 for current buffer or nil for all buffers

(You can add a document_diagnostics command as a shortcut for bufnr=0. I'd omit the workspace, though, and just have diagnostics for the global command. There can still be an lsp_workspace_diagnostic that just is a shortcut for the global command with the proper namespace passed.)

[Conni2461]

The thing is people will call Telescope diagnostics which will result in bufnr being nil. Depending on what we want we could flip this behavior.

Edit: I am not sure which default would be better

[clason]

Yes, I'm saying this should be default (Telescope diagnostics gets all diagnostics; add modifiers to filter it down to buffer or namespace; add convenience commands on top of that for prefiltered searches if that is wanted).

If you want to be really fancy, you can add :<foo>: parsing to filter ;)

[Conni2461]

Okay changelog is here: https://github.com/nvim-telescope/telescope.nvim/pull/1553/files#diff-16e85f7f2f133a7dfecf66783549b68319150bcd7b077190d22a3adb16629819

I think diagnostics for all open buffers and diagnostics bufnr=0 for current document works good. I dont think we need any shortcuts. For now lsp_document_diagnostics and lsp_workspace_diagnostics still exist and work as shortcuts as intended but they will log errors because i wanna remove them. Because like tj said here: #1250 (comment) it has nothing to do with lsp_* anymore :)

As always Thanks for your help :)

[clason]

Makes sense! And you're right, not much need for convenience commands in Telescope itself since users can just create their own bespoke mappings.

[fdschmidt93]

As a side note, I'm afraid README needs to be updated for this PR 😅

[Conni2461]

rm -rf all pickers from the readme 😭