ignore check event dir for ignore, cleanup tests

2 files changed, 129 insertions, 63 deletions

diff --git a/crates/ra_vfs/src/watcher.rs b/crates/ra_vfs/src/watcher.rs
index 9d552f886..f0ef9bc0e 100644
--- a/crates/ra_vfs/src/watcher.rs
+++ b/crates/ra_vfs/src/watcher.rs
@@ -1,7 +1,7 @@
 use crate::io;
 use crossbeam_channel::Sender;
 use drop_bomb::DropBomb;
-use ignore;
+use ignore::{gitignore::Gitignore, Walk};
 use notify::{DebouncedEvent, RecommendedWatcher, RecursiveMode, Watcher as NotifyWatcher};
 use parking_lot::Mutex;
 use std::{
@@ -40,8 +40,11 @@ fn handle_change_event(
             sender.send(io::Task::HandleChange(WatcherChange::Rescan))?;
         }
         DebouncedEvent::Create(path) => {
-            if path.is_dir() {
-                watch_recursive(watcher, &path);
+            // we have to check if `path` is ignored because Walk iterator doesn't check it
+            // also childs are only ignored if they match a pattern
+            // (see `matched` vs `matched_path_or_any_parents` in `Gitignore`)
+            if path.is_dir() && !should_ignore_dir(&path) {
+                watch_recursive(watcher, &path, Some(sender));
             }
             sender.send(io::Task::HandleChange(WatcherChange::Create(path)))?;
         }
@@ -63,15 +66,18 @@ fn handle_change_event(
     Ok(())
 }
 
-fn watch_one(watcher: &mut RecommendedWatcher, path: &Path) {
-    match watcher.watch(path, RecursiveMode::NonRecursive) {
-        Ok(()) => log::debug!("watching \"{}\"", path.display()),
-        Err(e) => log::warn!("could not watch \"{}\": {}", path.display(), e),
+fn watch_one(watcher: &mut RecommendedWatcher, dir: &Path) {
+    match watcher.watch(dir, RecursiveMode::NonRecursive) {
+        Ok(()) => log::debug!("watching \"{}\"", dir.display()),
+        Err(e) => log::warn!("could not watch \"{}\": {}", dir.display(), e),
     }
 }
 
-fn watch_recursive(watcher: &Arc<Mutex<Option<RecommendedWatcher>>>, path: &Path) {
-    log::debug!("watch_recursive \"{}\"", path.display());
+fn watch_recursive(
+    watcher: &Arc<Mutex<Option<RecommendedWatcher>>>,
+    dir: &Path,
+    sender: Option<&Sender<io::Task>>,
+) {
     let mut watcher = watcher.lock();
     let mut watcher = match *watcher {
         Some(ref mut watcher) => watcher,
@@ -80,20 +86,47 @@ fn watch_recursive(watcher: &Arc<Mutex<Option<RecommendedWatcher>>>, path: &Path
             return;
         }
     };
-    // TODO it seems path itself isn't checked against ignores
-    // check if path should be ignored before walking it
-    for res in ignore::Walk::new(path) {
+    for res in Walk::new(dir) {
         match res {
             Ok(entry) => {
                 if entry.path().is_dir() {
                     watch_one(&mut watcher, entry.path());
                 }
+                if let Some(sender) = sender {
+                    // emit as create because we haven't seen it yet
+                    if let Err(e) = sender.send(io::Task::HandleChange(WatcherChange::Create(
+                        entry.path().to_path_buf(),
+                    ))) {
+                        log::warn!("watcher error: {}", e)
+                    }
+                }
             }
             Err(e) => log::warn!("watcher error: {}", e),
         }
     }
 }
 
+fn should_ignore_dir(dir: &Path) -> bool {
+    let mut parent = dir;
+    loop {
+        parent = match parent.parent() {
+            Some(p) => p,
+            None => break,
+        };
+        let gitignore = parent.join(".gitignore");
+        if gitignore.exists() {
+            let gitignore = Gitignore::new(gitignore).0;
+            if gitignore.matched_path_or_any_parents(dir, true).is_ignore() {
+                log::debug!("ignored {}", dir.display());
+                return true;
+            }
+        }
+    }
+    false
+}
+
+const WATCHER_DELAY: Duration = Duration::from_millis(250);
+
 impl Watcher {
     pub(crate) fn start(
         output_sender: Sender<io::Task>,
@@ -101,7 +134,7 @@ impl Watcher {
         let (input_sender, input_receiver) = mpsc::channel();
         let watcher = Arc::new(Mutex::new(Some(notify::watcher(
             input_sender,
-            Duration::from_millis(250),
+            WATCHER_DELAY,
         )?)));
         let w = watcher.clone();
         let thread = thread::spawn(move || {
@@ -119,7 +152,7 @@ impl Watcher {
     }
 
     pub fn watch(&mut self, root: impl AsRef<Path>) {
-        watch_recursive(&self.watcher, root.as_ref());
+        watch_recursive(&self.watcher, root.as_ref(), None);
     }
 
     pub fn shutdown(mut self) -> thread::Result<()> {
diff --git a/crates/ra_vfs/tests/vfs.rs b/crates/ra_vfs/tests/vfs.rs
index 8266a0bd5..71b25a5c9 100644
--- a/crates/ra_vfs/tests/vfs.rs
+++ b/crates/ra_vfs/tests/vfs.rs
@@ -11,9 +11,21 @@ fn process_tasks(vfs: &mut Vfs, num_tasks: u32) {
     }
 }
 
+macro_rules! assert_match {
+    ($x:expr, $pat:pat) => {
+        assert_match!($x, $pat, assert!(true))
+    };
+    ($x:expr, $pat:pat, $assert:expr) => {
+        match $x {
+            $pat => $assert,
+            x => assert!(false, "Expected {}, got {:?}", stringify!($pat), x),
+        };
+    };
+}
+
 #[test]
 fn test_vfs_works() -> std::io::Result<()> {
-    // Logger::with_str("debug").start().unwrap();
+    // Logger::with_str("vfs=debug,ra_vfs=debug").start().unwrap();
 
     let files = [
         ("a/foo.rs", "hello"),
@@ -21,13 +33,16 @@ fn test_vfs_works() -> std::io::Result<()> {
         ("a/b/baz.rs", "nested hello"),
     ];
 
-    let dir = tempdir()?;
+    let dir = tempdir().unwrap();
     for (path, text) in files.iter() {
         let file_path = dir.path().join(path);
-        fs::create_dir_all(file_path.parent().unwrap())?;
+        fs::create_dir_all(file_path.parent().unwrap()).unwrap();
         fs::write(file_path, text)?
     }
 
+    let gitignore = dir.path().join("a/.gitignore");
+    fs::write(gitignore, "/target").unwrap();
+
     let a_root = dir.path().join("a");
     let b_root = dir.path().join("a/b");
 
@@ -62,79 +77,97 @@ fn test_vfs_works() -> std::io::Result<()> {
     }
 
     fs::write(&dir.path().join("a/b/baz.rs"), "quux").unwrap();
-    // 2 tasks per watcher change, first for HandleChange then for LoadChange
+    // 2 tasks per change, HandleChange and then LoadChange
     process_tasks(&mut vfs, 2);
-    match vfs.commit_changes().as_slice() {
-        [VfsChange::ChangeFile { text, .. }] => assert_eq!(text.as_str(), "quux"),
-        xs => panic!("unexpected changes {:?}", xs),
-    }
+    assert_match!(
+        vfs.commit_changes().as_slice(),
+        [VfsChange::ChangeFile { text, .. }],
+        assert_eq!(text.as_str(), "quux")
+    );
 
     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"),
-        xs => panic!("unexpected changes {:?}", xs),
-    }
+    assert_match!(
+        vfs.commit_changes().as_slice(),
+        [VfsChange::ChangeFile { text, .. }],
+        assert_eq!(text.as_str(), "m")
+    );
 
     // 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"),
-        xs => panic!("unexpected changes {:?}", xs),
-    }
+    assert_match!(
+        vfs.commit_changes().as_slice(),
+        [VfsChange::ChangeFile { text, .. }],
+        assert_eq!(text.as_str(), "quux")
+    );
 
     vfs.add_file_overlay(&dir.path().join("a/b/spam.rs"), "spam".to_string());
-    match vfs.commit_changes().as_slice() {
-        [VfsChange::AddFile { text, path, .. }] => {
+    assert_match!(
+        vfs.commit_changes().as_slice(),
+        [VfsChange::AddFile { text, path, .. }],
+        {
             assert_eq!(text.as_str(), "spam");
             assert_eq!(path, "spam.rs");
         }
-        xs => panic!("unexpected changes {:?}", xs),
-    }
+    );
 
     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"),
-        xs => panic!("unexpected changes {:?}", xs),
-    }
-
-    fs::write(&dir.path().join("a/new.rs"), "new hello").unwrap();
-    process_tasks(&mut vfs, 2);
-    match vfs.commit_changes().as_slice() {
-        [VfsChange::AddFile { text, path, .. }] => {
+    assert_match!(
+        vfs.commit_changes().as_slice(),
+        [VfsChange::RemoveFile { path, .. }],
+        assert_eq!(path, "spam.rs")
+    );
+
+    fs::create_dir_all(dir.path().join("a/c")).unwrap();
+    fs::write(dir.path().join("a/c/new.rs"), "new hello").unwrap();
+    process_tasks(&mut vfs, 4);
+    assert_match!(
+        vfs.commit_changes().as_slice(),
+        [VfsChange::AddFile { text, path, .. }],
+        {
             assert_eq!(text.as_str(), "new hello");
-            assert_eq!(path, "new.rs");
+            assert_eq!(path, "c/new.rs");
         }
-        xs => panic!("unexpected changes {:?}", xs),
-    }
+    );
 
-    fs::rename(&dir.path().join("a/new.rs"), &dir.path().join("a/new1.rs")).unwrap();
+    fs::rename(
+        &dir.path().join("a/c/new.rs"),
+        &dir.path().join("a/c/new1.rs"),
+    )
+    .unwrap();
     process_tasks(&mut vfs, 4);
-    match vfs.commit_changes().as_slice() {
+    assert_match!(
+        vfs.commit_changes().as_slice(),
         [VfsChange::RemoveFile {
             path: removed_path, ..
         }, VfsChange::AddFile {
             text,
             path: added_path,
             ..
-        }] => {
-            assert_eq!(removed_path, "new.rs");
-            assert_eq!(added_path, "new1.rs");
+        }],
+        {
+            assert_eq!(removed_path, "c/new.rs");
+            assert_eq!(added_path, "c/new1.rs");
             assert_eq!(text.as_str(), "new hello");
         }
-        xs => panic!("unexpected changes {:?}", xs),
-    }
+    );
 
-    fs::remove_file(&dir.path().join("a/new1.rs")).unwrap();
+    fs::remove_file(&dir.path().join("a/c/new1.rs")).unwrap();
     process_tasks(&mut vfs, 2);
-    match vfs.commit_changes().as_slice() {
-        [VfsChange::RemoveFile { path, .. }] => assert_eq!(path, "new1.rs"),
-        xs => panic!("unexpected changes {:?}", xs),
-    }
-
-    match vfs.task_receiver().try_recv() {
-        Err(crossbeam_channel::TryRecvError::Empty) => (),
-        res => panic!("unexpected {:?}", res),
-    }
+    assert_match!(
+        vfs.commit_changes().as_slice(),
+        [VfsChange::RemoveFile { path, .. }],
+        assert_eq!(path, "c/new1.rs")
+    );
+
+    fs::create_dir_all(dir.path().join("a/target")).unwrap();
+    // should be ignored
+    fs::write(&dir.path().join("a/target/new.rs"), "ignore me").unwrap();
+    process_tasks(&mut vfs, 1); // 1 task because no LoadChange will happen, just HandleChange for dir creation
+
+    assert_match!(
+        vfs.task_receiver().try_recv(),
+        Err(crossbeam_channel::TryRecvError::Empty)
+    );
 
     vfs.shutdown().unwrap();
     Ok(())