From 7f137ed3ddb90de232ec32eb3fef8fbffc738644 Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Wed, 3 May 2023 16:36:14 +0200 Subject: [PATCH] Compute view ancestry at layout time --- 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(-) diff --git a/crates/collab/src/tests.rs b/crates/collab/src/tests.rs index 1001493242..81a505cc4e 100644 --- a/crates/collab/src/tests.rs +++ b/crates/collab/src/tests.rs @@ -462,8 +462,8 @@ impl TestClient { project: &ModelHandle, cx: &mut TestAppContext, ) -> ViewHandle { - 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)) } } diff --git a/crates/collab/src/tests/integration_tests.rs b/crates/collab/src/tests/integration_tests.rs index 7d6199b109..c6f51a9dc4 100644 --- a/crates/collab/src/tests/integration_tests.rs +++ b/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) }); diff --git a/crates/command_palette/src/command_palette.rs b/crates/command_palette/src/command_palette.rs index 441fbb84a6..123c33cba1 100644 --- a/crates/command_palette/src/command_palette.rs +++ b/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 diff --git a/crates/diagnostics/src/diagnostics.rs b/crates/diagnostics/src/diagnostics.rs index 17d142ba4b..d82c653a09 100644 --- a/crates/diagnostics/src/diagnostics.rs +++ b/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) }); diff --git a/crates/editor/src/editor_tests.rs b/crates/editor/src/editor_tests.rs index af4bc1674b..76c34c63d5 100644 --- a/crates/editor/src/editor_tests.rs +++ b/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(); diff --git a/crates/editor/src/items.rs b/crates/editor/src/items.rs index dcd49607fb..342967136b 100644 --- a/crates/editor/src/items.rs +++ b/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::() .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))) diff --git a/crates/gpui/src/app.rs b/crates/gpui/src/app.rs index f8d6459cf4..32ac815232 100644 --- a/crates/gpui/src/app.rs +++ b/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 b pub struct AppContext { models: HashMap>, views: HashMap<(usize, usize), Box>, - pub(crate) parents: HashMap<(usize, usize), ParentId>, windows: HashMap, globals: HashMap>, element_states: HashMap>, @@ -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(&mut self, parent: &AnyViewHandle, build_view: F) -> ViewHandle - where - S: View, - F: FnOnce(&mut ViewContext) -> 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(&self, handle: &ViewHandle) -> &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::>() { 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(&mut self, build_view: F) -> ViewHandle - where - S: View, - F: FnOnce(&mut ViewContext) -> S, - { - self.window_context - .build_and_insert_view(ParentId::View(self.view_id), |cx| Some(build_view(cx))) - .unwrap() - } - - pub fn add_option_view(&mut self, build_view: F) -> Option> - where - S: View, - F: FnOnce(&mut ViewContext) -> Option, - { - 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(&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>>, + child: Option, } impl Entity for View { @@ -5259,8 +5220,11 @@ mod tests { } impl super::View for View { - fn render(&mut self, _: &mut ViewContext) -> AnyElement { - Empty::new().into_any() + fn render(&mut self, cx: &mut ViewContext) -> AnyElement { + 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>> = 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, } impl Entity for ViewA { @@ -5440,8 +5409,11 @@ mod tests { } impl View for ViewA { - fn render(&mut self, _: &mut ViewContext) -> AnyElement { - Empty::new().into_any() + fn render(&mut self, cx: &mut ViewContext) -> AnyElement { + 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, } impl Entity for ViewB { @@ -5458,8 +5431,11 @@ mod tests { } impl View for ViewB { - fn render(&mut self, _: &mut ViewContext) -> AnyElement { - Empty::new().into_any() + fn render(&mut self, cx: &mut ViewContext) -> AnyElement { + 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, } impl Entity for View { @@ -5606,8 +5630,11 @@ mod tests { } impl super::View for View { - fn render(&mut self, _: &mut ViewContext) -> AnyElement { - Empty::new().into_any() + fn render(&mut self, cx: &mut ViewContext) -> AnyElement { + 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, + } struct View2 {} impl Entity for View1 { @@ -5733,8 +5769,8 @@ mod tests { } impl super::View for View1 { - fn render(&mut self, _: &mut ViewContext) -> AnyElement { - Empty::new().into_any() + fn render(&mut self, cx: &mut ViewContext) -> AnyElement { + 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!( diff --git a/crates/gpui/src/app/menu.rs b/crates/gpui/src/app/menu.rs index ed23296618..a2ac13984b 100644 --- a/crates/gpui/src/app/menu.rs +++ b/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 diff --git a/crates/gpui/src/app/test_app_context.rs b/crates/gpui/src/app/test_app_context.rs index 2d079a6042..af01eec1be 100644 --- a/crates/gpui/src/app/test_app_context.rs +++ b/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(&mut self, parent_handle: &AnyViewHandle, build_view: F) -> ViewHandle + pub fn add_view(&mut self, window_id: usize, build_view: F) -> ViewHandle where T: View, F: FnOnce(&mut ViewContext) -> 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(&mut self, callback: F) -> Subscription diff --git a/crates/gpui/src/app/window.rs b/crates/gpui/src/app/window.rs index f9ccef7ae0..3d04b46e06 100644 --- a/crates/gpui/src/app/window.rs +++ b/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, pub(crate) focused_view_id: Option, + pub(crate) parents: HashMap, pub(crate) is_active: bool, pub(crate) is_fullscreen: bool, pub(crate) invalidation: Option, @@ -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, - action: &dyn Action, - ) -> bool { + pub(crate) fn dispatch_action(&mut self, view_id: Option, 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 { - 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, { - 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( - &mut self, - parent_id: ParentId, - build_view: F, - ) -> Option> + pub fn add_view(&mut self, build_view: F) -> ViewHandle + where + T: View, + F: FnOnce(&mut ViewContext) -> T, + { + self.add_option_view(|cx| Some(build_view(cx))).unwrap() + } + + pub fn add_option_view(&mut self, build_view: F) -> Option> where T: View, F: FnOnce(&mut ViewContext) -> Option, { 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 Element for ChildView { cx: &mut LayoutContext, ) -> (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, diff --git a/crates/project_symbols/src/project_symbols.rs b/crates/project_symbols/src/project_symbols.rs index 6720ef93de..25828f17ca 100644 --- a/crates/project_symbols/src/project_symbols.rs +++ b/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, diff --git a/crates/search/src/buffer_search.rs b/crates/search/src/buffer_search.rs index ee5a2e8332..91284a545f 100644 --- a/crates/search/src/buffer_search.rs +++ b/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); diff --git a/crates/search/src/project_search.rs b/crates/search/src/project_search.rs index ea29f9cfda..6266c571bf 100644 --- a/crates/search/src/project_search.rs +++ b/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::()) { - 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 { diff --git a/crates/terminal_view/src/terminal_view.rs b/crates/terminal_view/src/terminal_view.rs index 5e04fc9825..a40cb4508d 100644 --- a/crates/terminal_view/src/terminal_view.rs +++ b/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)) + })?) }) } diff --git a/crates/workspace/src/pane.rs b/crates/workspace/src/pane.rs index 599f04166a..8b0107d368 100644 --- a/crates/workspace/src/pane.rs +++ b/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; diff --git a/crates/workspace/src/sidebar.rs b/crates/workspace/src/sidebar.rs index 2b114d83ec..f37bd6da5f 100644 --- a/crates/workspace/src/sidebar.rs +++ b/crates/workspace/src/sidebar.rs @@ -146,7 +146,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 c31acc2f99..b4de6b3575 100644 --- a/crates/workspace/src/status_bar.rs +++ b/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(); } diff --git a/crates/workspace/src/workspace.rs b/crates/workspace/src/workspace.rs index 5a00853737..f9a86d3dcc 100644 --- a/crates/workspace/src/workspace.rs +++ b/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::>(); - 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());