Prompt for paths asynchronously to avoid double borrow

Antonio Scandurra created

Change summary

Cargo.lock                        |  1 
gpui/Cargo.toml                   |  1 
gpui/src/app.rs                   | 18 ++++++++++++
gpui/src/platform/mac/platform.rs | 46 ++++++++++++++++++++------------
gpui/src/platform/mod.rs          |  6 +++
gpui/src/platform/test.rs         |  7 +++-
zed/src/workspace/mod.rs          | 26 +++++++++---------
7 files changed, 70 insertions(+), 35 deletions(-)

Detailed changes

Cargo.lock 🔗

@@ -903,6 +903,7 @@ dependencies = [
  "async-std",
  "async-task",
  "bindgen",
+ "block",
  "cc",
  "cocoa",
  "core-foundation",

gpui/Cargo.toml 🔗

@@ -37,6 +37,7 @@ simplelog = "0.9"
 
 [target.'cfg(target_os = "macos")'.dependencies]
 anyhow = "1"
+block = "0.1"
 cocoa = "0.24"
 core-foundation = "0.9"
 core-graphics = "0.22.2"

gpui/src/app.rs 🔗

@@ -5,7 +5,7 @@ use crate::{
     platform::{self, WindowOptions},
     presenter::Presenter,
     util::post_inc,
-    AssetCache, AssetSource, ClipboardItem, FontCache, TextLayoutCache,
+    AssetCache, AssetSource, ClipboardItem, FontCache, PathPromptOptions, TextLayoutCache,
 };
 use anyhow::{anyhow, Result};
 use async_std::sync::Condvar;
@@ -570,6 +570,22 @@ impl MutableAppContext {
         self.platform.set_menus(menus);
     }
 
+    pub fn prompt_for_paths<F>(&self, options: PathPromptOptions, done_fn: F)
+    where
+        F: 'static + FnOnce(Option<Vec<PathBuf>>, &mut MutableAppContext),
+    {
+        let app = self.weak_self.as_ref().unwrap().upgrade().unwrap();
+        let foreground = self.foreground.clone();
+        self.platform().prompt_for_paths(
+            options,
+            Box::new(move |paths| {
+                foreground
+                    .spawn(async move { (done_fn)(paths, &mut *app.borrow_mut()) })
+                    .detach();
+            }),
+        );
+    }
+
     pub fn dispatch_action<T: 'static + Any>(
         &mut self,
         window_id: usize,

gpui/src/platform/mac/platform.rs 🔗

@@ -1,5 +1,6 @@
 use super::{BoolExt as _, Dispatcher, FontSystem, Window};
 use crate::{executor, keymap::Keystroke, platform, ClipboardItem, Event, Menu, MenuItem};
+use block::ConcreteBlock;
 use cocoa::{
     appkit::{
         NSApplication, NSApplicationActivationPolicy::NSApplicationActivationPolicyRegular,
@@ -20,7 +21,7 @@ use objc::{
 use ptr::null_mut;
 use std::{
     any::Any,
-    cell::RefCell,
+    cell::{Cell, RefCell},
     convert::TryInto,
     ffi::{c_void, CStr},
     os::raw::c_char,
@@ -267,31 +268,40 @@ impl platform::Platform for MacPlatform {
     fn prompt_for_paths(
         &self,
         options: platform::PathPromptOptions,
-    ) -> Option<Vec<std::path::PathBuf>> {
+        done_fn: Box<dyn FnOnce(Option<Vec<std::path::PathBuf>>)>,
+    ) {
         unsafe {
             let panel = NSOpenPanel::openPanel(nil);
             panel.setCanChooseDirectories_(options.directories.to_objc());
             panel.setCanChooseFiles_(options.files.to_objc());
             panel.setAllowsMultipleSelection_(options.multiple.to_objc());
             panel.setResolvesAliases_(false.to_objc());
-            let response = panel.runModal();
-            if response == NSModalResponse::NSModalResponseOk {
-                let mut result = Vec::new();
-                let urls = panel.URLs();
-                for i in 0..urls.count() {
-                    let url = urls.objectAtIndex(i);
-                    let string = url.absoluteString();
-                    let string = std::ffi::CStr::from_ptr(string.UTF8String())
-                        .to_string_lossy()
-                        .to_string();
-                    if let Some(path) = string.strip_prefix("file://") {
-                        result.push(PathBuf::from(path));
+            let done_fn = Cell::new(Some(done_fn));
+            let block = ConcreteBlock::new(move |response: NSModalResponse| {
+                let result = if response == NSModalResponse::NSModalResponseOk {
+                    let mut result = Vec::new();
+                    let urls = panel.URLs();
+                    for i in 0..urls.count() {
+                        let url = urls.objectAtIndex(i);
+                        let string = url.absoluteString();
+                        let string = std::ffi::CStr::from_ptr(string.UTF8String())
+                            .to_string_lossy()
+                            .to_string();
+                        if let Some(path) = string.strip_prefix("file://") {
+                            result.push(PathBuf::from(path));
+                        }
                     }
+                    Some(result)
+                } else {
+                    None
+                };
+
+                if let Some(done_fn) = done_fn.take() {
+                    (done_fn)(result);
                 }
-                Some(result)
-            } else {
-                None
-            }
+            });
+            let block = block.copy();
+            let _: () = msg_send![panel, beginWithCompletionHandler: block];
         }
     }
 

gpui/src/platform/mod.rs 🔗

@@ -40,7 +40,11 @@ pub trait Platform {
         executor: Rc<executor::Foreground>,
     ) -> Box<dyn Window>;
     fn key_window_id(&self) -> Option<usize>;
-    fn prompt_for_paths(&self, options: PathPromptOptions) -> Option<Vec<PathBuf>>;
+    fn prompt_for_paths(
+        &self,
+        options: PathPromptOptions,
+        done_fn: Box<dyn FnOnce(Option<Vec<std::path::PathBuf>>)>,
+    );
     fn quit(&self);
     fn write_to_clipboard(&self, item: ClipboardItem);
     fn read_from_clipboard(&self) -> Option<ClipboardItem>;

gpui/src/platform/test.rs 🔗

@@ -70,8 +70,11 @@ impl super::Platform for Platform {
 
     fn quit(&self) {}
 
-    fn prompt_for_paths(&self, _: super::PathPromptOptions) -> Option<Vec<std::path::PathBuf>> {
-        None
+    fn prompt_for_paths(
+        &self,
+        _: super::PathPromptOptions,
+        _: Box<dyn FnOnce(Option<Vec<std::path::PathBuf>>)>,
+    ) {
     }
 
     fn write_to_clipboard(&self, item: ClipboardItem) {

zed/src/workspace/mod.rs 🔗

@@ -29,19 +29,19 @@ pub struct OpenParams {
 }
 
 fn open(settings: &Receiver<Settings>, ctx: &mut MutableAppContext) {
-    if let Some(paths) = ctx.platform().prompt_for_paths(PathPromptOptions {
-        files: true,
-        directories: true,
-        multiple: true,
-    }) {
-        ctx.dispatch_global_action(
-            "workspace:open_paths",
-            OpenParams {
-                paths,
-                settings: settings.clone(),
-            },
-        );
-    }
+    let settings = settings.clone();
+    ctx.prompt_for_paths(
+        PathPromptOptions {
+            files: true,
+            directories: true,
+            multiple: true,
+        },
+        move |paths, ctx| {
+            if let Some(paths) = paths {
+                ctx.dispatch_global_action("workspace:open_paths", OpenParams { paths, settings });
+            }
+        },
+    );
 }
 
 fn open_paths(params: &OpenParams, app: &mut MutableAppContext) {