From 76bf7498aa88c4de4517f4eb1218807fdfc7071b Mon Sep 17 00:00:00 2001 From: Bernardo Date: Sat, 12 Jan 2019 18:17:52 +0100 Subject: [PATCH] handle watched events filtering in `Vfs`add `is_overlayed`load changed files contents in `io` --- Cargo.lock | 2 +- crates/ra_vfs/src/io.rs | 74 +++++++++------ crates/ra_vfs/src/lib.rs | 171 ++++++++++++++++++++++++++--------- crates/ra_vfs/src/watcher.rs | 85 +++++++---------- crates/ra_vfs/tests/vfs.rs | 35 +++---- 5 files changed, 221 insertions(+), 146 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index a694c2d9166..b8c840c6857 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1009,7 +1009,7 @@ version = "0.1.0" dependencies = [ "crossbeam-channel 0.3.6 (registry+https://github.com/rust-lang/crates.io-index)", "drop_bomb 0.1.4 (registry+https://github.com/rust-lang/crates.io-index)", - "flexi_logger 0.10.3 (registry+https://github.com/rust-lang/crates.io-index)", + "flexi_logger 0.10.4 (registry+https://github.com/rust-lang/crates.io-index)", "log 0.4.6 (registry+https://github.com/rust-lang/crates.io-index)", "notify 4.0.7 (registry+https://github.com/rust-lang/crates.io-index)", "ra_arena 0.1.0", diff --git a/crates/ra_vfs/src/io.rs b/crates/ra_vfs/src/io.rs index 79dea5dc725..a0c87fb258b 100644 --- a/crates/ra_vfs/src/io.rs +++ b/crates/ra_vfs/src/io.rs @@ -1,14 +1,13 @@ use std::{ - fmt, - fs, + fmt, fs, path::{Path, PathBuf}, }; -use walkdir::{DirEntry, WalkDir}; -use thread_worker::{WorkerHandle}; use relative_path::RelativePathBuf; +use thread_worker::WorkerHandle; +use walkdir::{DirEntry, WalkDir}; -use crate::{VfsRoot, has_rs_extension}; +use crate::{has_rs_extension, watcher::WatcherChange, VfsRoot}; pub(crate) enum Task { AddRoot { @@ -16,7 +15,7 @@ pub(crate) enum Task { path: PathBuf, filter: Box bool + Send>, }, - WatcherChange(crate::watcher::WatcherChange), + LoadChange(crate::watcher::WatcherChange), } #[derive(Debug)] @@ -26,29 +25,16 @@ pub struct AddRootResult { } #[derive(Debug)] -pub enum WatcherChangeResult { - Create { - path: PathBuf, - text: String, - }, - Write { - path: PathBuf, - text: String, - }, - Remove { - path: PathBuf, - }, - // can this be replaced and use Remove and Create instead? - Rename { - src: PathBuf, - dst: PathBuf, - text: String, - }, +pub enum WatcherChangeData { + Create { path: PathBuf, text: String }, + Write { path: PathBuf, text: String }, + Remove { path: PathBuf }, } pub enum TaskResult { AddRoot(AddRootResult), - WatcherChange(WatcherChangeResult), + HandleChange(WatcherChange), + LoadChange(Option), } impl fmt::Debug for TaskResult { @@ -77,9 +63,10 @@ fn handle_task(task: Task) -> TaskResult { log::debug!("... loaded {}", path.as_path().display()); TaskResult::AddRoot(AddRootResult { root, files }) } - Task::WatcherChange(change) => { - // TODO - unimplemented!() + Task::LoadChange(change) => { + log::debug!("loading {:?} ...", change); + let data = load_change(change); + TaskResult::LoadChange(data) } } } @@ -113,3 +100,34 @@ fn load_root(root: &Path, filter: &dyn Fn(&DirEntry) -> bool) -> Vec<(RelativePa } res } + +fn load_change(change: WatcherChange) -> Option { + let data = match change { + WatcherChange::Create(path) => { + let text = match fs::read_to_string(&path) { + Ok(text) => text, + Err(e) => { + log::warn!("watcher error: {}", e); + return None; + } + }; + WatcherChangeData::Create { path, text } + } + WatcherChange::Write(path) => { + let text = match fs::read_to_string(&path) { + Ok(text) => text, + Err(e) => { + log::warn!("watcher error: {}", e); + return None; + } + }; + WatcherChangeData::Write { path, text } + } + WatcherChange::Remove(path) => WatcherChangeData::Remove { path }, + WatcherChange::Rescan => { + // this should be handled by Vfs::handle_task + return None; + } + }; + Some(data) +} diff --git a/crates/ra_vfs/src/lib.rs b/crates/ra_vfs/src/lib.rs index 889ed788dd7..2930f6b8056 100644 --- a/crates/ra_vfs/src/lib.rs +++ b/crates/ra_vfs/src/lib.rs @@ -75,6 +75,7 @@ pub(crate) fn has_rs_extension(p: &Path) -> bool { struct VfsFileData { root: VfsRoot, path: RelativePathBuf, + is_overlayed: bool, text: Arc, } @@ -170,7 +171,7 @@ pub fn load(&mut self, path: &Path) -> Option { } else { let text = fs::read_to_string(path).unwrap_or_default(); let text = Arc::new(text); - let file = self.add_file(root, rel_path.clone(), Arc::clone(&text)); + let file = self.add_file(root, rel_path.clone(), Arc::clone(&text), false); let change = VfsChange::AddFile { file, text, @@ -205,7 +206,7 @@ pub fn handle_task(&mut self, task: io::TaskResult) { continue; } let text = Arc::new(text); - let file = self.add_file(task.root, path.clone(), Arc::clone(&text)); + let file = self.add_file(task.root, path.clone(), Arc::clone(&text), false); files.push((file, path, text)); } @@ -215,63 +216,132 @@ pub fn handle_task(&mut self, task: io::TaskResult) { }; self.pending_changes.push(change); } - io::TaskResult::WatcherChange(change) => { - // TODO - unimplemented!() - } + io::TaskResult::HandleChange(change) => match &change { + watcher::WatcherChange::Create(path) + | watcher::WatcherChange::Remove(path) + | watcher::WatcherChange::Write(path) => { + if self.should_handle_change(&path) { + self.worker.inp.send(io::Task::LoadChange(change)).unwrap() + } + } + watcher::WatcherChange::Rescan => { + // TODO send Task::AddRoot? + } + }, + io::TaskResult::LoadChange(None) => {} + io::TaskResult::LoadChange(Some(change)) => match change { + io::WatcherChangeData::Create { path, text } + | io::WatcherChangeData::Write { path, text } => { + if let Some((root, path, file)) = self.find_root(&path) { + if let Some(file) = file { + self.do_change_file(file, text, false); + } else { + self.do_add_file(root, path, text, false); + } + } + } + io::WatcherChangeData::Remove { path } => { + if let Some((root, path, file)) = self.find_root(&path) { + if let Some(file) = file { + self.do_remove_file(root, path, file, false); + } + } + } + }, } } - pub fn add_file_overlay(&mut self, path: &Path, text: String) -> Option { - let mut res = None; - if let Some((root, rel_path, file)) = self.find_root(path) { - let text = Arc::new(text); - let change = if let Some(file) = file { - res = Some(file); - self.change_file(file, Arc::clone(&text)); - VfsChange::ChangeFile { file, text } - } else { - let file = self.add_file(root, rel_path.clone(), Arc::clone(&text)); - res = Some(file); - VfsChange::AddFile { - file, - text, - root, - path: rel_path, + fn should_handle_change(&self, path: &Path) -> bool { + if let Some((_root, _rel_path, file)) = self.find_root(&path) { + if let Some(file) = file { + if self.files[file].is_overlayed { + // file is overlayed + return false; } - }; - self.pending_changes.push(change); + } + true + } else { + // file doesn't belong to any root + false + } + } + + fn do_add_file( + &mut self, + root: VfsRoot, + path: RelativePathBuf, + text: String, + is_overlay: bool, + ) -> Option { + let text = Arc::new(text); + let file = self.add_file(root, path.clone(), text.clone(), is_overlay); + self.pending_changes.push(VfsChange::AddFile { + file, + root, + path, + text, + }); + Some(file) + } + + fn do_change_file(&mut self, file: VfsFile, text: String, is_overlay: bool) { + if !is_overlay && self.files[file].is_overlayed { + return; + } + let text = Arc::new(text); + self.change_file(file, text.clone(), is_overlay); + self.pending_changes + .push(VfsChange::ChangeFile { file, text }); + } + + fn do_remove_file( + &mut self, + root: VfsRoot, + path: RelativePathBuf, + file: VfsFile, + is_overlay: bool, + ) { + if !is_overlay && self.files[file].is_overlayed { + return; + } + self.remove_file(file); + self.pending_changes + .push(VfsChange::RemoveFile { root, path, file }); + } + + pub fn add_file_overlay(&mut self, path: &Path, text: String) -> Option { + if let Some((root, rel_path, file)) = self.find_root(path) { + if let Some(file) = file { + self.do_change_file(file, text, true); + Some(file) + } else { + self.do_add_file(root, rel_path, text, true) + } + } else { + None } - res } pub fn change_file_overlay(&mut self, path: &Path, new_text: String) { if let Some((_root, _path, file)) = self.find_root(path) { let file = file.expect("can't change a file which wasn't added"); - let text = Arc::new(new_text); - self.change_file(file, Arc::clone(&text)); - let change = VfsChange::ChangeFile { file, text }; - self.pending_changes.push(change); + self.do_change_file(file, new_text, true); } } pub fn remove_file_overlay(&mut self, path: &Path) -> Option { - let mut res = None; if let Some((root, path, file)) = self.find_root(path) { let file = file.expect("can't remove a file which wasn't added"); - res = Some(file); let full_path = path.to_path(&self.roots[root].root); - let change = if let Ok(text) = fs::read_to_string(&full_path) { - let text = Arc::new(text); - self.change_file(file, Arc::clone(&text)); - VfsChange::ChangeFile { file, text } + if let Ok(text) = fs::read_to_string(&full_path) { + self.do_change_file(file, text, true); } else { - self.remove_file(file); - VfsChange::RemoveFile { root, file, path } - }; - self.pending_changes.push(change); + self.do_remove_file(root, path, file, true); + } + Some(file) + } else { + None } - res } pub fn commit_changes(&mut self) -> Vec { @@ -285,15 +355,28 @@ pub fn shutdown(self) -> thread::Result<()> { self.worker_handle.shutdown() } - fn add_file(&mut self, root: VfsRoot, path: RelativePathBuf, text: Arc) -> VfsFile { - let data = VfsFileData { root, path, text }; + fn add_file( + &mut self, + root: VfsRoot, + path: RelativePathBuf, + text: Arc, + is_overlayed: bool, + ) -> VfsFile { + let data = VfsFileData { + root, + path, + text, + is_overlayed, + }; let file = self.files.alloc(data); self.root2files.get_mut(&root).unwrap().insert(file); file } - fn change_file(&mut self, file: VfsFile, new_text: Arc) { - self.files[file].text = new_text; + fn change_file(&mut self, file: VfsFile, new_text: Arc, is_overlayed: bool) { + let mut file_data = &mut self.files[file]; + file_data.text = new_text; + file_data.is_overlayed = is_overlayed; } fn remove_file(&mut self, file: VfsFile) { diff --git a/crates/ra_vfs/src/watcher.rs b/crates/ra_vfs/src/watcher.rs index a6d0496c038..a5401869ce7 100644 --- a/crates/ra_vfs/src/watcher.rs +++ b/crates/ra_vfs/src/watcher.rs @@ -5,10 +5,10 @@ time::Duration, }; +use crate::io; use crossbeam_channel::Sender; use drop_bomb::DropBomb; use notify::{DebouncedEvent, RecommendedWatcher, RecursiveMode, Watcher as NotifyWatcher}; -use crate::{has_rs_extension, io}; pub struct Watcher { watcher: RecommendedWatcher, @@ -21,59 +21,41 @@ pub enum WatcherChange { Create(PathBuf), Write(PathBuf), Remove(PathBuf), - // can this be replaced and use Remove and Create instead? - Rename(PathBuf, PathBuf), + Rescan, } -impl WatcherChange { - fn try_from_debounced_event(ev: DebouncedEvent) -> Option { - match ev { - DebouncedEvent::NoticeWrite(_) - | DebouncedEvent::NoticeRemove(_) - | DebouncedEvent::Chmod(_) => { - // ignore - None - } - DebouncedEvent::Rescan => { - // TODO should we rescan the root? - None - } - DebouncedEvent::Create(path) => { - if has_rs_extension(&path) { - Some(WatcherChange::Create(path)) - } else { - None - } - } - DebouncedEvent::Write(path) => { - if has_rs_extension(&path) { - Some(WatcherChange::Write(path)) - } else { - None - } - } - DebouncedEvent::Remove(path) => { - if has_rs_extension(&path) { - Some(WatcherChange::Remove(path)) - } else { - None - } - } - DebouncedEvent::Rename(src, dst) => { - match (has_rs_extension(&src), has_rs_extension(&dst)) { - (true, true) => Some(WatcherChange::Rename(src, dst)), - (true, false) => Some(WatcherChange::Remove(src)), - (false, true) => Some(WatcherChange::Create(dst)), - (false, false) => None, - } - } - DebouncedEvent::Error(err, path) => { - // TODO should we reload the file contents? - log::warn!("watch error {}, {:?}", err, path); - None - } +fn send_change_events( + ev: DebouncedEvent, + sender: &Sender, +) -> Result<(), Box> { + match ev { + DebouncedEvent::NoticeWrite(_) + | DebouncedEvent::NoticeRemove(_) + | DebouncedEvent::Chmod(_) => { + // ignore + } + DebouncedEvent::Rescan => { + sender.send(io::Task::LoadChange(WatcherChange::Rescan))?; + } + DebouncedEvent::Create(path) => { + sender.send(io::Task::LoadChange(WatcherChange::Create(path)))?; + } + DebouncedEvent::Write(path) => { + sender.send(io::Task::LoadChange(WatcherChange::Write(path)))?; + } + DebouncedEvent::Remove(path) => { + sender.send(io::Task::LoadChange(WatcherChange::Remove(path)))?; + } + DebouncedEvent::Rename(src, dst) => { + sender.send(io::Task::LoadChange(WatcherChange::Remove(src)))?; + sender.send(io::Task::LoadChange(WatcherChange::Create(dst)))?; + } + DebouncedEvent::Error(err, path) => { + // TODO should we reload the file contents? + log::warn!("watcher error {}, {:?}", err, path); } } + Ok(()) } impl Watcher { @@ -86,8 +68,7 @@ pub(crate) fn start( input_receiver .into_iter() // forward relevant events only - .filter_map(WatcherChange::try_from_debounced_event) - .try_for_each(|change| output_sender.send(io::Task::WatcherChange(change))) + .try_for_each(|change| send_change_events(change, &output_sender)) .unwrap() }); Ok(Watcher { diff --git a/crates/ra_vfs/tests/vfs.rs b/crates/ra_vfs/tests/vfs.rs index 21d5633b12c..9cde2bed7fa 100644 --- a/crates/ra_vfs/tests/vfs.rs +++ b/crates/ra_vfs/tests/vfs.rs @@ -4,6 +4,13 @@ use ra_vfs::{Vfs, VfsChange}; use tempfile::tempdir; +fn process_tasks(vfs: &mut Vfs, num_tasks: u32) { + for _ in 0..num_tasks { + let task = vfs.task_receiver().recv().unwrap(); + vfs.handle_task(task); + } +} + #[test] fn test_vfs_works() -> std::io::Result<()> { Logger::with_str("debug").start().unwrap(); @@ -25,10 +32,7 @@ fn test_vfs_works() -> std::io::Result<()> { let b_root = dir.path().join("a/b"); let (mut vfs, _) = Vfs::new(vec![a_root, b_root]); - for _ in 0..2 { - let task = vfs.task_receiver().recv().unwrap(); - vfs.handle_task(task); - } + process_tasks(&mut vfs, 2); { let files = vfs .commit_changes() @@ -57,30 +61,26 @@ fn test_vfs_works() -> std::io::Result<()> { assert_eq!(files, expected_files); } - // on disk change fs::write(&dir.path().join("a/b/baz.rs"), "quux").unwrap(); - let task = vfs.task_receiver().recv().unwrap(); - vfs.handle_task(task); + process_tasks(&mut vfs, 1); match vfs.commit_changes().as_slice() { [VfsChange::ChangeFile { text, .. }] => assert_eq!(text.as_str(), "quux"), _ => panic!("unexpected changes"), } - // in memory change vfs.change_file_overlay(&dir.path().join("a/b/baz.rs"), "m".to_string()); match vfs.commit_changes().as_slice() { [VfsChange::ChangeFile { text, .. }] => assert_eq!(text.as_str(), "m"), _ => panic!("unexpected changes"), } - // in memory remove, restores data on disk + // removing overlay restores data on disk vfs.remove_file_overlay(&dir.path().join("a/b/baz.rs")); match vfs.commit_changes().as_slice() { [VfsChange::ChangeFile { text, .. }] => assert_eq!(text.as_str(), "quux"), _ => panic!("unexpected changes"), } - // in memory add vfs.add_file_overlay(&dir.path().join("a/b/spam.rs"), "spam".to_string()); match vfs.commit_changes().as_slice() { [VfsChange::AddFile { text, path, .. }] => { @@ -90,17 +90,14 @@ fn test_vfs_works() -> std::io::Result<()> { _ => panic!("unexpected changes"), } - // in memory remove vfs.remove_file_overlay(&dir.path().join("a/b/spam.rs")); match vfs.commit_changes().as_slice() { [VfsChange::RemoveFile { path, .. }] => assert_eq!(path, "spam.rs"), _ => panic!("unexpected changes"), } - // on disk add fs::write(&dir.path().join("a/new.rs"), "new hello").unwrap(); - let task = vfs.task_receiver().recv().unwrap(); - vfs.handle_task(task); + process_tasks(&mut vfs, 1); match vfs.commit_changes().as_slice() { [VfsChange::AddFile { text, path, .. }] => { assert_eq!(text.as_str(), "new hello"); @@ -109,10 +106,8 @@ fn test_vfs_works() -> std::io::Result<()> { _ => panic!("unexpected changes"), } - // on disk rename fs::rename(&dir.path().join("a/new.rs"), &dir.path().join("a/new1.rs")).unwrap(); - let task = vfs.task_receiver().recv().unwrap(); - vfs.handle_task(task); + process_tasks(&mut vfs, 2); match vfs.commit_changes().as_slice() { [VfsChange::RemoveFile { path: removed_path, .. @@ -125,13 +120,11 @@ fn test_vfs_works() -> std::io::Result<()> { assert_eq!(added_path, "new1.rs"); assert_eq!(text.as_str(), "new hello"); } - _ => panic!("unexpected changes"), + xs => panic!("unexpected changes {:?}", xs), } - // on disk remove fs::remove_file(&dir.path().join("a/new1.rs")).unwrap(); - let task = vfs.task_receiver().recv().unwrap(); - vfs.handle_task(task); + process_tasks(&mut vfs, 1); match vfs.commit_changes().as_slice() { [VfsChange::RemoveFile { path, .. }] => assert_eq!(path, "new1.rs"), _ => panic!("unexpected changes"),