x/tools/gopls: make eglot+gopls awesome

[adonovan]

@stapelberg and I plan to work together to make Emacs+eglot smoother, faster, and better documented.

Some specific items (feel free to add or suggest more):

• Code Lenses: support them, or, better, make gopls move all those features to Code Actions, which are better anyway.
• Add a nice "run this test" UI (see x/tools/gopls: better "run this test" UX #67400 and feature: eglot-execute to optionally redirect executeCommand progress messages to a compilation-mode buffer joaotavora/eglot#1402)
  • Approach 1: output certain executeCommands to a compilation-mode buffer. Needs no LSP changes initially, just a list of command IDs to treat this way; ultimately the Command would provide this information. Prototype implemented.
  • Approach 2: express it entirely within the WorkDoneProgress mechanism: change the server to indicate that a given progress stream should be directed to a buffer, and formalize the final success/failure protocol.
• support didChangeWatchedFiles for unopened files on macOS (see x/tools/gopls: unable to organize imports after go mod tidy / go get #62330)
  • This Emacs bug report explains that the underlying cause is a limitation of using kqueue on macOS: https://debbugs.gnu.org/cgi/bugreport.cgi?bug=51146 — possibly contributing fsevents support to Emacs’s filenotify package would fix the problem.
• fix didChangeWatchedFiles to work recursively? The underlying Emacs filenotify package does not watch directories recursively, but gopls’s glob pattern requests recursive matching.
  • didChangeWatchedFiles does not work recursively, LSP server goes out of sync for new directories joaotavora/eglot#1411
• update go-mode.el with various nice short go-* helper commands for features otherwise accessible though more cumbersome eglot commands (e.g. go-doc, go-test, go-freesymbols, etc); see add go-* shortcuts for various hard-to-find eglot operations dominikh/go-mode.el#436
  • mailed Add go-{browse-{freesymbols,doc,assembly},rename} dominikh/go-mode.el#439
• Expose textDocument/selectionRange as an interactive command joaotavora/eglot#1220 (comment)
  • Awaiting approval to merge the new code.
• Fix bug in (global-set-key (kbd "s-<mouse-1>") #'xref-find-definitions-at-mouse).
  • Already fixed in Emacs 30; see https://lists.gnu.org/archive/html/bug-gnu-emacs/2024-06/msg00550.html
• Ensure the gopls emacs documentation is accurate, and add useful gopls-specific tips and tricks.
• use LSP for organize-imports + formatting, not goimports command
  • eglot-format-and-organize-imports: format a file and organize its imports joaotavora/eglot#1409

[findleyr]

From #62330: one thing to look at would be watched file support (didChangeWatchedFiles).

[danp]

Re didChangeWatchedFiles, at least for macOS there's the possibly related emacs-lsp/lsp-mode#3296 (comment) for lsp-mode. Looks like eglot does use file-notify-add-watch like lsp-mode, which on macOS can't detect changes to existing files when the path given to file-notify-add-watch is a directory.

[jidicula]

I'm so glad to see this area getting some investment 😁. lsp-mode is nice, but the eglot experience works much better with TRAMP (especially in concert with https://github.com/patrickt/codespaces.el) and GitHub Codespaces.

[stapelberg]

Regarding the didChangeWatchedFiles issues, I have updated the first comment with a summary of what I understand is the problem.

Specifically, I can reproduce issue #62330 on macOS, but not on Linux.

On Linux, the (inotify-based) file content watching works and if I update go.mod’s contents (e.g. with echo "// test" >> go.mod), I do see an event being sent to gopls from Eglot in the *EGLOT…* buffer. Be careful when testing: touch go.mod is not sufficient, because Eglot is watching for file content changes, not file attribute changes.

On macOS, indeed I get no event at all. AFAIK, Emacs does its own file watching (falling back to polling, even) for opened files, which is why having a file open papers over the problem. I linked Emacs bug#51146, which I’ll summarize as: This is an inherent limitation of kqueue-based watching. Adding fsevents support (a macOS-specific file watching API) to Emacs would probably be the best way to fix this bug.

---

With that said, there is another problem regarding didChangeWatchedFiles, even on Linux, which is that Emacs’s filenotify package does not work recursively. This means that creating extra.go next to go.mod results in an event, but creating a new directory (mkdir internal) and creating a file within that directory (e.g. internal/extra.go) does not result in an event.

To better understand what’s happening, I instrumented Emacs like so:

(defun my-file-notify-add-watch-logger (file event &rest args)
  "log a message whenever file-notify-add-watch is called"
  (message "file-notify-add-watch called with file: %s, event: %s" file event))

(advice-add 'file-notify-add-watch :before #'my-file-notify-add-watch-logger)

When opening hello1.go, I see that Eglot expands the glob pattern on all known project files:

[eglot] Connected! Server `gopls' now managing `go-mode' buffers in project `hello1'.
[eglot] Server reports (type=4): Loading packages...
[eglot] Server reports (type=3): Finished loading packages.
file-notify-add-watch called with file: /home/stapelberg/repro-emacs/hello1/, event: (change)
file-notify-add-watch called with file: /home/stapelberg/repro-emacs/hello1/internal/foo/, event: (change)
file-notify-add-watch called with file: /home/stapelberg/repro-emacs/hello1/internal/, event: (change)

Creating a new directory does not result in a new expansion of the glob pattern, so the new directory is unwatched.

---

I have filed a bug report about this with Eglot, but given the rather dire state of client-side watching, are we sure that we don’t want to reconsider server-side watching? :)

[findleyr]

I have joaotavora/eglot#1411, but given the rather dire state of client-side watching, are we sure that we don’t want to reconsider server-side watching? :)

Two years ago I would have responded with a philosophical argument that clients should be responsible for all state changes, etc etc, including the arguments here.

However, after years of struggling with client-side watching, I am at the point where the only barrier to switching to server-side watching is momentum; we have other high priorities, and file watching sort of works in most clients.

Perhaps I'm being overly pessimistic, though. In theory, it shouldn't be that hard to link a cross platform file watching library, and provide a gopls option to use server-side file watching. It may be worth an experiment.

[stapelberg]

:)

I had a quick look and it seems like https://github.com/fsnotify/fsnotify is the most popular Go package for file watching. It does support recursive watching since last month (see fsnotify/fsnotify@a618f07), but not on macOS (it also does not support the fsevents API on macOS).

So I suppose by using the fsnotify package as of today, we would fix file watching on macOS in general, and recursive file watching on Linux, but not on macOS.

[findleyr]

You inspired me to file #67995, dumping my thoughts on this issue.

If you are interested, feel free to give that a try. I think the three bullets listed there are the basic code pointers necessary, though I'm happy to provide more guidance. This is a relatively self-contained experiment, and doesn't require deep gopls knowledge.