diff --git a/gpui/src/app.rs b/gpui/src/app.rs index 863cb94f270d0ec5561144d5ae4c31affc452e68..178b8f9b69f4b1e43065bfb145d4968b0d482d9a 100644 --- a/gpui/src/app.rs +++ b/gpui/src/app.rs @@ -1620,7 +1620,7 @@ impl<'a, T: Entity> ModelContext<'a, T> { }); } - fn handle(&self) -> ModelHandle { + pub fn handle(&self) -> ModelHandle { ModelHandle::new(self.model_id, &self.app.ctx.ref_counts) } diff --git a/zed/src/editor/buffer/mod.rs b/zed/src/editor/buffer/mod.rs index 64aa91dfdc84ba2031eee84836ff1534707e32b4..d42c3460cf470565a5f5f66f30abadf35f43bea4 100644 --- a/zed/src/editor/buffer/mod.rs +++ b/zed/src/editor/buffer/mod.rs @@ -377,7 +377,12 @@ impl Buffer { ctx: &mut ModelContext, ) -> Self { if let Some(file) = file.as_ref() { - file.observe_from_model(ctx, |_, _, ctx| ctx.emit(Event::FileHandleChanged)); + file.observe_from_model(ctx, |this, file, ctx| { + if this.version == this.saved_version && file.is_deleted() { + ctx.emit(Event::Dirtied); + } + ctx.emit(Event::FileHandleChanged); + }); } let mut insertion_splits = HashMap::default(); @@ -501,7 +506,7 @@ impl Buffer { } pub fn is_dirty(&self) -> bool { - self.version > self.saved_version + self.version > self.saved_version || self.file.as_ref().map_or(false, |f| f.is_deleted()) } pub fn version(&self) -> time::Global { @@ -2376,11 +2381,15 @@ impl ToPoint for usize { #[cfg(test)] mod tests { use super::*; + use crate::{test::temp_tree, worktree::Worktree}; use cmp::Ordering; use gpui::App; - use std::{cell::RefCell, rc::Rc}; + use serde_json::json; use std::{ + cell::RefCell, collections::BTreeMap, + fs, + rc::Rc, sync::atomic::{self, AtomicUsize}, }; @@ -2979,14 +2988,27 @@ mod tests { } #[test] - fn test_is_modified() { - App::test((), |app| { - let model = app.add_model(|ctx| Buffer::new(0, "abc", ctx)); + fn test_is_dirty() { + use crate::worktree::WorktreeHandle; + + App::test_async((), |mut app| async move { + let dir = temp_tree(json!({ + "file1": "", + "file2": "", + "file3": "", + })); + let tree = app.add_model(|ctx| Worktree::new(dir.path(), ctx)); + app.read(|ctx| tree.read(ctx).scan_complete()).await; + + let file1 = app.read(|ctx| tree.file("file1", ctx)); + let buffer1 = app.add_model(|ctx| { + Buffer::from_history(0, History::new("abc".into()), Some(file1), ctx) + }); let events = Rc::new(RefCell::new(Vec::new())); // initially, the buffer isn't dirty. - model.update(app, |buffer, ctx| { - ctx.subscribe(&model, { + buffer1.update(&mut app, |buffer, ctx| { + ctx.subscribe(&buffer1, { let events = events.clone(); move |_, event, _| events.borrow_mut().push(event.clone()) }); @@ -2998,7 +3020,7 @@ mod tests { }); // after the first edit, the buffer is dirty, and emits a dirtied event. - model.update(app, |buffer, ctx| { + buffer1.update(&mut app, |buffer, ctx| { assert!(buffer.text() == "ac"); assert!(buffer.is_dirty()); assert_eq!(*events.borrow(), &[Event::Edited, Event::Dirtied]); @@ -3008,7 +3030,7 @@ mod tests { }); // after saving, the buffer is not dirty, and emits a saved event. - model.update(app, |buffer, ctx| { + buffer1.update(&mut app, |buffer, ctx| { assert!(!buffer.is_dirty()); assert_eq!(*events.borrow(), &[Event::Saved]); events.borrow_mut().clear(); @@ -3018,7 +3040,7 @@ mod tests { }); // after editing again, the buffer is dirty, and emits another dirty event. - model.update(app, |buffer, ctx| { + buffer1.update(&mut app, |buffer, ctx| { assert!(buffer.text() == "aBDc"); assert!(buffer.is_dirty()); assert_eq!( @@ -3034,9 +3056,52 @@ mod tests { assert!(buffer.is_dirty()); }); - model.update(app, |_, _| { - assert_eq!(*events.borrow(), &[Event::Edited]); + assert_eq!(*events.borrow(), &[Event::Edited]); + + // When a file is deleted, the buffer is considered dirty. + let events = Rc::new(RefCell::new(Vec::new())); + let file2 = app.read(|ctx| tree.file("file2", ctx)); + let buffer2 = app.add_model(|ctx: &mut ModelContext| { + ctx.subscribe(&ctx.handle(), { + let events = events.clone(); + move |_, event, _| events.borrow_mut().push(event.clone()) + }); + + Buffer::from_history(0, History::new("abc".into()), Some(file2), ctx) + }); + + tree.flush_fs_events(&app).await; + fs::remove_file(dir.path().join("file2")).unwrap(); + tree.update(&mut app, |tree, ctx| tree.next_scan_complete(ctx)) + .await; + assert_eq!( + *events.borrow(), + &[Event::Dirtied, Event::FileHandleChanged] + ); + app.read(|ctx| assert!(buffer2.read(ctx).is_dirty())); + + // When a file is already dirty when deleted, we don't emit a Dirtied event. + let events = Rc::new(RefCell::new(Vec::new())); + let file3 = app.read(|ctx| tree.file("file3", ctx)); + let buffer3 = app.add_model(|ctx: &mut ModelContext| { + ctx.subscribe(&ctx.handle(), { + let events = events.clone(); + move |_, event, _| events.borrow_mut().push(event.clone()) + }); + + Buffer::from_history(0, History::new("abc".into()), Some(file3), ctx) + }); + + tree.flush_fs_events(&app).await; + buffer3.update(&mut app, |buffer, ctx| { + buffer.edit(Some(0..0), "x", Some(ctx)).unwrap(); }); + events.borrow_mut().clear(); + fs::remove_file(dir.path().join("file3")).unwrap(); + tree.update(&mut app, |tree, ctx| tree.next_scan_complete(ctx)) + .await; + assert_eq!(*events.borrow(), &[Event::FileHandleChanged]); + app.read(|ctx| assert!(buffer3.read(ctx).is_dirty())); }); } diff --git a/zed/src/workspace.rs b/zed/src/workspace.rs index b94f1d5fb15f863bbf9ec7d862fa88faee7c7c68..7253e0a5af9777bf50e3cd0d1fc56ff9159b2745 100644 --- a/zed/src/workspace.rs +++ b/zed/src/workspace.rs @@ -344,17 +344,17 @@ impl Workspace { pub fn open_paths( &mut self, - paths: &[PathBuf], + abs_paths: &[PathBuf], ctx: &mut ViewContext, ) -> impl Future { - let entries = paths + let entries = abs_paths .iter() .cloned() .map(|path| self.file_for_path(&path, ctx)) .collect::>(); let bg = ctx.background_executor().clone(); - let tasks = paths + let tasks = abs_paths .iter() .cloned() .zip(entries.into_iter()) @@ -489,11 +489,6 @@ impl Workspace { }; let file = worktree.file(path.clone(), ctx.as_ref()); - if file.is_deleted() { - log::error!("path {:?} does not exist", path); - return None; - } - if let Entry::Vacant(entry) = self.loading_items.entry(entry.clone()) { let (mut tx, rx) = postage::watch::channel(); entry.insert(rx); @@ -737,7 +732,6 @@ mod tests { use gpui::App; use serde_json::json; use std::collections::HashSet; - use std::time; use tempdir::TempDir; #[test] @@ -935,14 +929,16 @@ mod tests { }) .await; app.read(|ctx| { - workspace - .read(ctx) - .active_pane() - .read(ctx) - .active_item() - .unwrap() - .title(ctx) - == "a.txt" + assert_eq!( + workspace + .read(ctx) + .active_pane() + .read(ctx) + .active_item() + .unwrap() + .title(ctx), + "a.txt" + ); }); // Open a file outside of any existing worktree. @@ -952,7 +948,7 @@ mod tests { }) }) .await; - app.update(|ctx| { + app.read(|ctx| { let worktree_roots = workspace .read(ctx) .worktrees() @@ -965,16 +961,16 @@ mod tests { .into_iter() .collect(), ); - }); - app.read(|ctx| { - workspace - .read(ctx) - .active_pane() - .read(ctx) - .active_item() - .unwrap() - .title(ctx) - == "b.txt" + assert_eq!( + workspace + .read(ctx) + .active_pane() + .read(ctx) + .active_item() + .unwrap() + .title(ctx), + "b.txt" + ); }); }); } @@ -989,7 +985,7 @@ mod tests { workspace.add_worktree(dir.path(), ctx); workspace }); - let worktree = app.read(|ctx| { + let tree = app.read(|ctx| { workspace .read(ctx) .worktrees() @@ -998,6 +994,7 @@ mod tests { .unwrap() .clone() }); + tree.flush_fs_events(&app).await; // Create a new untitled buffer let editor = workspace.update(&mut app, |workspace, ctx| { @@ -1030,15 +1027,12 @@ mod tests { }); // When the save completes, the buffer's title is updated. - editor - .condition(&app, |editor, ctx| !editor.is_dirty(ctx)) - .await; - worktree - .condition_with_duration(time::Duration::from_millis(500), &app, |worktree, _| { - worktree.inode_for_path("the-new-name").is_some() - }) + tree.update(&mut app, |tree, ctx| tree.next_scan_complete(ctx)) .await; - app.read(|ctx| assert_eq!(editor.title(ctx), "the-new-name")); + app.read(|ctx| { + assert!(!editor.is_dirty(ctx)); + assert_eq!(editor.title(ctx), "the-new-name"); + }); // Edit the file and save it again. This time, there is no filename prompt. editor.update(&mut app, |editor, ctx| { @@ -1060,7 +1054,7 @@ mod tests { workspace.open_new_file(&(), ctx); workspace.split_pane(workspace.active_pane().clone(), SplitDirection::Right, ctx); assert!(workspace - .open_entry((worktree.id(), Path::new("the-new-name").into()), ctx) + .open_entry((tree.id(), Path::new("the-new-name").into()), ctx) .is_none()); }); let editor2 = workspace.update(&mut app, |workspace, ctx| { diff --git a/zed/src/worktree.rs b/zed/src/worktree.rs index 135fb3bedf4799a953a199243bb33472c0e941fd..1478daaff3ab11ae7614bff97ef570c5fe47c38c 100644 --- a/zed/src/worktree.rs +++ b/zed/src/worktree.rs @@ -114,19 +114,17 @@ impl Worktree { } } - pub fn next_scan_complete(&self) -> impl Future { - let mut scan_state_rx = self.scan_state.1.clone(); - let mut did_scan = matches!(*scan_state_rx.borrow(), ScanState::Scanning); - async move { - loop { - if let ScanState::Scanning = *scan_state_rx.borrow() { - did_scan = true; - } else if did_scan { - break; + pub fn next_scan_complete(&self, ctx: &mut ModelContext) -> impl Future { + let scan_id = self.snapshot.scan_id; + ctx.spawn_stream( + self.scan_state.1.clone(), + move |this, scan_state, ctx| { + if matches!(scan_state, ScanState::Idle) && this.snapshot.scan_id > scan_id { + ctx.halt_stream(); } - scan_state_rx.recv().await; - } - } + }, + |_, _| {}, + ) } fn observe_scan_state(&mut self, scan_state: ScanState, ctx: &mut ModelContext) { @@ -168,16 +166,20 @@ impl Worktree { path.starts_with(&self.snapshot.abs_path) } + fn absolutize(&self, path: &Path) -> PathBuf { + if path.file_name().is_some() { + self.snapshot.abs_path.join(path) + } else { + self.snapshot.abs_path.to_path_buf() + } + } + pub fn load_history( &self, path: &Path, ctx: &AppContext, ) -> impl Future> { - let abs_path = if path.file_name().is_some() { - self.snapshot.abs_path.join(path) - } else { - self.snapshot.abs_path.to_path_buf() - }; + let abs_path = self.absolutize(path); ctx.background_executor().spawn(async move { let mut file = std::fs::File::open(&abs_path)?; let mut base_text = String::new(); @@ -192,7 +194,7 @@ impl Worktree { content: BufferSnapshot, ctx: &AppContext, ) -> Task> { - let abs_path = self.snapshot.abs_path.join(path); + let abs_path = self.absolutize(path); ctx.background_executor().spawn(async move { let buffer_size = content.text_summary().bytes.min(10 * 1024); let file = std::fs::File::create(&abs_path)?; @@ -273,6 +275,25 @@ impl Snapshot { &self.root_name } + fn path_is_pending(&self, path: impl AsRef) -> bool { + if self.entries.is_empty() { + return true; + } + let path = path.as_ref(); + let mut cursor = self.entries.cursor::<_, ()>(); + if cursor.seek(&PathSearch::Exact(path), SeekBias::Left, &()) { + let entry = cursor.item().unwrap(); + if entry.path.as_ref() == path { + return matches!(entry.kind, EntryKind::PendingDir); + } + } + if let Some(entry) = cursor.prev_item() { + matches!(entry.kind, EntryKind::PendingDir) && path.starts_with(entry.path.as_ref()) + } else { + false + } + } + fn entry_for_path(&self, path: impl AsRef) -> Option<&Entry> { let mut cursor = self.entries.cursor::<_, ()>(); if cursor.seek(&PathSearch::Exact(path.as_ref()), SeekBias::Left, &()) { @@ -766,6 +787,7 @@ impl BackgroundScanner { }); } + self.mark_deleted_file_handles(); Ok(()) } @@ -977,19 +999,7 @@ impl BackgroundScanner { }); self.update_ignore_statuses(); - - let mut handles = self.handles.lock(); - let snapshot = self.snapshot.lock(); - handles.retain(|path, handle_state| { - if let Some(handle_state) = Weak::upgrade(&handle_state) { - let mut handle_state = handle_state.lock(); - handle_state.is_deleted = snapshot.entry_for_path(&path).is_none(); - true - } else { - false - } - }); - + self.mark_deleted_file_handles(); true } @@ -1079,6 +1089,20 @@ impl BackgroundScanner { self.snapshot.lock().entries.edit(edits, &()); } + fn mark_deleted_file_handles(&self) { + let mut handles = self.handles.lock(); + let snapshot = self.snapshot.lock(); + handles.retain(|path, handle_state| { + if let Some(handle_state) = Weak::upgrade(&handle_state) { + let mut handle_state = handle_state.lock(); + handle_state.is_deleted = snapshot.entry_for_path(&path).is_none(); + true + } else { + false + } + }); + } + fn fs_entry_for_path(&self, path: Arc, abs_path: &Path) -> Result> { let metadata = match fs::metadata(&abs_path) { Err(err) => { @@ -1137,6 +1161,12 @@ struct UpdateIgnoreStatusJob { pub trait WorktreeHandle { fn file(&self, path: impl AsRef, app: &AppContext) -> FileHandle; + + #[cfg(test)] + fn flush_fs_events<'a>( + &self, + app: &'a gpui::TestAppContext, + ) -> futures_core::future::LocalBoxFuture<'a, ()>; } impl WorktreeHandle for ModelHandle { @@ -1155,7 +1185,7 @@ impl WorktreeHandle for ModelHandle { } else { FileHandleState { path: path.into(), - is_deleted: true, + is_deleted: !tree.path_is_pending(path), } }; @@ -1169,6 +1199,40 @@ impl WorktreeHandle for ModelHandle { state, } } + + // When the worktree's FS event stream sometimes delivers "redundant" events for FS changes that + // occurred before the worktree was constructed. These events can cause the worktree to perfrom + // extra directory scans, and emit extra scan-state notifications. + // + // This function mutates the worktree's directory and waits for those mutations to be picked up, + // to ensure that all redundant FS events have already been processed. + #[cfg(test)] + fn flush_fs_events<'a>( + &self, + app: &'a gpui::TestAppContext, + ) -> futures_core::future::LocalBoxFuture<'a, ()> { + use smol::future::FutureExt; + + let filename = "fs-event-sentinel"; + let root_path = app.read(|ctx| self.read(ctx).abs_path.clone()); + let tree = self.clone(); + async move { + fs::write(root_path.join(filename), "").unwrap(); + tree.condition_with_duration(Duration::from_secs(5), &app, |tree, _| { + tree.entry_for_path(filename).is_some() + }) + .await; + + fs::remove_file(root_path.join(filename)).unwrap(); + tree.condition_with_duration(Duration::from_secs(5), &app, |tree, _| { + tree.entry_for_path(filename).is_none() + }) + .await; + + app.read(|ctx| tree.read(ctx).scan_complete()).await; + } + .boxed_local() + } } pub enum FileIter<'a> { @@ -1282,7 +1346,7 @@ mod tests { use crate::editor::Buffer; use crate::test::*; use anyhow::Result; - use gpui::{App, TestAppContext}; + use gpui::App; use rand::prelude::*; use serde_json::json; use std::env; @@ -1384,6 +1448,31 @@ mod tests { }); } + #[test] + fn test_save_in_single_file_worktree() { + App::test_async((), |mut app| async move { + let dir = temp_tree(json!({ + "file1": "the old contents", + })); + + let tree = app.add_model(|ctx| Worktree::new(dir.path().join("file1"), ctx)); + app.read(|ctx| tree.read(ctx).scan_complete()).await; + app.read(|ctx| assert_eq!(tree.read(ctx).file_count(), 1)); + + let buffer = + app.add_model(|ctx| Buffer::new(1, "a line of text.\n".repeat(10 * 1024), ctx)); + + let file = app.read(|ctx| tree.file("", ctx)); + app.update(|ctx| { + assert_eq!(file.path().file_name(), None); + smol::block_on(file.save(buffer.read(ctx).snapshot(), ctx.as_ref())).unwrap(); + }); + + let history = app.read(|ctx| file.load_history(ctx)).await.unwrap(); + app.read(|ctx| assert_eq!(history.base_text.as_ref(), buffer.read(ctx).text())); + }); + } + #[test] fn test_rescan_simple() { App::test_async((), |mut app| async move { @@ -1402,23 +1491,39 @@ mod tests { })); let tree = app.add_model(|ctx| Worktree::new(dir.path(), ctx)); - app.read(|ctx| tree.read(ctx).scan_complete()).await; - flush_fs_events(&tree, &app).await; - - let (file2, file3, file4, file5) = app.read(|ctx| { + let (file2, file3, file4, file5, non_existent_file) = app.read(|ctx| { ( tree.file("a/file2", ctx), tree.file("a/file3", ctx), tree.file("b/c/file4", ctx), tree.file("b/c/file5", ctx), + tree.file("a/filex", ctx), ) }); + // The worktree hasn't scanned the directories containing these paths, + // so it can't determine that the paths are deleted. + assert!(!file2.is_deleted()); + assert!(!file3.is_deleted()); + assert!(!file4.is_deleted()); + assert!(!file5.is_deleted()); + assert!(!non_existent_file.is_deleted()); + + // After scanning, the worktree knows which files exist and which don't. + app.read(|ctx| tree.read(ctx).scan_complete()).await; + assert!(!file2.is_deleted()); + assert!(!file3.is_deleted()); + assert!(!file4.is_deleted()); + assert!(!file5.is_deleted()); + assert!(non_existent_file.is_deleted()); + + tree.flush_fs_events(&app).await; std::fs::rename(dir.path().join("a/file3"), dir.path().join("b/c/file3")).unwrap(); std::fs::remove_file(dir.path().join("b/c/file5")).unwrap(); std::fs::rename(dir.path().join("b/c"), dir.path().join("d")).unwrap(); std::fs::rename(dir.path().join("a/file2"), dir.path().join("a/file2.new")).unwrap(); - app.read(|ctx| tree.read(ctx).next_scan_complete()).await; + tree.update(&mut app, |tree, ctx| tree.next_scan_complete(ctx)) + .await; app.read(|ctx| { assert_eq!( @@ -1469,7 +1574,7 @@ mod tests { let tree = app.add_model(|ctx| Worktree::new(dir.path(), ctx)); app.read(|ctx| tree.read(ctx).scan_complete()).await; - flush_fs_events(&tree, &app).await; + tree.flush_fs_events(&app).await; app.read(|ctx| { let tree = tree.read(ctx); let tracked = tree.entry_for_path("tracked-dir/tracked-file1").unwrap(); @@ -1480,7 +1585,8 @@ mod tests { fs::write(dir.path().join("tracked-dir/tracked-file2"), "").unwrap(); fs::write(dir.path().join("ignored-dir/ignored-file2"), "").unwrap(); - app.read(|ctx| tree.read(ctx).next_scan_complete()).await; + tree.update(&mut app, |tree, ctx| tree.next_scan_complete(ctx)) + .await; app.read(|ctx| { let tree = tree.read(ctx); let dot_git = tree.entry_for_path(".git").unwrap(); @@ -1493,6 +1599,59 @@ mod tests { }); } + #[test] + fn test_path_is_pending() { + let mut snapshot = Snapshot { + id: 0, + scan_id: 0, + abs_path: Path::new("").into(), + entries: Default::default(), + ignores: Default::default(), + root_name: Default::default(), + }; + + snapshot.entries.edit( + vec![ + Edit::Insert(Entry { + path: Path::new("b").into(), + kind: EntryKind::Dir, + inode: 0, + is_ignored: false, + is_symlink: false, + }), + Edit::Insert(Entry { + path: Path::new("b/a").into(), + kind: EntryKind::Dir, + inode: 0, + is_ignored: false, + is_symlink: false, + }), + Edit::Insert(Entry { + path: Path::new("b/c").into(), + kind: EntryKind::PendingDir, + inode: 0, + is_ignored: false, + is_symlink: false, + }), + Edit::Insert(Entry { + path: Path::new("b/e").into(), + kind: EntryKind::Dir, + inode: 0, + is_ignored: false, + is_symlink: false, + }), + ], + &(), + ); + + assert!(!snapshot.path_is_pending("b/a")); + assert!(!snapshot.path_is_pending("b/b")); + assert!(snapshot.path_is_pending("b/c")); + assert!(snapshot.path_is_pending("b/c/x")); + assert!(!snapshot.path_is_pending("b/d")); + assert!(!snapshot.path_is_pending("b/e")); + } + #[test] fn test_mounted_volume_paths() { let paths = mounted_volume_paths(); @@ -1776,29 +1935,4 @@ mod tests { paths } } - - // When the worktree's FS event stream sometimes delivers "redundant" events for FS changes that - // occurred before the worktree was constructed. These events can cause the worktree to perfrom - // extra directory scans, and emit extra scan-state notifications. - // - // This function mutates the worktree's directory and waits for those mutations to be picked up, - // to ensure that all redundant FS events have already been processed. - async fn flush_fs_events(tree: &ModelHandle, app: &TestAppContext) { - let filename = "fs-event-sentinel"; - let root_path = app.read(|ctx| tree.read(ctx).abs_path.clone()); - - fs::write(root_path.join(filename), "").unwrap(); - tree.condition_with_duration(Duration::from_secs(5), &app, |tree, _| { - tree.entry_for_path(filename).is_some() - }) - .await; - - fs::remove_file(root_path.join(filename)).unwrap(); - tree.condition_with_duration(Duration::from_secs(5), &app, |tree, _| { - tree.entry_for_path(filename).is_none() - }) - .await; - - app.read(|ctx| tree.read(ctx).scan_complete()).await; - } }