Port buffer reload bug fixes back to gpui1 crates

Max Brunsfeld created

Change summary

crates/copilot/src/copilot.rs  | 20 +++----
crates/language/src/buffer.rs  | 90 ++++++++++++++++++++----------------
crates/project/src/project.rs  |  4 
crates/project/src/worktree.rs |  2 
crates/rope/src/rope.rs        |  6 ++
5 files changed, 67 insertions(+), 55 deletions(-)

Detailed changes

crates/copilot/src/copilot.rs 🔗

@@ -1051,17 +1051,15 @@ mod tests {
         );
 
         // Ensure updates to the file are reflected in the LSP.
-        buffer_1
-            .update(cx, |buffer, cx| {
-                buffer.file_updated(
-                    Arc::new(File {
-                        abs_path: "/root/child/buffer-1".into(),
-                        path: Path::new("child/buffer-1").into(),
-                    }),
-                    cx,
-                )
-            })
-            .await;
+        buffer_1.update(cx, |buffer, cx| {
+            buffer.file_updated(
+                Arc::new(File {
+                    abs_path: "/root/child/buffer-1".into(),
+                    path: Path::new("child/buffer-1").into(),
+                }),
+                cx,
+            )
+        });
         assert_eq!(
             lsp.receive_notification::<lsp::notification::DidCloseTextDocument>()
                 .await,

crates/language/src/buffer.rs 🔗

@@ -17,7 +17,7 @@ use crate::{
 };
 use anyhow::{anyhow, Result};
 pub use clock::ReplicaId;
-use futures::FutureExt as _;
+use futures::channel::oneshot;
 use gpui::{fonts::HighlightStyle, AppContext, Entity, ModelContext, Task};
 use lsp::LanguageServerId;
 use parking_lot::Mutex;
@@ -45,7 +45,7 @@ pub use text::{Buffer as TextBuffer, BufferSnapshot as TextBufferSnapshot, *};
 use theme::SyntaxTheme;
 #[cfg(any(test, feature = "test-support"))]
 use util::RandomCharIter;
-use util::{RangeExt, TryFutureExt as _};
+use util::RangeExt;
 
 #[cfg(any(test, feature = "test-support"))]
 pub use {tree_sitter_rust, tree_sitter_typescript};
@@ -62,6 +62,7 @@ pub struct Buffer {
     saved_mtime: SystemTime,
     transaction_depth: usize,
     was_dirty_before_starting_transaction: Option<bool>,
+    reload_task: Option<Task<Result<()>>>,
     language: Option<Arc<Language>>,
     autoindent_requests: Vec<Arc<AutoindentRequest>>,
     pending_autoindent: Option<Task<()>>,
@@ -509,6 +510,7 @@ impl Buffer {
             saved_mtime,
             saved_version: buffer.version(),
             saved_version_fingerprint: buffer.as_rope().fingerprint(),
+            reload_task: None,
             transaction_depth: 0,
             was_dirty_before_starting_transaction: None,
             text: buffer,
@@ -608,37 +610,52 @@ impl Buffer {
         cx.notify();
     }
 
-    pub fn reload(&mut self, cx: &mut ModelContext<Self>) -> Task<Result<Option<Transaction>>> {
-        cx.spawn(|this, mut cx| async move {
-            if let Some((new_mtime, new_text)) = this.read_with(&cx, |this, cx| {
+    pub fn reload(
+        &mut self,
+        cx: &mut ModelContext<Self>,
+    ) -> oneshot::Receiver<Option<Transaction>> {
+        let (tx, rx) = futures::channel::oneshot::channel();
+        let prev_version = self.text.version();
+        self.reload_task = Some(cx.spawn(|this, mut cx| async move {
+            let Some((new_mtime, new_text)) = this.update(&mut cx, |this, cx| {
                 let file = this.file.as_ref()?.as_local()?;
                 Some((file.mtime(), file.load(cx)))
-            }) {
-                let new_text = new_text.await?;
-                let diff = this
-                    .read_with(&cx, |this, cx| this.diff(new_text, cx))
-                    .await;
-                this.update(&mut cx, |this, cx| {
-                    if this.version() == diff.base_version {
-                        this.finalize_last_transaction();
-                        this.apply_diff(diff, cx);
-                        if let Some(transaction) = this.finalize_last_transaction().cloned() {
-                            this.did_reload(
-                                this.version(),
-                                this.as_rope().fingerprint(),
-                                this.line_ending(),
-                                new_mtime,
-                                cx,
-                            );
-                            return Ok(Some(transaction));
-                        }
-                    }
-                    Ok(None)
-                })
-            } else {
-                Ok(None)
-            }
-        })
+            }) else {
+                return Ok(());
+            };
+
+            let new_text = new_text.await?;
+            let diff = this
+                .update(&mut cx, |this, cx| this.diff(new_text.clone(), cx))
+                .await;
+            this.update(&mut cx, |this, cx| {
+                if this.version() == diff.base_version {
+                    this.finalize_last_transaction();
+                    this.apply_diff(diff, cx);
+                    tx.send(this.finalize_last_transaction().cloned()).ok();
+
+                    this.did_reload(
+                        this.version(),
+                        this.as_rope().fingerprint(),
+                        this.line_ending(),
+                        new_mtime,
+                        cx,
+                    );
+                } else {
+                    this.did_reload(
+                        prev_version,
+                        Rope::text_fingerprint(&new_text),
+                        this.line_ending(),
+                        new_mtime,
+                        cx,
+                    );
+                }
+
+                this.reload_task.take();
+            });
+            Ok(())
+        }));
+        rx
     }
 
     pub fn did_reload(
@@ -667,13 +684,8 @@ impl Buffer {
         cx.notify();
     }
 
-    pub fn file_updated(
-        &mut self,
-        new_file: Arc<dyn File>,
-        cx: &mut ModelContext<Self>,
-    ) -> Task<()> {
+    pub fn file_updated(&mut self, new_file: Arc<dyn File>, cx: &mut ModelContext<Self>) {
         let mut file_changed = false;
-        let mut task = Task::ready(());
 
         if let Some(old_file) = self.file.as_ref() {
             if new_file.path() != old_file.path() {
@@ -693,8 +705,7 @@ impl Buffer {
                     file_changed = true;
 
                     if !self.is_dirty() {
-                        let reload = self.reload(cx).log_err().map(drop);
-                        task = cx.foreground().spawn(reload);
+                        self.reload(cx).close();
                     }
                 }
             }
@@ -708,7 +719,6 @@ impl Buffer {
             cx.emit(Event::FileHandleChanged);
             cx.notify();
         }
-        task
     }
 
     pub fn diff_base(&self) -> Option<&str> {

crates/project/src/project.rs 🔗

@@ -6190,7 +6190,7 @@ impl Project {
                                 .log_err();
                         }
 
-                        buffer.file_updated(Arc::new(new_file), cx).detach();
+                        buffer.file_updated(Arc::new(new_file), cx);
                     }
                 }
             });
@@ -7182,7 +7182,7 @@ impl Project {
                     .ok_or_else(|| anyhow!("no such worktree"))?;
                 let file = File::from_proto(file, worktree, cx)?;
                 buffer.update(cx, |buffer, cx| {
-                    buffer.file_updated(Arc::new(file), cx).detach();
+                    buffer.file_updated(Arc::new(file), cx);
                 });
                 this.detect_language_for_buffer(&buffer, cx);
             }

crates/project/src/worktree.rs 🔗

@@ -959,7 +959,7 @@ impl LocalWorktree {
 
                 buffer_handle.update(&mut cx, |buffer, cx| {
                     if has_changed_file {
-                        buffer.file_updated(new_file, cx).detach();
+                        buffer.file_updated(new_file, cx);
                     }
                 });
             }

crates/rope/src/rope.rs 🔗

@@ -41,6 +41,10 @@ impl Rope {
         Self::default()
     }
 
+    pub fn text_fingerprint(text: &str) -> RopeFingerprint {
+        bromberg_sl2::hash_strict(text.as_bytes())
+    }
+
     pub fn append(&mut self, rope: Rope) {
         let mut chunks = rope.chunks.cursor::<()>();
         chunks.next(&());
@@ -931,7 +935,7 @@ impl<'a> From<&'a str> for ChunkSummary {
     fn from(text: &'a str) -> Self {
         Self {
             text: TextSummary::from(text),
-            fingerprint: bromberg_sl2::hash_strict(text.as_bytes()),
+            fingerprint: Rope::text_fingerprint(text),
         }
     }
 }