Compute view ancestry at layout time

Antonio Scandurra created

Change summary

crates/collab/src/tests.rs                    |   4 
crates/collab/src/tests/integration_tests.rs  |  20 
crates/command_palette/src/command_palette.rs |   4 
crates/diagnostics/src/diagnostics.rs         |   8 
crates/editor/src/editor_tests.rs             |   4 
crates/editor/src/items.rs                    |  11 
crates/gpui/src/app.rs                        | 281 ++++++++++++--------
crates/gpui/src/app/menu.rs                   |   2 
crates/gpui/src/app/test_app_context.rs       |  13 
crates/gpui/src/app/window.rs                 |  49 +--
crates/project_symbols/src/project_symbols.rs |   4 
crates/search/src/buffer_search.rs            |   8 
crates/search/src/project_search.rs           |   2 
crates/terminal_view/src/terminal_view.rs     |  17 
crates/workspace/src/pane.rs                  |   2 
crates/workspace/src/sidebar.rs               |   2 
crates/workspace/src/status_bar.rs            |   2 
crates/workspace/src/workspace.rs             |  38 +-
18 files changed, 246 insertions(+), 225 deletions(-)

Detailed changes

crates/collab/src/tests.rs 🔗

@@ -462,8 +462,8 @@ impl TestClient {
         project: &ModelHandle<Project>,
         cx: &mut TestAppContext,
     ) -> ViewHandle<Workspace> {
-        let (_, root_view) = cx.add_window(|_| EmptyView);
-        cx.add_view(&root_view, |cx| Workspace::test_new(project.clone(), cx))
+        let (window_id, _) = cx.add_window(|_| EmptyView);
+        cx.add_view(window_id, |cx| Workspace::test_new(project.clone(), cx))
     }
 }
 

crates/collab/src/tests/integration_tests.rs 🔗

@@ -1202,7 +1202,7 @@ async fn test_share_project(
     cx_c: &mut TestAppContext,
 ) {
     deterministic.forbid_parking();
-    let (_, window_b) = cx_b.add_window(|_| EmptyView);
+    let (window_b, _) = cx_b.add_window(|_| EmptyView);
     let mut server = TestServer::start(&deterministic).await;
     let client_a = server.create_client(cx_a, "user_a").await;
     let client_b = server.create_client(cx_b, "user_b").await;
@@ -1289,7 +1289,7 @@ async fn test_share_project(
         .await
         .unwrap();
 
-    let editor_b = cx_b.add_view(&window_b, |cx| Editor::for_buffer(buffer_b, None, cx));
+    let editor_b = cx_b.add_view(window_b, |cx| Editor::for_buffer(buffer_b, None, cx));
 
     // Client A sees client B's selection
     deterministic.run_until_parked();
@@ -3076,13 +3076,13 @@ async fn test_newline_above_or_below_does_not_move_guest_cursor(
         .update(cx_a, |p, cx| p.open_buffer((worktree_id, "a.txt"), cx))
         .await
         .unwrap();
-    let (_, window_a) = cx_a.add_window(|_| EmptyView);
-    let editor_a = cx_a.add_view(&window_a, |cx| {
+    let (window_a, _) = cx_a.add_window(|_| EmptyView);
+    let editor_a = cx_a.add_view(window_a, |cx| {
         Editor::for_buffer(buffer_a, Some(project_a), cx)
     });
     let mut editor_cx_a = EditorTestContext {
         cx: cx_a,
-        window_id: window_a.id(),
+        window_id: window_a,
         editor: editor_a,
     };
 
@@ -3091,13 +3091,13 @@ async fn test_newline_above_or_below_does_not_move_guest_cursor(
         .update(cx_b, |p, cx| p.open_buffer((worktree_id, "a.txt"), cx))
         .await
         .unwrap();
-    let (_, window_b) = cx_b.add_window(|_| EmptyView);
-    let editor_b = cx_b.add_view(&window_b, |cx| {
+    let (window_b, _) = cx_b.add_window(|_| EmptyView);
+    let editor_b = cx_b.add_view(window_b, |cx| {
         Editor::for_buffer(buffer_b, Some(project_b), cx)
     });
     let mut editor_cx_b = EditorTestContext {
         cx: cx_b,
-        window_id: window_b.id(),
+        window_id: window_b,
         editor: editor_b,
     };
 
@@ -3832,8 +3832,8 @@ async fn test_collaborating_with_completion(
         .update(cx_b, |p, cx| p.open_buffer((worktree_id, "main.rs"), cx))
         .await
         .unwrap();
-    let (_, window_b) = cx_b.add_window(|_| EmptyView);
-    let editor_b = cx_b.add_view(&window_b, |cx| {
+    let (window_b, _) = cx_b.add_window(|_| EmptyView);
+    let editor_b = cx_b.add_view(window_b, |cx| {
         Editor::for_buffer(buffer_b.clone(), Some(project_b.clone()), cx)
     });
 

crates/command_palette/src/command_palette.rs 🔗

@@ -304,8 +304,8 @@ mod tests {
         });
 
         let project = Project::test(app_state.fs.clone(), [], cx).await;
-        let (_, workspace) = cx.add_window(|cx| Workspace::test_new(project.clone(), cx));
-        let editor = cx.add_view(&workspace, |cx| {
+        let (window_id, workspace) = cx.add_window(|cx| Workspace::test_new(project.clone(), cx));
+        let editor = cx.add_view(window_id, |cx| {
             let mut editor = Editor::single_line(None, cx);
             editor.set_text("abc", cx);
             editor

crates/diagnostics/src/diagnostics.rs 🔗

@@ -852,7 +852,7 @@ mod tests {
 
         let language_server_id = LanguageServerId(0);
         let project = Project::test(fs.clone(), ["/test".as_ref()], cx).await;
-        let (_, workspace) = cx.add_window(|cx| Workspace::test_new(project.clone(), cx));
+        let (window_id, workspace) = cx.add_window(|cx| Workspace::test_new(project.clone(), cx));
 
         // Create some diagnostics
         project.update(cx, |project, cx| {
@@ -939,7 +939,7 @@ mod tests {
         });
 
         // Open the project diagnostics view while there are already diagnostics.
-        let view = cx.add_view(&workspace, |cx| {
+        let view = cx.add_view(window_id, |cx| {
             ProjectDiagnosticsEditor::new(project.clone(), workspace.downgrade(), cx)
         });
 
@@ -1244,9 +1244,9 @@ mod tests {
         let server_id_1 = LanguageServerId(100);
         let server_id_2 = LanguageServerId(101);
         let project = Project::test(fs.clone(), ["/test".as_ref()], cx).await;
-        let (_, workspace) = cx.add_window(|cx| Workspace::test_new(project.clone(), cx));
+        let (window_id, workspace) = cx.add_window(|cx| Workspace::test_new(project.clone(), cx));
 
-        let view = cx.add_view(&workspace, |cx| {
+        let view = cx.add_view(window_id, |cx| {
             ProjectDiagnosticsEditor::new(project.clone(), workspace.downgrade(), cx)
         });
 

crates/editor/src/editor_tests.rs 🔗

@@ -493,9 +493,9 @@ async fn test_navigation_history(cx: &mut TestAppContext) {
 
     let fs = FakeFs::new(cx.background());
     let project = Project::test(fs, [], cx).await;
-    let (_, workspace) = cx.add_window(|cx| Workspace::test_new(project, cx));
+    let (window_id, workspace) = cx.add_window(|cx| Workspace::test_new(project, cx));
     let pane = workspace.read_with(cx, |workspace, _| workspace.active_pane().clone());
-    cx.add_view(&pane, |cx| {
+    cx.add_view(window_id, |cx| {
         let buffer = MultiBuffer::build_simple(&sample_text(300, 5, 'a'), cx);
         let mut editor = build_editor(buffer.clone(), cx);
         let handle = cx.handle();

crates/editor/src/items.rs 🔗

@@ -3,7 +3,7 @@ use crate::{
     movement::surrounding_word, persistence::DB, scroll::ScrollAnchor, Anchor, Autoscroll, Editor,
     Event, ExcerptId, ExcerptRange, MultiBuffer, MultiBufferSnapshot, NavigationData, ToPoint as _,
 };
-use anyhow::{anyhow, Context, Result};
+use anyhow::{Context, Result};
 use collections::HashSet;
 use futures::future::try_join_all;
 use gpui::{
@@ -864,16 +864,13 @@ impl Item for Editor {
                     let buffer = project_item
                         .downcast::<Buffer>()
                         .context("Project item at stored path was not a buffer")?;
-                    let pane = pane
-                        .upgrade(&cx)
-                        .ok_or_else(|| anyhow!("pane was dropped"))?;
-                    Ok(cx.update(|cx| {
-                        cx.add_view(&pane, |cx| {
+                    Ok(pane.update(&mut cx, |_, cx| {
+                        cx.add_view(|cx| {
                             let mut editor = Editor::for_buffer(buffer, Some(project), cx);
                             editor.read_scroll_position_from_db(item_id, workspace_id, cx);
                             editor
                         })
-                    }))
+                    })?)
                 })
             })
             .unwrap_or_else(|error| Task::ready(Err(error)))

crates/gpui/src/app.rs 🔗

@@ -335,7 +335,7 @@ impl AsyncAppContext {
         self.0
             .borrow_mut()
             .update_window(window_id, |window| {
-                window.handle_dispatch_action_from_effect(Some(view_id), action);
+                window.dispatch_action(Some(view_id), action);
             })
             .ok_or_else(|| anyhow!("window not found"))
     }
@@ -440,7 +440,6 @@ type WindowShouldCloseSubscriptionCallback = Box<dyn FnMut(&mut AppContext) -> b
 pub struct AppContext {
     models: HashMap<usize, Box<dyn AnyModel>>,
     views: HashMap<(usize, usize), Box<dyn AnyView>>,
-    pub(crate) parents: HashMap<(usize, usize), ParentId>,
     windows: HashMap<usize, Window>,
     globals: HashMap<TypeId, Box<dyn Any>>,
     element_states: HashMap<ElementStateId, Box<dyn Any>>,
@@ -502,7 +501,6 @@ impl AppContext {
         Self {
             models: Default::default(),
             views: Default::default(),
-            parents: Default::default(),
             windows: Default::default(),
             globals: Default::default(),
             element_states: Default::default(),
@@ -1380,18 +1378,6 @@ impl AppContext {
         self.update_window(window_id, |cx| cx.replace_root_view(build_root_view))
     }
 
-    pub fn add_view<S, F>(&mut self, parent: &AnyViewHandle, build_view: F) -> ViewHandle<S>
-    where
-        S: View,
-        F: FnOnce(&mut ViewContext<S>) -> S,
-    {
-        self.update_window(parent.window_id, |cx| {
-            cx.build_and_insert_view(ParentId::View(parent.view_id), |cx| Some(build_view(cx)))
-                .unwrap()
-        })
-        .unwrap()
-    }
-
     pub fn read_view<T: View>(&self, handle: &ViewHandle<T>) -> &T {
         if let Some(view) = self.views.get(&(handle.window_id, handle.view_id)) {
             view.as_any().downcast_ref().expect("downcast is type safe")
@@ -1451,6 +1437,7 @@ impl AppContext {
                 let mut view = self.views.remove(&(window_id, view_id)).unwrap();
                 view.release(self);
                 let change_focus_to = self.windows.get_mut(&window_id).and_then(|window| {
+                    window.parents.remove(&view_id);
                     window
                         .invalidation
                         .get_or_insert_with(Default::default)
@@ -1462,10 +1449,12 @@ impl AppContext {
                         None
                     }
                 });
-                self.parents.remove(&(window_id, view_id));
 
                 if let Some(view_id) = change_focus_to {
-                    self.handle_focus_effect(window_id, Some(view_id));
+                    self.pending_effects.push_back(Effect::Focus {
+                        window_id,
+                        view_id: Some(view_id),
+                    });
                 }
 
                 self.pending_effects
@@ -1487,7 +1476,9 @@ impl AppContext {
 
             let mut refreshing = false;
             let mut updated_windows = HashSet::default();
+            let mut focused_views = HashMap::default();
             loop {
+                self.remove_dropped_entities();
                 if let Some(effect) = self.pending_effects.pop_front() {
                     match effect {
                         Effect::Subscription {
@@ -1561,7 +1552,7 @@ impl AppContext {
                         }
 
                         Effect::Focus { window_id, view_id } => {
-                            self.handle_focus_effect(window_id, view_id);
+                            focused_views.insert(window_id, view_id);
                         }
 
                         Effect::FocusObservation {
@@ -1661,10 +1652,7 @@ impl AppContext {
                         ),
                     }
                     self.pending_notifications.clear();
-                    self.remove_dropped_entities();
                 } else {
-                    self.remove_dropped_entities();
-
                     for window_id in self.windows.keys().cloned().collect::<Vec<_>>() {
                         self.update_window(window_id, |cx| {
                             let invalidation = if refreshing {
@@ -1688,6 +1676,10 @@ impl AppContext {
                         });
                     }
 
+                    for (window_id, view_id) in focused_views.drain() {
+                        self.handle_focus_effect(window_id, view_id);
+                    }
+
                     if self.pending_effects.is_empty() {
                         for callback in after_window_update_callbacks.drain(..) {
                             callback(self);
@@ -2882,38 +2874,6 @@ impl<'a, 'b, V: View> ViewContext<'a, 'b, V> {
             });
     }
 
-    pub fn add_view<S, F>(&mut self, build_view: F) -> ViewHandle<S>
-    where
-        S: View,
-        F: FnOnce(&mut ViewContext<S>) -> S,
-    {
-        self.window_context
-            .build_and_insert_view(ParentId::View(self.view_id), |cx| Some(build_view(cx)))
-            .unwrap()
-    }
-
-    pub fn add_option_view<S, F>(&mut self, build_view: F) -> Option<ViewHandle<S>>
-    where
-        S: View,
-        F: FnOnce(&mut ViewContext<S>) -> Option<S>,
-    {
-        self.window_context
-            .build_and_insert_view(ParentId::View(self.view_id), build_view)
-    }
-
-    pub fn reparent(&mut self, view_handle: &AnyViewHandle) {
-        if self.window_id != view_handle.window_id {
-            panic!("Can't reparent view to a view from a different window");
-        }
-        self.parents
-            .remove(&(view_handle.window_id, view_handle.view_id));
-        let new_parent_id = self.view_id;
-        self.parents.insert(
-            (view_handle.window_id, view_handle.view_id),
-            ParentId::View(new_parent_id),
-        );
-    }
-
     pub fn subscribe<E, H, F>(&mut self, handle: &H, mut callback: F) -> Subscription
     where
         E: Entity,
@@ -4562,9 +4522,9 @@ mod tests {
             }
         }
 
-        let (_, root_view) = cx.add_window(|cx| View::new(None, cx));
-        let handle_1 = cx.add_view(&root_view, |cx| View::new(None, cx));
-        let handle_2 = cx.add_view(&root_view, |cx| View::new(Some(handle_1.clone()), cx));
+        let (window_id, _root_view) = cx.add_window(|cx| View::new(None, cx));
+        let handle_1 = cx.add_view(window_id, |cx| View::new(None, cx));
+        let handle_2 = cx.add_view(window_id, |cx| View::new(Some(handle_1.clone()), cx));
         assert_eq!(cx.read(|cx| cx.views.len()), 3);
 
         handle_1.update(cx, |view, cx| {
@@ -4724,8 +4684,8 @@ mod tests {
             type Event = String;
         }
 
-        let (_, handle_1) = cx.add_window(|_| TestView::default());
-        let handle_2 = cx.add_view(&handle_1, |_| TestView::default());
+        let (window_id, handle_1) = cx.add_window(|_| TestView::default());
+        let handle_2 = cx.add_view(window_id, |_| TestView::default());
         let handle_3 = cx.add_model(|_| Model);
 
         handle_1.update(cx, |_, cx| {
@@ -4951,9 +4911,9 @@ mod tests {
             type Event = ();
         }
 
-        let (_, root_view) = cx.add_window(|_| TestView::default());
-        let observing_view = cx.add_view(&root_view, |_| TestView::default());
-        let emitting_view = cx.add_view(&root_view, |_| TestView::default());
+        let (window_id, _root_view) = cx.add_window(|_| TestView::default());
+        let observing_view = cx.add_view(window_id, |_| TestView::default());
+        let emitting_view = cx.add_view(window_id, |_| TestView::default());
         let observing_model = cx.add_model(|_| Model);
         let observed_model = cx.add_model(|_| Model);
 
@@ -5078,8 +5038,8 @@ mod tests {
             type Event = ();
         }
 
-        let (_, root_view) = cx.add_window(|_| TestView::default());
-        let observing_view = cx.add_view(&root_view, |_| TestView::default());
+        let (window_id, _root_view) = cx.add_window(|_| TestView::default());
+        let observing_view = cx.add_view(window_id, |_| TestView::default());
         let observing_model = cx.add_model(|_| Model);
         let observed_model = cx.add_model(|_| Model);
 
@@ -5201,9 +5161,9 @@ mod tests {
             }
         }
 
-        let (_, root_view) = cx.add_window(|_| View);
-        let observing_view = cx.add_view(&root_view, |_| View);
-        let observed_view = cx.add_view(&root_view, |_| View);
+        let (window_id, _root_view) = cx.add_window(|_| View);
+        let observing_view = cx.add_view(window_id, |_| View);
+        let observed_view = cx.add_view(window_id, |_| View);
 
         let observation_count = Rc::new(RefCell::new(0));
         observing_view.update(cx, |_, cx| {
@@ -5252,6 +5212,7 @@ mod tests {
         struct View {
             name: String,
             events: Arc<Mutex<Vec<String>>>,
+            child: Option<AnyViewHandle>,
         }
 
         impl Entity for View {
@@ -5259,8 +5220,11 @@ mod tests {
         }
 
         impl super::View for View {
-            fn render(&mut self, _: &mut ViewContext<Self>) -> AnyElement<Self> {
-                Empty::new().into_any()
+            fn render(&mut self, cx: &mut ViewContext<Self>) -> AnyElement<Self> {
+                self.child
+                    .as_ref()
+                    .map(|child| ChildView::new(child, cx).into_any())
+                    .unwrap_or(Empty::new().into_any())
             }
 
             fn ui_name() -> &'static str {
@@ -5284,11 +5248,22 @@ mod tests {
         let (window_id, view_1) = cx.add_window(|_| View {
             events: view_events.clone(),
             name: "view 1".to_string(),
+            child: None,
         });
-        let view_2 = cx.add_view(&view_1, |_| View {
-            events: view_events.clone(),
-            name: "view 2".to_string(),
-        });
+        let view_2 = cx
+            .update_window(window_id, |cx| {
+                let view_2 = cx.add_view(|_| View {
+                    events: view_events.clone(),
+                    name: "view 2".to_string(),
+                    child: None,
+                });
+                view_1.update(cx, |view_1, cx| {
+                    view_1.child = Some(view_2.clone().into_any());
+                    cx.notify();
+                });
+                view_2
+            })
+            .unwrap();
 
         let observed_events: Arc<Mutex<Vec<String>>> = Default::default();
         view_1.update(cx, |_, cx| {
@@ -5325,7 +5300,7 @@ mod tests {
         assert_eq!(mem::take(&mut *observed_events.lock()), Vec::<&str>::new());
 
         view_1.update(cx, |_, cx| {
-            // Ensure focus events are sent for all intermediate focuses
+            // Ensure only the last focus event is honored.
             cx.focus(&view_2);
             cx.focus(&view_1);
             cx.focus(&view_2);
@@ -5337,22 +5312,11 @@ mod tests {
         });
         assert_eq!(
             mem::take(&mut *view_events.lock()),
-            [
-                "view 1 blurred",
-                "view 2 focused",
-                "view 2 blurred",
-                "view 1 focused",
-                "view 1 blurred",
-                "view 2 focused"
-            ],
+            ["view 1 blurred", "view 2 focused"],
         );
         assert_eq!(
             mem::take(&mut *observed_events.lock()),
             [
-                "view 2 observed view 1's blur",
-                "view 1 observed view 2's focus",
-                "view 1 observed view 2's blur",
-                "view 2 observed view 1's focus",
                 "view 2 observed view 1's blur",
                 "view 1 observed view 2's focus"
             ]
@@ -5388,7 +5352,11 @@ mod tests {
             ]
         );
 
-        view_1.update(cx, |_, _| drop(view_2));
+        println!("=====================");
+        view_1.update(cx, |view, _| {
+            drop(view_2);
+            view.child = None;
+        });
         assert_eq!(mem::take(&mut *view_events.lock()), ["view 1 focused"]);
         assert_eq!(mem::take(&mut *observed_events.lock()), Vec::<&str>::new());
     }
@@ -5433,6 +5401,7 @@ mod tests {
     fn test_dispatch_action(cx: &mut AppContext) {
         struct ViewA {
             id: usize,
+            child: Option<AnyViewHandle>,
         }
 
         impl Entity for ViewA {
@@ -5440,8 +5409,11 @@ mod tests {
         }
 
         impl View for ViewA {
-            fn render(&mut self, _: &mut ViewContext<Self>) -> AnyElement<Self> {
-                Empty::new().into_any()
+            fn render(&mut self, cx: &mut ViewContext<Self>) -> AnyElement<Self> {
+                self.child
+                    .as_ref()
+                    .map(|child| ChildView::new(child, cx).into_any())
+                    .unwrap_or(Empty::new().into_any())
             }
 
             fn ui_name() -> &'static str {
@@ -5451,6 +5423,7 @@ mod tests {
 
         struct ViewB {
             id: usize,
+            child: Option<AnyViewHandle>,
         }
 
         impl Entity for ViewB {
@@ -5458,8 +5431,11 @@ mod tests {
         }
 
         impl View for ViewB {
-            fn render(&mut self, _: &mut ViewContext<Self>) -> AnyElement<Self> {
-                Empty::new().into_any()
+            fn render(&mut self, cx: &mut ViewContext<Self>) -> AnyElement<Self> {
+                self.child
+                    .as_ref()
+                    .map(|child| ChildView::new(child, cx).into_any())
+                    .unwrap_or(Empty::new().into_any())
             }
 
             fn ui_name() -> &'static str {
@@ -5496,7 +5472,7 @@ mod tests {
                 if view.id != 1 {
                     cx.add_view(|cx| {
                         cx.propagate_action(); // Still works on a nested ViewContext
-                        ViewB { id: 5 }
+                        ViewB { id: 5, child: None }
                     });
                 }
                 actions.borrow_mut().push(format!("{} b", view.id));
@@ -5534,13 +5510,41 @@ mod tests {
         })
         .detach();
 
-        let (window_id, view_1) = cx.add_window(Default::default(), |_| ViewA { id: 1 });
-        let view_2 = cx.add_view(&view_1, |_| 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 });
+        let (window_id, view_1) =
+            cx.add_window(Default::default(), |_| ViewA { id: 1, child: None });
+        let view_2 = cx
+            .update_window(window_id, |cx| {
+                let child = cx.add_view(|_| ViewB { id: 2, child: None });
+                view_1.update(cx, |view, cx| {
+                    view.child = Some(child.clone().into_any());
+                    cx.notify();
+                });
+                child
+            })
+            .unwrap();
+        let view_3 = cx
+            .update_window(window_id, |cx| {
+                let child = cx.add_view(|_| ViewA { id: 3, child: None });
+                view_2.update(cx, |view, cx| {
+                    view.child = Some(child.clone().into_any());
+                    cx.notify();
+                });
+                child
+            })
+            .unwrap();
+        let view_4 = cx
+            .update_window(window_id, |cx| {
+                let child = cx.add_view(|_| ViewB { id: 4, child: None });
+                view_3.update(cx, |view, cx| {
+                    view.child = Some(child.clone().into_any());
+                    cx.notify();
+                });
+                child
+            })
+            .unwrap();
 
         cx.update_window(window_id, |cx| {
-            cx.handle_dispatch_action_from_effect(Some(view_4.id()), &Action("bar".to_string()))
+            cx.dispatch_action(Some(view_4.id()), &Action("bar".to_string()))
         });
 
         assert_eq!(
@@ -5561,13 +5565,32 @@ mod tests {
 
         // Remove view_1, which doesn't propagate the action
 
-        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 });
+        let (window_id, view_2) =
+            cx.add_window(Default::default(), |_| ViewB { id: 2, child: None });
+        let view_3 = cx
+            .update_window(window_id, |cx| {
+                let child = cx.add_view(|_| ViewA { id: 3, child: None });
+                view_2.update(cx, |view, cx| {
+                    view.child = Some(child.clone().into_any());
+                    cx.notify();
+                });
+                child
+            })
+            .unwrap();
+        let view_4 = cx
+            .update_window(window_id, |cx| {
+                let child = cx.add_view(|_| ViewB { id: 4, child: None });
+                view_3.update(cx, |view, cx| {
+                    view.child = Some(child.clone().into_any());
+                    cx.notify();
+                });
+                child
+            })
+            .unwrap();
 
         actions.borrow_mut().clear();
         cx.update_window(window_id, |cx| {
-            cx.handle_dispatch_action_from_effect(Some(view_4.id()), &Action("bar".to_string()))
+            cx.dispatch_action(Some(view_4.id()), &Action("bar".to_string()))
         });
 
         assert_eq!(
@@ -5599,6 +5622,7 @@ mod tests {
         struct View {
             id: usize,
             keymap_context: KeymapContext,
+            child: Option<AnyViewHandle>,
         }
 
         impl Entity for View {
@@ -5606,8 +5630,11 @@ mod tests {
         }
 
         impl super::View for View {
-            fn render(&mut self, _: &mut ViewContext<Self>) -> AnyElement<Self> {
-                Empty::new().into_any()
+            fn render(&mut self, cx: &mut ViewContext<Self>) -> AnyElement<Self> {
+                self.child
+                    .as_ref()
+                    .map(|child| ChildView::new(child, cx).into_any())
+                    .unwrap_or(Empty::new().into_any())
             }
 
             fn ui_name() -> &'static str {
@@ -5624,6 +5651,7 @@ mod tests {
                 View {
                     id,
                     keymap_context: KeymapContext::default(),
+                    child: None,
                 }
             }
         }
@@ -5638,11 +5666,17 @@ mod tests {
         view_3.keymap_context.add_identifier("b");
         view_3.keymap_context.add_identifier("c");
 
-        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, |cx| {
-            cx.focus_self();
-            view_3
+        let (window_id, _view_1) = cx.add_window(Default::default(), |cx| {
+            let view_2 = cx.add_view(|cx| {
+                let view_3 = cx.add_view(|cx| {
+                    cx.focus_self();
+                    view_3
+                });
+                view_2.child = Some(view_3.into_any());
+                view_2
+            });
+            view_1.child = Some(view_2.into_any());
+            view_1
         });
 
         // This binding only dispatches an action on view 2 because that view will have
@@ -5722,7 +5756,9 @@ mod tests {
     fn test_keystrokes_for_action(cx: &mut AppContext) {
         actions!(test, [Action1, Action2, GlobalAction]);
 
-        struct View1 {}
+        struct View1 {
+            child: ViewHandle<View2>,
+        }
         struct View2 {}
 
         impl Entity for View1 {
@@ -5733,8 +5769,8 @@ mod tests {
         }
 
         impl super::View for View1 {
-            fn render(&mut self, _: &mut ViewContext<Self>) -> AnyElement<Self> {
-                Empty::new().into_any()
+            fn render(&mut self, cx: &mut ViewContext<Self>) -> AnyElement<Self> {
+                ChildView::new(&self.child, cx).into_any()
             }
             fn ui_name() -> &'static str {
                 "View1"
@@ -5749,11 +5785,14 @@ mod tests {
             }
         }
 
-        let (window_id, view_1) = cx.add_window(Default::default(), |_| View1 {});
-        let view_2 = cx.add_view(&view_1, |cx| {
-            cx.focus_self();
-            View2 {}
+        let (window_id, view_1) = cx.add_window(Default::default(), |cx| {
+            let view_2 = cx.add_view(|cx| {
+                cx.focus_self();
+                View2 {}
+            });
+            View1 { child: view_2 }
         });
+        let view_2 = view_1.read(cx).child.clone();
 
         cx.add_action(|_: &mut View1, _: &Action1, _cx| {});
         cx.add_action(|_: &mut View2, _: &Action2, _cx| {});
@@ -5952,8 +5991,8 @@ mod tests {
     #[crate::test(self)]
     #[should_panic(expected = "view dropped with pending condition")]
     async fn test_view_condition_panic_on_drop(cx: &mut TestAppContext) {
-        let (_, root_view) = cx.add_window(|_| TestView::default());
-        let view = cx.add_view(&root_view, |_| TestView::default());
+        let (window_id, _root_view) = cx.add_window(|_| TestView::default());
+        let view = cx.add_view(window_id, |_| TestView::default());
 
         let condition = view.condition(cx, |_, _| false);
         cx.update(|_| drop(view));
@@ -5986,10 +6025,12 @@ mod tests {
             );
         });
 
-        let view = cx.add_view(&root_view, |cx| {
-            cx.refresh_windows();
-            View(0)
-        });
+        let view = cx
+            .update_window(window_id, |cx| {
+                cx.refresh_windows();
+                cx.add_view(|_| View(0))
+            })
+            .unwrap();
 
         cx.update_window(window_id, |cx| {
             assert_eq!(

crates/gpui/src/app/menu.rs 🔗

@@ -81,7 +81,7 @@ pub(crate) fn setup_menu_handlers(foreground_platform: &dyn ForegroundPlatform,
                 let dispatched = cx
                     .update_window(main_window_id, |cx| {
                         if let Some(view_id) = cx.focused_view_id() {
-                            cx.handle_dispatch_action_from_effect(Some(view_id), action);
+                            cx.dispatch_action(Some(view_id), action);
                             true
                         } else {
                             false

crates/gpui/src/app/test_app_context.rs 🔗

@@ -4,9 +4,9 @@ use crate::{
     keymap_matcher::Keystroke,
     platform,
     platform::{Event, InputHandler, KeyDownEvent, Platform},
-    Action, AnyViewHandle, AppContext, BorrowAppContext, BorrowWindowContext, Entity, FontCache,
-    Handle, ModelContext, ModelHandle, Subscription, Task, View, ViewContext, ViewHandle,
-    WeakHandle, WindowContext,
+    Action, AppContext, BorrowAppContext, BorrowWindowContext, Entity, FontCache, Handle,
+    ModelContext, ModelHandle, Subscription, Task, View, ViewContext, ViewHandle, WeakHandle,
+    WindowContext,
 };
 use collections::BTreeMap;
 use futures::Future;
@@ -75,7 +75,7 @@ impl TestAppContext {
         self.cx
             .borrow_mut()
             .update_window(window_id, |window| {
-                window.handle_dispatch_action_from_effect(window.focused_view_id(), &action);
+                window.dispatch_action(window.focused_view_id(), &action);
             })
             .expect("window not found");
     }
@@ -153,12 +153,13 @@ impl TestAppContext {
         (window_id, view)
     }
 
-    pub fn add_view<T, F>(&mut self, parent_handle: &AnyViewHandle, build_view: F) -> ViewHandle<T>
+    pub fn add_view<T, F>(&mut self, window_id: usize, build_view: F) -> ViewHandle<T>
     where
         T: View,
         F: FnOnce(&mut ViewContext<T>) -> T,
     {
-        self.cx.borrow_mut().add_view(parent_handle, build_view)
+        self.update_window(window_id, |cx| cx.add_view(build_view))
+            .expect("window not found")
     }
 
     pub fn observe_global<E, F>(&mut self, callback: F) -> Subscription

crates/gpui/src/app/window.rs 🔗

@@ -14,8 +14,8 @@ use crate::{
     text_layout::TextLayoutCache,
     util::post_inc,
     Action, AnyView, AnyViewHandle, AppContext, BorrowAppContext, BorrowWindowContext, Effect,
-    Element, Entity, Handle, LayoutContext, MouseRegion, MouseRegionId, ParentId, SceneBuilder,
-    Subscription, View, ViewContext, ViewHandle, WindowInvalidation,
+    Element, Entity, Handle, LayoutContext, MouseRegion, MouseRegionId, SceneBuilder, Subscription,
+    View, ViewContext, ViewHandle, WindowInvalidation,
 };
 use anyhow::{anyhow, bail, Result};
 use collections::{HashMap, HashSet};
@@ -39,6 +39,7 @@ use super::Reference;
 pub struct Window {
     pub(crate) root_view: Option<AnyViewHandle>,
     pub(crate) focused_view_id: Option<usize>,
+    pub(crate) parents: HashMap<usize, usize>,
     pub(crate) is_active: bool,
     pub(crate) is_fullscreen: bool,
     pub(crate) invalidation: Option<WindowInvalidation>,
@@ -72,6 +73,7 @@ impl Window {
         let mut window = Self {
             root_view: None,
             focused_view_id: None,
+            parents: Default::default(),
             is_active: false,
             invalidation: None,
             is_fullscreen: false,
@@ -90,9 +92,7 @@ impl Window {
         };
 
         let mut window_context = WindowContext::mutable(cx, &mut window, window_id);
-        let root_view = window_context
-            .build_and_insert_view(ParentId::Root, |cx| Some(build_view(cx)))
-            .unwrap();
+        let root_view = window_context.add_view(|cx| build_view(cx));
         if let Some(invalidation) = window_context.window.invalidation.take() {
             window_context.invalidate(invalidation, appearance);
         }
@@ -471,8 +471,7 @@ impl<'a> WindowContext<'a> {
                 MatchResult::Pending => true,
                 MatchResult::Matches(matches) => {
                     for (view_id, action) in matches {
-                        if self.handle_dispatch_action_from_effect(Some(*view_id), action.as_ref())
-                        {
+                        if self.dispatch_action(Some(*view_id), action.as_ref()) {
                             self.keystroke_matcher.clear_pending();
                             handled_by = Some(action.boxed_clone());
                             break;
@@ -943,6 +942,7 @@ impl<'a> WindowContext<'a> {
             self,
         )?;
 
+        self.window.parents = new_parents;
         self.window
             .rendered_views
             .insert(root_view_id, rendered_root);
@@ -1017,14 +1017,11 @@ impl<'a> WindowContext<'a> {
         self.window.is_fullscreen
     }
 
-    pub(crate) fn handle_dispatch_action_from_effect(
-        &mut self,
-        view_id: Option<usize>,
-        action: &dyn Action,
-    ) -> bool {
+    pub(crate) fn dispatch_action(&mut self, view_id: Option<usize>, action: &dyn Action) -> bool {
         if let Some(view_id) = view_id {
             self.halt_action_dispatch = false;
             self.visit_dispatch_path(view_id, |view_id, capture_phase, cx| {
+                dbg!(view_id);
                 cx.update_any_view(view_id, |view, cx| {
                     let type_id = view.as_any().type_id();
                     if let Some((name, mut handlers)) = cx
@@ -1067,9 +1064,7 @@ impl<'a> WindowContext<'a> {
         std::iter::once(view_id)
             .into_iter()
             .chain(std::iter::from_fn(move || {
-                if let Some(ParentId::View(parent_id)) =
-                    self.parents.get(&(self.window_id, view_id))
-                {
+                if let Some(parent_id) = self.window.parents.get(&view_id) {
                     view_id = *parent_id;
                     Some(view_id)
                 } else {
@@ -1081,7 +1076,7 @@ impl<'a> WindowContext<'a> {
     /// Returns the id of the parent of the given view, or none if the given
     /// view is the root.
     pub(crate) fn parent(&self, view_id: usize) -> Option<usize> {
-        if let Some(ParentId::View(view_id)) = self.parents.get(&(self.window_id, view_id)) {
+        if let Some(view_id) = self.window.parents.get(&view_id) {
             Some(*view_id)
         } else {
             None
@@ -1170,27 +1165,27 @@ impl<'a> WindowContext<'a> {
         V: View,
         F: FnOnce(&mut ViewContext<V>) -> V,
     {
-        let root_view = self
-            .build_and_insert_view(ParentId::Root, |cx| Some(build_root_view(cx)))
-            .unwrap();
+        let root_view = self.add_view(|cx| build_root_view(cx));
         self.window.root_view = Some(root_view.clone().into_any());
         self.window.focused_view_id = Some(root_view.id());
         root_view
     }
 
-    pub(crate) fn build_and_insert_view<T, F>(
-        &mut self,
-        parent_id: ParentId,
-        build_view: F,
-    ) -> Option<ViewHandle<T>>
+    pub fn add_view<T, F>(&mut self, build_view: F) -> ViewHandle<T>
+    where
+        T: View,
+        F: FnOnce(&mut ViewContext<T>) -> T,
+    {
+        self.add_option_view(|cx| Some(build_view(cx))).unwrap()
+    }
+
+    pub fn add_option_view<T, F>(&mut self, build_view: F) -> Option<ViewHandle<T>>
     where
         T: View,
         F: FnOnce(&mut ViewContext<T>) -> Option<T>,
     {
         let window_id = self.window_id;
         let view_id = post_inc(&mut self.next_entity_id);
-        // Make sure we can tell child views about their parentu
-        self.parents.insert((window_id, view_id), parent_id);
         let mut cx = ViewContext::mutable(self, view_id);
         let handle = if let Some(view) = build_view(&mut cx) {
             self.views.insert((window_id, view_id), Box::new(view));
@@ -1201,7 +1196,6 @@ impl<'a> WindowContext<'a> {
                 .insert(view_id);
             Some(ViewHandle::new(window_id, view_id, &self.ref_counts))
         } else {
-            self.parents.remove(&(window_id, view_id));
             None
         };
         handle
@@ -1385,6 +1379,7 @@ impl<V: View> Element<V> for ChildView {
         cx: &mut LayoutContext<V>,
     ) -> (Vector2F, Self::LayoutState) {
         if let Some(mut rendered_view) = cx.window.rendered_views.remove(&self.view_id) {
+            cx.new_parents.insert(self.view_id, cx.view_id());
             let size = rendered_view
                 .layout(
                     constraint,

crates/project_symbols/src/project_symbols.rs 🔗

@@ -318,10 +318,10 @@ mod tests {
             },
         );
 
-        let (_, workspace) = cx.add_window(|cx| Workspace::test_new(project.clone(), cx));
+        let (window_id, workspace) = cx.add_window(|cx| Workspace::test_new(project.clone(), cx));
 
         // Create the project symbols view.
-        let symbols = cx.add_view(&workspace, |cx| {
+        let symbols = cx.add_view(window_id, |cx| {
             ProjectSymbols::new(
                 ProjectSymbolsDelegate::new(workspace.downgrade(), project.clone()),
                 cx,

crates/search/src/buffer_search.rs 🔗

@@ -670,13 +670,11 @@ mod tests {
                 cx,
             )
         });
-        let (_, root_view) = cx.add_window(|_| EmptyView);
+        let (window_id, _root_view) = cx.add_window(|_| EmptyView);
 
-        let editor = cx.add_view(&root_view, |cx| {
-            Editor::for_buffer(buffer.clone(), None, cx)
-        });
+        let editor = cx.add_view(window_id, |cx| Editor::for_buffer(buffer.clone(), None, cx));
 
-        let search_bar = cx.add_view(&root_view, |cx| {
+        let search_bar = cx.add_view(window_id, |cx| {
             let mut search_bar = BufferSearchBar::new(cx);
             search_bar.set_active_pane_item(Some(&editor), cx);
             search_bar.show(false, true, cx);

crates/search/src/project_search.rs 🔗

@@ -939,8 +939,6 @@ impl ToolbarItemView for ProjectSearchBar {
         self.subscription = None;
         self.active_project_search = None;
         if let Some(search) = active_pane_item.and_then(|i| i.downcast::<ProjectSearchView>()) {
-            let query_editor = search.read(cx).query_editor.clone();
-            cx.reparent(&query_editor);
             self.subscription = Some(cx.observe(&search, |_, _, cx| cx.notify()));
             self.active_project_search = Some(search);
             ToolbarItemLocation::PrimaryLeft {

crates/terminal_view/src/terminal_view.rs 🔗

@@ -2,7 +2,6 @@ mod persistence;
 pub mod terminal_button;
 pub mod terminal_element;
 
-use anyhow::anyhow;
 use context_menu::{ContextMenu, ContextMenuItem};
 use dirs::home_dir;
 use gpui::{
@@ -628,16 +627,12 @@ impl Item for TerminalView {
                     })
                 });
 
-            let pane = pane
-                .upgrade(&cx)
-                .ok_or_else(|| anyhow!("pane was dropped"))?;
-            cx.update(|cx| {
-                let terminal = project.update(cx, |project, cx| {
-                    project.create_terminal(cwd, window_id, cx)
-                })?;
-
-                Ok(cx.add_view(&pane, |cx| TerminalView::new(terminal, workspace_id, cx)))
-            })
+            let terminal = project.update(&mut cx, |project, cx| {
+                project.create_terminal(cwd, window_id, cx)
+            })?;
+            Ok(pane.update(&mut cx, |_, cx| {
+                cx.add_view(|cx| TerminalView::new(terminal, workspace_id, cx))
+            })?)
         })
     }
 

crates/workspace/src/pane.rs 🔗

@@ -537,7 +537,6 @@ impl Pane {
             // If the item already exists, move it to the desired destination and activate it
             pane.update(cx, |pane, cx| {
                 if existing_item_index != insertion_index {
-                    cx.reparent(item.as_any());
                     let existing_item_is_active = existing_item_index == pane.active_item_index;
 
                     // If the caller didn't specify a destination and the added item is already
@@ -567,7 +566,6 @@ impl Pane {
             });
         } else {
             pane.update(cx, |pane, cx| {
-                cx.reparent(item.as_any());
                 pane.items.insert(insertion_index, item);
                 if insertion_index <= pane.active_item_index {
                     pane.active_item_index += 1;

crates/workspace/src/status_bar.rs 🔗

@@ -93,7 +93,6 @@ impl StatusBar {
     where
         T: 'static + StatusItemView,
     {
-        cx.reparent(item.as_any());
         self.left_items.push(Box::new(item));
         cx.notify();
     }
@@ -102,7 +101,6 @@ impl StatusBar {
     where
         T: 'static + StatusItemView,
     {
-        cx.reparent(item.as_any());
         self.right_items.push(Box::new(item));
         cx.notify();
     }

crates/workspace/src/workspace.rs 🔗

@@ -3118,10 +3118,10 @@ mod tests {
 
         let fs = FakeFs::new(cx.background());
         let project = Project::test(fs, [], cx).await;
-        let (_, workspace) = cx.add_window(|cx| Workspace::test_new(project.clone(), cx));
+        let (window_id, workspace) = cx.add_window(|cx| Workspace::test_new(project.clone(), cx));
 
         // Adding an item with no ambiguity renders the tab without detail.
-        let item1 = cx.add_view(&workspace, |_| {
+        let item1 = cx.add_view(window_id, |_| {
             let mut item = TestItem::new();
             item.tab_descriptions = Some(vec!["c", "b1/c", "a/b1/c"]);
             item
@@ -3133,7 +3133,7 @@ mod tests {
 
         // Adding an item that creates ambiguity increases the level of detail on
         // both tabs.
-        let item2 = cx.add_view(&workspace, |_| {
+        let item2 = cx.add_view(window_id, |_| {
             let mut item = TestItem::new();
             item.tab_descriptions = Some(vec!["c", "b2/c", "a/b2/c"]);
             item
@@ -3147,7 +3147,7 @@ mod tests {
         // Adding an item that creates ambiguity increases the level of detail only
         // on the ambiguous tabs. In this case, the ambiguity can't be resolved so
         // we stop at the highest detail available.
-        let item3 = cx.add_view(&workspace, |_| {
+        let item3 = cx.add_view(window_id, |_| {
             let mut item = TestItem::new();
             item.tab_descriptions = Some(vec!["c", "b2/c", "a/b2/c"]);
             item
@@ -3187,10 +3187,10 @@ mod tests {
             project.worktrees(cx).next().unwrap().read(cx).id()
         });
 
-        let item1 = cx.add_view(&workspace, |cx| {
+        let item1 = cx.add_view(window_id, |cx| {
             TestItem::new().with_project_items(&[TestProjectItem::new(1, "one.txt", cx)])
         });
-        let item2 = cx.add_view(&workspace, |cx| {
+        let item2 = cx.add_view(window_id, |cx| {
             TestItem::new().with_project_items(&[TestProjectItem::new(2, "two.txt", cx)])
         });
 
@@ -3275,15 +3275,15 @@ mod tests {
         let (window_id, workspace) = cx.add_window(|cx| Workspace::test_new(project.clone(), cx));
 
         // When there are no dirty items, there's nothing to do.
-        let item1 = cx.add_view(&workspace, |_| TestItem::new());
+        let item1 = cx.add_view(window_id, |_| TestItem::new());
         workspace.update(cx, |w, cx| w.add_item(Box::new(item1.clone()), cx));
         let task = workspace.update(cx, |w, cx| w.prepare_to_close(false, cx));
         assert!(task.await.unwrap());
 
         // When there are dirty untitled items, prompt to save each one. If the user
         // cancels any prompt, then abort.
-        let item2 = cx.add_view(&workspace, |_| TestItem::new().with_dirty(true));
-        let item3 = cx.add_view(&workspace, |cx| {
+        let item2 = cx.add_view(window_id, |_| TestItem::new().with_dirty(true));
+        let item3 = cx.add_view(window_id, |cx| {
             TestItem::new()
                 .with_dirty(true)
                 .with_project_items(&[TestProjectItem::new(1, "1.txt", cx)])
@@ -3309,24 +3309,24 @@ mod tests {
         let project = Project::test(fs, None, cx).await;
         let (window_id, workspace) = cx.add_window(|cx| Workspace::test_new(project, cx));
 
-        let item1 = cx.add_view(&workspace, |cx| {
+        let item1 = cx.add_view(window_id, |cx| {
             TestItem::new()
                 .with_dirty(true)
                 .with_project_items(&[TestProjectItem::new(1, "1.txt", cx)])
         });
-        let item2 = cx.add_view(&workspace, |cx| {
+        let item2 = cx.add_view(window_id, |cx| {
             TestItem::new()
                 .with_dirty(true)
                 .with_conflict(true)
                 .with_project_items(&[TestProjectItem::new(2, "2.txt", cx)])
         });
-        let item3 = cx.add_view(&workspace, |cx| {
+        let item3 = cx.add_view(window_id, |cx| {
             TestItem::new()
                 .with_dirty(true)
                 .with_conflict(true)
                 .with_project_items(&[TestProjectItem::new(3, "3.txt", cx)])
         });
-        let item4 = cx.add_view(&workspace, |cx| {
+        let item4 = cx.add_view(window_id, |cx| {
             TestItem::new()
                 .with_dirty(true)
                 .with_project_items(&[TestProjectItem::new_untitled(cx)])
@@ -3420,7 +3420,7 @@ mod tests {
         // workspace items with multiple project entries.
         let single_entry_items = (0..=4)
             .map(|project_entry_id| {
-                cx.add_view(&workspace, |cx| {
+                cx.add_view(window_id, |cx| {
                     TestItem::new()
                         .with_dirty(true)
                         .with_project_items(&[TestProjectItem::new(
@@ -3431,7 +3431,7 @@ mod tests {
                 })
             })
             .collect::<Vec<_>>();
-        let item_2_3 = cx.add_view(&workspace, |cx| {
+        let item_2_3 = cx.add_view(window_id, |cx| {
             TestItem::new()
                 .with_dirty(true)
                 .with_singleton(false)
@@ -3440,7 +3440,7 @@ mod tests {
                     single_entry_items[3].read(cx).project_items[0].clone(),
                 ])
         });
-        let item_3_4 = cx.add_view(&workspace, |cx| {
+        let item_3_4 = cx.add_view(window_id, |cx| {
             TestItem::new()
                 .with_dirty(true)
                 .with_singleton(false)
@@ -3523,7 +3523,7 @@ mod tests {
         let project = Project::test(fs, [], cx).await;
         let (window_id, workspace) = cx.add_window(|cx| Workspace::test_new(project, cx));
 
-        let item = cx.add_view(&workspace, |cx| {
+        let item = cx.add_view(window_id, |cx| {
             TestItem::new().with_project_items(&[TestProjectItem::new(1, "1.txt", cx)])
         });
         let item_id = item.id();
@@ -3638,9 +3638,9 @@ mod tests {
         let fs = FakeFs::new(cx.background());
 
         let project = Project::test(fs, [], cx).await;
-        let (_, workspace) = cx.add_window(|cx| Workspace::test_new(project, cx));
+        let (window_id, workspace) = cx.add_window(|cx| Workspace::test_new(project, cx));
 
-        let item = cx.add_view(&workspace, |cx| {
+        let item = cx.add_view(window_id, |cx| {
             TestItem::new().with_project_items(&[TestProjectItem::new(1, "1.txt", cx)])
         });
         let pane = workspace.read_with(cx, |workspace, _| workspace.active_pane().clone());