From 5e7c74c7b6c809df7acdea14fadcd0d2bf354619 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Wed, 20 Dec 2023 16:01:52 -0800 Subject: [PATCH] Ensure that on_release callbacks are called even if view outlives its window --- crates/go_to_line2/src/go_to_line.rs | 28 ++++++++++++++++------------ crates/gpui2/src/window.rs | 8 ++++++-- crates/vim2/src/editor_events.rs | 6 +++--- crates/workspace2/src/workspace2.rs | 2 +- crates/zed2/src/open_listener.rs | 2 +- 5 files changed, 27 insertions(+), 19 deletions(-) diff --git a/crates/go_to_line2/src/go_to_line.rs b/crates/go_to_line2/src/go_to_line.rs index d6b464cf0e..02fcd47716 100644 --- a/crates/go_to_line2/src/go_to_line.rs +++ b/crates/go_to_line2/src/go_to_line.rs @@ -1,8 +1,8 @@ use editor::{display_map::ToDisplayPoint, scroll::autoscroll::Autoscroll, Editor}; use gpui::{ - actions, div, prelude::*, AppContext, DismissEvent, Div, EventEmitter, FocusHandle, - FocusableView, Render, SharedString, Styled, Subscription, View, ViewContext, VisualContext, - WindowContext, + actions, div, prelude::*, AnyWindowHandle, AppContext, DismissEvent, Div, EventEmitter, + FocusHandle, FocusableView, Render, SharedString, Styled, Subscription, View, ViewContext, + VisualContext, }; use text::{Bias, Point}; use theme::ActiveTheme; @@ -74,15 +74,19 @@ impl GoToLine { } } - fn release(&mut self, cx: &mut WindowContext) { - let scroll_position = self.prev_scroll_position.take(); - self.active_editor.update(cx, |editor, cx| { - editor.highlight_rows(None); - if let Some(scroll_position) = scroll_position { - editor.set_scroll_position(scroll_position, cx); - } - cx.notify(); - }) + fn release(&mut self, window: AnyWindowHandle, cx: &mut AppContext) { + window + .update(cx, |_, cx| { + let scroll_position = self.prev_scroll_position.take(); + self.active_editor.update(cx, |editor, cx| { + editor.highlight_rows(None); + if let Some(scroll_position) = scroll_position { + editor.set_scroll_position(scroll_position, cx); + } + cx.notify(); + }) + }) + .ok(); } fn on_line_editor_event( diff --git a/crates/gpui2/src/window.rs b/crates/gpui2/src/window.rs index 8546e094d4..9e8347c783 100644 --- a/crates/gpui2/src/window.rs +++ b/crates/gpui2/src/window.rs @@ -2422,16 +2422,20 @@ impl<'a, V: 'static> ViewContext<'a, V> { subscription } + /// Register a callback to be invoked when the view is released. + /// + /// The callback receives a handle to the view's window. This handle may be + /// invalid, if the window was closed before the view was released. pub fn on_release( &mut self, - on_release: impl FnOnce(&mut V, &mut WindowContext) + 'static, + on_release: impl FnOnce(&mut V, AnyWindowHandle, &mut AppContext) + 'static, ) -> Subscription { let window_handle = self.window.handle; let (subscription, activate) = self.app.release_listeners.insert( self.view.model.entity_id, Box::new(move |this, cx| { let this = this.downcast_mut().expect("invalid entity type"); - let _ = window_handle.update(cx, |_, cx| on_release(this, cx)); + on_release(this, window_handle, cx) }), ); activate(); diff --git a/crates/vim2/src/editor_events.rs b/crates/vim2/src/editor_events.rs index 0e2a1451fe..70f486f298 100644 --- a/crates/vim2/src/editor_events.rs +++ b/crates/vim2/src/editor_events.rs @@ -13,7 +13,7 @@ pub fn init(cx: &mut AppContext) { .detach(); let id = cx.view().entity_id(); - cx.on_release(move |_, cx| released(id, cx)).detach(); + cx.on_release(move |_, _, cx| released(id, cx)).detach(); }) .detach(); } @@ -51,8 +51,8 @@ fn blurred(editor: View, cx: &mut WindowContext) { }); } -fn released(entity_id: EntityId, cx: &mut WindowContext) { - Vim::update(cx, |vim, _| { +fn released(entity_id: EntityId, cx: &mut AppContext) { + cx.update_global(|vim: &mut Vim, _| { if vim .active_editor .as_ref() diff --git a/crates/workspace2/src/workspace2.rs b/crates/workspace2/src/workspace2.rs index 797e95088d..c8712205cb 100644 --- a/crates/workspace2/src/workspace2.rs +++ b/crates/workspace2/src/workspace2.rs @@ -642,7 +642,7 @@ impl Workspace { this.serialize_workspace(cx); cx.notify(); }), - cx.on_release(|this, cx| { + cx.on_release(|this, _, cx| { this.app_state.workspace_store.update(cx, |store, _| { store.workspaces.remove(&this.window_self); }) diff --git a/crates/zed2/src/open_listener.rs b/crates/zed2/src/open_listener.rs index 4c961a2b31..b45254f717 100644 --- a/crates/zed2/src/open_listener.rs +++ b/crates/zed2/src/open_listener.rs @@ -253,7 +253,7 @@ pub async fn handle_cli_connection( let (done_tx, done_rx) = oneshot::channel(); let _subscription = workspace.update(&mut cx, |workspace, cx| { - cx.on_release(move |_, _| { + cx.on_release(move |_, _, _| { let _ = done_tx.send(()); }) });