Not sending textDocument/didSave notifications to all LSPs

[jpaju]

Summary

When using multiple LSPs, sometimes Helix does not send textDocument/didSave notifications to some LSPs. Other similar notifications, such as didOpen, didClose and didChange seem to be sent.

This is problematic with LSPs that only compile code and provide diagnostics on save (for example metals, LSP for Scala). One has to always manually trigger recompilation to get up-to-date diagnostics from metals after a modification. In practice I have to trigger compilation manually after every save 😳

I think this problem has only started during this spring, it's possibly connected to the event system that was added recently?

Reproduction Steps

I wasn't able to reproduce the issue deterministically. However, when using Helix normally this happens tens of times per day. It might be related to using metals for autoformatting, but I wasn't able to confirm this suspicion.

I expected this to happen/Instead, this happened:
Helix should send didSave notification to all LSPs, but it doesn't. The log below demonstrates that didSave is only sent to scls and not metals.

Here is relevant part of my languages.toml:

[language-server.scls]
command = "simple-completion-language-server"

[language-server.metals]
command = "metals"

[[language]]
name = "scala"
auto-format = true
language-servers = ["metals", "scls"]

Helix log

~/.cache/helix/helix.log

2024-04-04T12:02:26.172 helix_lsp::transport [INFO] metals <- {"capabilities":{"callHierarchyProvider":true,"codeActionProvider":{"codeActionKinds":["quickfix","refactor","source.organizeImports"]},"codeLensProvider":{"resolveProvider":false},"completionProvider":{"resolveProvider":true,"triggerCharacters":[".","*"]},"definitionProvider":true,"documentFormattingProvider":true,"documentHighlightProvider":true,"documentOnTypeFormattingProvider":{"firstTriggerCharacter":"\n","moreTriggerCharacter":["\""]},"documentRangeFormattingProvider":true,"documentSymbolProvider":true,"executeCommandProvider":{"commands":["analyze-stacktrace","zip-reports","list-build-targets","extract-method","debug-adapter-start","new-scala-file","build-connect","reset-workspace","doctor-run","insert-inferred-type","build-restart","discover-jvm-run-command","generate-bsp-config","build-disconnect","copy-worksheet-output","presentation-compiler-restart","browser-open-url:https://github.com/scalameta/metals-feature-requests/issues/new?template=feature-request.yml","reset-choice","goto","open-new-github-issue","new-scala-project","ammonite-stop","scalafix-run","build-import","inline-value","sources-scan","bsp-switch","new-java-file","reset-notifications","extract-member-definition","ammonite-start","compile-cancel","goto-super-method","goto-position","compile-cascade","convert-to-named-arguments","discover-tests","scala-cli-stop","super-method-hierarchy","scala-cli-start","file-decode","compile-clean","scalafix-run-only"]},"experimental":{"rangeHoverProvider":true},"foldingRangeProvider":true,"hoverProvider":true,"implementationProvider":true,"referencesProvider":true,"renameProvider":{"prepareProvider":true},"selectionRangeProvider":true,"semanticTokensProvider":{"full":true,"legend":{"tokenModifiers":["declaration","definition","readonly","static","deprecated","abstract","async","modification","documentation","defaultLibrary"],"tokenTypes":["namespace","type","class","enum","interface","struct","typeParameter","parameter","variable","property","enumMember","event","function","method","macro","keyword","modifier","comment","string","number","regexp","operator","decorator"]},"range":false},"signatureHelpProvider":{"triggerCharacters":["(","[",","]},"textDocumentSync":{"change":1,"openClose":true,"save":{"includeText":true}},"typeDefinitionProvider":true,"workspace":{"fileOperations":{"willRename":{"filters":[{"pattern":{"glob":"**/*.scala","matches":"file"}},{"pattern":{"glob":"**/","matches":"folder"}}]}},"workspaceFolders":{"changeNotifications":true,"supported":true}},"workspaceSymbolProvider":true},"serverInfo":{"name":"Metals","version":"1.2.2"}}
2024-04-04T12:08:32.610 helix_lsp::transport [INFO] scls -> {"jsonrpc":"2.0","method":"textDocument/didChange","params":{"contentChanges":[{"range":{"end":{"character":39,"line":70},"start":{"character":34,"line":70}},"text":""}],"textDocument":{"uri":"file://<path>","version":543}}}
2024-04-04T12:08:32.610 helix_lsp::transport [INFO] metals -> {"jsonrpc":"2.0","method":"textDocument/didChange","params":{"contentChanges":[{"text":"<content>"}],"textDocument":{"uri":"file://<path>","version":543}}}
2024-04-04T12:08:32.618 helix_lsp::transport [INFO] metals <- {"jsonrpc":"2.0","method":"textDocument/publishDiagnostics","params":{"uri":"file://<path>","diagnostics":[{"range":{"start":{"line":67,"character":6},"end":{"line":70,"character":34}},"severity":1,"source":"bloop","message":"<message>","data":{"actions":[]}},{"range":{"start":{"line":57,"character":10},"end":{"line":59,"character":63}},"severity":2,"source":"bloop","message":"dead code following this construct","data":{"actions":[]}}]}}
2024-04-04T12:08:32.623 helix_vcs::diff::worker [INFO] Diff computation timed out, update of diffs might appear delayed
2024-04-04T12:08:33.354 helix_lsp::transport [INFO] metals -> {"jsonrpc":"2.0","method":"textDocument/formatting","params":{"options":{"insertSpaces":true,"tabSize":2},"textDocument":{"uri":"file://<path>"}},"id":631}
2024-04-04T12:08:33.388 helix_lsp::transport [INFO] metals <- {"jsonrpc":"2.0","id":631,"result":[]}
2024-04-04T12:08:33.388 helix_lsp::transport [INFO] metals <- []
2024-04-04T12:08:33.390 helix_lsp::transport [INFO] scls -> {"jsonrpc":"2.0","method":"textDocument/didSave","params":{"textDocument":{"uri":"file://<path>"}}}
!!! NO didSave message sent to metals !!!
2024-04-04T12:12:38.312 helix_lsp::transport [INFO] metals -> {"jsonrpc":"2.0","method":"workspace/executeCommand","params":{"arguments":[],"command":"compile-cascade"},"id":671}

Platform

macOS

Terminal Emulator

wezterm 20240203-110809-5046fc22

Installation Method

flake

Helix Version

Helix 24.03 (bb0bfa2)

[the-mikedavis]

Could you give https://github.com/helix-editor/helix/tree/handle-partial-lsp-did-save-failure a try? If you can reproduce with a smaller log, it would be interesting to see a full / "from scratch" log.

This is most likely unrelated to the event system - document save notifications haven't been migrated to use events yet. The part of Document::save_impl that handles didSave hasn't been modified since #2507 actually so this is more likely a relatively old bug.

[jpaju]

Hey, thanks for the quick response again! 🙏 I used helix built from the branch for a full work day and it seems to fix the issue completely! Could these changes be merged to master anytime soon ?