I think we can ignore any of the directories that match these cases: https://github.com/golang/tools/blob/ccabf82fa1aef57afc4686de4b7881718f7b7d27/internal/lsp/cache/view.go#L662.

Ignoring something like node_modules is more difficult, and likely not doable if you open a module that contains a node_modules directory.

I think we can ignore any of the directories that match these cases

When you say "we", do you mean gopls? If so, that's too late because the client will already have setup watchers on those directories, and that's the cost I'm raising here.

Ignoring something like node_modules is more difficult, and likely not doable if you open a module that contains a node_modules directory.

As I explained in the original description, the advice with node_modules directories is to place a go.mod file in that directory (rather than special case ignore it), hence pruning it from the watch space.

I think we can ignore any of the directories that match these cases

When you say "we", do you mean gopls? If so, that's too late because the client will already have setup watchers on those directories, and that's the cost I'm raising here.

Yep, sorry for not being clearer - I linked that function to show the different cases that we know the Go command ignores and that we could also ignore in the GlobPattern.

Ignoring something like node_modules is more difficult, and likely not doable if you open a module that contains a node_modules directory.

As I explained in the original description, the advice with node_modules directories is to place a go.mod file in that directory (rather than special case ignore it), hence pruning it from the watch space.

Yep--I thought you might be asking for an alternative method of ignoring it.

@myitcv: Would you be willing to contribute a CL for this? We're happy to do it, but it likely won't be prioritized for a while. The file watching glob patterns can be found here, and the cases that should be ignored are here.

@stamblerre I'm very unlikely to get to this for some time unfortunately. So please feel free to offer to anyone else who might be interested in taking this on. Thanks for checking in any case.

Hello, I am interested in taking on this issue.

I took the time to look around and run some tests on my own, and it appears that the glob patterns are not flexible enough to match recursively while excluding the pattern the go command excludes (_, . and testdata) (1).

AFAIK, the remaining approach I see is to watch every folder individually (like it's currently done to handle the deletion events on certain clients) with a less restrictive glob, allowing to match subfolder creation in order to update the watches accordingly.

Because we're manually iterating through all known folders of the workspace, we can reject the ones that need to be ignored. Here is a snippet to illustrate this explanation:

(context: https://github.com/golang/tools/blob/master/internal/lsp/cache/snapshot.go#L674)

var dirNames []string
for uri := range s.allKnownSubdirs(ctx) {
	// only watch it if it's not to be ignored, starts with _, . or is named testdata. 
	if !s.IgnoredFile(uri) { 
		dirNames = append(dirNames, uri.Filename())
	} 
}

// yes, we're watching a/b/* because we want subfolders creation events to refresh the watcher. 
patterns[fmt.Sprintf("{%s}{,/*}", strings.Join(dirNames, ","))] = struct{}{}

Ending the glob with a wildcard (a/b/*) means that gopls will receive unwanted filesystem events. We'd have to discard them obviously (the handler seems the right place to do so).

@stamblerre, Is that the kind of solution you had in mind back then? I am unsure about this trade-off, reducing the cost on the client-side with narrower globs but add unnecessary notifications on the server-side.

I don't want to create unnecessary noise, so if that's ok I can go on and submit a CL!

@jhchabran: Thank you so much for doing this detailed investigation! Really appreciate that you've done a deep dive here. I think you are right that we don't want to begin watching everything with the * pattern--that would be too expensive if users have monorepos with lots of different types of files.

And in fact, I don't think it should be necessary to switch to the * pattern instead of the *.{go,mod,sum} pattern--we don't actually need to worry about creation events for new folders (and we currently don't). We should only need to care when a new Go file is created in that subfolder, and at that point, we can update the glob patterns to add that folder (if it's not ignored). Does that make sense? Please let me know if I am missing something.

Thanks for your feedback 🙏

TL;DR: I found out that I missed an aspect of gopls behavior in my tests which prevented me to see why your above suggestion works. Still, there's a slight behavior change, but that may be inconsequential. I kept my original explanation because it showcases the subtle change in question.

➡️ With that in mind, and assuming that it's ok to not have gopls be aware of new files in a newly created package until there's a reference toward it (1) then the costly ** can be removed and we can drop the ignored filenames along the way.

Please tell me if I missed anything!

(1) typing the package name followed by a dot in VScode is enough to achieve that

I'd be interested to test any glob changes with emacs and lsp-mode/eglot as I'm able. Both have had some trouble in the past (see joaotavora/eglot#602 and emacs-lsp/lsp-mode#1979).

@jhchabran: Thank you for continuing to test out these complicated cases! I didn't realize that those glob patterns wouldn't get file creation events for subdirectories like the /home/me/code/path1/bar/foo.go case you described. That would be a regression I think--gopls needs to get that file creation event even if the file is never opened.

It's starting to seem like it would be unnecessarily complicated to get this working, and I bet that other language servers have run into this too. I filed an issue with VS Code to ask how to ignore certain directories in a Glob Pattern: microsoft/language-server-protocol#1210. Let's wait for a response there before moving forward here. Sorry that this issue has turned out to be so complicated! If you'd like to find another issue to work on, that's totally fine and encouraged :)

Ok, it totally makes sense! I'll wait for some updates on this and will have a look at the other issues in the meantime then 😊👍

@stamblerre No worries, the issue being more complicated than it looked was part of the fun for me, I'm glad that I may have helped to move further on this topic by digging into it.

I'm quite curious to see what will they answer on the issue in the eventuality of not having missed anything in what can be done with the glob patterns or elsewhere. I believe the topic had been previously discussed over here microsoft/language-server-protocol#354.