Assign new file handles on buffers when their files change on disk

Co-Authored-By: Antonio Scandurra <me@as-cii.com>
Co-Authored-By: Nathan Sobo <nathan@zed.dev>

Cargo.lock

@@ -2815,6 +2815,7 @@ dependencies = [
  "anyhow",
  "buffer",
  "clock",
+ "futures",
  "gpui",
  "lazy_static",
  "log",
@@ -2828,6 +2829,7 @@ dependencies = [
  "tree-sitter",
  "tree-sitter-rust",
  "unindent",
+ "util",
 ]
 
 [[package]]

crates/buffer/src/lib.rs

@@ -17,12 +17,10 @@ pub use point::*;
 pub use random_char_iter::*;
 pub use rope::{Chunks, Rope, TextSummary};
 use rpc::proto;
-use seahash::SeaHasher;
 pub use selection::*;
 use std::{
     cmp,
     convert::{TryFrom, TryInto},
-    hash::BuildHasher,
     iter::Iterator,
     ops::Range,
     str,
@@ -32,14 +30,16 @@ use std::{
 pub use sum_tree::Bias;
 use sum_tree::{FilterCursor, SumTree};
 
+#[cfg(any(test, feature = "test-support"))]
 #[derive(Clone, Default)]
 struct DeterministicState;
 
-impl BuildHasher for DeterministicState {
+#[cfg(any(test, feature = "test-support"))]
-    type Hasher = SeaHasher;
+impl std::hash::BuildHasher for DeterministicState {
+    type Hasher = seahash::SeaHasher;
 
     fn build_hasher(&self) -> Self::Hasher {
-        SeaHasher::new()
+        seahash::SeaHasher::new()
     }
 }
 

crates/language/Cargo.toml

@@ -4,7 +4,7 @@ version = "0.1.0"
 edition = "2018"
 
 [features]
-test-support = ["rand"]
+test-support = ["rand", "buffer/test-support"]
 
 [dependencies]
 buffer = { path = "../buffer" }
@@ -12,7 +12,9 @@ clock = { path = "../clock" }
 gpui = { path = "../gpui" }
 rpc = { path = "../rpc" }
 theme = { path = "../theme" }
+util = { path = "../util" }
 anyhow = "1.0.38"
+futures = "0.3"
 lazy_static = "1.4"
 log = "0.4"
 parking_lot = "0.11.1"

crates/language/src/lib.rs

@@ -10,6 +10,7 @@ pub use self::{
 use anyhow::{anyhow, Result};
 pub use buffer::{Buffer as TextBuffer, *};
 use clock::ReplicaId;
+use futures::FutureExt as _;
 use gpui::{AppContext, Entity, ModelContext, MutableAppContext, Task};
 use lazy_static::lazy_static;
 use parking_lot::Mutex;
@@ -31,6 +32,7 @@ use std::{
     time::{Duration, Instant, SystemTime, UNIX_EPOCH},
 };
 use tree_sitter::{InputEdit, Parser, QueryCursor, Tree};
+use util::TryFutureExt as _;
 
 thread_local! {
     static PARSER: RefCell<Parser> = RefCell::new(Parser::new());
@@ -83,16 +85,10 @@ pub trait File {
 
     fn entry_id(&self) -> Option<usize>;
 
-    fn set_entry_id(&mut self, entry_id: Option<usize>);
-
     fn mtime(&self) -> SystemTime;
 
-    fn set_mtime(&mut self, mtime: SystemTime);
-
     fn path(&self) -> &Arc<Path>;
 
-    fn set_path(&mut self, path: Arc<Path>);
-
     fn full_path(&self, cx: &AppContext) -> PathBuf;
 
     /// Returns the last component of this handle's absolute path. If this handle refers to the root
@@ -109,6 +105,8 @@ pub trait File {
         cx: &mut MutableAppContext,
     ) -> Task<Result<(clock::Global, SystemTime)>>;
 
+    fn load_local(&self, cx: &AppContext) -> Option<Task<Result<String>>>;
+
     fn buffer_updated(&self, buffer_id: u64, operation: Operation, cx: &mut MutableAppContext);
 
     fn buffer_removed(&self, buffer_id: u64, cx: &mut MutableAppContext);
@@ -256,10 +254,6 @@ impl Buffer {
         self.file.as_deref()
     }
 
-    pub fn file_mut(&mut self) -> Option<&mut dyn File> {
-        self.file.as_mut().map(|f| f.deref_mut() as &mut dyn File)
-    }
-
     pub fn save(
         &mut self,
         cx: &mut ModelContext<Self>,
@@ -300,41 +294,64 @@ impl Buffer {
         cx.emit(Event::Saved);
     }
 
-    pub fn file_renamed(&self, cx: &mut ModelContext<Self>) {
-        cx.emit(Event::FileHandleChanged);
-    }
-
     pub fn file_updated(
         &mut self,
-        new_text: String,
+        new_file: Box<dyn File>,
         cx: &mut ModelContext<Self>,
     ) -> Option<Task<()>> {
-        if let Some(file) = self.file.as_ref() {
+        let old_file = self.file.as_ref()?;
-            cx.emit(Event::FileHandleChanged);
+        let mut file_changed = false;
-            let mtime = file.mtime();
+        let mut task = None;
-            if self.version == self.saved_version {
+
-                return Some(cx.spawn(|this, mut cx| async move {
+        if new_file.path() != old_file.path() {
-                    let diff = this
+            file_changed = true;
-                        .read_with(&cx, |this, cx| this.diff(new_text.into(), cx))
+        }
-                        .await;
+
-                    this.update(&mut cx, |this, cx| {
+        if new_file.is_deleted() {
-                        if this.apply_diff(diff, cx) {
+            if !old_file.is_deleted() {
-                            this.saved_version = this.version.clone();
+                file_changed = true;
-                            this.saved_mtime = mtime;
+                if !self.is_dirty() {
-                            cx.emit(Event::Reloaded);
+                    cx.emit(Event::Dirtied);
+                }
+            }
+        } else {
+            let new_mtime = new_file.mtime();
+            if new_mtime != old_file.mtime() {
+                file_changed = true;
+
+                if !self.is_dirty() {
+                    task = Some(cx.spawn(|this, mut cx| {
+                        async move {
+                            let new_text = this.read_with(&cx, |this, cx| {
+                                this.file.as_ref().and_then(|file| file.load_local(cx))
+                            });
+                            if let Some(new_text) = new_text {
+                                let new_text = new_text.await?;
+                                let diff = this
+                                    .read_with(&cx, |this, cx| this.diff(new_text.into(), cx))
+                                    .await;
+                                this.update(&mut cx, |this, cx| {
+                                    if this.apply_diff(diff, cx) {
+                                        this.saved_version = this.version.clone();
+                                        this.saved_mtime = new_mtime;
+                                        cx.emit(Event::Reloaded);
+                                    }
+                                });
+                            }
+                            Ok(())
                         }
-                    });
+                        .log_err()
-                }));
+                        .map(drop)
+                    }));
+                }
             }
         }
-        None
-    }
 
-    pub fn file_deleted(&mut self, cx: &mut ModelContext<Self>) {
+        if file_changed {
-        if self.version == self.saved_version {
+            cx.emit(Event::FileHandleChanged);
-            cx.emit(Event::Dirtied);
         }
-        cx.emit(Event::FileHandleChanged);
+        self.file = Some(new_file);
+        task
     }
 
     pub fn close(&mut self, cx: &mut ModelContext<Self>) {

crates/project/Cargo.toml

@@ -4,7 +4,7 @@ version = "0.1.0"
 edition = "2018"
 
 [features]
-test-support = []
+test-support = ["language/test-support", "buffer/test-support"]
 
 [dependencies]
 buffer = { path = "../buffer" }
@@ -34,6 +34,7 @@ toml = "0.5"
 
 [dev-dependencies]
 client = { path = "../client", features = ["test-support"] }
+gpui = { path = "../gpui", features = ["test-support"] }
 language = { path = "../language", features = ["test-support"] }
 util = { path = "../util", features = ["test-support"] }
 rpc = { path = "../rpc", features = ["test-support"] }

crates/project/src/worktree.rs

@@ -587,43 +587,40 @@ impl Worktree {
             }
         };
 
+        let worktree_handle = cx.handle();
         let mut buffers_to_delete = Vec::new();
         for (buffer_id, buffer) in open_buffers {
             if let Some(buffer) = buffer.upgrade(cx) {
                 buffer.update(cx, |buffer, cx| {
-                    let buffer_is_clean = !buffer.is_dirty();
+                    if let Some(old_file) = buffer.file() {
-
+                        let new_file = if let Some(entry) = old_file
-                    if let Some(file) = buffer.file_mut() {
-                        if let Some(entry) = file
                             .entry_id()
                             .and_then(|entry_id| self.entry_for_id(entry_id))
                         {
-                            if entry.mtime != file.mtime() {
+                            File {
-                                file.set_mtime(entry.mtime);
+                                entry_id: Some(entry.id),
-                                if let Some(worktree) = self.as_local() {
+                                mtime: entry.mtime,
-                                    if buffer_is_clean {
+                                path: entry.path.clone(),
-                                        let abs_path = worktree.absolutize(file.path().as_ref());
+                                worktree: worktree_handle.clone(),
-                                        refresh_buffer(abs_path, &worktree.fs, cx);
-                                    }
-                                }
                             }
+                        } else if let Some(entry) = self.entry_for_path(old_file.path().as_ref()) {
+                            File {
+                                entry_id: Some(entry.id),
+                                mtime: entry.mtime,
+                                path: entry.path.clone(),
+                                worktree: worktree_handle.clone(),
+                            }
+                        } else {
+                            File {
+                                entry_id: None,
+                                path: old_file.path().clone(),
+                                mtime: old_file.mtime(),
+                                worktree: worktree_handle.clone(),
+                            }
+                        };
 
-                            if entry.path != *file.path() {
+                        if let Some(task) = buffer.file_updated(Box::new(new_file), cx) {
-                                file.set_path(entry.path.clone());
+                            task.detach();
-                                buffer.file_renamed(cx);
-                            }
-                        } else if let Some(entry) = self.entry_for_path(file.path().as_ref()) {
-                            file.set_entry_id(Some(entry.id));
-                            file.set_mtime(entry.mtime);
-                            if let Some(worktree) = self.as_local() {
-                                if buffer_is_clean {
-                                    let abs_path = worktree.absolutize(file.path().as_ref());
-                                    refresh_buffer(abs_path, &worktree.fs, cx);
-                                }
-                            }
-                        } else if !file.is_deleted() {
-                            file.set_entry_id(None);
-                            buffer.file_deleted(cx);
                         }
                     }
                 });
@@ -1172,23 +1169,6 @@ fn build_gitignore(abs_path: &Path, fs: &dyn Fs) -> Result<Gitignore> {
     Ok(builder.build()?)
 }
 
-pub fn refresh_buffer(abs_path: PathBuf, fs: &Arc<dyn Fs>, cx: &mut ModelContext<Buffer>) {
-    let fs = fs.clone();
-    cx.spawn(|buffer, mut cx| async move {
-        match fs.load(&abs_path).await {
-            Err(error) => log::error!("error refreshing buffer after file changed: {}", error),
-            Ok(new_text) => {
-                if let Some(task) =
-                    buffer.update(&mut cx, |buffer, cx| buffer.file_updated(new_text, cx))
-                {
-                    task.await;
-                }
-            }
-        }
-    })
-    .detach()
-}
-
 impl Deref for LocalWorktree {
     type Target = Snapshot;
 
@@ -1787,26 +1767,14 @@ impl language::File for File {
         self.entry_id
     }
 
-    fn set_entry_id(&mut self, entry_id: Option<usize>) {
-        self.entry_id = entry_id;
-    }
-
     fn mtime(&self) -> SystemTime {
         self.mtime
     }
 
-    fn set_mtime(&mut self, mtime: SystemTime) {
-        self.mtime = mtime;
-    }
-
     fn path(&self) -> &Arc<Path> {
         &self.path
     }
 
-    fn set_path(&mut self, path: Arc<Path>) {
-        self.path = path;
-    }
-
     fn full_path(&self, cx: &AppContext) -> PathBuf {
         let worktree = self.worktree.read(cx);
         let mut full_path = PathBuf::new();
@@ -1875,6 +1843,16 @@ impl language::File for File {
         })
     }
 
+    fn load_local(&self, cx: &AppContext) -> Option<Task<Result<String>>> {
+        let worktree = self.worktree.read(cx).as_local()?;
+        let abs_path = worktree.absolutize(&self.path);
+        let fs = worktree.fs.clone();
+        Some(
+            cx.background()
+                .spawn(async move { fs.load(&abs_path).await }),
+        )
+    }
+
     fn buffer_updated(&self, buffer_id: u64, operation: Operation, cx: &mut MutableAppContext) {
         self.worktree.update(cx, |worktree, cx| {
             if let Some((rpc, remote_id)) = match worktree {
@@ -3430,12 +3408,13 @@ mod tests {
         buffer.update(&mut cx, |buffer, cx| {
             buffer.edit(vec![0..0], " ", cx);
             assert!(buffer.is_dirty());
+            assert!(!buffer.has_conflict());
         });
 
         // Change the file on disk again, adding blank lines to the beginning.
         fs::write(&abs_path, "\n\n\nAAAA\naaa\nBB\nbbbbb\n").unwrap();
 
-        // Becaues the buffer is modified, it doesn't reload from disk, but is
+        // Because the buffer is modified, it doesn't reload from disk, but is
         // marked as having a conflict.
         buffer
             .condition(&cx, |buffer, _| buffer.has_conflict())