Wait to update bufferview's file handle until save has completed

Max Brunsfeld created

Change summary

zed/src/editor/buffer/mod.rs  |   3 
zed/src/editor/buffer_view.rs |  21 ++++--
zed/src/workspace.rs          | 116 ++++++++++++++++++++++++++----------
3 files changed, 96 insertions(+), 44 deletions(-)

Detailed changes

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

@@ -3123,8 +3123,7 @@ mod tests {
                 let mut buffers = Vec::new();
                 let mut network = Network::new();
                 for i in 0..PEERS {
-                    let buffer =
-                        ctx.add_model(|_| Buffer::new(i as ReplicaId, base_text.as_str()));
+                    let buffer = ctx.add_model(|_| Buffer::new(i as ReplicaId, base_text.as_str()));
                     buffers.push(buffer);
                     replica_ids.push(i as u16);
                     network.add_peer(i as u16);

zed/src/editor/buffer_view.rs 🔗

@@ -13,7 +13,7 @@ use gpui::{
 use parking_lot::Mutex;
 use serde::{Deserialize, Serialize};
 use smallvec::SmallVec;
-use smol::Timer;
+use smol::{future::FutureExt, Timer};
 use std::{
     cmp::{self, Ordering},
     fmt::Write,
@@ -2136,15 +2136,20 @@ impl workspace::ItemView for BufferView {
 
     fn save(
         &mut self,
-        file: Option<FileHandle>,
+        new_file: Option<FileHandle>,
         ctx: &mut ViewContext<Self>,
     ) -> LocalBoxFuture<'static, Result<()>> {
-        if file.is_some() {
-            self.file = file;
-        }
-        if let Some(file) = self.file.as_ref() {
-            self.buffer
-                .update(ctx, |buffer, ctx| buffer.save(file, ctx))
+        if let Some(file) = new_file.as_ref().or(self.file.as_ref()) {
+            let save = self.buffer.update(ctx, |b, ctx| b.save(file, ctx));
+            ctx.spawn(save, move |this, result, ctx| {
+                if new_file.is_some() && result.is_ok() {
+                    this.file = new_file;
+                    ctx.emit(Event::FileHandleChanged);
+                    ctx.notify();
+                }
+                result
+            })
+            .boxed_local()
         } else {
             Box::pin(async { Ok(()) })
         }

zed/src/workspace.rs 🔗

@@ -369,11 +369,11 @@ impl Workspace {
 
     pub fn open_new_file(&mut self, _: &(), ctx: &mut ViewContext<Self>) {
         let buffer = ctx.add_model(|_| Buffer::new(self.replica_id, ""));
-        let buffer_view = Box::new(ctx.add_view(|ctx| {
+        let buffer_view = ctx.add_view(|ctx| {
             BufferView::for_buffer(buffer.clone(), None, self.settings.clone(), ctx)
-        }));
+        });
         self.untitled_buffers.insert(buffer);
-        self.add_item(buffer_view, ctx);
+        self.add_item(Box::new(buffer_view), ctx);
     }
 
     #[must_use]
@@ -468,41 +468,45 @@ impl Workspace {
         ))
     }
 
+    pub fn active_item(&self, ctx: &ViewContext<Self>) -> Option<Box<dyn ItemViewHandle>> {
+        self.active_pane().read(ctx).active_item()
+    }
+
     pub fn save_active_item(&mut self, _: &(), ctx: &mut ViewContext<Self>) {
-        let handle = ctx.handle();
-        let first_worktree = self.worktrees.iter().next();
-        self.active_pane.update(ctx, move |pane, ctx| {
-            if let Some(item) = pane.active_item() {
-                if item.entry_id(ctx.as_ref()).is_none() {
-                    let start_path = first_worktree
-                        .map_or(Path::new(""), |h| h.read(ctx).abs_path())
-                        .to_path_buf();
-                    ctx.prompt_for_new_path(&start_path, move |path, ctx| {
-                        if let Some(path) = path {
-                            handle.update(ctx, move |this, ctx| {
-                                let file = this.file_for_path(&path, ctx);
-                                let task = item.save(Some(file), ctx.as_mut());
-                                ctx.spawn(task, |_, result, _| {
-                                    if let Err(e) = result {
-                                        error!("failed to save item: {:?}, ", e);
-                                    }
-                                })
-                                .detach()
+        if let Some(item) = self.active_item(ctx) {
+            if item.entry_id(ctx.as_ref()).is_none() {
+                let handle = ctx.handle();
+                let start_path = self
+                    .worktrees
+                    .iter()
+                    .next()
+                    .map_or(Path::new(""), |h| h.read(ctx).abs_path())
+                    .to_path_buf();
+                ctx.prompt_for_new_path(&start_path, move |path, ctx| {
+                    if let Some(path) = path {
+                        handle.update(ctx, move |this, ctx| {
+                            let file = this.file_for_path(&path, ctx);
+                            let task = item.save(Some(file), ctx.as_mut());
+                            ctx.spawn(task, |_, result, _| {
+                                if let Err(e) = result {
+                                    error!("failed to save item: {:?}, ", e);
+                                }
                             })
-                        }
-                    });
-                    return;
-                }
-
-                let task = item.save(None, ctx.as_mut());
-                ctx.spawn(task, |_, result, _| {
-                    if let Err(e) = result {
-                        error!("failed to save item: {:?}, ", e);
+                            .detach()
+                        })
                     }
-                })
-                .detach()
+                });
+                return;
             }
-        });
+
+            let task = item.save(None, ctx.as_mut());
+            ctx.spawn(task, |_, result, _| {
+                if let Err(e) = result {
+                    error!("failed to save item: {:?}, ", e);
+                }
+            })
+            .detach()
+        }
     }
 
     pub fn debug_elements(&mut self, _: &(), ctx: &mut ViewContext<Self>) {
@@ -660,6 +664,7 @@ mod tests {
     use gpui::App;
     use serde_json::json;
     use std::{collections::HashSet, os::unix};
+    use tempdir::TempDir;
 
     #[test]
     fn test_open_paths_action() {
@@ -942,6 +947,49 @@ mod tests {
         });
     }
 
+    #[test]
+    fn test_open_and_save_new_file() {
+        App::test_async((), |mut app| async move {
+            let dir = TempDir::new("test-new-file").unwrap();
+            let settings = settings::channel(&app.font_cache()).unwrap().1;
+            let (_, workspace) = app.add_window(|ctx| {
+                let mut workspace = Workspace::new(0, settings, ctx);
+                workspace.add_worktree(dir.path(), ctx);
+                workspace
+            });
+
+            // Create a new untitled buffer
+            let editor = workspace.update(&mut app, |workspace, ctx| {
+                workspace.open_new_file(&(), ctx);
+                workspace
+                    .active_item(ctx)
+                    .unwrap()
+                    .to_any()
+                    .downcast::<BufferView>()
+                    .unwrap()
+            });
+            editor.update(&mut app, |editor, ctx| {
+                assert_eq!(editor.title(ctx.as_ref()), "untitled");
+                editor.insert(&"hi".to_string(), ctx)
+            });
+
+            // Save the buffer, selecting a filename
+            workspace.update(&mut app, |workspace, ctx| {
+                workspace.save_active_item(&(), ctx)
+            });
+            app.simulate_new_path_selection(|parent_dir| {
+                assert_eq!(parent_dir, dir.path());
+                Some(parent_dir.join("the-new-name"))
+            });
+            app.read(|ctx| assert_eq!(editor.title(ctx), "untitled"));
+
+            // When the save completes, the buffer's title is updated.
+            editor
+                .condition(&app, |editor, ctx| editor.title(ctx) == "the-new-name")
+                .await;
+        });
+    }
+
     #[test]
     fn test_pane_actions() {
         App::test_async((), |mut app| async move {