From 178705f8f97414b1c0f7e81d962120c84eed5f50 Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Thu, 6 May 2021 18:49:41 +0200 Subject: [PATCH] Add `FoldMap::folds_in_range` to randomized test and fix issues it found --- zed/src/editor/display_map/fold_map.rs | 80 +++++++---- zed/src/sum_tree/cursor.rs | 181 +++++++++---------------- zed/src/sum_tree/mod.rs | 2 +- 3 files changed, 120 insertions(+), 143 deletions(-) diff --git a/zed/src/editor/display_map/fold_map.rs b/zed/src/editor/display_map/fold_map.rs index c2892fa1d34ceaf89c2b5e8daae74f1909c57751..c9e350cba86acddee31a677b5d5e7f010114af47 100644 --- a/zed/src/editor/display_map/fold_map.rs +++ b/zed/src/editor/display_map/fold_map.rs @@ -3,7 +3,7 @@ use super::{ Anchor, Buffer, DisplayPoint, Edit, Point, TextSummary, ToOffset, }; use crate::{ - sum_tree::{self, Cursor, SeekBias, SumTree}, + sum_tree::{self, Cursor, FilterCursor, SeekBias, SumTree}, time, }; use anyhow::{anyhow, Result}; @@ -80,15 +80,7 @@ impl FoldMap { where T: ToOffset, { - let buffer = self.buffer.read(ctx); - let range = buffer.anchor_before(range.start)?..buffer.anchor_before(range.end)?; - Ok(self - .folds - .filter::<_, usize>(move |summary| { - range.start.cmp(&summary.max_end, buffer).unwrap() < Ordering::Equal - && range.end.cmp(&summary.min_start, buffer).unwrap() >= Ordering::Equal - }) - .map(|f| &f.0)) + Ok(self.intersecting_folds(range, ctx)?.map(|f| &f.0)) } pub fn fold( @@ -150,25 +142,17 @@ impl FoldMap { let mut edits = Vec::new(); let mut fold_ixs_to_delete = Vec::new(); for range in ranges.into_iter() { - let start = buffer.anchor_before(range.start.to_offset(buffer)?)?; - let end = buffer.anchor_after(range.end.to_offset(buffer)?)?; - let range = start..end; - // Remove intersecting folds and add their ranges to edits that are passed to apply_edits - let mut cursor = self.folds.filter::<_, usize>(|summary| { - range.start.cmp(&summary.max_end, buffer).unwrap() < Ordering::Equal - && range.end.cmp(&summary.min_start, buffer).unwrap() >= Ordering::Equal - }); - - while let Some(fold) = cursor.item() { + let mut folds_cursor = self.intersecting_folds(range, ctx)?; + while let Some(fold) = folds_cursor.item() { let offset_range = fold.0.start.to_offset(buffer).unwrap()..fold.0.end.to_offset(buffer).unwrap(); edits.push(Edit { old_range: offset_range.clone(), new_range: offset_range, }); - fold_ixs_to_delete.push(*cursor.start()); - cursor.next(); + fold_ixs_to_delete.push(*folds_cursor.start()); + folds_cursor.next(); } } fold_ixs_to_delete.sort_unstable(); @@ -192,6 +176,23 @@ impl FoldMap { Ok(()) } + fn intersecting_folds<'a, T>( + &self, + range: Range, + ctx: &'a AppContext, + ) -> Result bool, Fold, usize>> + where + T: ToOffset, + { + let buffer = self.buffer.read(ctx); + let start = buffer.anchor_before(range.start.to_offset(buffer)?)?; + let end = buffer.anchor_after(range.end.to_offset(buffer)?)?; + Ok(self.folds.filter::<_, usize>(move |summary| { + start.cmp(&summary.max_end, buffer).unwrap() <= Ordering::Equal + && end.cmp(&summary.min_start, buffer).unwrap() >= Ordering::Equal + })) + } + pub fn is_line_folded(&self, display_row: u32, ctx: &AppContext) -> bool { let transforms = self.sync(ctx); let mut cursor = transforms.cursor::(); @@ -502,7 +503,7 @@ impl Default for FoldSummary { Self { start: Anchor::Start, end: Anchor::End, - min_start: Anchor::Start, + min_start: Anchor::End, max_end: Anchor::Start, count: 0, } @@ -521,12 +522,12 @@ impl sum_tree::Summary for FoldSummary { self.max_end = other.max_end.clone(); } - if cfg!(debug_assertions) { + #[cfg(debug_assertions)] + { let start_comparison = self.start.cmp(&other.start, buffer).unwrap(); - let end_comparison = self.end.cmp(&other.end, buffer).unwrap(); assert!(start_comparison <= Ordering::Equal); if start_comparison == Ordering::Equal { - assert!(end_comparison >= Ordering::Equal); + assert!(self.end.cmp(&other.end, buffer).unwrap() >= Ordering::Equal); } } self.start = other.start.clone(); @@ -821,6 +822,7 @@ mod tests { fold_ranges, vec![ Point::new(0, 2)..Point::new(2, 2), + Point::new(0, 4)..Point::new(1, 0), Point::new(1, 2)..Point::new(3, 2) ] ); @@ -848,7 +850,7 @@ mod tests { }; for seed in seed_range { - println!("{:?}", seed); + dbg!(seed); let mut rng = StdRng::seed_from_u64(seed); App::test((), |app| { @@ -973,6 +975,30 @@ mod tests { ); assert!(map.is_line_folded(display_point.row(), app.as_ref())); } + + for _ in 0..5 { + let end = rng.gen_range(0..=buffer.len()); + let start = rng.gen_range(0..=end); + let expected_folds = map + .folds + .items() + .into_iter() + .filter(|fold| { + let fold_start = fold.0.start.to_offset(buffer).unwrap(); + let fold_end = fold.0.end.to_offset(buffer).unwrap(); + start <= fold_end && end >= fold_start + }) + .map(|fold| fold.0) + .collect::>(); + + assert_eq!( + map.folds_in_range(start..end, app.as_ref()) + .unwrap() + .cloned() + .collect::>(), + expected_folds + ); + } } }); } diff --git a/zed/src/sum_tree/cursor.rs b/zed/src/sum_tree/cursor.rs index d90702ab6e924114ff5fe8c8fd25f6d319342b2b..67e585c1343c8443d0b274168606d88184221cfd 100644 --- a/zed/src/sum_tree/cursor.rs +++ b/zed/src/sum_tree/cursor.rs @@ -199,9 +199,6 @@ where } pub fn next(&mut self) { - if !self.did_seek { - self.descend_to_first_item(self.tree, |_| true) - } self.next_internal(|_| true) } @@ -209,67 +206,88 @@ where where F: Fn(&T::Summary) -> bool, { - assert!(self.did_seek, "Must seek before calling this method"); + let mut descend = false; - if self.stack.is_empty() { - if !self.at_end { - self.descend_to_first_item(self.tree, filter_node); - } - } else { - while self.stack.len() > 0 { - let new_subtree = { - let entry = self.stack.last_mut().unwrap(); - match entry.tree.0.as_ref() { - Node::Internal { - child_trees, - child_summaries, - .. - } => { - while entry.index < child_summaries.len() { - entry - .seek_dimension - .add_summary(&child_summaries[entry.index]); - entry - .sum_dimension - .add_summary(&child_summaries[entry.index]); - - entry.index += 1; - if let Some(next_summary) = child_summaries.get(entry.index) { - if filter_node(next_summary) { - break; - } else { - self.seek_dimension.add_summary(next_summary); - self.sum_dimension.add_summary(next_summary); - } - } - } + if !self.did_seek { + self.stack.push(StackEntry { + tree: self.tree, + index: 0, + seek_dimension: S::default(), + sum_dimension: U::default(), + }); + descend = true; + self.did_seek = true; + } + + while self.stack.len() > 0 { + let new_subtree = { + let entry = self.stack.last_mut().unwrap(); + match entry.tree.0.as_ref() { + Node::Internal { + child_trees, + child_summaries, + .. + } => { + if !descend { + let summary = &child_summaries[entry.index]; + entry.seek_dimension.add_summary(summary); + entry.sum_dimension.add_summary(summary); + entry.index += 1; + } - child_trees.get(entry.index) + while entry.index < child_summaries.len() { + let next_summary = &child_summaries[entry.index]; + if filter_node(next_summary) { + break; + } else { + self.seek_dimension.add_summary(next_summary); + self.sum_dimension.add_summary(next_summary); + } + entry.index += 1; } - Node::Leaf { item_summaries, .. } => loop { + + child_trees.get(entry.index) + } + Node::Leaf { item_summaries, .. } => { + if !descend { let item_summary = &item_summaries[entry.index]; self.seek_dimension.add_summary(item_summary); entry.seek_dimension.add_summary(item_summary); self.sum_dimension.add_summary(item_summary); entry.sum_dimension.add_summary(item_summary); entry.index += 1; + } + + loop { if let Some(next_item_summary) = item_summaries.get(entry.index) { if filter_node(next_item_summary) { return; + } else { + self.seek_dimension.add_summary(next_item_summary); + entry.seek_dimension.add_summary(next_item_summary); + self.sum_dimension.add_summary(next_item_summary); + entry.sum_dimension.add_summary(next_item_summary); + entry.index += 1; } } else { break None; } - }, + } } - }; - - if let Some(subtree) = new_subtree { - self.descend_to_first_item(subtree, filter_node); - break; - } else { - self.stack.pop(); } + }; + + if let Some(subtree) = new_subtree { + descend = true; + self.stack.push(StackEntry { + tree: subtree, + index: 0, + seek_dimension: self.seek_dimension.clone(), + sum_dimension: self.sum_dimension.clone(), + }); + } else { + descend = false; + self.stack.pop(); } } @@ -277,67 +295,6 @@ where debug_assert!(self.stack.is_empty() || self.stack.last().unwrap().tree.0.is_leaf()); } - pub fn descend_to_first_item(&mut self, mut subtree: &'a SumTree, filter_node: F) - where - F: Fn(&T::Summary) -> bool, - { - self.did_seek = true; - loop { - subtree = match *subtree.0 { - Node::Internal { - ref child_trees, - ref child_summaries, - .. - } => { - let mut new_index = None; - for (index, summary) in child_summaries.iter().enumerate() { - if filter_node(summary) { - new_index = Some(index); - break; - } - self.seek_dimension.add_summary(summary); - self.sum_dimension.add_summary(summary); - } - - if let Some(new_index) = new_index { - self.stack.push(StackEntry { - tree: subtree, - index: new_index, - seek_dimension: self.seek_dimension.clone(), - sum_dimension: self.sum_dimension.clone(), - }); - &child_trees[new_index] - } else { - break; - } - } - Node::Leaf { - ref item_summaries, .. - } => { - let mut new_index = None; - for (index, item_summary) in item_summaries.iter().enumerate() { - if filter_node(item_summary) { - new_index = Some(index); - break; - } - self.seek_dimension.add_summary(item_summary); - self.sum_dimension.add_summary(item_summary); - } - - if let Some(new_index) = new_index { - self.stack.push(StackEntry { - tree: subtree, - index: new_index, - seek_dimension: self.seek_dimension.clone(), - sum_dimension: self.sum_dimension.clone(), - }); - } - break; - } - } - } - } - fn descend_to_last_item(&mut self, mut subtree: &'a SumTree) { self.did_seek = true; loop { @@ -744,7 +701,7 @@ where fn next(&mut self) -> Option { if !self.did_seek { - self.descend_to_first_item(self.tree, |_| true); + self.next(); } if let Some(item) = self.item() { @@ -769,13 +726,7 @@ where { pub fn new(tree: &'a SumTree, filter_node: F) -> Self { let mut cursor = tree.cursor::<(), U>(); - if filter_node(&tree.summary()) { - cursor.descend_to_first_item(tree, &filter_node); - } else { - cursor.did_seek = true; - cursor.at_end = true; - } - + cursor.next_internal(&filter_node); Self { cursor, filter_node, diff --git a/zed/src/sum_tree/mod.rs b/zed/src/sum_tree/mod.rs index 375e2d7e2a53f7fd05bea747650bda62d5e8e54c..14ad59552f257546ca7a229fb5a81e8087df1909 100644 --- a/zed/src/sum_tree/mod.rs +++ b/zed/src/sum_tree/mod.rs @@ -73,7 +73,6 @@ impl SumTree { #[allow(unused)] pub fn items(&self) -> Vec { let mut cursor = self.cursor::<(), ()>(); - cursor.descend_to_first_item(self, |_| true); cursor.cloned().collect() } @@ -566,6 +565,7 @@ mod tests { for seed in 0..100 { use rand::{distributions, prelude::*}; + dbg!(seed); let rng = &mut StdRng::seed_from_u64(seed); let mut tree = SumTree::::new();