From 6442ec59e71f44fb44920e6d1dec326338e9279b Mon Sep 17 00:00:00 2001 From: K Simmons Date: Fri, 5 Aug 2022 12:38:05 -0700 Subject: [PATCH] Switch action dispatch to use MutableAppContext parent utilities and delete parent map from presenter --- crates/gpui/src/app.rs | 101 +++++++++++++++-------------- crates/gpui/src/presenter.rs | 41 +++++++++++- crates/workspace/src/pane.rs | 1 + crates/workspace/src/sidebar.rs | 1 + crates/workspace/src/status_bar.rs | 2 + crates/workspace/src/workspace.rs | 6 ++ 6 files changed, 100 insertions(+), 52 deletions(-) diff --git a/crates/gpui/src/app.rs b/crates/gpui/src/app.rs index a1b782039a..ac9ef6f829 100644 --- a/crates/gpui/src/app.rs +++ b/crates/gpui/src/app.rs @@ -230,7 +230,7 @@ impl App { let mut cx = cx.borrow_mut(); if let Some(key_window_id) = cx.cx.platform.key_window_id() { if let Some(view_id) = cx.focused_view_id(key_window_id) { - cx.handle_dispatch_action_any_effect(key_window_id, Some(view_id), action); + cx.handle_dispatch_action_from_effect(key_window_id, Some(view_id), action); return; } } @@ -457,7 +457,7 @@ impl TestAppContext { pub fn dispatch_action(&self, window_id: usize, action: A) { let mut cx = self.cx.borrow_mut(); if let Some(view_id) = cx.focused_view_id(window_id) { - cx.handle_dispatch_action_any_effect(window_id, Some(view_id), &action); + cx.handle_dispatch_action_from_effect(window_id, Some(view_id), &action); } } @@ -477,6 +477,7 @@ impl TestAppContext { if cx.dispatch_keystroke(window_id, &keystroke) { return true; } + if presenter.borrow_mut().dispatch_event( Event::KeyDown(KeyDownEvent { keystroke: keystroke.clone(), @@ -1606,6 +1607,12 @@ impl MutableAppContext { } } + pub(crate) fn name_for_view(&self, window_id: usize, view_id: usize) -> Option<&str> { + self.views + .get(&(window_id, view_id)) + .map(|view| view.ui_name()) + } + pub fn all_action_names<'a>(&'a self) -> impl Iterator + 'a { self.action_deserializers.keys().copied() } @@ -1685,29 +1692,6 @@ impl MutableAppContext { None } - // pub fn dispatch_action_at(&mut self, window_id: usize, view_id: usize, action: &dyn Action) { - // let presenter = self - // .presenters_and_platform_windows - // .get(&window_id) - // .unwrap() - // .0 - // .clone(); - // let mut dispatch_path = Vec::new(); - // presenter - // .borrow() - // .compute_dispatch_path_from(view_id, &mut dispatch_path); - // self.dispatch_action_any(window_id, &dispatch_path, action); - // } - - // pub fn dispatch_action( - // &mut self, - // window_id: usize, - // dispatch_path: Vec, - // action: &A, - // ) { - // self.dispatch_action_any(window_id, &dispatch_path, action); - // } - // Traverses the parent tree. Walks down the tree toward the passed // view calling visit with true. Then walks back up the tree calling visit with false. // If `visit` returns false this function will immediately return. @@ -1719,12 +1703,7 @@ impl MutableAppContext { mut visit: impl FnMut(usize, bool, &mut MutableAppContext) -> bool, ) -> bool { // List of view ids from the leaf to the root of the window - let mut path = vec![view_id]; - let mut current_view = view_id; - while let Some(ParentId::View(parent_id)) = self.parents.get(&(window_id, current_view)) { - current_view = *parent_id; - path.push(current_view); - } + let path = self.parents(window_id, view_id).collect::>(); // Walk down from the root to the leaf calling visit with capture_phase = true for view_id in path.iter().rev() { @@ -1746,15 +1725,18 @@ impl MutableAppContext { // Returns an iterator over all of the view ids from the passed view up to the root of the window // Includes the passed view itself fn parents(&self, window_id: usize, mut view_id: usize) -> impl Iterator + '_ { - std::iter::from_fn(move || { - if let Some(ParentId::View(parent_id)) = self.parents.get(&(window_id, view_id)) { - view_id = *parent_id; - Some(view_id) - } else { - None - } - }) + std::iter::once(view_id) + .into_iter() + .chain(std::iter::from_fn(move || { + if let Some(ParentId::View(parent_id)) = self.parents.get(&(window_id, view_id)) { + view_id = *parent_id; + Some(view_id) + } else { + None + } + })) } + fn actions_mut( &mut self, capture_phase: bool, @@ -1793,8 +1775,8 @@ impl MutableAppContext { pub fn dispatch_keystroke(&mut self, window_id: usize, keystroke: &Keystroke) -> bool { let mut pending = false; - if let Some(view_id) = self.focused_view_id(window_id) { - for view_id in self.parents(window_id, view_id).collect::>() { + if let Some(focused_view_id) = self.focused_view_id(window_id) { + for view_id in self.parents(window_id, focused_view_id).collect::>() { let keymap_context = self .cx .views @@ -1810,7 +1792,7 @@ impl MutableAppContext { MatchResult::None => {} MatchResult::Pending => pending = true, MatchResult::Action(action) => { - if self.handle_dispatch_action_any_effect( + if self.handle_dispatch_action_from_effect( window_id, Some(view_id), action.as_ref(), @@ -2303,7 +2285,7 @@ impl MutableAppContext { view_id, action, } => { - self.handle_dispatch_action_any_effect( + self.handle_dispatch_action_from_effect( window_id, Some(view_id), action.as_ref(), @@ -2579,7 +2561,7 @@ impl MutableAppContext { }) } - fn handle_dispatch_action_any_effect( + fn handle_dispatch_action_from_effect( &mut self, window_id: usize, view_id: Option, @@ -2587,6 +2569,7 @@ impl MutableAppContext { ) -> bool { self.update(|this| { if let Some(view_id) = view_id { + this.halt_action_dispatch = false; this.visit_dispatch_path(window_id, view_id, |view_id, capture_phase, this| { if let Some(mut view) = this.cx.views.remove(&(window_id, view_id)) { let type_id = view.as_any().type_id(); @@ -2809,6 +2792,7 @@ impl Deref for MutableAppContext { } } +#[derive(Debug)] pub enum ParentId { View(usize), Root, @@ -2817,7 +2801,7 @@ pub enum ParentId { pub struct AppContext { models: HashMap>, views: HashMap<(usize, usize), Box>, - parents: HashMap<(usize, usize), ParentId>, + pub(crate) parents: HashMap<(usize, usize), ParentId>, windows: HashMap, globals: HashMap>, element_states: HashMap>, @@ -3733,6 +3717,21 @@ impl<'a, T: View> ViewContext<'a, T> { .build_and_insert_view(self.window_id, ParentId::View(self.view_id), build_view) } + pub fn reparent(&mut self, view_handle: impl Into) { + let view_handle = view_handle.into(); + if self.window_id != view_handle.window_id { + panic!("Can't reparent view to a view from a different window"); + } + self.cx + .parents + .remove(&(view_handle.window_id, view_handle.view_id)); + let new_parent_id = self.view_id; + self.cx.parents.insert( + (view_handle.window_id, view_handle.view_id), + ParentId::View(new_parent_id), + ); + } + pub fn replace_root_view(&mut self, build_root_view: F) -> ViewHandle where V: View, @@ -6914,7 +6913,7 @@ mod tests { let view_3 = cx.add_view(&view_2, |_| ViewA { id: 3 }); let view_4 = cx.add_view(&view_3, |_| ViewB { id: 4 }); - cx.handle_dispatch_action_any_effect( + cx.handle_dispatch_action_from_effect( window_id, Some(view_4.id()), &Action("bar".to_string()), @@ -6938,12 +6937,12 @@ mod tests { // Remove view_1, which doesn't propagate the action - let (window_id, view_2) = cx.add_window(Default::default(), |_| ViewA { id: 1 }); + let (window_id, view_2) = cx.add_window(Default::default(), |_| ViewB { id: 2 }); let view_3 = cx.add_view(&view_2, |_| ViewA { id: 3 }); let view_4 = cx.add_view(&view_3, |_| ViewB { id: 4 }); actions.borrow_mut().clear(); - cx.handle_dispatch_action_any_effect( + cx.handle_dispatch_action_from_effect( window_id, Some(view_4.id()), &Action("bar".to_string()), @@ -7019,8 +7018,10 @@ mod tests { let (window_id, view_1) = cx.add_window(Default::default(), |_| view_1); let view_2 = cx.add_view(&view_1, |_| view_2); - let view_3 = cx.add_view(&view_2, |_| view_3); - cx.focus(window_id, Some(view_3.id())); + let view_3 = cx.add_view(&view_2, |cx| { + cx.focus_self(); + view_3 + }); // This keymap's only binding dispatches an action on view 2 because that view will have // "a" and "b" in its context, but not "c". diff --git a/crates/gpui/src/presenter.rs b/crates/gpui/src/presenter.rs index 57d3a1a3fd..9cba57810b 100644 --- a/crates/gpui/src/presenter.rs +++ b/crates/gpui/src/presenter.rs @@ -10,8 +10,8 @@ use crate::{ text_layout::TextLayoutCache, Action, AnyModelHandle, AnyViewHandle, AnyWeakModelHandle, AssetCache, ElementBox, Entity, FontSystem, ModelHandle, MouseButtonEvent, MouseMovedEvent, MouseRegion, MouseRegionId, - ReadModel, ReadView, RenderContext, RenderParams, Scene, UpgradeModelHandle, UpgradeViewHandle, - View, ViewHandle, WeakModelHandle, WeakViewHandle, + ParentId, ReadModel, ReadView, RenderContext, RenderParams, Scene, UpgradeModelHandle, + UpgradeViewHandle, View, ViewHandle, WeakModelHandle, WeakViewHandle, }; use collections::{HashMap, HashSet}; use pathfinder_geometry::vector::{vec2f, Vector2F}; @@ -482,6 +482,43 @@ impl<'a> LayoutContext<'a> { } fn layout(&mut self, view_id: usize, constraint: SizeConstraint) -> Vector2F { + let print_error = |view_id| { + format!( + "{} with id {}", + self.app.name_for_view(self.window_id, view_id).unwrap(), + view_id, + ) + }; + match ( + self.view_stack.last(), + self.app.parents.get(&(self.window_id, view_id)), + ) { + (Some(layout_parent), Some(ParentId::View(app_parent))) => { + if layout_parent != app_parent { + panic!( + "View {} was laid out with parent {} when it was constructed with parent {}", + print_error(view_id), + print_error(*layout_parent), + print_error(*app_parent)) + } + } + (None, Some(ParentId::View(app_parent))) => panic!( + "View {} was laid out without a parent when it was constructed with parent {}", + print_error(view_id), + print_error(*app_parent) + ), + (Some(layout_parent), Some(ParentId::Root)) => panic!( + "View {} was laid out with parent {} when it was constructed as a window root", + print_error(view_id), + print_error(*layout_parent), + ), + (_, None) => panic!( + "View {} did not have a registered parent in the app context", + print_error(view_id), + ), + _ => {} + } + self.view_stack.push(view_id); let mut rendered_view = self.rendered_views.remove(&view_id).unwrap(); let size = rendered_view.layout(constraint, self); diff --git a/crates/workspace/src/pane.rs b/crates/workspace/src/pane.rs index 354331974f..572e1032b3 100644 --- a/crates/workspace/src/pane.rs +++ b/crates/workspace/src/pane.rs @@ -441,6 +441,7 @@ impl Pane { pane.active_item_index = usize::MAX; }; + cx.reparent(&item); pane.items.insert(item_ix, item); pane.activate_item(item_ix, activate_pane, focus_item, false, cx); cx.notify(); diff --git a/crates/workspace/src/sidebar.rs b/crates/workspace/src/sidebar.rs index 542cd51cb6..b7ed8140c9 100644 --- a/crates/workspace/src/sidebar.rs +++ b/crates/workspace/src/sidebar.rs @@ -140,6 +140,7 @@ impl Sidebar { } }), ]; + cx.reparent(&view); self.items.push(Item { icon_path, tooltip, diff --git a/crates/workspace/src/status_bar.rs b/crates/workspace/src/status_bar.rs index f84940e8fb..ca44cf7c27 100644 --- a/crates/workspace/src/status_bar.rs +++ b/crates/workspace/src/status_bar.rs @@ -81,6 +81,7 @@ impl StatusBar { where T: 'static + StatusItemView, { + cx.reparent(&item); self.left_items.push(Box::new(item)); cx.notify(); } @@ -89,6 +90,7 @@ impl StatusBar { where T: 'static + StatusItemView, { + cx.reparent(&item); self.right_items.push(Box::new(item)); cx.notify(); } diff --git a/crates/workspace/src/workspace.rs b/crates/workspace/src/workspace.rs index d1a239cbb4..02ad18eb3d 100644 --- a/crates/workspace/src/workspace.rs +++ b/crates/workspace/src/workspace.rs @@ -708,6 +708,12 @@ impl Into for Box { } } +impl Into for &Box { + fn into(self) -> AnyViewHandle { + self.to_any() + } +} + impl Clone for Box { fn clone(&self) -> Box { self.boxed_clone()