handle watched events filtering in `Vfs`add `is_overlayed`load changed files contents in `io`

4 files changed, 217 insertions, 142 deletions

diff --git a/crates/ra_vfs/src/io.rs b/crates/ra_vfs/src/io.rs
index 79dea5dc7..a0c87fb25 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<Fn(&DirEntry) -> 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<WatcherChangeData>),
 }
 
 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<WatcherChangeData> {
+    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 889ed788d..2930f6b80 100644
--- a/crates/ra_vfs/src/lib.rs
+++ b/crates/ra_vfs/src/lib.rs
@@ -75,6 +75,7 @@ impl_arena_id!(VfsFile);
 struct VfsFileData {
     root: VfsRoot,
     path: RelativePathBuf,
+    is_overlayed: bool,
     text: Arc<String>,
 }
 
@@ -170,7 +171,7 @@ impl Vfs {
             } 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 @@ impl Vfs {
                         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 @@ impl Vfs {
                 };
                 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);
+                        }
+                    }
+                }
+            },
+        }
+    }
+
+    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;
+                }
             }
+            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<VfsFile> {
+        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<VfsFile> {
-        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 }
+            if let Some(file) = file {
+                self.do_change_file(file, text, true);
+                Some(file)
             } else {
-                let file = self.add_file(root, rel_path.clone(), Arc::clone(&text));
-                res = Some(file);
-                VfsChange::AddFile {
-                    file,
-                    text,
-                    root,
-                    path: rel_path,
-                }
-            };
-            self.pending_changes.push(change);
+                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<VfsFile> {
-        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<VfsChange> {
@@ -285,15 +355,28 @@ impl Vfs {
         self.worker_handle.shutdown()
     }
 
-    fn add_file(&mut self, root: VfsRoot, path: RelativePathBuf, text: Arc<String>) -> VfsFile {
-        let data = VfsFileData { root, path, text };
+    fn add_file(
+        &mut self,
+        root: VfsRoot,
+        path: RelativePathBuf,
+        text: Arc<String>,
+        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<String>) {
-        self.files[file].text = new_text;
+    fn change_file(&mut self, file: VfsFile, new_text: Arc<String>, 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 a6d0496c0..a5401869c 100644
--- a/crates/ra_vfs/src/watcher.rs
+++ b/crates/ra_vfs/src/watcher.rs
@@ -5,10 +5,10 @@ use std::{
     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<WatcherChange> {
-        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<io::Task>,
+) -> Result<(), Box<std::error::Error>> {
+    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 @@ impl Watcher {
             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 21d5633b1..9cde2bed7 100644
--- a/crates/ra_vfs/tests/vfs.rs
+++ b/crates/ra_vfs/tests/vfs.rs
@@ -4,6 +4,13 @@ use flexi_logger::Logger;
 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"),