Use the new buffer subscription API to keep `DisplayMap` in sync

Antonio Scandurra and Nathan Sobo created 4 years ago

Co-Authored-By: Nathan Sobo <[e-mail]>

Change summary

crates/editor/src/display_map.rs | 55 ++++++++-------------------------
crates/editor/src/editor.rs      |  2 
crates/language/src/buffer.rs    |  8 +++-
crates/language/src/tests.rs     | 27 +---------------
crates/project/src/worktree.rs   | 28 +++--------------
crates/text/src/text.rs          | 17 +++------
6 files changed, 34 insertions(+), 103 deletions(-)

Detailed changes

crates/editor/src/display_map.rs

@@ -10,11 +10,9 @@ use gpui::{
     fonts::{FontId, HighlightStyle},
     AppContext, Entity, ModelContext, ModelHandle,
 };
-use language::{Anchor, Buffer, Patch, Point, ToOffset, ToPoint};
-use parking_lot::Mutex;
+use language::{Anchor, Buffer, Point, Subscription as BufferSubscription, ToOffset, ToPoint};
 use std::{
     collections::{HashMap, HashSet},
-    mem,
     ops::Range,
 };
 use sum_tree::Bias;
@@ -29,11 +27,11 @@ pub trait ToDisplayPoint {
 
 pub struct DisplayMap {
     buffer: ModelHandle<Buffer>,
+    buffer_subscription: BufferSubscription,
     fold_map: FoldMap,
     tab_map: TabMap,
     wrap_map: ModelHandle<WrapMap>,
     block_map: BlockMap,
-    buffer_edits_since_sync: Mutex<Patch<usize>>,
 }
 
 impl Entity for DisplayMap {
@@ -49,28 +47,26 @@ impl DisplayMap {
         wrap_width: Option<f32>,
         cx: &mut ModelContext<Self>,
     ) -> Self {
+        let buffer_subscription = buffer.update(cx, |buffer, _| buffer.subscribe());
         let (fold_map, snapshot) = FoldMap::new(buffer.read(cx).snapshot());
         let (tab_map, snapshot) = TabMap::new(snapshot, tab_size);
         let (wrap_map, snapshot) = WrapMap::new(snapshot, font_id, font_size, wrap_width, cx);
         let block_map = BlockMap::new(buffer.clone(), snapshot);
         cx.observe(&wrap_map, |_, _, cx| cx.notify()).detach();
-        cx.subscribe(&buffer, Self::handle_buffer_event).detach();
         DisplayMap {
             buffer,
+            buffer_subscription,
             fold_map,
             tab_map,
             wrap_map,
             block_map,
-            buffer_edits_since_sync: Default::default(),
         }
     }
 
     pub fn snapshot(&self, cx: &mut ModelContext<Self>) -> DisplayMapSnapshot {
         let buffer_snapshot = self.buffer.read(cx).snapshot();
-        let (folds_snapshot, edits) = self.fold_map.read(
-            buffer_snapshot,
-            mem::take(&mut *self.buffer_edits_since_sync.lock()).into_inner(),
-        );
+        let edits = self.buffer_subscription.consume().into_inner();
+        let (folds_snapshot, edits) = self.fold_map.read(buffer_snapshot, edits);
         let (tabs_snapshot, edits) = self.tab_map.sync(folds_snapshot.clone(), edits);
         let (wraps_snapshot, edits) = self
             .wrap_map
@@ -92,10 +88,8 @@ impl DisplayMap {
         cx: &mut ModelContext<Self>,
     ) {
         let snapshot = self.buffer.read(cx).snapshot();
-        let (mut fold_map, snapshot, edits) = self.fold_map.write(
-            snapshot,
-            mem::take(&mut *self.buffer_edits_since_sync.lock()).into_inner(),
-        );
+        let edits = self.buffer_subscription.consume().into_inner();
+        let (mut fold_map, snapshot, edits) = self.fold_map.write(snapshot, edits);
         let (snapshot, edits) = self.tab_map.sync(snapshot, edits);
         let (snapshot, edits) = self
             .wrap_map
@@ -115,10 +109,8 @@ impl DisplayMap {
         cx: &mut ModelContext<Self>,
     ) {
         let snapshot = self.buffer.read(cx).snapshot();
-        let (mut fold_map, snapshot, edits) = self.fold_map.write(
-            snapshot,
-            mem::take(&mut *self.buffer_edits_since_sync.lock()).into_inner(),
-        );
+        let edits = self.buffer_subscription.consume().into_inner();
+        let (mut fold_map, snapshot, edits) = self.fold_map.write(snapshot, edits);
         let (snapshot, edits) = self.tab_map.sync(snapshot, edits);
         let (snapshot, edits) = self
             .wrap_map
@@ -142,10 +134,8 @@ impl DisplayMap {
         T: Into<Rope> + Clone,
     {
         let snapshot = self.buffer.read(cx).snapshot();
-        let (snapshot, edits) = self.fold_map.read(
-            snapshot,
-            mem::take(&mut *self.buffer_edits_since_sync.lock()).into_inner(),
-        );
+        let edits = self.buffer_subscription.consume().into_inner();
+        let (snapshot, edits) = self.fold_map.read(snapshot, edits);
         let (snapshot, edits) = self.tab_map.sync(snapshot, edits);
         let (snapshot, edits) = self
             .wrap_map
@@ -164,10 +154,8 @@ impl DisplayMap {
 
     pub fn remove_blocks(&mut self, ids: HashSet<BlockId>, cx: &mut ModelContext<Self>) {
         let snapshot = self.buffer.read(cx).snapshot();
-        let (snapshot, edits) = self.fold_map.read(
-            snapshot,
-            mem::take(&mut *self.buffer_edits_since_sync.lock()).into_inner(),
-        );
+        let edits = self.buffer_subscription.consume().into_inner();
+        let (snapshot, edits) = self.fold_map.read(snapshot, edits);
         let (snapshot, edits) = self.tab_map.sync(snapshot, edits);
         let (snapshot, edits) = self
             .wrap_map
@@ -190,21 +178,6 @@ impl DisplayMap {
     pub fn is_rewrapping(&self, cx: &gpui::AppContext) -> bool {
         self.wrap_map.read(cx).is_rewrapping()
     }
-
-    fn handle_buffer_event(
-        &mut self,
-        _: ModelHandle<Buffer>,
-        event: &language::Event,
-        _: &mut ModelContext<Self>,
-    ) {
-        match event {
-            language::Event::Edited(patch) => {
-                let mut edits_since_sync = self.buffer_edits_since_sync.lock();
-                *edits_since_sync = edits_since_sync.compose(patch);
-            }
-            _ => {}
-        }
-    }
 }
 
 pub struct DisplayMapSnapshot {

crates/editor/src/editor.rs

@@ -3466,7 +3466,7 @@ impl Editor {
         cx: &mut ViewContext<Self>,
     ) {
         match event {
-            language::Event::Edited(_) => cx.emit(Event::Edited),
+            language::Event::Edited => cx.emit(Event::Edited),
             language::Event::Dirtied => cx.emit(Event::Dirtied),
             language::Event::Saved => cx.emit(Event::Saved),
             language::Event::FileHandleChanged => cx.emit(Event::FileHandleChanged),

crates/language/src/buffer.rs

@@ -109,7 +109,7 @@ pub enum Operation {
 
 #[derive(Clone, Debug, Eq, PartialEq)]
 pub enum Event {
-    Edited(Patch<usize>),
+    Edited,
     Dirtied,
     Saved,
     FileHandleChanged,
@@ -1138,6 +1138,10 @@ impl Buffer {
                 .map_or(false, |file| file.mtime() > self.saved_mtime)
     }
 
+    pub fn subscribe(&mut self) -> Subscription {
+        self.text.subscribe()
+    }
+
     pub fn start_transaction(
         &mut self,
         selection_set_ids: impl IntoIterator<Item = SelectionSetId>,
@@ -1330,7 +1334,7 @@ impl Buffer {
         self.reparse(cx);
         self.update_language_server();
 
-        cx.emit(Event::Edited(patch));
+        cx.emit(Event::Edited);
         if !was_dirty {
             cx.emit(Event::Dirtied);
         }

crates/language/src/tests.rs

@@ -110,34 +110,11 @@ fn test_edit_events(cx: &mut gpui::MutableAppContext) {
     let buffer_1_events = buffer_1_events.borrow();
     assert_eq!(
         *buffer_1_events,
-        vec![
-            Event::Edited(Patch::new(vec![Edit {
-                old: 2..4,
-                new: 2..5
-            }])),
-            Event::Dirtied,
-            Event::Edited(Patch::new(vec![Edit {
-                old: 5..5,
-                new: 5..7
-            }])),
-            Event::Edited(Patch::new(vec![Edit {
-                old: 5..7,
-                new: 5..5,
-            }]))
-        ]
+        vec![Event::Edited, Event::Dirtied, Event::Edited, Event::Edited]
     );
 
     let buffer_2_events = buffer_2_events.borrow();
-    assert_eq!(
-        *buffer_2_events,
-        vec![
-            Event::Edited(Patch::new(vec![Edit {
-                old: 2..4,
-                new: 2..5
-            }])),
-            Event::Dirtied
-        ]
-    );
+    assert_eq!(*buffer_2_events, vec![Event::Edited, Event::Dirtied]);
 }
 
 #[gpui::test]

crates/project/src/worktree.rs

@@ -3016,7 +3016,7 @@ mod tests {
         fmt::Write,
         time::{SystemTime, UNIX_EPOCH},
     };
-    use text::{Patch, Point};
+    use text::Point;
     use util::test::temp_tree;
 
     #[gpui::test]
@@ -3469,13 +3469,7 @@ mod tests {
             assert!(buffer.is_dirty());
             assert_eq!(
                 *events.borrow(),
-                &[
-                    language::Event::Edited(Patch::new(vec![text::Edit {
-                        old: 1..2,
-                        new: 1..1
-                    }])),
-                    language::Event::Dirtied
-                ]
+                &[language::Event::Edited, language::Event::Dirtied]
             );
             events.borrow_mut().clear();
             buffer.did_save(buffer.version(), buffer.file().unwrap().mtime(), None, cx);
@@ -3498,15 +3492,9 @@ mod tests {
             assert_eq!(
                 *events.borrow(),
                 &[
-                    language::Event::Edited(Patch::new(vec![text::Edit {
-                        old: 1..1,
-                        new: 1..2
-                    }])),
+                    language::Event::Edited,
                     language::Event::Dirtied,
-                    language::Event::Edited(Patch::new(vec![text::Edit {
-                        old: 2..2,
-                        new: 2..3
-                    }])),
+                    language::Event::Edited,
                 ],
             );
             events.borrow_mut().clear();
@@ -3518,13 +3506,7 @@ mod tests {
             assert!(buffer.is_dirty());
         });
 
-        assert_eq!(
-            *events.borrow(),
-            &[language::Event::Edited(Patch::new(vec![text::Edit {
-                old: 1..3,
-                new: 1..1
-            }]))]
-        );
+        assert_eq!(*events.borrow(), &[language::Event::Edited]);
 
         // When a file is deleted, the buffer is considered dirty.
         let events = Rc::new(RefCell::new(Vec::new()));

crates/text/src/text.rs

@@ -46,7 +46,7 @@ pub struct Buffer {
     remote_id: u64,
     local_clock: clock::Local,
     lamport_clock: clock::Lamport,
-    subscriptions: Vec<Weak<Subscription>>,
+    subscriptions: Vec<Weak<Mutex<Vec<Patch<usize>>>>>,
 }
 
 #[derive(Clone)]
@@ -342,7 +342,7 @@ impl<D1, D2> Edit<(D1, D2)> {
     }
 }
 
-#[derive(Default)]
+#[derive(Clone, Default)]
 pub struct Subscription(Arc<Mutex<Vec<Patch<usize>>>>);
 
 impl Subscription {
@@ -354,11 +354,6 @@ impl Subscription {
         }
         changes
     }
-
-    pub fn publish(&self, patch: Patch<usize>) {
-        let mut changes = self.0.lock();
-        changes.push(patch);
-    }
 }
 
 #[derive(Copy, Clone, Debug, Default, Eq, PartialEq)]
@@ -1200,16 +1195,16 @@ impl Buffer {
         })
     }
 
-    pub fn subscribe(&mut self) -> Arc<Subscription> {
-        let subscription = Arc::new(Default::default());
-        self.subscriptions.push(Arc::downgrade(&subscription));
+    pub fn subscribe(&mut self) -> Subscription {
+        let subscription = Subscription(Default::default());
+        self.subscriptions.push(Arc::downgrade(&subscription.0));
         subscription
     }
 
     fn update_subscriptions(&mut self, edits: Patch<usize>) {
         self.subscriptions.retain(|subscription| {
             if let Some(subscription) = subscription.upgrade() {
-                subscription.publish(edits.clone());
+                subscription.lock().push(edits.clone());
                 true
             } else {
                 false