gopls: add a new "subdirWatchPatterns" setting
As discovered in golang/go#60089, file watching patterns behave very
differently in different clients. We avoided a bad client-side bug in VS
Code by splitting our subdirectory watch pattern, but this appears to be
very expensive in other clients (notably coc.nvim, or any vim client
that uses watchman).
The subdirectory watch patterns were only known to be necessary for VS
Code, due to microsoft/vscode#109754. Other clients work as expected
when we watch e.g. **/*.go. For that reason, let's revert all other
clients to just use simple watch patterns, and only specialize to have
subdirectory watch patterns for VS Code.
It's truly unfortunate to have to specialize in this way. To paper over
this hole in the wall, add an internal setting that allows clients to
configure this behavior explicitly. The new "subdirWatchPatterns"
setting may accepts the following values:
- "on": request watch patterns for each subdirectory (as before)
- "off": do not request subdirectory watch patterns
- "auto": same as "on" for VS Code, "off" for all others, based on the
provided initializeParams.clientInfo.Name.
Includes some minor cleanup for the fake editor, and fixes some stale
comments.
Updates golang/go#golang/go#60089
Fixes golang/go#59635
Change-Id: I1eab5c08790bd86a5910657169edcb20511c0280
Reviewed-on: https://go-review.googlesource.com/c/tools/+/496835
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Alan Donovan <adonovan@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
diff --git a/gopls/internal/lsp/cache/snapshot.go b/gopls/internal/lsp/cache/snapshot.go
index 948912a..de9524b 100644
--- a/gopls/internal/lsp/cache/snapshot.go
+++ b/gopls/internal/lsp/cache/snapshot.go
@@ -936,30 +936,56 @@
patterns[fmt.Sprintf("%s/**/*.{%s}", dirName, extensions)] = struct{}{}
}
- // Some clients (e.g. VSCode) do not send notifications for
- // changes to directories that contain Go code (golang/go#42348).
- // To handle this, explicitly watch all of the directories in
- // the workspace. We find them by adding the directories of
- // every file in the snapshot's workspace directories.
- // There may be thousands of patterns, each a single directory.
- //
- // (A previous iteration created a single glob pattern holding a
- // union of all the directories, but this was found to cause
- // VSCode to get stuck for several minutes after a buffer was
- // saved twice in a workspace that had >8000 watched directories.)
- //
- // Some clients (notably coc.nvim, which uses watchman for
- // globs) perform poorly with a large list of individual
- // directories, though they work fine with one large
- // comma-separated element. Sadly no one size fits all, so we
- // may have to resort to sniffing the client to determine the
- // best behavior, though that would set a poor precedent.
- // TODO(adonovan): improve the nvim situation.
- s.addKnownSubdirs(patterns, dirs)
+ if s.watchSubdirs() {
+ // Some clients (e.g. VS Code) do not send notifications for changes to
+ // directories that contain Go code (golang/go#42348). To handle this,
+ // explicitly watch all of the directories in the workspace. We find them
+ // by adding the directories of every file in the snapshot's workspace
+ // directories. There may be thousands of patterns, each a single
+ // directory.
+ //
+ // (A previous iteration created a single glob pattern holding a union of
+ // all the directories, but this was found to cause VS Code to get stuck
+ // for several minutes after a buffer was saved twice in a workspace that
+ // had >8000 watched directories.)
+ //
+ // Some clients (notably coc.nvim, which uses watchman for globs) perform
+ // poorly with a large list of individual directories.
+ s.addKnownSubdirs(patterns, dirs)
+ }
return patterns
}
+// watchSubdirs reports whether gopls should request separate file watchers for
+// each relevant subdirectory. This is necessary only for clients (namely VS
+// Code) that do not send notifications for individual files in a directory
+// when the entire directory is deleted.
+func (s *snapshot) watchSubdirs() bool {
+ opts := s.view.Options()
+ switch p := opts.SubdirWatchPatterns; p {
+ case source.SubdirWatchPatternsOn:
+ return true
+ case source.SubdirWatchPatternsOff:
+ return false
+ case source.SubdirWatchPatternsAuto:
+ // See the documentation of InternalOptions.SubdirWatchPatterns for an
+ // explanation of why VS Code gets a different default value here.
+ //
+ // Unfortunately, there is no authoritative list of client names, nor any
+ // requirements that client names do not change. We should update the VS
+ // Code extension to set a default value of "subdirWatchPatterns" to "on",
+ // so that this workaround is only temporary.
+ if opts.ClientInfo != nil && opts.ClientInfo.Name == "Visual Studio Code" {
+ return true
+ }
+ return false
+ default:
+ bug.Reportf("invalid subdirWatchPatterns: %q", p)
+ return false
+ }
+}
+
func (s *snapshot) addKnownSubdirs(patterns map[string]struct{}, wsDirs []span.URI) {
s.mu.Lock()
defer s.mu.Unlock()
