Skip to content

feat: fix file change handling and auto-detect file open on registrations#132

Open
defacedvr wants to merge 1 commit into
isaacphi:mainfrom
defacedvr:fix/lsp-behavior
Open

feat: fix file change handling and auto-detect file open on registrations#132
defacedvr wants to merge 1 commit into
isaacphi:mainfrom
defacedvr:fix/lsp-behavior

Conversation

@defacedvr
Copy link
Copy Markdown

  • For open files, send didChange + didSave instead of didChange alone.
    didSave may triggers expensive server-side work (type checking, diagnostics).
    Atomic writes appear as Created events, now treated same as Changed.
  • Closed files and deletes use workspace/didChangeWatchedFiles (as per LSP spec).
  • OpenFilesOnRegistration now defaults to false and is auto-detected from ServerInfo. Name in the initialize response instead of always true hardcode.
  • Add NotifySave to LSPClient interface and Client.

@defacedvr
Copy link
Copy Markdown
Author

Hello.
I've been using mcp-language-server with https://github.com/WhatsApp/erlang-language-platform and found a couple of issues related to file event handling:

  1. the server gets flooded with didOpen notifications on startup
  2. file change events don't correctly follow the lsp spec for documents that aren't open in the editor.

I've put together a fix - happy to open a PR if there's interest.
jfyi It's my first dive into lsp protocols and specs.

…tion

- For open files, send didChange + didSave instead of didChange alone.
  didSave triggers expensive server-side work (type checking, diagnostics).
  Atomic writes appear as Created events, now treated same as Changed.
- Closed files and deletes use workspace/didChangeWatchedFiles (as per LSP spec).
- OpenFilesOnRegistration now defaults to false and is auto-detected from
  ServerInfo.Name in the initialize response instead of always true hardcode.
- Add NotifySave to LSPClient interface and Client.
@defacedvr
Copy link
Copy Markdown
Author

Sorry for the noise. I had to force-push after accidentally including unrelated glob pattern changes from #99. Used them when testing with the Erlang language server (elp). It uses *.{e,h}rl glob patterns that weren't matching correctly without it.

@defacedvr
Copy link
Copy Markdown
Author

I have a question about the interaction between OpenFilesOnRegistration and the existing TypeScript-specific initialization in client.go#L220-L221.

It seems like for typescript-language-server, files are already opened during initialization via initializeTypescriptLanguageServer, which hardcodes .ts and .tsx extensions (typescript.go#L48) and detects the server by command path.

With OpenFilesOnRegistration set to true for typescript-language-server, files would be opened a second time when watchers are registered - as I think was already the case before this PR, since it was always true.

My intention was to detect the server via the LSP protocol itself (using ServerInfo.Name from the initialize response) rather than hardcoding the command path, and to rely on the glob patterns registered by the server via client/registerCapability instead of hardcoding file extensions. This way the watcher opens exactly the files the server asks to watch, without any server-specific knowledge baked into the client.

Should the existing initializeTypescriptLanguageServer mechanism be replaced with this approach, or kept as is? Happy to adjust the PR either way.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant