From 23613a9de71758553876dd7e7149de04b026dae5 Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Thu, 21 Mar 2024 20:34:55 +0100 Subject: [PATCH] fix: Some file watching related vfs fixes --- .../src/handlers/notification.rs | 2 +- crates/vfs-notify/src/lib.rs | 2 +- crates/vfs/src/lib.rs | 52 +++++++++++++++---- crates/vfs/src/loader.rs | 2 +- 4 files changed, 46 insertions(+), 12 deletions(-) diff --git a/crates/rust-analyzer/src/handlers/notification.rs b/crates/rust-analyzer/src/handlers/notification.rs index ff213748b4f..b5c4a4f435e 100644 --- a/crates/rust-analyzer/src/handlers/notification.rs +++ b/crates/rust-analyzer/src/handlers/notification.rs @@ -241,7 +241,7 @@ pub(crate) fn handle_did_change_watched_files( state: &mut GlobalState, params: DidChangeWatchedFilesParams, ) -> anyhow::Result<()> { - for change in params.changes { + for change in params.changes.iter().unique_by(|&it| &it.uri) { if let Ok(path) = from_proto::abs_path(&change.uri) { state.loader.handle.invalidate(path); } diff --git a/crates/vfs-notify/src/lib.rs b/crates/vfs-notify/src/lib.rs index 94fd6cb78c9..1f25b0e5346 100644 --- a/crates/vfs-notify/src/lib.rs +++ b/crates/vfs-notify/src/lib.rs @@ -136,7 +136,7 @@ fn run(mut self, inbox: Receiver) { Message::Invalidate(path) => { let contents = read(path.as_path()); let files = vec![(path, contents)]; - self.send(loader::Message::Loaded { files }); + self.send(loader::Message::Changed { files }); } }, Event::NotifyEvent(event) => { diff --git a/crates/vfs/src/lib.rs b/crates/vfs/src/lib.rs index 9d723f8143b..ddd6bbe3f7a 100644 --- a/crates/vfs/src/lib.rs +++ b/crates/vfs/src/lib.rs @@ -91,12 +91,21 @@ impl nohash_hasher::IsEnabled for FileId {} pub struct Vfs { interner: PathInterner, data: Vec, + // FIXME: This should be a HashMap + // right now we do a nasty deduplication in GlobalState::process_changes that would be a lot + // easier to handle here on insertion. changes: Vec, + // The above FIXME would then also get rid of this probably + created_this_cycle: Vec, } -#[derive(Copy, PartialEq, PartialOrd, Clone)] +#[derive(Copy, Clone, Debug, PartialEq, PartialOrd)] pub enum FileState { + /// The file has been created this cycle. + Created, + /// The file exists. Exists, + /// The file is deleted. Deleted, } @@ -121,6 +130,11 @@ pub fn is_created_or_deleted(&self) -> bool { matches!(self.change, Change::Create(_) | Change::Delete) } + /// Returns `true` if the change is [`Create`](ChangeKind::Create). + pub fn is_created(&self) -> bool { + matches!(self.change, Change::Create(_)) + } + /// Returns `true` if the change is [`Modify`](ChangeKind::Modify). pub fn is_modified(&self) -> bool { matches!(self.change, Change::Modify(_)) @@ -160,7 +174,9 @@ pub enum ChangeKind { impl Vfs { /// Id of the given path if it exists in the `Vfs` and is not deleted. pub fn file_id(&self, path: &VfsPath) -> Option { - self.interner.get(path).filter(|&it| matches!(self.get(it), FileState::Exists)) + self.interner + .get(path) + .filter(|&it| matches!(self.get(it), FileState::Exists | FileState::Created)) } /// File path corresponding to the given `file_id`. @@ -178,7 +194,9 @@ pub fn file_path(&self, file_id: FileId) -> &VfsPath { pub fn iter(&self) -> impl Iterator + '_ { (0..self.data.len()) .map(|it| FileId(it as u32)) - .filter(move |&file_id| matches!(self.get(file_id), FileState::Exists)) + .filter(move |&file_id| { + matches!(self.get(file_id), FileState::Exists | FileState::Created) + }) .map(move |file_id| { let path = self.interner.lookup(file_id); (file_id, path) @@ -193,27 +211,43 @@ pub fn iter(&self) -> impl Iterator + '_ { /// [`FileId`] for it. pub fn set_file_contents(&mut self, path: VfsPath, contents: Option>) -> bool { let file_id = self.alloc_file_id(path); - let change_kind = match (self.get(file_id), contents) { + let state = self.get(file_id); + let change_kind = match (state, contents) { (FileState::Deleted, None) => return false, (FileState::Deleted, Some(v)) => Change::Create(v), - (FileState::Exists, None) => Change::Delete, - (FileState::Exists, Some(v)) => Change::Modify(v), + (FileState::Exists | FileState::Created, None) => Change::Delete, + (FileState::Exists | FileState::Created, Some(v)) => Change::Modify(v), + }; + self.data[file_id.0 as usize] = match change_kind { + Change::Create(_) => { + self.created_this_cycle.push(file_id); + FileState::Created + } + // If the file got created this cycle, make sure we keep it that way even + // if a modify comes in + Change::Modify(_) if matches!(state, FileState::Created) => FileState::Created, + Change::Modify(_) => FileState::Exists, + Change::Delete => FileState::Deleted, }; let changed_file = ChangedFile { file_id, change: change_kind }; - self.data[file_id.0 as usize] = - if changed_file.exists() { FileState::Exists } else { FileState::Deleted }; self.changes.push(changed_file); true } /// Drain and returns all the changes in the `Vfs`. pub fn take_changes(&mut self) -> Vec { + for file_id in self.created_this_cycle.drain(..) { + if self.data[file_id.0 as usize] == FileState::Created { + // downgrade the file from `Created` to `Exists` as the cycle is done + self.data[file_id.0 as usize] = FileState::Exists; + } + } mem::take(&mut self.changes) } /// Provides a panic-less way to verify file_id validity. pub fn exists(&self, file_id: FileId) -> bool { - matches!(self.get(file_id), FileState::Exists) + matches!(self.get(file_id), FileState::Exists | FileState::Created) } /// Returns the id associated with `path` diff --git a/crates/vfs/src/loader.rs b/crates/vfs/src/loader.rs index c3d1ff7271a..3af91b1af81 100644 --- a/crates/vfs/src/loader.rs +++ b/crates/vfs/src/loader.rs @@ -58,7 +58,7 @@ pub enum Message { /// The [`Config`] version. config_version: u32, }, - /// The handle loaded the following files' content. + /// The handle loaded the following files' content for the first time. Loaded { files: Vec<(AbsPathBuf, Option>)> }, /// The handle loaded the following files' content. Changed { files: Vec<(AbsPathBuf, Option>)> },