From 6b86f038d61752bbf306ed5dd9def74be3b5dcc1 Mon Sep 17 00:00:00 2001 From: Bernardo Date: Mon, 7 Jan 2019 21:35:18 +0100 Subject: refator to move all io to io module use same channel for scanner and watcher some implementations pending --- crates/ra_lsp_server/src/main_loop.rs | 17 +------- crates/ra_vfs/src/io.rs | 57 +++++++++++++++++++----- crates/ra_vfs/src/lib.rs | 81 ++++++++++++++--------------------- crates/ra_vfs/src/watcher.rs | 62 +++++++++++++++++++-------- crates/ra_vfs/tests/vfs.rs | 22 +++++----- 5 files changed, 136 insertions(+), 103 deletions(-) diff --git a/crates/ra_lsp_server/src/main_loop.rs b/crates/ra_lsp_server/src/main_loop.rs index e5a0603d1..4f984ebc7 100644 --- a/crates/ra_lsp_server/src/main_loop.rs +++ b/crates/ra_lsp_server/src/main_loop.rs @@ -113,7 +113,6 @@ enum Event { Msg(RawMessage), Task(Task), Vfs(VfsTask), - Watcher(WatcherChange), Lib(LibraryData), } @@ -150,7 +149,6 @@ impl fmt::Debug for Event { Event::Task(it) => fmt::Debug::fmt(it, f), Event::Vfs(it) => fmt::Debug::fmt(it, f), Event::Lib(it) => fmt::Debug::fmt(it, f), - Event::Watcher(it) => fmt::Debug::fmt(it, f), } } } @@ -185,10 +183,6 @@ fn main_loop_inner( Ok(task) => Event::Vfs(task), Err(RecvError) => bail!("vfs died"), }, - recv(state.vfs.read().change_receiver()) -> change => match change { - Ok(change) => Event::Watcher(change), - Err(RecvError) => bail!("vfs watcher died"), - }, recv(libdata_receiver) -> data => Event::Lib(data.unwrap()) }; log::info!("loop_turn = {:?}", event); @@ -200,10 +194,6 @@ fn main_loop_inner( state.vfs.write().handle_task(task); state_changed = true; } - Event::Watcher(change) => { - state.vfs.write().handle_change(change); - state_changed = true; - } Event::Lib(lib) => { feedback(internal_mode, "library loaded", msg_sender); state.add_lib(lib); @@ -375,7 +365,7 @@ fn on_notification( if let Some(file_id) = state .vfs .write() - .add_file_overlay(&path, Some(params.text_document.text)) + .add_file_overlay(&path, params.text_document.text) { subs.add_sub(FileId(file_id.0.into())); } @@ -394,10 +384,7 @@ fn on_notification( .pop() .ok_or_else(|| format_err!("empty changes"))? .text; - state - .vfs - .write() - .change_file_overlay(path.as_path(), Some(text)); + state.vfs.write().change_file_overlay(path.as_path(), text); return Ok(()); } Err(not) => not, diff --git a/crates/ra_vfs/src/io.rs b/crates/ra_vfs/src/io.rs index 80328ad18..79dea5dc7 100644 --- a/crates/ra_vfs/src/io.rs +++ b/crates/ra_vfs/src/io.rs @@ -10,17 +10,47 @@ use relative_path::RelativePathBuf; use crate::{VfsRoot, has_rs_extension}; -pub(crate) struct Task { - pub(crate) root: VfsRoot, - pub(crate) path: PathBuf, - pub(crate) filter: Box bool + Send>, +pub(crate) enum Task { + AddRoot { + root: VfsRoot, + path: PathBuf, + filter: Box bool + Send>, + }, + WatcherChange(crate::watcher::WatcherChange), } -pub struct TaskResult { +#[derive(Debug)] +pub struct AddRootResult { pub(crate) root: VfsRoot, pub(crate) files: Vec<(RelativePathBuf, String)>, } +#[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 TaskResult { + AddRoot(AddRootResult), + WatcherChange(WatcherChangeResult), +} + impl fmt::Debug for TaskResult { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { f.write_str("TaskResult { ... }") @@ -40,11 +70,18 @@ pub(crate) fn start() -> (Worker, WorkerHandle) { } fn handle_task(task: Task) -> TaskResult { - let Task { root, path, filter } = task; - log::debug!("loading {} ...", path.as_path().display()); - let files = load_root(path.as_path(), &*filter); - log::debug!("... loaded {}", path.as_path().display()); - TaskResult { root, files } + match task { + Task::AddRoot { root, path, filter } => { + log::debug!("loading {} ...", path.as_path().display()); + let files = load_root(path.as_path(), &*filter); + log::debug!("... loaded {}", path.as_path().display()); + TaskResult::AddRoot(AddRootResult { root, files }) + } + Task::WatcherChange(change) => { + // TODO + unimplemented!() + } + } } fn load_root(root: &Path, filter: &dyn Fn(&DirEntry) -> bool) -> Vec<(RelativePathBuf, String)> { diff --git a/crates/ra_vfs/src/lib.rs b/crates/ra_vfs/src/lib.rs index 1ca94dcd6..889ed788d 100644 --- a/crates/ra_vfs/src/lib.rs +++ b/crates/ra_vfs/src/lib.rs @@ -60,7 +60,7 @@ impl RootFilter { } } -fn has_rs_extension(p: &Path) -> bool { +pub(crate) fn has_rs_extension(p: &Path) -> bool { p.extension() == Some(OsStr::new("rs")) } @@ -98,7 +98,7 @@ impl Vfs { pub fn new(mut roots: Vec) -> (Vfs, Vec) { let (worker, worker_handle) = io::start(); - let watcher = Watcher::start().unwrap(); // TODO return Result? + let watcher = Watcher::start(worker.inp.clone()).unwrap(); // TODO return Result? let mut res = Vfs { roots: Arena::default(), @@ -127,7 +127,7 @@ impl Vfs { nested.iter().all(|it| it != entry.path()) } }; - let task = io::Task { + let task = io::Task::AddRoot { root, path: path.clone(), filter: Box::new(filter), @@ -188,58 +188,43 @@ impl Vfs { &self.worker.out } - pub fn change_receiver(&self) -> &Receiver { - &self.watcher.change_receiver() - } - pub fn handle_task(&mut self, task: io::TaskResult) { - let mut files = Vec::new(); - // While we were scanning the root in the backgound, a file might have - // been open in the editor, so we need to account for that. - let exising = self.root2files[&task.root] - .iter() - .map(|&file| (self.files[file].path.clone(), file)) - .collect::>(); - for (path, text) in task.files { - if let Some(&file) = exising.get(&path) { - let text = Arc::clone(&self.files[file].text); - files.push((file, path, text)); - continue; - } - let text = Arc::new(text); - let file = self.add_file(task.root, path.clone(), Arc::clone(&text)); - files.push((file, path, text)); - } - - let change = VfsChange::AddRoot { - root: task.root, - files, - }; - self.pending_changes.push(change); - } + match task { + io::TaskResult::AddRoot(task) => { + let mut files = Vec::new(); + // While we were scanning the root in the backgound, a file might have + // been open in the editor, so we need to account for that. + let exising = self.root2files[&task.root] + .iter() + .map(|&file| (self.files[file].path.clone(), file)) + .collect::>(); + for (path, text) in task.files { + if let Some(&file) = exising.get(&path) { + let text = Arc::clone(&self.files[file].text); + files.push((file, path, text)); + continue; + } + let text = Arc::new(text); + let file = self.add_file(task.root, path.clone(), Arc::clone(&text)); + files.push((file, path, text)); + } - pub fn handle_change(&mut self, change: WatcherChange) { - match change { - WatcherChange::Create(path) => { - self.add_file_overlay(&path, None); - } - WatcherChange::Remove(path) => { - self.remove_file_overlay(&path); - } - WatcherChange::Rename(src, dst) => { - self.remove_file_overlay(&src); - self.add_file_overlay(&dst, None); + let change = VfsChange::AddRoot { + root: task.root, + files, + }; + self.pending_changes.push(change); } - WatcherChange::Write(path) => { - self.change_file_overlay(&path, None); + io::TaskResult::WatcherChange(change) => { + // TODO + unimplemented!() } } } - pub fn add_file_overlay(&mut self, path: &Path, text: Option) -> Option { + 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 = text.unwrap_or_else(|| fs::read_to_string(&path).unwrap_or_default()); let text = Arc::new(text); let change = if let Some(file) = file { res = Some(file); @@ -260,10 +245,8 @@ impl Vfs { res } - pub fn change_file_overlay(&mut self, path: &Path, new_text: Option) { + pub fn change_file_overlay(&mut self, path: &Path, new_text: String) { if let Some((_root, _path, file)) = self.find_root(path) { - let new_text = - new_text.unwrap_or_else(|| fs::read_to_string(&path).unwrap_or_default()); 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)); diff --git a/crates/ra_vfs/src/watcher.rs b/crates/ra_vfs/src/watcher.rs index 1aac23616..a6d0496c0 100644 --- a/crates/ra_vfs/src/watcher.rs +++ b/crates/ra_vfs/src/watcher.rs @@ -5,12 +5,12 @@ use std::{ time::Duration, }; -use crossbeam_channel::Receiver; +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 { - receiver: Receiver, watcher: RecommendedWatcher, thread: thread::JoinHandle<()>, bomb: DropBomb, @@ -21,24 +21,54 @@ pub enum WatcherChange { Create(PathBuf), Write(PathBuf), Remove(PathBuf), + // can this be replaced and use Remove and Create instead? Rename(PathBuf, PathBuf), } impl WatcherChange { - fn from_debounced_event(ev: DebouncedEvent) -> Option { + fn try_from_debounced_event(ev: DebouncedEvent) -> Option { match ev { DebouncedEvent::NoticeWrite(_) | DebouncedEvent::NoticeRemove(_) - | DebouncedEvent::Chmod(_) - | DebouncedEvent::Rescan => { + | DebouncedEvent::Chmod(_) => { // ignore None } - DebouncedEvent::Create(path) => Some(WatcherChange::Create(path)), - DebouncedEvent::Write(path) => Some(WatcherChange::Write(path)), - DebouncedEvent::Remove(path) => Some(WatcherChange::Remove(path)), - DebouncedEvent::Rename(src, dst) => Some(WatcherChange::Rename(src, dst)), + 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 } @@ -47,20 +77,20 @@ impl WatcherChange { } impl Watcher { - pub fn start() -> Result> { + pub(crate) fn start( + output_sender: Sender, + ) -> Result> { let (input_sender, input_receiver) = mpsc::channel(); let watcher = notify::watcher(input_sender, Duration::from_millis(250))?; - let (output_sender, output_receiver) = crossbeam_channel::unbounded(); let thread = thread::spawn(move || { input_receiver .into_iter() // forward relevant events only - .filter_map(WatcherChange::from_debounced_event) - .try_for_each(|change| output_sender.send(change)) + .filter_map(WatcherChange::try_from_debounced_event) + .try_for_each(|change| output_sender.send(io::Task::WatcherChange(change))) .unwrap() }); Ok(Watcher { - receiver: output_receiver, watcher, thread, bomb: DropBomb::new(format!("Watcher was not shutdown")), @@ -72,10 +102,6 @@ impl Watcher { Ok(()) } - pub fn change_receiver(&self) -> &Receiver { - &self.receiver - } - pub fn shutdown(mut self) -> thread::Result<()> { self.bomb.defuse(); drop(self.watcher); diff --git a/crates/ra_vfs/tests/vfs.rs b/crates/ra_vfs/tests/vfs.rs index 8634be9c4..21d5633b1 100644 --- a/crates/ra_vfs/tests/vfs.rs +++ b/crates/ra_vfs/tests/vfs.rs @@ -59,15 +59,15 @@ fn test_vfs_works() -> std::io::Result<()> { // on disk change fs::write(&dir.path().join("a/b/baz.rs"), "quux").unwrap(); - let change = vfs.change_receiver().recv().unwrap(); - vfs.handle_change(change); + let task = vfs.task_receiver().recv().unwrap(); + vfs.handle_task(task); 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"), Some("m".to_string())); + 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"), @@ -81,7 +81,7 @@ fn test_vfs_works() -> std::io::Result<()> { } // in memory add - vfs.add_file_overlay(&dir.path().join("a/b/spam.rs"), Some("spam".to_string())); + 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_eq!(text.as_str(), "spam"); @@ -99,8 +99,8 @@ fn test_vfs_works() -> std::io::Result<()> { // on disk add fs::write(&dir.path().join("a/new.rs"), "new hello").unwrap(); - let change = vfs.change_receiver().recv().unwrap(); - vfs.handle_change(change); + let task = vfs.task_receiver().recv().unwrap(); + vfs.handle_task(task); match vfs.commit_changes().as_slice() { [VfsChange::AddFile { text, path, .. }] => { assert_eq!(text.as_str(), "new hello"); @@ -111,8 +111,8 @@ fn test_vfs_works() -> std::io::Result<()> { // on disk rename fs::rename(&dir.path().join("a/new.rs"), &dir.path().join("a/new1.rs")).unwrap(); - let change = vfs.change_receiver().recv().unwrap(); - vfs.handle_change(change); + let task = vfs.task_receiver().recv().unwrap(); + vfs.handle_task(task); match vfs.commit_changes().as_slice() { [VfsChange::RemoveFile { path: removed_path, .. @@ -130,14 +130,14 @@ fn test_vfs_works() -> std::io::Result<()> { // on disk remove fs::remove_file(&dir.path().join("a/new1.rs")).unwrap(); - let change = vfs.change_receiver().recv().unwrap(); - vfs.handle_change(change); + let task = vfs.task_receiver().recv().unwrap(); + vfs.handle_task(task); match vfs.commit_changes().as_slice() { [VfsChange::RemoveFile { path, .. }] => assert_eq!(path, "new1.rs"), _ => panic!("unexpected changes"), } - match vfs.change_receiver().try_recv() { + match vfs.task_receiver().try_recv() { Err(crossbeam_channel::TryRecvError::Empty) => (), res => panic!("unexpected {:?}", res), } -- cgit v1.2.3