Merge pull request #625 from zed-industries/go-to-prev-diagnostic

Antonio Scandurra created

Go to previous diagnostic via `shift-f8`

Change summary

Cargo.lock                                |   3 
crates/editor/src/display_map/fold_map.rs |  23 +-
crates/editor/src/editor.rs               |  66 ++++++--
crates/editor/src/items.rs                |   2 
crates/editor/src/multi_buffer.rs         |   8 
crates/gpui/src/elements/list.rs          |   6 
crates/language/src/buffer.rs             |   6 
crates/language/src/diagnostic_set.rs     |  35 ++-
crates/language/src/tests.rs              |   4 
crates/project/src/project.rs             |  10 
crates/server/src/rpc.rs                  |   2 
crates/sum_tree/Cargo.toml                |   3 
crates/sum_tree/src/cursor.rs             | 178 +++++++++++-------------
crates/sum_tree/src/sum_tree.rs           |  68 +++++++--
crates/text/src/rope.rs                   |   2 
crates/text/src/text.rs                   |   9 
16 files changed, 243 insertions(+), 182 deletions(-)

Detailed changes

Cargo.lock 🔗

@@ -4929,6 +4929,9 @@ name = "sum_tree"
 version = "0.1.0"
 dependencies = [
  "arrayvec 0.7.1",
+ "ctor",
+ "env_logger",
+ "log",
  "rand 0.8.3",
 ]
 

crates/editor/src/display_map/fold_map.rs 🔗

@@ -819,19 +819,18 @@ where
 {
     let start = buffer.anchor_before(range.start.to_offset(buffer));
     let end = buffer.anchor_after(range.end.to_offset(buffer));
-    folds.filter::<_, usize>(
-        move |summary| {
-            let start_cmp = start.cmp(&summary.max_end, buffer).unwrap();
-            let end_cmp = end.cmp(&summary.min_start, buffer).unwrap();
+    let mut cursor = folds.filter::<_, usize>(move |summary| {
+        let start_cmp = start.cmp(&summary.max_end, buffer).unwrap();
+        let end_cmp = end.cmp(&summary.min_start, buffer).unwrap();
 
-            if inclusive {
-                start_cmp <= Ordering::Equal && end_cmp >= Ordering::Equal
-            } else {
-                start_cmp == Ordering::Less && end_cmp == Ordering::Greater
-            }
-        },
-        buffer,
-    )
+        if inclusive {
+            start_cmp <= Ordering::Equal && end_cmp >= Ordering::Equal
+        } else {
+            start_cmp == Ordering::Less && end_cmp == Ordering::Greater
+        }
+    });
+    cursor.next(buffer);
+    cursor
 }
 
 fn consolidate_buffer_edits(edits: &mut Vec<text::Edit<usize>>) {

crates/editor/src/editor.rs 🔗

@@ -115,7 +115,7 @@ action!(ToggleComments);
 action!(SelectLargerSyntaxNode);
 action!(SelectSmallerSyntaxNode);
 action!(MoveToEnclosingBracket);
-action!(ShowNextDiagnostic);
+action!(GoToDiagnostic, Direction);
 action!(GoToDefinition);
 action!(FindAllReferences);
 action!(Rename);
@@ -136,6 +136,12 @@ action!(OpenExcerpts);
 enum DocumentHighlightRead {}
 enum DocumentHighlightWrite {}
 
+#[derive(Copy, Clone, PartialEq, Eq)]
+pub enum Direction {
+    Prev,
+    Next,
+}
+
 pub fn init(cx: &mut MutableAppContext, path_openers: &mut Vec<Box<dyn PathOpener>>) {
     path_openers.push(Box::new(items::BufferOpener));
     cx.add_bindings(vec![
@@ -250,7 +256,8 @@ pub fn init(cx: &mut MutableAppContext, path_openers: &mut Vec<Box<dyn PathOpene
         Binding::new("ctrl-w", SelectLargerSyntaxNode, Some("Editor")),
         Binding::new("alt-down", SelectSmallerSyntaxNode, Some("Editor")),
         Binding::new("ctrl-shift-W", SelectSmallerSyntaxNode, Some("Editor")),
-        Binding::new("f8", ShowNextDiagnostic, Some("Editor")),
+        Binding::new("f8", GoToDiagnostic(Direction::Next), Some("Editor")),
+        Binding::new("shift-f8", GoToDiagnostic(Direction::Prev), Some("Editor")),
         Binding::new("f2", Rename, Some("Editor")),
         Binding::new("f12", GoToDefinition, Some("Editor")),
         Binding::new("alt-shift-f12", FindAllReferences, Some("Editor")),
@@ -319,7 +326,7 @@ pub fn init(cx: &mut MutableAppContext, path_openers: &mut Vec<Box<dyn PathOpene
     cx.add_action(Editor::select_larger_syntax_node);
     cx.add_action(Editor::select_smaller_syntax_node);
     cx.add_action(Editor::move_to_enclosing_bracket);
-    cx.add_action(Editor::show_next_diagnostic);
+    cx.add_action(Editor::go_to_diagnostic);
     cx.add_action(Editor::go_to_definition);
     cx.add_action(Editor::page_up);
     cx.add_action(Editor::page_down);
@@ -4173,7 +4180,11 @@ impl Editor {
         self.update_selections(selections, Some(Autoscroll::Fit), cx);
     }
 
-    pub fn show_next_diagnostic(&mut self, _: &ShowNextDiagnostic, cx: &mut ViewContext<Self>) {
+    pub fn go_to_diagnostic(
+        &mut self,
+        &GoToDiagnostic(direction): &GoToDiagnostic,
+        cx: &mut ViewContext<Self>,
+    ) {
         let buffer = self.buffer.read(cx).snapshot(cx);
         let selection = self.newest_selection_with_snapshot::<usize>(&buffer);
         let mut active_primary_range = self.active_diagnostics.as_ref().map(|active_diagnostics| {
@@ -4193,20 +4204,23 @@ impl Editor {
         };
 
         loop {
-            let next_group = buffer
-                .diagnostics_in_range::<_, usize>(search_start..buffer.len())
-                .find_map(|entry| {
-                    if entry.diagnostic.is_primary
-                        && !entry.range.is_empty()
-                        && Some(entry.range.end) != active_primary_range.as_ref().map(|r| *r.end())
-                    {
-                        Some((entry.range, entry.diagnostic.group_id))
-                    } else {
-                        None
-                    }
-                });
+            let mut diagnostics = if direction == Direction::Prev {
+                buffer.diagnostics_in_range::<_, usize>(0..search_start, true)
+            } else {
+                buffer.diagnostics_in_range::<_, usize>(search_start..buffer.len(), false)
+            };
+            let group = diagnostics.find_map(|entry| {
+                if entry.diagnostic.is_primary
+                    && !entry.range.is_empty()
+                    && Some(entry.range.end) != active_primary_range.as_ref().map(|r| *r.end())
+                {
+                    Some((entry.range, entry.diagnostic.group_id))
+                } else {
+                    None
+                }
+            });
 
-            if let Some((primary_range, group_id)) = next_group {
+            if let Some((primary_range, group_id)) = group {
                 self.activate_diagnostics(group_id, cx);
                 self.update_selections(
                     vec![Selection {
@@ -4220,13 +4234,23 @@ impl Editor {
                     cx,
                 );
                 break;
-            } else if search_start == 0 {
-                break;
             } else {
                 // Cycle around to the start of the buffer, potentially moving back to the start of
                 // the currently active diagnostic.
-                search_start = 0;
                 active_primary_range.take();
+                if direction == Direction::Prev {
+                    if search_start == buffer.len() {
+                        break;
+                    } else {
+                        search_start = buffer.len();
+                    }
+                } else {
+                    if search_start == 0 {
+                        break;
+                    } else {
+                        search_start = 0;
+                    }
+                }
             }
         }
     }
@@ -4590,7 +4614,7 @@ impl Editor {
             let buffer = self.buffer.read(cx).snapshot(cx);
             let primary_range_start = active_diagnostics.primary_range.start.to_offset(&buffer);
             let is_valid = buffer
-                .diagnostics_in_range::<_, usize>(active_diagnostics.primary_range.clone())
+                .diagnostics_in_range::<_, usize>(active_diagnostics.primary_range.clone(), false)
                 .any(|entry| {
                     entry.diagnostic.is_primary
                         && !entry.range.is_empty()

crates/editor/src/items.rs 🔗

@@ -373,7 +373,7 @@ impl DiagnosticMessage {
             .head();
         let new_diagnostic = buffer
             .read(cx)
-            .diagnostics_in_range::<_, usize>(cursor_position..cursor_position)
+            .diagnostics_in_range::<_, usize>(cursor_position..cursor_position, false)
             .filter(|entry| !entry.range.is_empty())
             .min_by_key(|entry| (entry.diagnostic.severity, entry.range.len()))
             .map(|entry| entry.diagnostic);

crates/editor/src/multi_buffer.rs 🔗

@@ -1700,7 +1700,7 @@ impl MultiBufferSnapshot {
     }
 
     pub fn text_summary(&self) -> TextSummary {
-        self.excerpts.summary().text
+        self.excerpts.summary().text.clone()
     }
 
     pub fn text_summary_for_range<'a, D, O>(&'a self, range: Range<O>) -> D
@@ -2179,6 +2179,7 @@ impl MultiBufferSnapshot {
     pub fn diagnostics_in_range<'a, T, O>(
         &'a self,
         range: Range<T>,
+        reversed: bool,
     ) -> impl Iterator<Item = DiagnosticEntry<O>> + 'a
     where
         T: 'a + ToOffset,
@@ -2187,7 +2188,10 @@ impl MultiBufferSnapshot {
         self.as_singleton()
             .into_iter()
             .flat_map(move |(_, _, buffer)| {
-                buffer.diagnostics_in_range(range.start.to_offset(self)..range.end.to_offset(self))
+                buffer.diagnostics_in_range(
+                    range.start.to_offset(self)..range.end.to_offset(self),
+                    reversed,
+                )
             })
     }
 

crates/gpui/src/elements/list.rs 🔗

@@ -614,7 +614,7 @@ mod tests {
         let (size, _) = list.layout(constraint, &mut presenter.build_layout_context(false, cx));
         assert_eq!(size, vec2f(100., 40.));
         assert_eq!(
-            state.0.borrow().items.summary(),
+            state.0.borrow().items.summary().clone(),
             ListItemSummary {
                 count: 3,
                 rendered_count: 3,
@@ -649,7 +649,7 @@ mod tests {
         state.splice(1..2, 2);
         state.splice(4..4, 1);
         assert_eq!(
-            state.0.borrow().items.summary(),
+            state.0.borrow().items.summary().clone(),
             ListItemSummary {
                 count: 5,
                 rendered_count: 2,
@@ -662,7 +662,7 @@ mod tests {
             list.layout(constraint, &mut presenter.build_layout_context(false, cx));
         assert_eq!(size, vec2f(100., 40.));
         assert_eq!(
-            state.0.borrow().items.summary(),
+            state.0.borrow().items.summary().clone(),
             ListItemSummary {
                 count: 5,
                 rendered_count: 5,

crates/language/src/buffer.rs 🔗

@@ -1575,7 +1575,7 @@ impl BufferSnapshot {
         let mut diagnostic_endpoints = Vec::new();
         if language_aware {
             tree = self.tree.as_ref();
-            for entry in self.diagnostics_in_range::<_, usize>(range.clone()) {
+            for entry in self.diagnostics_in_range::<_, usize>(range.clone(), false) {
                 diagnostic_endpoints.push(DiagnosticEndpoint {
                     offset: entry.range.start,
                     is_start: true,
@@ -1838,12 +1838,14 @@ impl BufferSnapshot {
     pub fn diagnostics_in_range<'a, T, O>(
         &'a self,
         search_range: Range<T>,
+        reversed: bool,
     ) -> impl 'a + Iterator<Item = DiagnosticEntry<O>>
     where
         T: 'a + Clone + ToOffset,
         O: 'a + FromAnchor,
     {
-        self.diagnostics.range(search_range.clone(), self, true)
+        self.diagnostics
+            .range(search_range.clone(), self, true, reversed)
     }
 
     pub fn diagnostic_groups(&self) -> Vec<DiagnosticGroup<Anchor>> {

crates/language/src/diagnostic_set.rs 🔗

@@ -71,6 +71,7 @@ impl DiagnosticSet {
         range: Range<T>,
         buffer: &'a text::BufferSnapshot,
         inclusive: bool,
+        reversed: bool,
     ) -> impl 'a + Iterator<Item = DiagnosticEntry<O>>
     where
         T: 'a + ToOffset,
@@ -78,25 +79,31 @@ impl DiagnosticSet {
     {
         let end_bias = if inclusive { Bias::Right } else { Bias::Left };
         let range = buffer.anchor_before(range.start)..buffer.anchor_at(range.end, end_bias);
-        let mut cursor = self.diagnostics.filter::<_, ()>(
-            {
-                move |summary: &Summary| {
-                    let start_cmp = range.start.cmp(&summary.max_end, buffer).unwrap();
-                    let end_cmp = range.end.cmp(&summary.min_start, buffer).unwrap();
-                    if inclusive {
-                        start_cmp <= Ordering::Equal && end_cmp >= Ordering::Equal
-                    } else {
-                        start_cmp == Ordering::Less && end_cmp == Ordering::Greater
-                    }
+        let mut cursor = self.diagnostics.filter::<_, ()>({
+            move |summary: &Summary| {
+                let start_cmp = range.start.cmp(&summary.max_end, buffer).unwrap();
+                let end_cmp = range.end.cmp(&summary.min_start, buffer).unwrap();
+                if inclusive {
+                    start_cmp <= Ordering::Equal && end_cmp >= Ordering::Equal
+                } else {
+                    start_cmp == Ordering::Less && end_cmp == Ordering::Greater
                 }
-            },
-            buffer,
-        );
+            }
+        });
 
+        if reversed {
+            cursor.prev(buffer);
+        } else {
+            cursor.next(buffer);
+        }
         iter::from_fn({
             move || {
                 if let Some(diagnostic) = cursor.item() {
-                    cursor.next(buffer);
+                    if reversed {
+                        cursor.prev(buffer);
+                    } else {
+                        cursor.next(buffer);
+                    }
                     Some(diagnostic.resolve(buffer))
                 } else {
                     None

crates/language/src/tests.rs 🔗

@@ -768,10 +768,10 @@ fn test_random_collaboration(cx: &mut MutableAppContext, mut rng: StdRng) {
         );
         assert_eq!(
             buffer
-                .diagnostics_in_range::<_, usize>(0..buffer.len())
+                .diagnostics_in_range::<_, usize>(0..buffer.len(), false)
                 .collect::<Vec<_>>(),
             first_buffer
-                .diagnostics_in_range::<_, usize>(0..first_buffer.len())
+                .diagnostics_in_range::<_, usize>(0..first_buffer.len(), false)
                 .collect::<Vec<_>>(),
             "Replica {} diagnostics != Replica 0 diagnostics",
             buffer.replica_id()

crates/project/src/project.rs 🔗

@@ -4869,7 +4869,7 @@ mod tests {
         buffer.read_with(cx, |buffer, _| {
             let snapshot = buffer.snapshot();
             let diagnostics = snapshot
-                .diagnostics_in_range::<_, Point>(0..buffer.len())
+                .diagnostics_in_range::<_, Point>(0..buffer.len(), false)
                 .collect::<Vec<_>>();
             assert_eq!(
                 diagnostics,
@@ -4985,7 +4985,7 @@ mod tests {
             assert_eq!(
                 buffer
                     .snapshot()
-                    .diagnostics_in_range::<_, Point>(Point::new(3, 0)..Point::new(5, 0))
+                    .diagnostics_in_range::<_, Point>(Point::new(3, 0)..Point::new(5, 0), false)
                     .collect::<Vec<_>>(),
                 &[
                     DiagnosticEntry {
@@ -5063,7 +5063,7 @@ mod tests {
             assert_eq!(
                 buffer
                     .snapshot()
-                    .diagnostics_in_range::<_, Point>(Point::new(2, 0)..Point::new(3, 0))
+                    .diagnostics_in_range::<_, Point>(Point::new(2, 0)..Point::new(3, 0), false)
                     .collect::<Vec<_>>(),
                 &[
                     DiagnosticEntry {
@@ -5150,7 +5150,7 @@ mod tests {
             assert_eq!(
                 buffer
                     .snapshot()
-                    .diagnostics_in_range::<_, Point>(0..buffer.len())
+                    .diagnostics_in_range::<_, Point>(0..buffer.len(), false)
                     .collect::<Vec<_>>(),
                 &[
                     DiagnosticEntry {
@@ -6428,7 +6428,7 @@ mod tests {
 
         assert_eq!(
             buffer
-                .diagnostics_in_range::<_, Point>(0..buffer.len())
+                .diagnostics_in_range::<_, Point>(0..buffer.len(), false)
                 .collect::<Vec<_>>(),
             &[
                 DiagnosticEntry {

crates/server/src/rpc.rs 🔗

@@ -2061,7 +2061,7 @@ mod tests {
             assert_eq!(
                 buffer
                     .snapshot()
-                    .diagnostics_in_range::<_, Point>(0..buffer.len())
+                    .diagnostics_in_range::<_, Point>(0..buffer.len(), false)
                     .map(|entry| entry)
                     .collect::<Vec<_>>(),
                 &[

crates/sum_tree/Cargo.toml 🔗

@@ -9,6 +9,9 @@ doctest = false
 
 [dependencies]
 arrayvec = "0.7.1"
+log = "0.4"
 
 [dev-dependencies]
+ctor = "0.1"
+env_logger = "0.8"
 rand = "0.8.3"

crates/sum_tree/src/cursor.rs 🔗

@@ -60,7 +60,7 @@ where
     }
 
     pub fn item(&self) -> Option<&'a T> {
-        assert!(self.did_seek, "Must seek before calling this method");
+        self.assert_did_seek();
         if let Some(entry) = self.stack.last() {
             match *entry.tree.0 {
                 Node::Leaf { ref items, .. } => {
@@ -78,7 +78,7 @@ where
     }
 
     pub fn item_summary(&self) -> Option<&'a T::Summary> {
-        assert!(self.did_seek, "Must seek before calling this method");
+        self.assert_did_seek();
         if let Some(entry) = self.stack.last() {
             match *entry.tree.0 {
                 Node::Leaf {
@@ -98,7 +98,7 @@ where
     }
 
     pub fn prev_item(&self) -> Option<&'a T> {
-        assert!(self.did_seek, "Must seek before calling this method");
+        self.assert_did_seek();
         if let Some(entry) = self.stack.last() {
             if entry.index == 0 {
                 if let Some(prev_leaf) = self.prev_leaf() {
@@ -134,52 +134,69 @@ where
     }
 
     pub fn prev(&mut self, cx: &<T::Summary as Summary>::Context) {
-        assert!(self.did_seek, "Must seek before calling this method");
+        self.prev_internal(|_| true, cx)
+    }
+
+    fn prev_internal<F>(&mut self, mut filter_node: F, cx: &<T::Summary as Summary>::Context)
+    where
+        F: FnMut(&T::Summary) -> bool,
+    {
+        if !self.did_seek {
+            self.did_seek = true;
+            self.at_end = true;
+        }
 
         if self.at_end {
             self.position = D::default();
-            self.descend_to_last_item(self.tree, cx);
             self.at_end = self.tree.is_empty();
-        } else {
-            while let Some(entry) = self.stack.pop() {
-                if entry.index > 0 {
-                    let new_index = entry.index - 1;
+            if !self.tree.is_empty() {
+                self.stack.push(StackEntry {
+                    tree: self.tree,
+                    index: self.tree.0.child_summaries().len(),
+                    position: D::from_summary(self.tree.summary(), cx),
+                });
+            }
+        }
 
-                    if let Some(StackEntry { position, .. }) = self.stack.last() {
-                        self.position = position.clone();
-                    } else {
-                        self.position = D::default();
-                    }
+        let mut descending = false;
+        while !self.stack.is_empty() {
+            if let Some(StackEntry { position, .. }) = self.stack.iter().rev().skip(1).next() {
+                self.position = position.clone();
+            } else {
+                self.position = D::default();
+            }
 
-                    match entry.tree.0.as_ref() {
-                        Node::Internal {
-                            child_trees,
-                            child_summaries,
-                            ..
-                        } => {
-                            for summary in &child_summaries[0..new_index] {
-                                self.position.add_summary(summary, cx);
-                            }
-                            self.stack.push(StackEntry {
-                                tree: entry.tree,
-                                index: new_index,
-                                position: self.position.clone(),
-                            });
-                            self.descend_to_last_item(&child_trees[new_index], cx);
-                        }
-                        Node::Leaf { item_summaries, .. } => {
-                            for item_summary in &item_summaries[0..new_index] {
-                                self.position.add_summary(item_summary, cx);
-                            }
-                            self.stack.push(StackEntry {
-                                tree: entry.tree,
-                                index: new_index,
-                                position: self.position.clone(),
-                            });
-                        }
-                    }
+            let mut entry = self.stack.last_mut().unwrap();
+            if !descending {
+                if entry.index == 0 {
+                    self.stack.pop();
+                    continue;
+                } else {
+                    entry.index -= 1;
+                }
+            }
 
-                    break;
+            for summary in &entry.tree.0.child_summaries()[..entry.index] {
+                self.position.add_summary(summary, cx);
+            }
+            entry.position = self.position.clone();
+
+            descending = filter_node(&entry.tree.0.child_summaries()[entry.index]);
+            match entry.tree.0.as_ref() {
+                Node::Internal { child_trees, .. } => {
+                    if descending {
+                        let tree = &child_trees[entry.index];
+                        self.stack.push(StackEntry {
+                            position: D::default(),
+                            tree,
+                            index: tree.0.child_summaries().len() - 1,
+                        })
+                    }
+                }
+                Node::Leaf { .. } => {
+                    if descending {
+                        break;
+                    }
                 }
             }
         }
@@ -217,8 +234,8 @@ where
                         ..
                     } => {
                         if !descend {
-                            entry.position = self.position.clone();
                             entry.index += 1;
+                            entry.position = self.position.clone();
                         }
 
                         while entry.index < child_summaries.len() {
@@ -226,9 +243,10 @@ where
                             if filter_node(next_summary) {
                                 break;
                             } else {
+                                entry.index += 1;
+                                entry.position.add_summary(next_summary, cx);
                                 self.position.add_summary(next_summary, cx);
                             }
-                            entry.index += 1;
                         }
 
                         child_trees.get(entry.index)
@@ -236,9 +254,9 @@ where
                     Node::Leaf { item_summaries, .. } => {
                         if !descend {
                             let item_summary = &item_summaries[entry.index];
-                            self.position.add_summary(item_summary, cx);
-                            entry.position.add_summary(item_summary, cx);
                             entry.index += 1;
+                            entry.position.add_summary(item_summary, cx);
+                            self.position.add_summary(item_summary, cx);
                         }
 
                         loop {
@@ -246,9 +264,9 @@ where
                                 if filter_node(next_item_summary) {
                                     return;
                                 } else {
-                                    self.position.add_summary(next_item_summary, cx);
-                                    entry.position.add_summary(next_item_summary, cx);
                                     entry.index += 1;
+                                    entry.position.add_summary(next_item_summary, cx);
+                                    self.position.add_summary(next_item_summary, cx);
                                 }
                             } else {
                                 break None;
@@ -275,48 +293,11 @@ where
         debug_assert!(self.stack.is_empty() || self.stack.last().unwrap().tree.0.is_leaf());
     }
 
-    fn descend_to_last_item(
-        &mut self,
-        mut subtree: &'a SumTree<T>,
-        cx: &<T::Summary as Summary>::Context,
-    ) {
-        self.did_seek = true;
-        if subtree.is_empty() {
-            return;
-        }
-
-        loop {
-            match subtree.0.as_ref() {
-                Node::Internal {
-                    child_trees,
-                    child_summaries,
-                    ..
-                } => {
-                    for summary in &child_summaries[0..child_summaries.len() - 1] {
-                        self.position.add_summary(summary, cx);
-                    }
-
-                    self.stack.push(StackEntry {
-                        tree: subtree,
-                        index: child_trees.len() - 1,
-                        position: self.position.clone(),
-                    });
-                    subtree = child_trees.last().unwrap();
-                }
-                Node::Leaf { item_summaries, .. } => {
-                    let last_index = item_summaries.len() - 1;
-                    for item_summary in &item_summaries[0..last_index] {
-                        self.position.add_summary(item_summary, cx);
-                    }
-                    self.stack.push(StackEntry {
-                        tree: subtree,
-                        index: last_index,
-                        position: self.position.clone(),
-                    });
-                    break;
-                }
-            }
-        }
+    fn assert_did_seek(&self) {
+        assert!(
+            self.did_seek,
+            "Must call `seek`, `next` or `prev` before calling this method"
+        );
     }
 }
 
@@ -596,13 +577,8 @@ where
     T: Item,
     D: Dimension<'a, T::Summary>,
 {
-    pub fn new(
-        tree: &'a SumTree<T>,
-        mut filter_node: F,
-        cx: &<T::Summary as Summary>::Context,
-    ) -> Self {
-        let mut cursor = tree.cursor::<D>();
-        cursor.next_internal(&mut filter_node, cx);
+    pub fn new(tree: &'a SumTree<T>, filter_node: F) -> Self {
+        let cursor = tree.cursor::<D>();
         Self {
             cursor,
             filter_node,
@@ -624,6 +600,10 @@ where
     pub fn next(&mut self, cx: &<T::Summary as Summary>::Context) {
         self.cursor.next_internal(&mut self.filter_node, cx);
     }
+
+    pub fn prev(&mut self, cx: &<T::Summary as Summary>::Context) {
+        self.cursor.prev_internal(&mut self.filter_node, cx);
+    }
 }
 
 impl<'a, F, T, S, U> Iterator for FilterCursor<'a, F, T, U>
@@ -636,6 +616,10 @@ where
     type Item = &'a T;
 
     fn next(&mut self) -> Option<Self::Item> {
+        if !self.cursor.did_seek {
+            self.next(&());
+        }
+
         if let Some(item) = self.item() {
             self.cursor.next_internal(&self.filter_node, &());
             Some(item)

crates/sum_tree/src/sum_tree.rs 🔗

@@ -168,16 +168,12 @@ impl<T: Item> SumTree<T> {
         Cursor::new(self)
     }
 
-    pub fn filter<'a, F, U>(
-        &'a self,
-        filter_node: F,
-        cx: &<T::Summary as Summary>::Context,
-    ) -> FilterCursor<F, T, U>
+    pub fn filter<'a, F, U>(&'a self, filter_node: F) -> FilterCursor<F, T, U>
     where
         F: FnMut(&T::Summary) -> bool,
         U: Dimension<'a, T::Summary>,
     {
-        FilterCursor::new(self, filter_node, cx)
+        FilterCursor::new(self, filter_node)
     }
 
     #[allow(dead_code)]
@@ -242,10 +238,10 @@ impl<T: Item> SumTree<T> {
         extent
     }
 
-    pub fn summary(&self) -> T::Summary {
+    pub fn summary(&self) -> &T::Summary {
         match self.0.as_ref() {
-            Node::Internal { summary, .. } => summary.clone(),
-            Node::Leaf { summary, .. } => summary.clone(),
+            Node::Internal { summary, .. } => summary,
+            Node::Leaf { summary, .. } => summary,
         }
     }
 
@@ -678,6 +674,13 @@ mod tests {
     use rand::{distributions, prelude::*};
     use std::cmp;
 
+    #[ctor::ctor]
+    fn init_logger() {
+        if std::env::var("RUST_LOG").is_ok() {
+            env_logger::init();
+        }
+    }
+
     #[test]
     fn test_extend_and_push_tree() {
         let mut tree1 = SumTree::new();
@@ -703,8 +706,11 @@ mod tests {
         if let Ok(value) = std::env::var("ITERATIONS") {
             num_iterations = value.parse().expect("invalid ITERATIONS variable");
         }
+        let num_operations = std::env::var("OPERATIONS")
+            .map_or(5, |o| o.parse().expect("invalid OPERATIONS variable"));
 
         for seed in starting_seed..(starting_seed + num_iterations) {
+            dbg!(seed);
             let mut rng = StdRng::seed_from_u64(seed);
 
             let rng = &mut rng;
@@ -712,7 +718,7 @@ mod tests {
             let count = rng.gen_range(0..10);
             tree.extend(rng.sample_iter(distributions::Standard).take(count), &());
 
-            for _ in 0..5 {
+            for _ in 0..num_operations {
                 let splice_end = rng.gen_range(0..tree.extent::<Count>(&()).0 + 1);
                 let splice_start = rng.gen_range(0..splice_end + 1);
                 let count = rng.gen_range(0..3);
@@ -740,20 +746,48 @@ mod tests {
                     tree.cursor::<()>().collect::<Vec<_>>()
                 );
 
-                let mut filter_cursor =
-                    tree.filter::<_, Count>(|summary| summary.contains_even, &());
-                let mut reference_filter = tree
+                log::info!("tree items: {:?}", tree.items(&()));
+
+                let mut filter_cursor = tree.filter::<_, Count>(|summary| summary.contains_even);
+                let expected_filtered_items = tree
                     .items(&())
                     .into_iter()
                     .enumerate()
-                    .filter(|(_, item)| (item & 1) == 0);
-                while let Some(actual_item) = filter_cursor.item() {
-                    let (reference_index, reference_item) = reference_filter.next().unwrap();
+                    .filter(|(_, item)| (item & 1) == 0)
+                    .collect::<Vec<_>>();
+
+                let mut item_ix = if rng.gen() {
+                    filter_cursor.next(&());
+                    0
+                } else {
+                    filter_cursor.prev(&());
+                    expected_filtered_items.len().saturating_sub(1)
+                };
+                while item_ix < expected_filtered_items.len() {
+                    log::info!("filter_cursor, item_ix: {}", item_ix);
+                    let actual_item = filter_cursor.item().unwrap();
+                    let (reference_index, reference_item) =
+                        expected_filtered_items[item_ix].clone();
                     assert_eq!(actual_item, &reference_item);
                     assert_eq!(filter_cursor.start().0, reference_index);
+                    log::info!("next");
                     filter_cursor.next(&());
+                    item_ix += 1;
+
+                    while item_ix > 0 && rng.gen_bool(0.2) {
+                        log::info!("prev");
+                        filter_cursor.prev(&());
+                        item_ix -= 1;
+
+                        if item_ix == 0 && rng.gen_bool(0.2) {
+                            filter_cursor.prev(&());
+                            assert_eq!(filter_cursor.item(), None);
+                            assert_eq!(filter_cursor.start().0, 0);
+                            filter_cursor.next(&());
+                        }
+                    }
                 }
-                assert!(reference_filter.next().is_none());
+                assert_eq!(filter_cursor.item(), None);
 
                 let mut pos = rng.gen_range(0..tree.extent::<Count>(&()).0 + 1);
                 let mut before_start = false;

crates/text/src/rope.rs 🔗

@@ -115,7 +115,7 @@ impl Rope {
     }
 
     pub fn summary(&self) -> TextSummary {
-        self.chunks.summary()
+        self.chunks.summary().clone()
     }
 
     pub fn len(&self) -> usize {

crates/text/src/text.rs 🔗

@@ -1849,10 +1849,11 @@ impl BufferSnapshot {
         let fragments_cursor = if *since == self.version {
             None
         } else {
-            Some(self.fragments.filter(
-                move |summary| !since.observed_all(&summary.max_version),
-                &None,
-            ))
+            let mut cursor = self
+                .fragments
+                .filter(move |summary| !since.observed_all(&summary.max_version));
+            cursor.next(&None);
+            Some(cursor)
         };
         let mut cursor = self
             .fragments