Move file handle from buffer to buffer view

Max Brunsfeld created

Change summary

zed/src/editor/buffer/mod.rs        | 67 ++++++++---------------------
zed/src/editor/buffer_view.rs       | 49 +++++++++++++++------
zed/src/workspace/workspace.rs      | 25 +++++++----
zed/src/workspace/workspace_view.rs | 68 ++++++++++++++++++++++++++++--
zed/src/worktree.rs                 |  8 +-
5 files changed, 136 insertions(+), 81 deletions(-)

Detailed changes

zed/src/editor/buffer/mod.rs 🔗

@@ -18,16 +18,14 @@ use crate::{
     worktree::FileHandle,
 };
 use anyhow::{anyhow, Result};
-use gpui::{AppContext, Entity, ModelContext};
+use gpui::{Entity, ModelContext};
 use lazy_static::lazy_static;
 use rand::prelude::*;
 use std::{
     cmp,
-    ffi::OsString,
     hash::BuildHasher,
     iter::{self, Iterator},
     ops::{AddAssign, Range},
-    path::Path,
     str,
     sync::Arc,
     time::{Duration, Instant},
@@ -59,7 +57,6 @@ type HashMap<K, V> = std::collections::HashMap<K, V>;
 type HashSet<T> = std::collections::HashSet<T>;
 
 pub struct Buffer {
-    file: Option<FileHandle>,
     fragments: SumTree<Fragment>,
     insertion_splits: HashMap<time::Local, SumTree<InsertionSplit>>,
     pub version: time::Global,
@@ -359,24 +356,18 @@ impl Buffer {
         base_text: T,
         ctx: &mut ModelContext<Self>,
     ) -> Self {
-        Self::build(replica_id, None, History::new(base_text.into()), ctx)
+        Self::build(replica_id, History::new(base_text.into()), ctx)
     }
 
     pub fn from_history(
         replica_id: ReplicaId,
-        file: FileHandle,
         history: History,
         ctx: &mut ModelContext<Self>,
     ) -> Self {
-        Self::build(replica_id, Some(file), history, ctx)
+        Self::build(replica_id, history, ctx)
     }
 
-    fn build(
-        replica_id: ReplicaId,
-        file: Option<FileHandle>,
-        history: History,
-        ctx: &mut ModelContext<Self>,
-    ) -> Self {
+    fn build(replica_id: ReplicaId, history: History, _: &mut ModelContext<Self>) -> Self {
         let mut insertion_splits = HashMap::default();
         let mut fragments = SumTree::new();
 
@@ -425,12 +416,7 @@ impl Buffer {
             });
         }
 
-        if let Some(file) = file.as_ref() {
-            file.observe_from_model(ctx, |_, _, ctx| ctx.emit(Event::FileHandleChanged));
-        }
-
         Self {
-            file,
             fragments,
             insertion_splits,
             version: time::Global::new(),
@@ -448,41 +434,27 @@ impl Buffer {
         }
     }
 
-    pub fn file_name(&self, ctx: &AppContext) -> Option<OsString> {
-        self.file.as_ref().and_then(|file| file.file_name(ctx))
-    }
-
-    pub fn path(&self) -> Option<Arc<Path>> {
-        self.file.as_ref().map(|file| file.path())
-    }
-
-    pub fn entry_id(&self) -> Option<(usize, Arc<Path>)> {
-        self.file.as_ref().map(|file| file.entry_id())
-    }
-
     pub fn snapshot(&self) -> Snapshot {
         Snapshot {
             fragments: self.fragments.clone(),
         }
     }
 
-    pub fn save(&mut self, ctx: &mut ModelContext<Self>) -> LocalBoxFuture<'static, Result<()>> {
-        if let Some(file) = &self.file {
-            dbg!(file.path());
-
-            let snapshot = self.snapshot();
-            let version = self.version.clone();
-            let save_task = file.save(snapshot, ctx.as_ref());
-            let task = ctx.spawn(save_task, |me, save_result, ctx| {
-                if save_result.is_ok() {
-                    me.did_save(version, ctx);
-                }
-                save_result
-            });
-            Box::pin(task)
-        } else {
-            Box::pin(async { Ok(()) })
-        }
+    pub fn save(
+        &mut self,
+        file: &FileHandle,
+        ctx: &mut ModelContext<Self>,
+    ) -> LocalBoxFuture<'static, Result<()>> {
+        let snapshot = self.snapshot();
+        let version = self.version.clone();
+        let save_task = file.save(snapshot, ctx.as_ref());
+        let task = ctx.spawn(save_task, |me, save_result, ctx| {
+            if save_result.is_ok() {
+                me.did_save(version, ctx);
+            }
+            save_result
+        });
+        Box::pin(task)
     }
 
     fn did_save(&mut self, version: time::Global, ctx: &mut ModelContext<Buffer>) {
@@ -1763,7 +1735,6 @@ impl Buffer {
 impl Clone for Buffer {
     fn clone(&self) -> Self {
         Self {
-            file: self.file.clone(),
             fragments: self.fragments.clone(),
             insertion_splits: self.insertion_splits.clone(),
             version: self.version.clone(),

zed/src/editor/buffer_view.rs 🔗

@@ -2,7 +2,7 @@ use super::{
     buffer, movement, Anchor, Bias, Buffer, BufferElement, DisplayMap, DisplayPoint, Point,
     Selection, SelectionSetId, ToOffset, ToPoint,
 };
-use crate::{settings::Settings, watch, workspace};
+use crate::{settings::Settings, watch, workspace, worktree::FileHandle};
 use anyhow::Result;
 use futures_core::future::LocalBoxFuture;
 use gpui::{
@@ -98,6 +98,7 @@ pub enum SelectAction {
 pub struct BufferView {
     handle: WeakViewHandle<Self>,
     buffer: ModelHandle<Buffer>,
+    file: Option<FileHandle>,
     display_map: ModelHandle<DisplayMap>,
     selection_set_id: SelectionSetId,
     pending_selection: Option<Selection>,
@@ -120,18 +121,23 @@ struct ClipboardSelection {
 impl BufferView {
     pub fn single_line(settings: watch::Receiver<Settings>, ctx: &mut ViewContext<Self>) -> Self {
         let buffer = ctx.add_model(|ctx| Buffer::new(0, String::new(), ctx));
-        let mut view = Self::for_buffer(buffer, settings, ctx);
+        let mut view = Self::for_buffer(buffer, None, settings, ctx);
         view.single_line = true;
         view
     }
 
     pub fn for_buffer(
         buffer: ModelHandle<Buffer>,
+        file: Option<FileHandle>,
         settings: watch::Receiver<Settings>,
         ctx: &mut ViewContext<Self>,
     ) -> Self {
         settings.notify_view_on_change(ctx);
 
+        if let Some(file) = file.as_ref() {
+            file.observe_from_view(ctx, |_, _, ctx| ctx.emit(Event::FileHandleChanged));
+        }
+
         ctx.observe(&buffer, Self::on_buffer_changed);
         ctx.subscribe_to_model(&buffer, Self::on_buffer_event);
         let display_map = ctx.add_model(|ctx| {
@@ -157,6 +163,7 @@ impl BufferView {
         Self {
             handle: ctx.handle().downgrade(),
             buffer,
+            file,
             display_map,
             selection_set_id,
             pending_selection: None,
@@ -405,7 +412,7 @@ impl BufferView {
         Ok(())
     }
 
-    fn insert(&mut self, text: &String, ctx: &mut ViewContext<Self>) {
+    pub fn insert(&mut self, text: &String, ctx: &mut ViewContext<Self>) {
         let mut offset_ranges = SmallVec::<[Range<usize>; 32]>::new();
         {
             let buffer = self.buffer.read(ctx);
@@ -1343,9 +1350,10 @@ impl workspace::Item for Buffer {
     fn build_view(
         buffer: ModelHandle<Self>,
         settings: watch::Receiver<Settings>,
+        file: Option<FileHandle>,
         ctx: &mut ViewContext<Self::View>,
     ) -> Self::View {
-        BufferView::for_buffer(buffer, settings, ctx)
+        BufferView::for_buffer(buffer, file, settings, ctx)
     }
 }
 
@@ -1362,28 +1370,39 @@ impl workspace::ItemView for BufferView {
     }
 
     fn title(&self, app: &AppContext) -> std::string::String {
-        if let Some(name) = self.buffer.read(app).file_name(app) {
+        let filename = self.file.as_ref().and_then(|file| file.file_name(app));
+        if let Some(name) = filename {
             name.to_string_lossy().into()
         } else {
             "untitled".into()
         }
     }
 
-    fn entry_id(&self, app: &AppContext) -> Option<(usize, Arc<Path>)> {
-        self.buffer.read(app).entry_id()
+    fn entry_id(&self, _: &AppContext) -> Option<(usize, Arc<Path>)> {
+        self.file.as_ref().map(|file| file.entry_id())
     }
 
     fn clone_on_split(&self, ctx: &mut ViewContext<Self>) -> Option<Self>
     where
         Self: Sized,
     {
-        let clone = BufferView::for_buffer(self.buffer.clone(), self.settings.clone(), ctx);
+        let clone = BufferView::for_buffer(
+            self.buffer.clone(),
+            self.file.clone(),
+            self.settings.clone(),
+            ctx,
+        );
         *clone.scroll_position.lock() = *self.scroll_position.lock();
         Some(clone)
     }
 
     fn save(&self, ctx: &mut ViewContext<Self>) -> LocalBoxFuture<'static, Result<()>> {
-        self.buffer.update(ctx, |buffer, ctx| buffer.save(ctx))
+        if let Some(file) = self.file.as_ref() {
+            self.buffer
+                .update(ctx, |buffer, ctx| buffer.save(file, ctx))
+        } else {
+            Box::pin(async { Ok(()) })
+        }
     }
 
     fn is_dirty(&self, ctx: &AppContext) -> bool {
@@ -1406,7 +1425,7 @@ mod tests {
                 app.add_model(|ctx| Buffer::new(0, "aaaaaa\nbbbbbb\ncccccc\ndddddd\n", ctx));
             let settings = settings::channel(&app.font_cache()).unwrap().1;
             let (_, buffer_view) =
-                app.add_window(|ctx| BufferView::for_buffer(buffer, settings, ctx));
+                app.add_window(|ctx| BufferView::for_buffer(buffer, None, settings, ctx));
 
             buffer_view.update(app, |view, ctx| {
                 view.begin_selection(DisplayPoint::new(2, 2), false, ctx);
@@ -1521,7 +1540,7 @@ mod tests {
 
             let settings = settings::channel(&font_cache).unwrap().1;
             let (_, view) =
-                app.add_window(|ctx| BufferView::for_buffer(buffer.clone(), settings, ctx));
+                app.add_window(|ctx| BufferView::for_buffer(buffer.clone(), None, settings, ctx));
 
             let layouts = view
                 .read(app)
@@ -1560,7 +1579,7 @@ mod tests {
             });
             let settings = settings::channel(&app.font_cache()).unwrap().1;
             let (_, view) =
-                app.add_window(|ctx| BufferView::for_buffer(buffer.clone(), settings, ctx));
+                app.add_window(|ctx| BufferView::for_buffer(buffer.clone(), None, settings, ctx));
 
             view.update(app, |view, ctx| {
                 view.select_display_ranges(
@@ -1632,7 +1651,7 @@ mod tests {
             let buffer = app.add_model(|ctx| Buffer::new(0, sample_text(6, 6), ctx));
             let settings = settings::channel(&app.font_cache()).unwrap().1;
             let (_, view) =
-                app.add_window(|ctx| BufferView::for_buffer(buffer.clone(), settings, ctx));
+                app.add_window(|ctx| BufferView::for_buffer(buffer.clone(), None, settings, ctx));
 
             buffer.update(app, |buffer, ctx| {
                 buffer.edit(
@@ -1675,7 +1694,7 @@ mod tests {
             });
             let settings = settings::channel(&app.font_cache()).unwrap().1;
             let (_, view) =
-                app.add_window(|ctx| BufferView::for_buffer(buffer.clone(), settings, ctx));
+                app.add_window(|ctx| BufferView::for_buffer(buffer.clone(), None, settings, ctx));
 
             view.update(app, |view, ctx| {
                 view.select_display_ranges(
@@ -1706,7 +1725,7 @@ mod tests {
             let buffer = app.add_model(|ctx| Buffer::new(0, "one two three four five six ", ctx));
             let settings = settings::channel(&app.font_cache()).unwrap().1;
             let view = app
-                .add_window(|ctx| BufferView::for_buffer(buffer.clone(), settings, ctx))
+                .add_window(|ctx| BufferView::for_buffer(buffer.clone(), None, settings, ctx))
                 .1;
 
             // Cut with three selections. Clipboard text is divided into three slices.

zed/src/workspace/workspace.rs 🔗

@@ -4,7 +4,7 @@ use crate::{
     settings::Settings,
     time::ReplicaId,
     watch,
-    worktree::{Worktree, WorktreeHandle as _},
+    worktree::{FileHandle, Worktree, WorktreeHandle as _},
 };
 use anyhow::anyhow;
 use gpui::{AppContext, Entity, Handle, ModelContext, ModelHandle, MutableAppContext, ViewContext};
@@ -25,6 +25,7 @@ where
     fn build_view(
         handle: ModelHandle<Self>,
         settings: watch::Receiver<Settings>,
+        file: Option<FileHandle>,
         ctx: &mut ViewContext<Self::View>,
     ) -> Self::View;
 }
@@ -34,6 +35,7 @@ pub trait ItemHandle: Debug + Send + Sync {
         &self,
         window_id: usize,
         settings: watch::Receiver<Settings>,
+        file: Option<FileHandle>,
         app: &mut MutableAppContext,
     ) -> Box<dyn ItemViewHandle>;
     fn id(&self) -> usize;
@@ -45,9 +47,12 @@ impl<T: 'static + Item> ItemHandle for ModelHandle<T> {
         &self,
         window_id: usize,
         settings: watch::Receiver<Settings>,
+        file: Option<FileHandle>,
         app: &mut MutableAppContext,
     ) -> Box<dyn ItemViewHandle> {
-        Box::new(app.add_view(window_id, |ctx| T::build_view(self.clone(), settings, ctx)))
+        Box::new(app.add_view(window_id, |ctx| {
+            T::build_view(self.clone(), settings, file, ctx)
+        }))
     }
 
     fn id(&self) -> usize {
@@ -148,7 +153,7 @@ impl Workspace {
         &mut self,
         (worktree_id, path): (usize, Arc<Path>),
         ctx: &mut ModelContext<'_, Self>,
-    ) -> anyhow::Result<Pin<Box<dyn Future<Output = OpenResult> + Send>>> {
+    ) -> anyhow::Result<Pin<Box<dyn Future<Output = (OpenResult, FileHandle)> + Send>>> {
         let worktree = self
             .worktrees
             .get(&worktree_id)
@@ -160,18 +165,20 @@ impl Workspace {
             .inode_for_path(&path)
             .ok_or_else(|| anyhow!("path {:?} does not exist", path))?;
 
+        let file = worktree.file(path.clone(), ctx.as_ref())?;
+
         let item_key = (worktree_id, inode);
         if let Some(item) = self.items.get(&item_key).cloned() {
             return Ok(async move {
                 match item {
                     OpenedItem::Loaded(handle) => {
-                        return Ok(handle);
+                        return (Ok(handle), file);
                     }
                     OpenedItem::Loading(rx) => loop {
                         rx.updated().await;
 
                         if let Some(result) = smol::block_on(rx.read()).clone() {
-                            return result;
+                            return (result, file);
                         }
                     },
                 }
@@ -180,7 +187,6 @@ impl Workspace {
         }
 
         let replica_id = self.replica_id;
-        let file = worktree.file(path.clone(), ctx.as_ref())?;
         let history = file.load_history(ctx.as_ref());
 
         let (mut tx, rx) = watch::channel(None);
@@ -190,7 +196,7 @@ impl Workspace {
             move |me, history: anyhow::Result<History>, ctx| match history {
                 Ok(history) => {
                     let handle = Box::new(
-                        ctx.add_model(|ctx| Buffer::from_history(replica_id, file, history, ctx)),
+                        ctx.add_model(|ctx| Buffer::from_history(replica_id, history, ctx)),
                     ) as Box<dyn ItemHandle>;
                     me.items
                         .insert(item_key, OpenedItem::Loaded(handle.clone()));
@@ -282,14 +288,15 @@ mod tests {
                 )
             });
 
-            let handle_1 = future_1.await.unwrap();
-            let handle_2 = future_2.await.unwrap();
+            let handle_1 = future_1.await.0.unwrap();
+            let handle_2 = future_2.await.0.unwrap();
             assert_eq!(handle_1.id(), handle_2.id());
 
             // Open the same entry again now that it has loaded
             let handle_3 = workspace
                 .update(&mut app, |w, app| w.open_entry(entry, app).unwrap())
                 .await
+                .0
                 .unwrap();
 
             assert_eq!(handle_3.id(), handle_1.id());

zed/src/workspace/workspace_view.rs 🔗

@@ -250,13 +250,14 @@ impl WorkspaceView {
                 error!("{}", error);
                 None
             }
-            Ok(item) => {
+            Ok(future) => {
                 let settings = self.settings.clone();
-                Some(ctx.spawn(item, move |me, item, ctx| {
+                Some(ctx.spawn(future, move |me, (item, file), ctx| {
                     me.loading_entries.remove(&entry);
                     match item {
                         Ok(item) => {
-                            let item_view = item.add_view(ctx.window_id(), settings, ctx.as_mut());
+                            let item_view =
+                                item.add_view(ctx.window_id(), settings, Some(file), ctx.as_mut());
                             me.add_item(item_view, ctx);
                         }
                         Err(error) => {
@@ -417,10 +418,10 @@ impl View for WorkspaceView {
 #[cfg(test)]
 mod tests {
     use super::{pane, Workspace, WorkspaceView};
-    use crate::{settings, test::temp_tree, workspace::WorkspaceHandle as _};
+    use crate::{editor::BufferView, settings, test::temp_tree, workspace::WorkspaceHandle as _};
     use gpui::App;
     use serde_json::json;
-    use std::collections::HashSet;
+    use std::{collections::HashSet, os::unix};
 
     #[test]
     fn test_open_entry() {
@@ -575,6 +576,63 @@ mod tests {
         });
     }
 
+    #[test]
+    fn test_open_two_paths_to_the_same_file() {
+        use crate::workspace::ItemViewHandle;
+
+        App::test_async((), |mut app| async move {
+            // Create a worktree with a symlink:
+            //   dir
+            //   ├── hello.txt
+            //   └── hola.txt -> hello.txt
+            let temp_dir = temp_tree(json!({ "hello.txt": "hi" }));
+            let dir = temp_dir.path();
+            unix::fs::symlink(dir.join("hello.txt"), dir.join("hola.txt")).unwrap();
+
+            let workspace = app.add_model(|ctx| Workspace::new(vec![dir.into()], ctx));
+            let settings = settings::channel(&app.font_cache()).unwrap().1;
+            let (_, workspace_view) =
+                app.add_window(|ctx| WorkspaceView::new(workspace.clone(), settings, ctx));
+
+            // Simultaneously open both the original file and the symlink to the same file.
+            app.update(|ctx| {
+                workspace_view.update(ctx, |view, ctx| {
+                    view.open_paths(&[dir.join("hello.txt"), dir.join("hola.txt")], ctx)
+                })
+            })
+            .await;
+
+            // The same content shows up with two different editors.
+            let buffer_views = app.read(|ctx| {
+                workspace_view
+                    .read(ctx)
+                    .active_pane()
+                    .read(ctx)
+                    .items()
+                    .iter()
+                    .map(|i| i.to_any().downcast::<BufferView>().unwrap())
+                    .collect::<Vec<_>>()
+            });
+            app.read(|ctx| {
+                assert_eq!(buffer_views[0].title(ctx), "hello.txt");
+                assert_eq!(buffer_views[1].title(ctx), "hola.txt");
+                assert_eq!(buffer_views[0].read(ctx).text(ctx), "hi");
+                assert_eq!(buffer_views[1].read(ctx).text(ctx), "hi");
+            });
+
+            // When modifying one buffer, the changes appear in both editors.
+            app.update(|ctx| {
+                buffer_views[0].update(ctx, |buf, ctx| {
+                    buf.insert(&"oh, ".to_string(), ctx);
+                });
+            });
+            app.read(|ctx| {
+                assert_eq!(buffer_views[0].read(ctx).text(ctx), "oh, hi");
+                assert_eq!(buffer_views[1].read(ctx).text(ctx), "oh, hi");
+            });
+        });
+    }
+
     #[test]
     fn test_pane_actions() {
         App::test_async((), |mut app| async move {

zed/src/worktree.rs 🔗

@@ -9,7 +9,7 @@ use crate::{
 use ::ignore::gitignore::Gitignore;
 use anyhow::{anyhow, Context, Result};
 pub use fuzzy::{match_paths, PathMatch};
-use gpui::{scoped_pool, AppContext, Entity, ModelContext, ModelHandle, Task};
+use gpui::{scoped_pool, AppContext, Entity, ModelContext, ModelHandle, Task, View, ViewContext};
 use lazy_static::lazy_static;
 use parking_lot::Mutex;
 use postage::{
@@ -419,10 +419,10 @@ impl FileHandle {
         (self.worktree.id(), self.path())
     }
 
-    pub fn observe_from_model<T: Entity>(
+    pub fn observe_from_view<T: View>(
         &self,
-        ctx: &mut ModelContext<T>,
-        mut callback: impl FnMut(&mut T, FileHandle, &mut ModelContext<T>) + 'static,
+        ctx: &mut ViewContext<T>,
+        mut callback: impl FnMut(&mut T, FileHandle, &mut ViewContext<T>) + 'static,
     ) {
         let mut prev_state = self.state.lock().clone();
         let cur_state = Arc::downgrade(&self.state);