diagnostics not clearing due to bad behavior with version synchronization

[reillylm]

I'm using Vim 9 + COC as my LSP client, and I noticed that, if we're in a sketch ino file, and we fix all the diagnostics in the file, the last one doesn't clear out.

Looks like, when we get a textDocument/publishDiagnostics back from clangd, who we then transform and send back to the IDE (as part of INOLanguageServer.publishDiagnosticsNotifFromClangd), we're sending the document version as clangd sees it (the sketchMapper.CppText.Version) rather than the sketch's document version on the IDE side (COC's view of the document).

arduino-language-server/ls/ls_clang_to_ide.go

Lines 155 to 159 in a8a3511

	allIdeDiagsParams[ideURI] = &lsp.PublishDiagnosticsParams{
	URI: ideURI,
	Version: clangDiagsParams.Version,
	Diagnostics: []lsp.Diagnostic{},
	}

Since the clangd-side version advances more quickly than the IDE side (and gets kept "open," even if we close and reopen the ino file in the IDE), the versions get out of sync.

Here's the COC logic for handling a publishDiagnostics from the server:
https://github.com/neoclide/coc.nvim/blob/d7a56ab95bc18b1c01cd2a3e1e2c9636bad03552/src/language-client/client.ts#L1756
It matches up the uri with the currently-opened document in the IDE. Then, if the version from the notification doesn't match the IDE's document version, it throws away the notification.

However, this behavior is actually only happening when we want to clear diagnostics. When we get a notification from clangd with diagnostics, and send it back to the IDE, we actually omit the version altogether:

arduino-language-server/ls/ls.go

Lines 1254 to 1257 in a8a3511

	allIdeParams[ideInoURI] = &lsp.PublishDiagnosticsParams{
	URI: ideInoURI,
	Diagnostics: []lsp.Diagnostic{},
	}

This is why we can correctly see diagnostics appear in COC, but not clear. When the version isn't included, it just looks like we're an LSP server who doesn't send the version with diagnostics (it's optional per the spec), so COC just takes the most recent notification. However, when the version is included, it's the incorrect version, so the COC logic throws away the notification, and we never get our diagnostics cleared.

The full fix for this would probably be to pull back the proper IDE version (available in INOLanguageServer.trackedIdeDocs) and include it in all the publishDiagnostics we send to the IDE.

However, I think a better workaround would be to just always exclude the version. I poked around and AFAICT there would be no impact. The two major LSP clients are probably VSCode and Neovim. Looks like neither of them inspect the version from publishDiagnostics: VSCode and Neovim. Then, in Arduino IDE2, we have a config property realTimeDiagnostics (which hooks up to DisableRealTimeDiagnostics in the LSP server), which is disabled by default; however, even if it was enabled, Monaco has only just recently implemented an LSP client, and their diagnostics handling also doesn't look at the version. So I think it would be a safe change to make. If we think that's the right way to go, I can open a PR.

[reillylm]

Sorry for the spam, was trying to get the change onto my fork