feat(core): workspace watcher #5200
Conversation
CodSpeed Performance ReportMerging #5200 will not alter performanceComparing Summary
|
| @@ -204,3 +193,23 @@ impl Projects { | |||
| !is_feature_included | |||
| } | |||
| } | |||
|
|
|||
| #[derive(Clone, Copy, Debug, Deserialize, Eq, Hash, PartialEq, Serialize)] | |||
There was a problem hiding this comment.
I moved this block because it was annoying that the definition was in the middle between the Projects definition and its implementation.
5ade72a to
864cb7e
Compare
Fixes + test Fix Windows tests (no need for snapshots)
e705705 to
0d64c48
Compare
There was a problem hiding this comment.
That's awesome! Thank you for implementing this 🥇
I left some questions because the PR description doesn't cover much. Also some here:
- how do we handle symbolic links?
- is there some documentation that we can provide to our maintainers on how to debug things? A file watcher is a hassle, so we should at least give as much information as possible to our maintainers (me included! 🤣 ) to how to debug things and test things
| @@ -555,7 +559,7 @@ pub struct OpenFileParams { | |||
| pub project_key: ProjectKey, | |||
| pub path: BiomePath, | |||
| pub content: FileContent, | |||
| pub version: i32, | |||
| pub version: Option<i32>, | |||
There was a problem hiding this comment.
What's the reason behind this change? Maybe we should document it, so we know when the version should be provided
There was a problem hiding this comment.
Ah, yes, I'll add a comment for that! Basically, it doesn't really make sense to provide except in the LSP use case where versions are explicitly specified. We used to provide 0 everywhere else anyway.
There was a problem hiding this comment.
I'm thinking now I'll just move the version into the FileContent enum, so you only can provide it when also explicitly providing the content.
| pub(super) fn update_service_data_with_added_or_updated_path( | ||
| &self, | ||
| path: &Utf8Path, | ||
| ) -> Result<(), WorkspaceError> { | ||
| let path = BiomePath::from(path); | ||
| if path.is_manifest() { | ||
| self.update_project_layout_for_added_or_changed_path(&path)?; | ||
| } | ||
|
|
||
| self.update_dependency_graph_for_paths(&[path], &[]); | ||
|
|
||
| Ok(()) | ||
| } | ||
|
|
||
| pub(super) fn update_service_data_with_removed_path( |
There was a problem hiding this comment.
I wonder if we can unify these functions by using an enum to distinguish when a file is added/updated/removed:
enum Signal<'a> {
AddOrUpdate(&'a Utf8Path),
Remove(&'a Utf8Path)
}There was a problem hiding this comment.
Added a WatcherSignalKind. Saves a bit of duplication indeed :)
Co-authored-by: Emanuele Stoppa <my.burning@gmail.com>
|
@ematipico I've added a Regarding symlinks, I wouldn't be surprised if there are some bugs lurking there, probably more related to the scanner in general, than the watcher specifically. I'm not even sure how to go about testing all such scenarios except to see what users report 😅 |
|
Yeah I suppose we will have to roll up our sleeves and start fixing cases reported by the users as we go :) |
84a8fee to
72b354a
Compare
72b354a to
cf82f51
Compare
Summary
🎉
(it keeps the workspace in the daemon in sync with the FS)
Test Plan
Test added.