Remove `ReadView` and `UpdateView` traits

Antonio Scandurra and Nathan Sobo created

Co-Authored-By: Nathan Sobo <nathan@zed.dev>

Change summary

crates/editor/src/editor.rs                   |   6 
crates/editor/src/editor_tests.rs             |  12 
crates/editor/src/items.rs                    |   2 
crates/editor/src/test/editor_test_context.rs |   2 
crates/gpui/src/app.rs                        | 140 ++++++++------------
crates/gpui/src/app/test_app_context.rs       |  69 +++------
crates/gpui/src/app/window.rs                 |  62 +++++---
crates/vim/src/test/vim_test_context.rs       |   4 
crates/workspace/src/dock.rs                  |  23 +-
crates/workspace/src/pane.rs                  |   4 
crates/workspace/src/persistence/model.rs     |   5 
crates/workspace/src/workspace.rs             |  12 
12 files changed, 159 insertions(+), 182 deletions(-)

Detailed changes

crates/editor/src/editor.rs 🔗

@@ -2715,7 +2715,7 @@ impl Editor {
         title: String,
         mut cx: AsyncAppContext,
     ) -> Result<()> {
-        let replica_id = this.read_with(&cx, |this, cx| this.replica_id(cx));
+        let replica_id = this.read_with(&cx, |this, cx| this.replica_id(cx))?;
 
         let mut entries = transaction.0.into_iter().collect::<Vec<_>>();
         entries.sort_unstable_by_key(|(buffer, _)| {
@@ -2732,7 +2732,7 @@ impl Editor {
                         .buffer()
                         .read(cx)
                         .excerpt_containing(editor.selections.newest_anchor().head(), cx)
-                });
+                })?;
                 if let Some((_, excerpted_buffer, excerpt_range)) = excerpt {
                     if excerpted_buffer == *buffer {
                         let all_edits_within_excerpt = buffer.read_with(&cx, |buffer, _| {
@@ -5814,7 +5814,7 @@ impl Editor {
                     buffer_highlights
                         .next()
                         .map(|highlight| highlight.start.text_anchor..highlight.end.text_anchor)
-                })
+                })?
             };
             if let Some(rename_range) = rename_range {
                 let rename_buffer_range = rename_range.to_offset(&snapshot);

crates/editor/src/editor_tests.rs 🔗

@@ -5673,8 +5673,8 @@ async fn test_following_with_multiple_excerpts(cx: &mut gpui::TestAppContext) {
         .await
         .unwrap();
     assert_eq!(
-        follower_1.read_with(cx, Editor::text),
-        leader.read_with(cx, Editor::text)
+        follower_1.read_with(cx, |editor, cx| editor.text(cx)),
+        leader.read_with(cx, |editor, cx| editor.text(cx))
     );
     update_message.borrow_mut().take();
 
@@ -5697,8 +5697,8 @@ async fn test_following_with_multiple_excerpts(cx: &mut gpui::TestAppContext) {
         .await
         .unwrap();
     assert_eq!(
-        follower_2.read_with(cx, Editor::text),
-        leader.read_with(cx, Editor::text)
+        follower_2.read_with(cx, |editor, cx| editor.text(cx)),
+        leader.read_with(cx, |editor, cx| editor.text(cx))
     );
 
     // Remove some excerpts.
@@ -5725,8 +5725,8 @@ async fn test_following_with_multiple_excerpts(cx: &mut gpui::TestAppContext) {
         .unwrap();
     update_message.borrow_mut().take();
     assert_eq!(
-        follower_1.read_with(cx, Editor::text),
-        leader.read_with(cx, Editor::text)
+        follower_1.read_with(cx, |editor, cx| editor.text(cx)),
+        leader.read_with(cx, |editor, cx| editor.text(cx))
     );
 }
 

crates/editor/src/items.rs 🔗

@@ -78,7 +78,7 @@ impl FollowableItem for Editor {
                             == editor.read(cx).buffer.read(cx).as_singleton().as_ref();
                     ids_match || singleton_buffer_matches
                 })
-            });
+            })?;
 
             let editor = if let Some(editor) = editor {
                 editor

crates/editor/src/test/editor_test_context.rs 🔗

@@ -57,7 +57,7 @@ impl<'a> EditorTestContext<'a> {
 
     pub fn editor<F, T>(&self, read: F) -> T
     where
-        F: FnOnce(&Editor, &AppContext) -> T,
+        F: FnOnce(&Editor, &ViewContext<Editor>) -> T,
     {
         self.editor.read_with(self.cx, read)
     }

crates/gpui/src/app.rs 🔗

@@ -126,26 +126,19 @@ pub trait BorrowAppContext {
     fn update<T, F: FnOnce(&mut AppContext) -> T>(&mut self, f: F) -> T;
 }
 
-pub trait ReadViewWith {
-    fn read_view_with<V, T>(
-        &self,
-        handle: &ViewHandle<V>,
-        read: &mut dyn FnMut(&V, &AppContext) -> T,
-    ) -> T
-    where
-        V: View;
-}
+pub trait BorrowWindowContext {
+    type ReturnValue<T>;
 
-pub trait UpdateView {
-    type Output<S>;
-
-    fn update_view<T, S>(
+    fn read_with<T, F: FnOnce(&WindowContext) -> T>(
+        &self,
+        window_id: usize,
+        f: F,
+    ) -> Self::ReturnValue<T>;
+    fn update<T, F: FnOnce(&mut WindowContext) -> T>(
         &mut self,
-        handle: &ViewHandle<T>,
-        update: &mut dyn FnMut(&mut T, &mut ViewContext<T>) -> S,
-    ) -> Self::Output<S>
-    where
-        T: View;
+        window_id: usize,
+        f: F,
+    ) -> Self::ReturnValue<T>;
 }
 
 #[derive(Clone)]
@@ -388,36 +381,25 @@ impl BorrowAppContext for AsyncAppContext {
     }
 }
 
-impl UpdateView for AsyncAppContext {
-    type Output<S> = Result<S>;
+impl BorrowWindowContext for AsyncAppContext {
+    type ReturnValue<T> = Result<T>;
 
-    fn update_view<T, S>(
-        &mut self,
-        handle: &ViewHandle<T>,
-        update: &mut dyn FnMut(&mut T, &mut ViewContext<T>) -> S,
-    ) -> Result<S>
-    where
-        T: View,
-    {
+    fn read_with<T, F: FnOnce(&WindowContext) -> T>(&self, window_id: usize, f: F) -> Result<T> {
         self.0
-            .borrow_mut()
-            .update_window(handle.window_id, |cx| cx.update_view(handle, update))
+            .borrow()
+            .read_window(window_id, f)
             .ok_or_else(|| anyhow!("window was closed"))
     }
-}
 
-impl ReadViewWith for AsyncAppContext {
-    fn read_view_with<V, T>(
-        &self,
-        handle: &ViewHandle<V>,
-        read: &mut dyn FnMut(&V, &AppContext) -> T,
-    ) -> T
-    where
-        V: View,
-    {
-        let cx = self.0.borrow();
-        let cx = &*cx;
-        read(handle.read(cx), cx)
+    fn update<T, F: FnOnce(&mut WindowContext) -> T>(
+        &mut self,
+        window_id: usize,
+        f: F,
+    ) -> Result<T> {
+        self.0
+            .borrow_mut()
+            .update_window(window_id, f)
+            .ok_or_else(|| anyhow!("window was closed"))
     }
 }
 
@@ -3349,26 +3331,23 @@ impl<'a, 'b, V: View> ViewContext<'a, 'b, V> {
 
 impl<V> BorrowAppContext for ViewContext<'_, '_, V> {
     fn read_with<T, F: FnOnce(&AppContext) -> T>(&self, f: F) -> T {
-        self.window_context.read_with(f)
+        BorrowAppContext::read_with(&*self.window_context, f)
     }
 
     fn update<T, F: FnOnce(&mut AppContext) -> T>(&mut self, f: F) -> T {
-        self.window_context.update(f)
+        BorrowAppContext::update(&mut *self.window_context, f)
     }
 }
 
-impl<V: View> UpdateView for ViewContext<'_, '_, V> {
-    type Output<S> = S;
+impl<V> BorrowWindowContext for ViewContext<'_, '_, V> {
+    type ReturnValue<T> = T;
 
-    fn update_view<T, S>(
-        &mut self,
-        handle: &ViewHandle<T>,
-        update: &mut dyn FnMut(&mut T, &mut ViewContext<T>) -> S,
-    ) -> S
-    where
-        T: View,
-    {
-        self.window_context.update_view(handle, update)
+    fn read_with<T, F: FnOnce(&WindowContext) -> T>(&self, window_id: usize, f: F) -> T {
+        BorrowWindowContext::read_with(&*self.window_context, window_id, f)
+    }
+
+    fn update<T, F: FnOnce(&mut WindowContext) -> T>(&mut self, window_id: usize, f: F) -> T {
+        BorrowWindowContext::update(&mut *self.window_context, window_id, f)
     }
 }
 
@@ -3406,26 +3385,23 @@ impl<V: View> DerefMut for EventContext<'_, '_, '_, V> {
 
 impl<V: View> BorrowAppContext for EventContext<'_, '_, '_, V> {
     fn read_with<T, F: FnOnce(&AppContext) -> T>(&self, f: F) -> T {
-        self.view_context.read_with(f)
+        BorrowAppContext::read_with(&*self.view_context, f)
     }
 
     fn update<T, F: FnOnce(&mut AppContext) -> T>(&mut self, f: F) -> T {
-        self.view_context.update(f)
+        BorrowAppContext::update(&mut *self.view_context, f)
     }
 }
 
-impl<V: View> UpdateView for EventContext<'_, '_, '_, V> {
-    type Output<S> = S;
+impl<V: View> BorrowWindowContext for EventContext<'_, '_, '_, V> {
+    type ReturnValue<T> = T;
 
-    fn update_view<T, S>(
-        &mut self,
-        handle: &ViewHandle<T>,
-        update: &mut dyn FnMut(&mut T, &mut ViewContext<T>) -> S,
-    ) -> S
-    where
-        T: View,
-    {
-        self.view_context.update_view(handle, update)
+    fn read_with<T, F: FnOnce(&WindowContext) -> T>(&self, window_id: usize, f: F) -> T {
+        BorrowWindowContext::read_with(&*self.view_context, window_id, f)
+    }
+
+    fn update<T, F: FnOnce(&mut WindowContext) -> T>(&mut self, window_id: usize, f: F) -> T {
+        BorrowWindowContext::update(&mut *self.view_context, window_id, f)
     }
 }
 
@@ -3756,27 +3732,29 @@ impl<T: View> ViewHandle<T> {
         cx.read_view(self)
     }
 
-    pub fn read_with<C, F, S>(&self, cx: &C, read: F) -> S
+    pub fn read_with<C, F, S>(&self, cx: &C, read: F) -> C::ReturnValue<S>
     where
-        C: ReadViewWith,
-        F: FnOnce(&T, &AppContext) -> S,
+        C: BorrowWindowContext,
+        F: FnOnce(&T, &ViewContext<T>) -> S,
     {
-        let mut read = Some(read);
-        cx.read_view_with(self, &mut |view, cx| {
-            let read = read.take().unwrap();
-            read(view, cx)
+        cx.read_with(self.window_id, |cx| {
+            let cx = ViewContext::immutable(cx, self.view_id);
+            read(cx.read_view(self), &cx)
         })
     }
 
-    pub fn update<C, F, S>(&self, cx: &mut C, update: F) -> C::Output<S>
+    pub fn update<C, F, S>(&self, cx: &mut C, update: F) -> C::ReturnValue<S>
     where
-        C: UpdateView,
+        C: BorrowWindowContext,
         F: FnOnce(&mut T, &mut ViewContext<T>) -> S,
     {
         let mut update = Some(update);
-        cx.update_view(self, &mut |view, cx| {
-            let update = update.take().unwrap();
-            update(view, cx)
+
+        cx.update(self.window_id, |cx| {
+            cx.update_view(self, &mut |view, cx| {
+                let update = update.take().unwrap();
+                update(view, cx)
+            })
         })
     }
 

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

@@ -1,3 +1,18 @@
+use crate::{
+    executor,
+    geometry::vector::Vector2F,
+    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,
+};
+use collections::BTreeMap;
+use futures::Future;
+use itertools::Itertools;
+use parking_lot::{Mutex, RwLock};
+use smol::stream::StreamExt;
 use std::{
     any::Any,
     cell::RefCell,
@@ -11,23 +26,6 @@ use std::{
     time::Duration,
 };
 
-use futures::Future;
-use itertools::Itertools;
-use parking_lot::{Mutex, RwLock};
-use smol::stream::StreamExt;
-
-use crate::{
-    executor,
-    geometry::vector::Vector2F,
-    keymap_matcher::Keystroke,
-    platform,
-    platform::{Event, InputHandler, KeyDownEvent, Platform},
-    Action, AnyViewHandle, AppContext, BorrowAppContext, Entity, FontCache, Handle, ModelContext,
-    ModelHandle, ReadViewWith, Subscription, Task, UpdateView, View, ViewContext, ViewHandle,
-    WeakHandle, WindowContext,
-};
-use collections::BTreeMap;
-
 use super::{
     ref_counts::LeakDetector, window_input_handler::WindowInputHandler, AsyncAppContext, RefCounts,
 };
@@ -391,36 +389,21 @@ impl BorrowAppContext for TestAppContext {
     }
 }
 
-impl UpdateView for TestAppContext {
-    type Output<S> = S;
+impl BorrowWindowContext for TestAppContext {
+    type ReturnValue<T> = T;
 
-    fn update_view<T, S>(
-        &mut self,
-        handle: &ViewHandle<T>,
-        update: &mut dyn FnMut(&mut T, &mut ViewContext<T>) -> S,
-    ) -> S
-    where
-        T: View,
-    {
+    fn read_with<T, F: FnOnce(&WindowContext) -> T>(&self, window_id: usize, f: F) -> T {
         self.cx
-            .borrow_mut()
-            .update_window(handle.window_id, |cx| cx.update_view(handle, update))
-            .unwrap()
+            .borrow()
+            .read_window(window_id, f)
+            .expect("window was closed")
     }
-}
 
-impl ReadViewWith for TestAppContext {
-    fn read_view_with<V, T>(
-        &self,
-        handle: &ViewHandle<V>,
-        read: &mut dyn FnMut(&V, &AppContext) -> T,
-    ) -> T
-    where
-        V: View,
-    {
-        let cx = self.cx.borrow();
-        let cx = &*cx;
-        read(handle.read(cx), cx)
+    fn update<T, F: FnOnce(&mut WindowContext) -> T>(&mut self, window_id: usize, f: F) -> T {
+        self.cx
+            .borrow_mut()
+            .update_window(window_id, f)
+            .expect("window was closed")
     }
 }
 

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

@@ -13,9 +13,9 @@ use crate::{
     },
     text_layout::TextLayoutCache,
     util::post_inc,
-    Action, AnyView, AnyViewHandle, AppContext, BorrowAppContext, Effect, Element, Entity, Handle,
-    MouseRegion, MouseRegionId, ParentId, SceneBuilder, Subscription, UpdateView, View,
-    ViewContext, ViewHandle, WindowInvalidation,
+    Action, AnyView, AnyViewHandle, AppContext, BorrowAppContext, BorrowWindowContext, Effect,
+    Element, Entity, Handle, MouseRegion, MouseRegionId, ParentId, SceneBuilder, Subscription,
+    View, ViewContext, ViewHandle, WindowInvalidation,
 };
 use anyhow::{anyhow, bail, Result};
 use collections::{HashMap, HashSet};
@@ -141,27 +141,23 @@ impl BorrowAppContext for WindowContext<'_> {
     }
 }
 
-impl UpdateView for WindowContext<'_> {
-    type Output<S> = S;
+impl BorrowWindowContext for WindowContext<'_> {
+    type ReturnValue<T> = T;
 
-    fn update_view<T, S>(
-        &mut self,
-        handle: &ViewHandle<T>,
-        update: &mut dyn FnMut(&mut T, &mut ViewContext<T>) -> S,
-    ) -> S
-    where
-        T: View,
-    {
-        self.update_any_view(handle.view_id, |view, cx| {
-            let mut cx = ViewContext::mutable(cx, handle.view_id);
-            update(
-                view.as_any_mut()
-                    .downcast_mut()
-                    .expect("downcast is type safe"),
-                &mut cx,
-            )
-        })
-        .expect("view is already on the stack")
+    fn read_with<T, F: FnOnce(&WindowContext) -> T>(&self, window_id: usize, f: F) -> T {
+        if self.window_id == window_id {
+            f(self)
+        } else {
+            panic!("read_with called with id of window that does not belong to this context")
+        }
+    }
+
+    fn update<T, F: FnOnce(&mut WindowContext) -> T>(&mut self, window_id: usize, f: F) -> T {
+        if self.window_id == window_id {
+            f(self)
+        } else {
+            panic!("update called with id of window that does not belong to this context")
+        }
     }
 }
 
@@ -225,6 +221,26 @@ impl<'a> WindowContext<'a> {
         Some(result)
     }
 
+    pub(crate) fn update_view<T, S>(
+        &mut self,
+        handle: &ViewHandle<T>,
+        update: &mut dyn FnMut(&mut T, &mut ViewContext<T>) -> S,
+    ) -> S
+    where
+        T: View,
+    {
+        self.update_any_view(handle.view_id, |view, cx| {
+            let mut cx = ViewContext::mutable(cx, handle.view_id);
+            update(
+                view.as_any_mut()
+                    .downcast_mut()
+                    .expect("downcast is type safe"),
+                &mut cx,
+            )
+        })
+        .expect("view is already on the stack")
+    }
+
     pub fn defer(&mut self, callback: impl 'static + FnOnce(&mut WindowContext)) {
         let window_id = self.window_id;
         self.app_context.defer(move |cx| {

crates/vim/src/test/vim_test_context.rs 🔗

@@ -3,7 +3,7 @@ use std::ops::{Deref, DerefMut};
 use editor::test::{
     editor_lsp_test_context::EditorLspTestContext, editor_test_context::EditorTestContext,
 };
-use gpui::{AppContext, ContextHandle};
+use gpui::ContextHandle;
 use search::{BufferSearchBar, ProjectSearchBar};
 
 use crate::{state::Operator, *};
@@ -45,7 +45,7 @@ impl<'a> VimTestContext<'a> {
 
     pub fn workspace<F, T>(&mut self, read: F) -> T
     where
-        F: FnOnce(&Workspace, &AppContext) -> T,
+        F: FnOnce(&Workspace, &ViewContext<Workspace>) -> T,
     {
         self.cx.workspace.read_with(self.cx.cx.cx, read)
     }

crates/workspace/src/dock.rs 🔗

@@ -428,7 +428,7 @@ mod tests {
         path::PathBuf,
     };
 
-    use gpui::{AppContext, TestAppContext, UpdateView, View, ViewContext};
+    use gpui::{AppContext, BorrowWindowContext, TestAppContext, ViewContext, WindowContext};
     use project::{FakeFs, Project};
     use settings::Settings;
 
@@ -660,7 +660,7 @@ mod tests {
 
         pub fn workspace<F, T>(&self, read: F) -> T
         where
-            F: FnOnce(&Workspace, &AppContext) -> T,
+            F: FnOnce(&Workspace, &ViewContext<Workspace>) -> T,
         {
             self.workspace.read_with(self.cx, read)
         }
@@ -810,18 +810,15 @@ mod tests {
         }
     }
 
-    impl<'a> UpdateView for DockTestContext<'a> {
-        type Output<S> = S;
+    impl BorrowWindowContext for DockTestContext<'_> {
+        type ReturnValue<T> = T;
 
-        fn update_view<T, S>(
-            &mut self,
-            handle: &ViewHandle<T>,
-            update: &mut dyn FnMut(&mut T, &mut ViewContext<T>) -> S,
-        ) -> S
-        where
-            T: View,
-        {
-            handle.update(self.cx, update)
+        fn read_with<T, F: FnOnce(&WindowContext) -> T>(&self, window_id: usize, f: F) -> T {
+            BorrowWindowContext::read_with(self.cx, window_id, f)
+        }
+
+        fn update<T, F: FnOnce(&mut WindowContext) -> T>(&mut self, window_id: usize, f: F) -> T {
+            BorrowWindowContext::update(self.cx, window_id, f)
         }
     }
 }

crates/workspace/src/pane.rs 🔗

@@ -981,7 +981,7 @@ impl Pane {
                 // was started.
                 let (item_ix, mut project_item_ids) = pane.read_with(&cx, |pane, cx| {
                     (pane.index_for_item(&*item), item.project_item_model_ids(cx))
-                });
+                })?;
                 let item_ix = if let Some(ix) = item_ix {
                     ix
                 } else {
@@ -1001,7 +1001,7 @@ impl Pane {
                             project_item_ids.retain(|id| !other_project_item_ids.contains(id));
                         }
                     }
-                });
+                })?;
                 let should_save = project_item_ids
                     .iter()
                     .any(|id| saved_project_items_ids.insert(*id));

crates/workspace/src/persistence/model.rs 🔗

@@ -140,7 +140,10 @@ impl SerializedPaneGroup {
                     .await
                     .log_err()?;
 
-                if pane.read_with(cx, |pane, _| pane.items_len() != 0) {
+                if pane
+                    .read_with(cx, |pane, _| pane.items_len() != 0)
+                    .log_err()?
+                {
                     Some((Member::Pane(pane.clone()), active.then(|| pane)))
                 } else {
                     workspace

crates/workspace/src/workspace.rs 🔗

@@ -1226,7 +1226,7 @@ impl Workspace {
                     cx.read(|cx| (item.is_singleton(cx), item.project_entry_ids(cx)));
                 if singleton || !project_entry_ids.is_empty() {
                     if let Some(ix) =
-                        pane.read_with(&cx, |pane, _| pane.index_for_item(item.as_ref()))
+                        pane.read_with(&cx, |pane, _| pane.index_for_item(item.as_ref()))?
                     {
                         if !Pane::save_item(
                             project.clone(),
@@ -2298,7 +2298,7 @@ impl Workspace {
         this.read_with(&cx, |this, _| {
             this.leader_updates_tx
                 .unbounded_send((leader_id, envelope.payload))
-        })?;
+        })??;
         Ok(())
     }
 
@@ -2354,7 +2354,7 @@ impl Workspace {
                         .flat_map(|states_by_pane| states_by_pane.keys())
                         .cloned()
                         .collect()
-                });
+                })?;
                 Self::add_views_from_leader(this.clone(), leader_id, panes, vec![view], cx).await?;
             }
         }
@@ -2369,7 +2369,7 @@ impl Workspace {
         views: Vec<proto::View>,
         cx: &mut AsyncAppContext,
     ) -> Result<()> {
-        let project = this.read_with(cx, |this, _| this.project.clone());
+        let project = this.read_with(cx, |this, _| this.project.clone())?;
         let replica_id = project
             .read_with(cx, |project, _| {
                 project
@@ -2699,7 +2699,7 @@ impl Workspace {
                             workspace.dock_pane().clone(),
                             workspace.last_active_center_pane.clone(),
                         )
-                    });
+                    })?;
 
                 serialized_workspace
                     .dock_pane
@@ -2765,7 +2765,7 @@ impl Workspace {
                 })?;
 
                 // Serialize ourself to make sure our timestamps and any pane / item changes are replicated
-                workspace.read_with(&cx, |workspace, cx| workspace.serialize_workspace(cx))
+                workspace.read_with(&cx, |workspace, cx| workspace.serialize_workspace(cx))?
             }
             anyhow::Ok(())
         })