Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

x/tools/gopls: experiment with an option/capability for server-side file watching #67995

Open
findleyr opened this issue Jun 14, 2024 · 2 comments
Labels
gopls Issues related to the Go language server, gopls. Tools This label describes issues relating to any tools in the x/tools repository.
Milestone

Comments

@findleyr
Copy link
Contributor

findleyr commented Jun 14, 2024

Philosophically, file watching should be done by the client. In addition to the arguments here, client-side file watching means that the client is responsible for all state transitions on the server, which means the client owns the session lifecycle. This seems like a good idea.

Except that it doesn't work well. Not all clients support file watching, and those that do have spotty and inconsistent support. Sometimes, a file watching pattern that is performant for one client is prohibitively expensive on another, so much so that this is the only place in gopls where we have been forced to specialize behavior based on the client's user agent.

Recently, the cross platform fsnotify package gained support for recursive directory watching (on all platforms except macOS; thanks to @stapelberg for pointing this out). We've also heard from our friends maintaining the Dart LSP that server-side watching has worked well for them.

We should experiment with server-side file watching. In theory, it shouldn't be that hard to wire in:

  • Add a new internal gopls setting to control the experiment.
  • Hook into server.updateWatchedDirectories to manage glob patterns. I think it's sufficient to check server.Options inside updateWatchedDirectories, and perform state transitions based on the file watching setting in the previous bullet. (register/unregister client side watching as necessary, and manage the lifecycle of the server-side watcher).
  • Call server.didModifyFiles on file notifications. See server.DidChangeWatchedFiles. In theory, we could even call DidChangeWatchedFiles on change events, but that is a bit inaccurate as file watcher events are not LSP events.

Once that is done, we should perform a bake off with VS Code's client-side watching. For example: perform a branch switch, and see whether server-side watching or client-side watching gets it right. Also check the CPU cost of file watching.

One possibility is that neither server-side nor client-side watching gets it right. This could be due to gopls bugs around large numbers of simultaneous state changes, or it could be due to bugs in file watching. With server-side watching, we actually have an opportunity to get it right. For example, we could add heuristics by which we validate state with additional polling, thereby enforcing eventual correctness.

CC @adonovan

@findleyr findleyr added this to the gopls/v0.17.0 milestone Jun 14, 2024
@gopherbot gopherbot added Tools This label describes issues relating to any tools in the x/tools repository. gopls Issues related to the Go language server, gopls. labels Jun 14, 2024
@gabyhelp
Copy link

Similar Issues

(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.)

@rsc
Copy link
Contributor

rsc commented Jun 14, 2024

This would be really excellent for the long-tail editors. I added an easy "Restart" command to acme-lsp so I can reboot gopls after I've done things like branch switches, since it was too much work to change acme (a C program) to add all the necessary file watching.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
gopls Issues related to the Go language server, gopls. Tools This label describes issues relating to any tools in the x/tools repository.
Projects
None yet
Development

No branches or pull requests

4 participants