Replies: 9 comments
-
|
See #1125. The file watcher itself is the hard part of this and @pascalkuthe is working on it as time allows. There is already #2653 which uses the |
Beta Was this translation helpful? Give feedback.
-
|
Oops. I swear I looked before opening this, but didn't think to look for PRs. And dismissed #1125 as not quite addressing this. So I'm fine with nixing this issue, if this is covered by #1125 and others. Didn't want to add to the clutter. I'll look into the patch, thanks for the pointer. :) |
Beta Was this translation helpful? Give feedback.
-
|
After sleeping on this another night, I have some more thoughts: I do consider this a bug, because Helix advertises a feature but implements it incompletely. Can I suggest locking the Looks like this is the only issue that stumbled over it, so maybe it's not that big of a deal. Just throwing it out there. |
Beta Was this translation helpful? Give feedback.
-
|
Is this a practical issue with a language server or theoretical? I don't know of any language servers that choose to start their own file watchers if the client doesn't advertise support. Unless the current implementation causes problems I don't see us disabling it or adding a way to configure out of it. |
Beta Was this translation helpful? Give feedback.
-
|
I don't know of any either, other than the one I'm currently writing (and I only thought of doing it this way after opening this issue). That's why I said "no big deal". If you're on this and it'll straighten itself out eventually, that's fine. But I still think my point stands in principle: The whole point of the capability negotiation is that both sides can adapt their behavior based on what is advertised. If the server subscribes to file updates but doesn't get the ones it asked for, then the server will not function correctly, through no fault of its own. OK, so I had a look at rust-analyzer, and it looks like it behaves that way as well: pub fn files(&self) -> FilesConfig {
FilesConfig {
watcher: match self.files_watcher() {
FilesWatcherDef::Client if self.did_change_watched_files_dynamic_registration() => {
FilesWatcher::Client
}
_ => FilesWatcher::Server,
},
exclude: self.files_excludeDirs().iter().map(|it| self.root_path.join(it)).collect(),
}
}And then in |
Beta Was this translation helpful? Give feedback.
-
|
Rust analyzer does not have its own file watcher so it just does not do file watching in case the client doesn't supply it (because there is no file watcher of sufficient quality available in rust as writing one is genuienly very hard) |
Beta Was this translation helpful? Give feedback.
-
It does:
So I don't think this problem is theoretical anymore. |
Beta Was this translation helpful? Give feedback.
-
|
You can manually set this config option to enable Server side file watching but it's certainly not something recommended and not enabled by default. The watcher in RA is most likely buggy and just a placeholder: What we have right now us a good intermediary solution, that I don't want to hide behind a feature gate. If you want to remove it then you can patch it out in your fork. The right solution is to build a proper file watcher into helix (for which we already have an issue) |
Beta Was this translation helpful? Give feedback.
-
|
I just use plain RA that I've also read that comment, and it doesn't say that it's a placeholder, it says that it's likely buggy and that's why they prefer to rely on the client by default. But if the client doesn't offer it (by inspecting the client capabilites), it does the watching itself. Anyway, I'll just special case misbehaving clients until they're known to work. That seems more scalable than asking users to compile their own patched editors. |
Beta Was this translation helpful? Give feedback.
-
Summary
It seems that Helix only notifies language servers about files that it has currently open.
From the Spec, the intention seems to be that the client should inform the server about all files matching the glob given during registration:
That's how I interpret the spec, anyway. Otherwise I'm not sure I see a difference between this and the normal document sync mechanisms.
If you agree that this is something that Helix should do, then I could offer to implement it. I'm going to have to add file watching one way or another, and I'd much rather have it happen on the client side (for the reasons given in the spec).
Reproduction Steps
I tried this:
workspace/didChangeWatchedFilesnotificationsAwithin HelixAoutside HelixBoutside HelixI expected this to happen:
didChangeWatchedFilesnotifications for all three changesInstead, this happened:
Helix log
I've attached the log entries sent from my experimental LSP. I don't expect you want to go to the trouble of installing it (it's in a very works-on-my-machine kind of state). If you really want to, I'll amend the report with instructions.
~/.cache/helix/helix.log
Init params sent by Helix:
Init result sent by LSP:
Registering for all
*.inkfiles, recursively:After writing
showcase.inkfrom Helix (corresponding to step 2. above):(unhandled means there's no code to do anything with it yet, but the point is: the language server is notified)
Writing
showcase.inkorshowcase2.inkfrom another editor (steps 3. and 4. above) don't generate any such notifications.Platform
Fedora Linux
Terminal Emulator
Konsole (KDE)
Installation Method
Fedora repos (
dnf install helix)Helix Version
helix 24.7 (079f544)
Beta Was this translation helpful? Give feedback.
All reactions