From 7820fb38f439cf7eec94b668095ed32ae155c02f Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Tue, 12 Feb 2019 15:45:44 +0300 Subject: don't distinguish Create and Write events in VFS --- crates/ra_vfs/src/io.rs | 45 ++++++++++++++++++++++++++------------------- crates/ra_vfs/src/lib.rs | 35 ++++++++++++++++------------------- 2 files changed, 42 insertions(+), 38 deletions(-) diff --git a/crates/ra_vfs/src/io.rs b/crates/ra_vfs/src/io.rs index ff5ae3a19..dc0b84d5a 100644 --- a/crates/ra_vfs/src/io.rs +++ b/crates/ra_vfs/src/io.rs @@ -17,14 +17,28 @@ pub(crate) enum Task { AddRoot { root: VfsRoot, config: Arc }, } +/// `TaskResult` transfers files read on the IO thread to the VFS on the main +/// thread. #[derive(Debug)] pub enum TaskResult { + /// Emitted when we've recursively scanned a source root during the initial + /// load. BulkLoadRoot { root: VfsRoot, files: Vec<(RelativePathBuf, String)> }, - AddSingleFile { root: VfsRoot, path: RelativePathBuf, text: String }, - ChangeSingleFile { root: VfsRoot, path: RelativePathBuf, text: String }, - RemoveSingleFile { root: VfsRoot, path: RelativePathBuf }, + /// Emitted when we've noticed that a single file has changed. + /// + /// Note that this by design does not distinguish between + /// create/delete/write events, and instead specifies the *current* state of + /// the file. The idea is to guarantee that in the quiescent state the sum + /// of all results equals to the current state of the file system, while + /// allowing to skip intermediate events in non-quiescent states. + SingleFile { root: VfsRoot, path: RelativePathBuf, text: Option }, } +/// The kind of raw notification we've received from the notify library. +/// +/// Note that these are not necessary 100% precise (for example we might receive +/// `Create` instead of `Write`, see #734), but we try do distinguish `Create`s +/// to implement recursive watching of directories. #[derive(Debug)] enum ChangeKind { Create, @@ -45,7 +59,7 @@ impl Worker { // explained by the following concerns: // * we need to burn a thread translating from notify's mpsc to // crossbeam_channel. - // * we want to read all files from a single thread, to gurantee that + // * we want to read all files from a single thread, to guarantee that // we always get fresher versions and never go back in time. // * we want to tear down everything neatly during shutdown. let (worker, worker_handle) = thread_worker::spawn( @@ -63,7 +77,7 @@ impl Worker { let mut watcher = notify::watcher(notify_sender, WATCHER_DELAY) .map_err(|e| log::error!("failed to spawn notify {}", e)) .ok(); - // Start a silly thread to tranform between two channels + // Start a silly thread to transform between two channels let thread = thread::spawn(move || { notify_receiver .into_iter() @@ -98,7 +112,7 @@ impl Worker { } // Stopped the watcher drop(watcher.take()); - // Drain pending events: we are not inrerested in them anyways! + // Drain pending events: we are not interested in them anyways! watcher_receiver.into_iter().for_each(|_| ()); let res = thread.join(); @@ -199,23 +213,16 @@ fn handle_change( } paths .into_iter() - .filter_map(|rel_path| { + .try_for_each(|rel_path| { let abs_path = rel_path.to_path(&config.root); - let text = read_to_string(&abs_path)?; - Some((rel_path, text)) - }) - .try_for_each(|(path, text)| { - sender.send(TaskResult::AddSingleFile { root, path, text }) + let text = read_to_string(&abs_path); + sender.send(TaskResult::SingleFile { root, path: rel_path, text }) }) .unwrap() } - ChangeKind::Write => { - if let Some(text) = read_to_string(&path) { - sender.send(TaskResult::ChangeSingleFile { root, path: rel_path, text }).unwrap(); - } - } - ChangeKind::Remove => { - sender.send(TaskResult::RemoveSingleFile { root, path: rel_path }).unwrap() + ChangeKind::Write | ChangeKind::Remove => { + let text = read_to_string(&path); + sender.send(TaskResult::SingleFile { root, path: rel_path, text }).unwrap(); } } } diff --git a/crates/ra_vfs/src/lib.rs b/crates/ra_vfs/src/lib.rs index 3805be570..5d98d905c 100644 --- a/crates/ra_vfs/src/lib.rs +++ b/crates/ra_vfs/src/lib.rs @@ -37,8 +37,8 @@ impl_arena_id!(VfsRoot); /// Describes the contents of a single source root. /// -/// `RootConfig` can be thought of as a glob pattern like `src/**.rs` whihc -/// specifes the source root or as a function whihc takes a `PathBuf` and +/// `RootConfig` can be thought of as a glob pattern like `src/**.rs` which +/// specifies the source root or as a function which takes a `PathBuf` and /// returns `true` iff path belongs to the source root pub(crate) struct RootConfig { root: PathBuf, @@ -60,7 +60,7 @@ impl RootConfig { fn new(root: PathBuf, excluded_dirs: Vec) -> RootConfig { RootConfig { root, excluded_dirs } } - /// Cheks if root contains a path and returns a root-relative path. + /// Checks if root contains a path and returns a root-relative path. pub(crate) fn contains(&self, path: &Path) -> Option { // First, check excluded dirs if self.excluded_dirs.iter().any(|it| path.starts_with(it)) { @@ -210,7 +210,7 @@ impl Vfs { match task { TaskResult::BulkLoadRoot { root, files } => { let mut cur_files = Vec::new(); - // While we were scanning the root in the backgound, a file might have + // While we were scanning the root in the background, a file might have // been open in the editor, so we need to account for that. let exising = self.root2files[root] .iter() @@ -230,21 +230,18 @@ impl Vfs { let change = VfsChange::AddRoot { root, files: cur_files }; self.pending_changes.push(change); } - TaskResult::AddSingleFile { root, path, text } => { - if self.find_file(root, &path).is_none() { - self.do_add_file(root, path, text, false); - } - } - TaskResult::ChangeSingleFile { root, path, text } => { - if let Some(file) = self.find_file(root, &path) { - self.do_change_file(file, text, false); - } else { - self.do_add_file(root, path, text, false); - } - } - TaskResult::RemoveSingleFile { root, path } => { - if let Some(file) = self.find_file(root, &path) { - self.do_remove_file(root, path, file, false); + TaskResult::SingleFile { root, path, text } => { + match (self.find_file(root, &path), text) { + (Some(file), None) => { + self.do_remove_file(root, path, file, false); + } + (None, Some(text)) => { + self.do_add_file(root, path, text, false); + } + (Some(file), Some(text)) => { + self.do_change_file(file, text, false); + } + (None, None) => (), } } } -- cgit v1.2.3