Merge pull request #2191 from zed-industries/petros/z-53-reveal-in-finder-crashes-zed

Petros Amoiridis created 2 years ago

Move reveal_path to ForegroundPlatform

Change summary

crates/gpui/src/app.rs                    | 10 ++++
crates/gpui/src/platform.rs               |  2 
crates/gpui/src/platform/mac.rs           |  8 ++
crates/gpui/src/platform/mac/platform.rs  | 54 +++++++++++++++++-------
crates/gpui/src/platform/test.rs          |  4 
crates/project_panel/src/project_panel.rs |  3 
6 files changed, 57 insertions(+), 24 deletions(-)

Detailed changes

crates/gpui/src/app.rs

@@ -190,8 +190,8 @@ pub struct AsyncAppContext(Rc<RefCell<MutableAppContext>>);
 impl App {
     pub fn new(asset_source: impl AssetSource) -> Result<Self> {
         let platform = platform::current::platform();
-        let foreground_platform = platform::current::foreground_platform();
         let foreground = Rc::new(executor::Foreground::platform(platform.dispatcher())?);
+        let foreground_platform = platform::current::foreground_platform(foreground.clone());
         let app = Self(Rc::new(RefCell::new(MutableAppContext::new(
             foreground,
             Arc::new(executor::Background::new()),
@@ -900,6 +900,10 @@ impl MutableAppContext {
         self.foreground_platform.prompt_for_new_path(directory)
     }
 
+    pub fn reveal_path(&self, path: &Path) {
+        self.foreground_platform.reveal_path(path)
+    }
+
     pub fn emit_global<E: Any>(&mut self, payload: E) {
         self.pending_effects.push_back(Effect::GlobalEvent {
             payload: Box::new(payload),
@@ -3637,6 +3641,10 @@ impl<'a, T: View> ViewContext<'a, T> {
         self.app.prompt_for_new_path(directory)
     }
 
+    pub fn reveal_path(&self, path: &Path) {
+        self.app.reveal_path(path)
+    }
+
     pub fn debug_elements(&self) -> crate::json::Value {
         self.app.debug_elements(self.window_id).unwrap()
     }

crates/gpui/src/platform.rs

@@ -65,7 +65,6 @@ pub trait Platform: Send + Sync {
     fn write_to_clipboard(&self, item: ClipboardItem);
     fn read_from_clipboard(&self) -> Option<ClipboardItem>;
     fn open_url(&self, url: &str);
-    fn reveal_path(&self, path: &Path);
 
     fn write_credentials(&self, url: &str, username: &str, password: &[u8]) -> Result<()>;
     fn read_credentials(&self, url: &str) -> Result<Option<(String, Vec<u8>)>>;
@@ -100,6 +99,7 @@ pub(crate) trait ForegroundPlatform {
         options: PathPromptOptions,
     ) -> oneshot::Receiver<Option<Vec<PathBuf>>>;
     fn prompt_for_new_path(&self, directory: &Path) -> oneshot::Receiver<Option<PathBuf>>;
+    fn reveal_path(&self, path: &Path);
 }
 
 pub trait Dispatcher: Send + Sync {

crates/gpui/src/platform/mac.rs

@@ -23,12 +23,16 @@ pub use renderer::Surface;
 use std::{ops::Range, rc::Rc, sync::Arc};
 use window::Window;
 
+use crate::executor;
+
 pub(crate) fn platform() -> Arc<dyn super::Platform> {
     Arc::new(MacPlatform::new())
 }
 
-pub(crate) fn foreground_platform() -> Rc<dyn super::ForegroundPlatform> {
-    Rc::new(MacForegroundPlatform::default())
+pub(crate) fn foreground_platform(
+    foreground: Rc<executor::Foreground>,
+) -> Rc<dyn super::ForegroundPlatform> {
+    Rc::new(MacForegroundPlatform::new(foreground))
 }
 
 trait BoolExt {

crates/gpui/src/platform/mac/platform.rs

@@ -16,7 +16,7 @@ use cocoa::{
         NSEventModifierFlags, NSMenu, NSMenuItem, NSModalResponse, NSOpenPanel, NSPasteboard,
         NSPasteboardTypeString, NSSavePanel, NSWindow,
     },
-    base::{id, nil, selector, YES},
+    base::{id, nil, selector, BOOL, YES},
     foundation::{
         NSArray, NSAutoreleasePool, NSBundle, NSData, NSInteger, NSProcessInfo, NSString,
         NSUInteger, NSURL,
@@ -113,10 +113,8 @@ unsafe fn build_classes() {
     }
 }
 
-#[derive(Default)]
 pub struct MacForegroundPlatform(RefCell<MacForegroundPlatformState>);
 
-#[derive(Default)]
 pub struct MacForegroundPlatformState {
     become_active: Option<Box<dyn FnMut()>>,
     resign_active: Option<Box<dyn FnMut()>>,
@@ -128,9 +126,26 @@ pub struct MacForegroundPlatformState {
     open_urls: Option<Box<dyn FnMut(Vec<String>)>>,
     finish_launching: Option<Box<dyn FnOnce()>>,
     menu_actions: Vec<Box<dyn Action>>,
+    foreground: Rc<executor::Foreground>,
 }
 
 impl MacForegroundPlatform {
+    pub fn new(foreground: Rc<executor::Foreground>) -> Self {
+        Self(RefCell::new(MacForegroundPlatformState {
+            become_active: Default::default(),
+            resign_active: Default::default(),
+            quit: Default::default(),
+            event: Default::default(),
+            menu_command: Default::default(),
+            validate_menu_command: Default::default(),
+            will_open_menu: Default::default(),
+            open_urls: Default::default(),
+            finish_launching: Default::default(),
+            menu_actions: Default::default(),
+            foreground,
+        }))
+    }
+
     unsafe fn create_menu_bar(
         &self,
         menus: Vec<Menu>,
@@ -398,6 +413,26 @@ impl platform::ForegroundPlatform for MacForegroundPlatform {
             done_rx
         }
     }
+
+    fn reveal_path(&self, path: &Path) {
+        unsafe {
+            let path = path.to_path_buf();
+            self.0
+                .borrow()
+                .foreground
+                .spawn(async move {
+                    let full_path = ns_string(path.to_str().unwrap_or(""));
+                    let root_full_path = ns_string("");
+                    let workspace: id = msg_send![class!(NSWorkspace), sharedWorkspace];
+                    let _: BOOL = msg_send![
+                        workspace,
+                        selectFile: full_path
+                        inFileViewerRootedAtPath: root_full_path
+                    ];
+                })
+                .detach();
+        }
+    }
 }
 
 pub struct MacPlatform {
@@ -599,19 +634,6 @@ impl platform::Platform for MacPlatform {
         }
     }
 
-    fn reveal_path(&self, path: &Path) {
-        unsafe {
-            let full_path = ns_string(path.to_str().unwrap_or(""));
-            let root_full_path = ns_string("");
-            let workspace: id = msg_send![class!(NSWorkspace), sharedWorkspace];
-            msg_send![
-                workspace,
-                selectFile: full_path
-                inFileViewerRootedAtPath: root_full_path
-            ]
-        }
-    }
-
     fn write_credentials(&self, url: &str, username: &str, password: &[u8]) -> Result<()> {
         let url = CFString::from(url);
         let username = CFString::from(username);

crates/gpui/src/platform/test.rs

@@ -92,6 +92,8 @@ impl super::ForegroundPlatform for ForegroundPlatform {
         *self.last_prompt_for_new_path_args.borrow_mut() = Some((path.to_path_buf(), done_tx));
         done_rx
     }
+
+    fn reveal_path(&self, _: &Path) {}
 }
 
 pub fn platform() -> Platform {
@@ -173,8 +175,6 @@ impl super::Platform for Platform {
 
     fn open_url(&self, _: &str) {}
 
-    fn reveal_path(&self, _: &Path) {}
-
     fn write_credentials(&self, _: &str, _: &str, _: &[u8]) -> Result<()> {
         Ok(())
     }

crates/project_panel/src/project_panel.rs

@@ -792,8 +792,7 @@ impl ProjectPanel {
 
     fn reveal_in_finder(&mut self, _: &RevealInFinder, cx: &mut ViewContext<Self>) {
         if let Some((worktree, entry)) = self.selected_entry(cx) {
-            cx.platform()
-                .reveal_path(&worktree.abs_path().join(&entry.path));
+            cx.reveal_path(&worktree.abs_path().join(&entry.path));
         }
     }