File watcher implementation & LSP DidChangeWatchedFiles notification

[kyrime]

Closes #2479 and is a prerequisite for #1125. Also probably supersedes #588.

I don't think this is quite ready to merge but it's definitely due for some reviewing. Functionality works fine for closing the intended issue but I've got concerns when it comes to other issues that depend on file watching.

notify is the main dependency added. I've reused globset as a dependency to parse the LSP spec's glob patterns, it's already included through ignore.

There are a couple different choices with regard to what directories to set notify watching:

1. Recursively watch the current working directory.
2. Set watches only on the specific files that are of interest.
3. Same as (1) but mix in non-recursive watching to allow for ignoring certain directories.

The issue with (1) is that there's no way to outright ignore directories: it seems likely that the amount of file changes rust-analyzer generates in the target directory on every write through cargo check could cause stuttering on lower-specced machines. The issue with (2) is that it can't detect new files as it's not watching for them, and the LSP may be interested in events from files that aren't open in the editor. So I've gone with (3) here: non-recursively watching the root directory, ignoring those matching some default list of build/cache directories ala Rust's target directory, and recursively watching the rest. This approach is taken from emacs-lsp, which has a seemingly pretty comprehensive list of ignored directories. The downside is that this adds some complexity with keeping track of directory creations/deletions/renames in the root directory, but it's not bad.

File watching works as follows: Watcher sets up notify watches on open LSP workspaces. Interested parties register a callback to be called when the watcher receives events matching a path filter in a specific workspace.

Some considerations:

• DidChangeWatchedFiles can only be registered dynamically, so supporting it requires handling RegisterCapability and UnregisterCapability. I haven't yet added support for unregistering the DidChangeWatchedFiles capability as I'm not sure where to store the required state: it seems to belong with the LSP client, but mutating the client seems to require interior mutability right now which gives me pause.
• Helix supports having multiple LSPs in use which all may have their own root workspace folders, so I've attempted to handle the case of multiple workspaces in the file watcher. I'm not really sure if that was the right move.
• Something weird here is that you might expect the watcher to default to watching everything from the current working directory down, but right now it just works with LSP workspaces. The current model also might cause issues with something like automatic reload when file changes externally #1125 where any file could be open in the editor, even outside any workspace or the working directory, so seems like that would require mixing in some functionality to facilitate choice (2) above. Any thoughts would be welcome.
• I haven't wired up LSP clients to add/remove their workspaces to/from the watcher's list of workspaces when they're created or dropped yet, mostly because I'm not really sure if I'm taking the right approach with how workspaces are handled right now.
• The only build directory I have ignored right now is target. Happy to add others from emacs-lsp's list in if maintainers are fine with this approach. Something nagging me is that since these directories are per-language it would also make sense to me if these were configured per-language, rather than one big list.
• I've used the pre-release version of notify which has been around for a while and seems pretty stable. It doesn't support debouncing yet but it seems like it's not too far off either. I haven't added debouncing to this PR yet either, it's possible it's best to just leave it as is and wait until notify's debounce support hits if the lack of debouncing isn't causing any issues.

That's all for now off the top of my head, but there's probably other stuff lurking around in the PR.

[Philipp-M]

... ignoring those matching some default list of build/cache directories ala Rust's target directory

I think we can just use the .gitignore (or other .ignore files) to filter relevant files/folders.

They are used anyway already for the file-picker etc.

I think there's rarely a case where no git or something a like is used...

[kyrime]

I'm inclined to agree that .gitignore is the better approach now that I've thought about it some more. Originally I imagined that could lead to some surprising behaviour in edge cases depending on what the user's chosen to add to their .gitignore, but thinking about it now I can't really imagine many sane scenarios where something bad could happen.

For Rust and other languages that use such an integrated project/build system it should work well in the vast majority of cases, since of course cargo new automatically sets up a repo and gitignores target. Other languages I'm not so sure, but we can hope that the amount of cases where people would have an extremely active build directory that isn't present in .gitignore would be comparatively low.

This would also eliminate some edge cases: like if a Rust project were configured to use a different name than target for its build directory.

[loewenheim]

I think this should say "unregister"?

Suggested change

	"attempt to register unsupported capability"
	"attempt to unregister unsupported capability"

[loewenheim]

Even leaving aside moving the glob logic to the LSP side, a filter closure subsumes it anyway, right?

[archseer]

@kyrime Sorry for the delay on this! I've been meaning to review this for a while.

The issue with (1) is that there's no way to outright ignore directories: it seems likely that the amount of file changes rust-analyzer generates in the target directory on every write through cargo check could cause stuttering on lower-specced machines.

Hmm isn't this how watchman would handle this internally? A recursive watch, but a way to drop unwanted notifications very early to avoid sending it through the notification system.

lsp-mode seems to also do recursive watching: https://github.com/emacs-lsp/lsp-mode/blob/8091f5d13efbde5bfabc774970dd689487053333/lsp-mode.el#L1871-L1910

I'd start with a simple recursive watch and implement something more complicated only if it becomes necessary.

it seems to belong with the LSP client, but mutating the client seems to require interior mutability right now which gives me pause.

We could use an ArcSwap on the server capabilities, similar to how we do the editor config. That way it stays immutable but we can swap it out with another updated capability struct.

Something weird here is that you might expect the watcher to default to watching everything from the current working directory down, but right now it just works with LSP workspaces.

We should watch from the root directory down, then if a filepath is also inside the LSP workspace, we can also trigger a LSP notification.

Happy to add others from emacs-lsp's list in if maintainers are fine with this approach. Something nagging me is that since these directories are per-language it would also make sense to me if these were configured per-language, rather than one big list.

I think we could use a global ignore list. The problem with per-language setting is that in a monorepo we might have a mix of different languages. Just because I'm editing a rust file doesn't mean there isn't a node_modules present in the same tree.

[krobelus]

The issue with (1) is that there's no way to outright ignore directories
Happy to add others from emacs-lsp's list in if maintainers are fine with this approach. Something nagging me is that since these directories are per-language it would also make sense to me if these were configured per-language, rather than one big list.
I think we could use a global ignore list. The problem with per-language setting is that in a monorepo we might have a mix of different languages. Just because I'm editing a rust file doesn't mean there isn't a node_modules present in the same tree.

yeah this is annoying. I wish there was an editor-independent (in LSP or outside) way of marking paths as "not containing sources". Would be nice to piggy-back on gitignore but that's not exactly the same thing.

I'd start with a simple recursive watch and implement something more complicated only if it becomes necessary.

I agree. This is also what kak-lsp does because I couldn't reproduce performance problems.
I still feel iffy about watching everything but maybe the OS-imposed limits take care of that

[violin0622]

Any updates recently?

[yesudeep]

This feature is the only one preventing me from switching over to helix completely. I love helix, but being out of sync with the state of the file on the disk is a little jarring sometimes. What needs to be done for this to work?

[krobelus]

On a quick glance this seems reasonable apart from the needlessly-complex recursive watch logic.
It's probably appropriate to use notify-debouncer-full.

You should know that kakoune-lsp recently disabled the file watcher by default because it caused very high latency for pyright-langserver startup in a repo with many files.
(Didn't root cause it yet. Very odd because the file watcher runs in its own thread.)
I'm happy to test your implementation for this issue.

[glehmann]

part of the work done here has already been merged with #7665