From 79c91de2f48ae58aca4f1b98b81e0bb5a0192286 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Fri, 14 May 2021 12:47:19 -0700 Subject: [PATCH 01/12] Get tests passing with RopeBuilder Co-Authored-By: Nathan Sobo --- zed/src/editor/buffer/mod.rs | 394 +++++++++++++++------------------- zed/src/editor/buffer/rope.rs | 11 +- 2 files changed, 179 insertions(+), 226 deletions(-) diff --git a/zed/src/editor/buffer/mod.rs b/zed/src/editor/buffer/mod.rs index 633d0e8a525ab1972518d174eb7475030fc915a6..3b5c657f1fa2268530700ad69a1928a31961efd3 100644 --- a/zed/src/editor/buffer/mod.rs +++ b/zed/src/editor/buffer/mod.rs @@ -12,7 +12,7 @@ use similar::{ChangeTag, TextDiff}; use crate::{ operation_queue::{self, OperationQueue}, - sum_tree::{self, FilterCursor, SeekBias, SumTree}, + sum_tree::{self, FilterCursor, Item, SeekBias, SumTree}, time::{self, ReplicaId}, util::RandomCharIter, worktree::FileHandle, @@ -25,6 +25,7 @@ use std::{ cmp, hash::BuildHasher, iter::{self, Iterator}, + mem, ops::Range, str, sync::Arc, @@ -1016,24 +1017,26 @@ impl Buffer { let start_fragment_id = self.resolve_fragment_id(start_id, start_offset)?; let end_fragment_id = self.resolve_fragment_id(end_id, end_offset)?; - let old_fragments = self.fragments.clone(); - let old_visible_text = self.visible_text.clone(); - let old_deleted_text = self.deleted_text.clone(); + let mut old_visible_text = Rope::new(); + let mut old_deleted_text = Rope::new(); + let mut old_fragments = SumTree::new(); + mem::swap(&mut old_visible_text, &mut self.visible_text); + mem::swap(&mut old_deleted_text, &mut self.deleted_text); + mem::swap(&mut old_fragments, &mut self.fragments); - let mut builder = RopeBuilder::new(old_visible_text.cursor(0), new_visible_text.cursor(0)); + let mut new_ropes = + RopeBuilder::new(old_visible_text.cursor(0), old_deleted_text.cursor(0)); let mut fragments_cursor = old_fragments.cursor::(); let mut new_fragments = fragments_cursor.slice(&FragmentIdRef::new(&start_fragment_id), SeekBias::Left, &()); - - builder.keep_to( - new_fragments.summary().text.visible, - new_fragments.summary().text.deleted, - ); + new_ropes.keep(new_fragments.summary().text); let start_fragment = fragments_cursor.item().unwrap(); if start_offset == start_fragment.range_in_insertion.end { - new_fragments.push(fragments_cursor.item().unwrap().clone(), &()); + let fragment = fragments_cursor.item().unwrap().clone(); + new_ropes.keep(fragment.summary().text); + new_fragments.push(fragment, &()); fragments_cursor.next(); } @@ -1073,35 +1076,31 @@ impl Buffer { None }; if let Some(fragment) = before_range { + new_ropes.keep(fragment.summary().text); new_fragments.push(fragment, &()); } if let Some(fragment) = insertion { - new_visible_text - .append(visible_text_cursor.slice(new_fragments.summary().text.visible)); + new_ropes.insert(new_text.take().unwrap()); new_fragments.push(fragment, &()); - new_visible_text.push(new_text.take().unwrap()); } if let Some(mut fragment) = within_range { + let fragment_was_visible = fragment.visible; if fragment.was_visible(&version_in_range, &self.undo_map) { fragment.deletions.insert(local_timestamp); if fragment.visible { fragment.visible = false; - new_visible_text.append( - visible_text_cursor.slice(new_fragments.summary().text.visible), - ); - new_deleted_text.append( - deleted_text_cursor.slice(new_fragments.summary().text.deleted), - ); - new_deleted_text.append( - visible_text_cursor - .slice(new_fragments.summary().text.visible + fragment.len()), - ); } } + if fragment_was_visible && !fragment.visible { + new_ropes.delete(fragment.len()); + } else { + new_ropes.keep(fragment.summary().text); + } new_fragments.push(fragment, &()); } if let Some(fragment) = after_range { + new_ropes.keep(fragment.summary().text); new_fragments.push(fragment, &()); } } else { @@ -1114,31 +1113,25 @@ impl Buffer { local_timestamp, lamport_timestamp, ); - new_visible_text - .append(visible_text_cursor.slice(new_fragments.summary().text.visible)); + new_ropes.insert(new_text); new_fragments.push(fragment, &()); - new_visible_text.push(new_text); } + let fragment_was_visible = fragment.visible; if fragment.id < end_fragment_id && fragment.was_visible(&version_in_range, &self.undo_map) { fragment.deletions.insert(local_timestamp); if fragment.visible { fragment.visible = false; - new_visible_text.append( - visible_text_cursor.slice(new_fragments.summary().text.visible), - ); - new_deleted_text.append( - deleted_text_cursor.slice(new_fragments.summary().text.deleted), - ); - new_deleted_text.append( - visible_text_cursor - .slice(new_fragments.summary().text.visible + fragment.len()), - ); } } + if fragment_was_visible && !fragment.visible { + new_ropes.delete(fragment.len()); + } else { + new_ropes.keep(fragment.summary().text); + } new_fragments.push(fragment, &()); } @@ -1153,19 +1146,16 @@ impl Buffer { local_timestamp, lamport_timestamp, ); - new_visible_text - .append(visible_text_cursor.slice(new_fragments.summary().text.visible)); + new_ropes.insert(new_text); new_fragments.push(fragment, &()); - new_visible_text.push(new_text); } + let (visible_text, deleted_text) = new_ropes.finish(); new_fragments.push_tree(fragments_cursor.suffix(&()), &()); - new_visible_text.append(visible_text_cursor.suffix()); - new_deleted_text.append(deleted_text_cursor.suffix()); self.fragments = new_fragments; - self.visible_text = new_visible_text; - self.deleted_text = new_deleted_text; + self.visible_text = visible_text; + self.deleted_text = deleted_text; self.local_clock.observe(local_timestamp); self.lamport_clock.observe(lamport_timestamp); Ok(()) @@ -1240,8 +1230,12 @@ impl Buffer { fn apply_undo(&mut self, undo: UndoOperation) -> Result<()> { let mut new_fragments; - let mut new_visible_text = Rope::new(); - let mut new_deleted_text = Rope::new(); + let mut old_visible_text = Rope::new(); + let mut old_deleted_text = Rope::new(); + mem::swap(&mut old_visible_text, &mut self.visible_text); + mem::swap(&mut old_deleted_text, &mut self.deleted_text); + let mut new_ropes = + RopeBuilder::new(old_visible_text.cursor(0), old_deleted_text.cursor(0)); self.undo_map.insert(undo); let edit = &self.history.ops[&undo.edit_id]; @@ -1249,8 +1243,6 @@ impl Buffer { let end_fragment_id = self.resolve_fragment_id(edit.end_id, edit.end_offset)?; let mut fragments_cursor = self.fragments.cursor::(); - let mut visible_text_cursor = self.visible_text.cursor(0); - let mut deleted_text_cursor = self.deleted_text.cursor(0); if edit.start_id == edit.end_id && edit.start_offset == edit.end_offset { let splits = &self.insertion_splits[&undo.edit_id]; @@ -1259,10 +1251,8 @@ impl Buffer { let first_split_id = insertion_splits.next().unwrap(); new_fragments = fragments_cursor.slice(&FragmentIdRef::new(first_split_id), SeekBias::Left, &()); - new_visible_text - .append(visible_text_cursor.slice(new_fragments.summary().text.visible)); - new_deleted_text - .append(deleted_text_cursor.slice(new_fragments.summary().text.deleted)); + new_ropes.keep(new_fragments.summary().text); + assert_eq!(new_ropes.summary(), new_fragments.summary().text); loop { let mut fragment = fragments_cursor.item().unwrap().clone(); @@ -1270,32 +1260,23 @@ impl Buffer { fragment.visible = fragment.is_visible(&self.undo_map); fragment.max_undos.observe(undo.id); - if fragment.visible != was_visible { - new_visible_text - .append(visible_text_cursor.slice(new_fragments.summary().text.visible)); - new_deleted_text - .append(deleted_text_cursor.slice(new_fragments.summary().text.deleted)); - } - if fragment.visible && !was_visible { - new_visible_text.append( - deleted_text_cursor - .slice(new_fragments.summary().text.deleted + fragment.len()), - ); + new_ropes.restore(fragment.len()); } else if !fragment.visible && was_visible { - new_deleted_text.append( - visible_text_cursor - .slice(new_fragments.summary().text.visible + fragment.len()), - ); + new_ropes.delete(fragment.len()); + } else { + new_ropes.keep(fragment.summary().text); } - new_fragments.push(fragment, &()); + new_fragments.push(fragment.clone(), &()); + assert_eq!(new_ropes.summary(), new_fragments.summary().text,); + fragments_cursor.next(); if let Some(split_id) = insertion_splits.next() { - new_fragments.push_tree( - fragments_cursor.slice(&FragmentIdRef::new(split_id), SeekBias::Left, &()), - &(), - ); + let slice = + fragments_cursor.slice(&FragmentIdRef::new(split_id), SeekBias::Left, &()); + new_ropes.keep(slice.summary().text); + new_fragments.push_tree(slice, &()); } else { break; } @@ -1306,6 +1287,9 @@ impl Buffer { SeekBias::Left, &(), ); + new_ropes.keep(new_fragments.summary().text); + assert_eq!(new_ropes.summary(), new_fragments.summary().text); + while let Some(fragment) = fragments_cursor.item() { if fragment.id > end_fragment_id { break; @@ -1318,42 +1302,32 @@ impl Buffer { fragment.visible = fragment.is_visible(&self.undo_map); fragment.max_undos.observe(undo.id); - if fragment.visible != was_visible { - new_visible_text.append( - visible_text_cursor.slice(new_fragments.summary().text.visible), - ); - new_deleted_text.append( - deleted_text_cursor.slice(new_fragments.summary().text.deleted), - ); - } - if fragment.visible && !was_visible { - new_visible_text.append( - deleted_text_cursor - .slice(new_fragments.summary().text.deleted + fragment.len()), - ); + new_ropes.restore(fragment.len()); } else if !fragment.visible && was_visible { - new_deleted_text.append( - visible_text_cursor - .slice(new_fragments.summary().text.visible + fragment.len()), - ); + new_ropes.delete(fragment.len()); + } else { + new_ropes.keep(fragment.summary().text); } + } else { + new_ropes.keep(fragment.summary().text); } new_fragments.push(fragment, &()); + assert_eq!(new_ropes.summary(), new_fragments.summary().text); + fragments_cursor.next(); } } } new_fragments.push_tree(fragments_cursor.suffix(&()), &()); - new_visible_text.append(visible_text_cursor.suffix()); - new_deleted_text.append(deleted_text_cursor.suffix()); + let (visible_text, deleted_text) = new_ropes.finish(); drop(fragments_cursor); self.fragments = new_fragments; - self.visible_text = new_visible_text; - self.deleted_text = new_deleted_text; + self.visible_text = visible_text; + self.deleted_text = deleted_text; Ok(()) } @@ -1434,24 +1408,21 @@ impl Buffer { let mut ops = Vec::with_capacity(old_ranges.size_hint().0); - let old_fragments = self.fragments.clone(); - let old_visible_text = self.visible_text.clone(); - let old_deleted_text = self.deleted_text.clone(); + let mut old_fragments = SumTree::new(); + let mut old_visible_text = Rope::new(); + let mut old_deleted_text = Rope::new(); + mem::swap(&mut old_visible_text, &mut self.visible_text); + mem::swap(&mut old_deleted_text, &mut self.deleted_text); + mem::swap(&mut old_fragments, &mut self.fragments); let mut fragments_cursor = old_fragments.cursor::(); - let mut visible_text_cursor = old_visible_text.cursor(0); - let mut deleted_text_cursor = old_deleted_text.cursor(0); - - let mut new_fragments = SumTree::new(); - let mut new_visible_text = Rope::new(); - let mut new_deleted_text = Rope::new(); + let mut new_fragments = + fragments_cursor.slice(&cur_range.as_ref().unwrap().start, SeekBias::Right, &()); - new_fragments.push_tree( - fragments_cursor.slice(&cur_range.as_ref().unwrap().start, SeekBias::Right, &()), - &(), - ); - new_visible_text.append(visible_text_cursor.slice(new_fragments.summary().text.visible)); - new_deleted_text.append(deleted_text_cursor.slice(new_fragments.summary().text.deleted)); + let mut new_ropes = + RopeBuilder::new(old_visible_text.cursor(0), old_deleted_text.cursor(0)); + new_ropes.keep(new_fragments.summary().text); + assert_eq!(new_ropes.summary(), new_fragments.summary().text); let mut start_id = None; let mut start_offset = None; @@ -1467,6 +1438,7 @@ impl Buffer { let fragment_summary = fragments_cursor.item_summary().unwrap(); let mut fragment_start = *fragments_cursor.start(); let mut fragment_end = fragment_start + fragment.visible_len(); + let fragment_was_visible = fragment.visible; let old_split_tree = self .insertion_splits @@ -1488,7 +1460,11 @@ impl Buffer { prefix.id = FragmentId::between(&new_fragments.last().unwrap().id, &fragment.id); fragment.range_in_insertion.start = prefix.range_in_insertion.end; + + new_ropes.keep(prefix.summary().text); new_fragments.push(prefix.clone(), &()); + assert_eq!(new_ropes.summary(), new_fragments.summary().text); + new_split_tree.push( InsertionSplit { extent: prefix.range_in_insertion.end - prefix.range_in_insertion.start, @@ -1520,11 +1496,9 @@ impl Buffer { lamport_timestamp, ); - new_visible_text.append( - visible_text_cursor.slice(new_fragments.summary().text.visible), - ); + new_ropes.insert(&new_text); new_fragments.push(new_fragment, &()); - new_visible_text.push(&new_text); + assert_eq!(new_ropes.summary(), new_fragments.summary().text); } } @@ -1539,20 +1513,14 @@ impl Buffer { if prefix.visible { prefix.deletions.insert(local_timestamp); prefix.visible = false; - - new_visible_text.append( - visible_text_cursor.slice(new_fragments.summary().text.visible), - ); - new_deleted_text.append( - deleted_text_cursor.slice(new_fragments.summary().text.deleted), - ); - new_deleted_text.append( - visible_text_cursor - .slice(new_fragments.summary().text.visible + prefix.len()), - ); + new_ropes.delete(prefix.len()); + } else { + new_ropes.keep(prefix.summary().text); } fragment.range_in_insertion.start = prefix.range_in_insertion.end; new_fragments.push(prefix.clone(), &()); + assert_eq!(new_ropes.summary(), new_fragments.summary().text); + new_split_tree.push( InsertionSplit { extent: prefix.range_in_insertion.end @@ -1570,17 +1538,6 @@ impl Buffer { if fragment.visible { fragment.deletions.insert(local_timestamp); fragment.visible = false; - - new_visible_text.append( - visible_text_cursor.slice(new_fragments.summary().text.visible), - ); - new_deleted_text.append( - deleted_text_cursor.slice(new_fragments.summary().text.deleted), - ); - new_deleted_text.append( - visible_text_cursor - .slice(new_fragments.summary().text.visible + fragment.len()), - ); } } @@ -1629,7 +1586,14 @@ impl Buffer { ); self.insertion_splits .insert(fragment.insertion.id, new_split_tree); + + if fragment_was_visible && !fragment.visible { + new_ropes.delete(fragment.len()); + } else { + new_ropes.keep(fragment.summary().text); + } new_fragments.push(fragment, &()); + assert_eq!(new_ropes.summary(), new_fragments.summary().text); // Scan forward until we find a fragment that is not fully contained by the current splice. fragments_cursor.next(); @@ -1644,18 +1608,9 @@ impl Buffer { if new_fragment.visible { new_fragment.deletions.insert(local_timestamp); new_fragment.visible = false; - - new_visible_text.append( - visible_text_cursor.slice(new_fragments.summary().text.visible), - ); - new_deleted_text.append( - deleted_text_cursor.slice(new_fragments.summary().text.deleted), - ); - new_deleted_text.append( - visible_text_cursor.slice( - new_fragments.summary().text.visible + new_fragment.len(), - ), - ); + new_ropes.delete(new_fragment.len()); + } else { + new_ropes.keep(new_fragment.summary().text); } new_fragments.push(new_fragment, &()); fragments_cursor.next(); @@ -1698,14 +1653,13 @@ impl Buffer { // that the cursor is parked at, we should seek to the next splice's start range // and push all the fragments in between into the new tree. if cur_range.as_ref().map_or(false, |r| r.start > fragment_end) { - new_fragments.push_tree( - fragments_cursor.slice( - &cur_range.as_ref().unwrap().start, - SeekBias::Right, - &(), - ), + let slice = fragments_cursor.slice( + &cur_range.as_ref().unwrap().start, + SeekBias::Right, &(), ); + new_ropes.keep(slice.summary().text); + new_fragments.push_tree(slice, &()); } } } @@ -1738,20 +1692,17 @@ impl Buffer { lamport_timestamp, ); - new_visible_text - .append(visible_text_cursor.slice(new_fragments.summary().text.visible)); + new_ropes.insert(&new_text); new_fragments.push(new_fragment, &()); - new_visible_text.push(&new_text); } } new_fragments.push_tree(fragments_cursor.suffix(&()), &()); - new_visible_text.append(visible_text_cursor.suffix()); - new_deleted_text.append(deleted_text_cursor.suffix()); + let (visible_text, deleted_text) = new_ropes.finish(); self.fragments = new_fragments; - self.visible_text = new_visible_text; - self.deleted_text = new_deleted_text; + self.visible_text = visible_text; + self.deleted_text = deleted_text; ops } @@ -2043,6 +1994,66 @@ impl Clone for Buffer { } } +struct RopeBuilder<'a> { + old_visible_cursor: rope::Cursor<'a>, + old_deleted_cursor: rope::Cursor<'a>, + new_visible: Rope, + new_deleted: Rope, +} + +impl<'a> RopeBuilder<'a> { + fn new(old_visible_cursor: rope::Cursor<'a>, old_deleted_cursor: rope::Cursor<'a>) -> Self { + Self { + old_visible_cursor, + old_deleted_cursor, + new_visible: Rope::new(), + new_deleted: Rope::new(), + } + } + + fn keep(&mut self, len: FragmentTextSummary) { + let visible_text = self + .old_visible_cursor + .slice(self.old_visible_cursor.offset() + len.visible); + let deleted_text = self + .old_deleted_cursor + .slice(self.old_deleted_cursor.offset() + len.deleted); + self.new_visible.append(visible_text); + self.new_deleted.append(deleted_text); + } + + fn delete(&mut self, deleted_len: usize) { + let deleted = self + .old_visible_cursor + .slice(self.old_visible_cursor.offset() + deleted_len); + self.new_deleted.append(deleted); + } + + fn restore(&mut self, restored_len: usize) { + let restored = self + .old_deleted_cursor + .slice(self.old_deleted_cursor.offset() + restored_len); + self.new_visible.append(restored); + } + + fn insert(&mut self, text: &str) { + self.new_visible.push(text); + } + + fn summary(&self) -> FragmentTextSummary { + FragmentTextSummary { + visible: self.new_visible.len(), + deleted: self.new_deleted.len(), + } + } + + fn finish(mut self) -> (Rope, Rope) { + self.new_visible.append(self.old_visible_cursor.suffix()); + self.new_deleted.append(self.old_deleted_cursor.suffix()); + (self.new_visible, self.new_deleted) + } +} + #[derive(Clone, Debug, Eq, PartialEq)] pub enum Event { Edited, @@ -3511,68 +3522,3 @@ mod tests { lengths } } - -struct RopeBuilder<'a> { - visible_delta: isize, - deleted_delta: isize, - old_visible_cursor: rope::Cursor<'a>, - old_deleted_cursor: rope::Cursor<'a>, - new_visible: Rope, - new_deleted: Rope, -} - -impl<'a> RopeBuilder<'a> { - fn new(old_visible_cursor: rope::Cursor<'a>, old_deleted_cursor: rope::Cursor<'a>) -> Self { - Self { - visible_delta: 0, - deleted_delta: 0, - old_visible_cursor, - old_deleted_cursor, - new_visible: Rope::new(), - new_deleted: Rope::new(), - } - } - - fn keep_to(&mut self, sum: FragmentTextSummary) { - self.new_visible.append( - self.old_visible_cursor - .slice((sum.visible as isize + self.visible_delta) as usize), - ); - self.new_deleted.append( - self.old_deleted_cursor - .slice((sum.deleted as isize + self.deleted_delta) as usize), - ); - } - - fn delete_to(&mut self, offset: usize) { - let deleted = self - .old_visible_cursor - .slice((offset as isize + self.visible_delta) as usize); - let deleted_len = deleted.len(); - self.new_deleted.append(deleted); - self.visible_delta += deleted_len as isize; - self.deleted_delta -= deleted_len as isize; - } - - fn restore_to(&mut self, offset: usize) { - let restored = self - .old_deleted_cursor - .slice((offset as isize + self.deleted_delta) as usize); - let restored_len = restored.len(); - self.new_visible.append(restored); - self.visible_delta -= restored_len as isize; - self.deleted_delta += restored_len as isize; - } - - fn insert(&mut self, text: &str) { - let old_len = self.new_visible.len(); - self.new_visible.push(text); - self.visible_delta -= (self.new_visible.len() - old_len) as isize; - } - - fn finish(self) -> (Rope, Rope) { - self.new_visible.append(self.old_visible_cursor.suffix()); - self.new_deleted.append(self.old_deleted_cursor.suffix()); - (self.new_visible, self.new_deleted) - } -} diff --git a/zed/src/editor/buffer/rope.rs b/zed/src/editor/buffer/rope.rs index c08733f62512019ef5c68f187cf8651e04f25130..74a680286bc0ad2a0430094308d3040f8499f966 100644 --- a/zed/src/editor/buffer/rope.rs +++ b/zed/src/editor/buffer/rope.rs @@ -129,7 +129,10 @@ impl Rope { let mut cursor = self.chunks.cursor::(); cursor.seek(&offset, SeekBias::Left, &()); let overshoot = offset - cursor.start().chars; - Ok(cursor.start().lines + cursor.item().unwrap().to_point(overshoot)) + Ok(cursor.start().lines + + cursor + .item() + .map_or(Point::zero(), |chunk| chunk.to_point(overshoot))) } else { Err(anyhow!("offset out of bounds")) } @@ -141,7 +144,7 @@ impl Rope { let mut cursor = self.chunks.cursor::(); cursor.seek(&point, SeekBias::Left, &()); let overshoot = point - cursor.start().lines; - Ok(cursor.start().chars + cursor.item().unwrap().to_offset(overshoot)) + Ok(cursor.start().chars + cursor.item().map_or(0, |chunk| chunk.to_offset(overshoot))) } else { Err(anyhow!("offset out of bounds")) } @@ -207,6 +210,10 @@ impl<'a> Cursor<'a> { pub fn suffix(mut self) -> Rope { self.slice(self.rope.chunks.extent()) } + + pub fn offset(&self) -> usize { + self.offset + } } #[derive(Clone, Debug, Default)] From fc2533555c8411ae8667bf681a57dbbbbda4144a Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Fri, 14 May 2021 14:07:25 -0700 Subject: [PATCH 02/12] Remove inline assertions about RopeBuilder invariants --- zed/src/editor/buffer/mod.rs | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/zed/src/editor/buffer/mod.rs b/zed/src/editor/buffer/mod.rs index 3b5c657f1fa2268530700ad69a1928a31961efd3..f235deafefed4ac85b65eaee9e3e7301a1b6ded0 100644 --- a/zed/src/editor/buffer/mod.rs +++ b/zed/src/editor/buffer/mod.rs @@ -1252,7 +1252,6 @@ impl Buffer { new_fragments = fragments_cursor.slice(&FragmentIdRef::new(first_split_id), SeekBias::Left, &()); new_ropes.keep(new_fragments.summary().text); - assert_eq!(new_ropes.summary(), new_fragments.summary().text); loop { let mut fragment = fragments_cursor.item().unwrap().clone(); @@ -1269,7 +1268,6 @@ impl Buffer { } new_fragments.push(fragment.clone(), &()); - assert_eq!(new_ropes.summary(), new_fragments.summary().text,); fragments_cursor.next(); if let Some(split_id) = insertion_splits.next() { @@ -1288,7 +1286,6 @@ impl Buffer { &(), ); new_ropes.keep(new_fragments.summary().text); - assert_eq!(new_ropes.summary(), new_fragments.summary().text); while let Some(fragment) = fragments_cursor.item() { if fragment.id > end_fragment_id { @@ -1314,8 +1311,6 @@ impl Buffer { } new_fragments.push(fragment, &()); - assert_eq!(new_ropes.summary(), new_fragments.summary().text); - fragments_cursor.next(); } } @@ -1422,7 +1417,6 @@ impl Buffer { let mut new_ropes = RopeBuilder::new(old_visible_text.cursor(0), old_deleted_text.cursor(0)); new_ropes.keep(new_fragments.summary().text); - assert_eq!(new_ropes.summary(), new_fragments.summary().text); let mut start_id = None; let mut start_offset = None; @@ -1463,8 +1457,6 @@ impl Buffer { new_ropes.keep(prefix.summary().text); new_fragments.push(prefix.clone(), &()); - assert_eq!(new_ropes.summary(), new_fragments.summary().text); - new_split_tree.push( InsertionSplit { extent: prefix.range_in_insertion.end - prefix.range_in_insertion.start, @@ -1498,7 +1490,6 @@ impl Buffer { new_ropes.insert(&new_text); new_fragments.push(new_fragment, &()); - assert_eq!(new_ropes.summary(), new_fragments.summary().text); } } @@ -1519,8 +1510,6 @@ impl Buffer { } fragment.range_in_insertion.start = prefix.range_in_insertion.end; new_fragments.push(prefix.clone(), &()); - assert_eq!(new_ropes.summary(), new_fragments.summary().text); - new_split_tree.push( InsertionSplit { extent: prefix.range_in_insertion.end @@ -1593,7 +1582,6 @@ impl Buffer { new_ropes.keep(fragment.summary().text); } new_fragments.push(fragment, &()); - assert_eq!(new_ropes.summary(), new_fragments.summary().text); // Scan forward until we find a fragment that is not fully contained by the current splice. fragments_cursor.next(); From 614e96b95706c1463c1dc4a95129f05136600aa9 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Fri, 14 May 2021 14:48:19 -0700 Subject: [PATCH 03/12] Clarify how the ropes are kept consistent with the fragments --- zed/src/editor/buffer/mod.rs | 132 +++++++++++++---------------------- 1 file changed, 47 insertions(+), 85 deletions(-) diff --git a/zed/src/editor/buffer/mod.rs b/zed/src/editor/buffer/mod.rs index f235deafefed4ac85b65eaee9e3e7301a1b6ded0..78c7c2f2af4cbe6ac77a736a0e18282803f1e67c 100644 --- a/zed/src/editor/buffer/mod.rs +++ b/zed/src/editor/buffer/mod.rs @@ -12,7 +12,7 @@ use similar::{ChangeTag, TextDiff}; use crate::{ operation_queue::{self, OperationQueue}, - sum_tree::{self, FilterCursor, Item, SeekBias, SumTree}, + sum_tree::{self, FilterCursor, SeekBias, SumTree}, time::{self, ReplicaId}, util::RandomCharIter, worktree::FileHandle, @@ -1024,18 +1024,18 @@ impl Buffer { mem::swap(&mut old_deleted_text, &mut self.deleted_text); mem::swap(&mut old_fragments, &mut self.fragments); - let mut new_ropes = - RopeBuilder::new(old_visible_text.cursor(0), old_deleted_text.cursor(0)); let mut fragments_cursor = old_fragments.cursor::(); let mut new_fragments = fragments_cursor.slice(&FragmentIdRef::new(&start_fragment_id), SeekBias::Left, &()); - new_ropes.keep(new_fragments.summary().text); + let mut new_ropes = + RopeBuilder::new(old_visible_text.cursor(0), old_deleted_text.cursor(0)); + new_ropes.push_tree(new_fragments.summary().text); let start_fragment = fragments_cursor.item().unwrap(); if start_offset == start_fragment.range_in_insertion.end { let fragment = fragments_cursor.item().unwrap().clone(); - new_ropes.keep(fragment.summary().text); + new_ropes.push_fragment(&fragment, fragment.visible); new_fragments.push(fragment, &()); fragments_cursor.next(); } @@ -1076,11 +1076,11 @@ impl Buffer { None }; if let Some(fragment) = before_range { - new_ropes.keep(fragment.summary().text); + new_ropes.push_fragment(&fragment, fragment.visible); new_fragments.push(fragment, &()); } if let Some(fragment) = insertion { - new_ropes.insert(new_text.take().unwrap()); + new_ropes.push_str(new_text.take().unwrap()); new_fragments.push(fragment, &()); } if let Some(mut fragment) = within_range { @@ -1092,15 +1092,11 @@ impl Buffer { } } - if fragment_was_visible && !fragment.visible { - new_ropes.delete(fragment.len()); - } else { - new_ropes.keep(fragment.summary().text); - } + new_ropes.push_fragment(&fragment, fragment_was_visible); new_fragments.push(fragment, &()); } if let Some(fragment) = after_range { - new_ropes.keep(fragment.summary().text); + new_ropes.push_fragment(&fragment, fragment.visible); new_fragments.push(fragment, &()); } } else { @@ -1113,7 +1109,7 @@ impl Buffer { local_timestamp, lamport_timestamp, ); - new_ropes.insert(new_text); + new_ropes.push_str(new_text); new_fragments.push(fragment, &()); } @@ -1127,11 +1123,7 @@ impl Buffer { } } - if fragment_was_visible && !fragment.visible { - new_ropes.delete(fragment.len()); - } else { - new_ropes.keep(fragment.summary().text); - } + new_ropes.push_fragment(&fragment, fragment_was_visible); new_fragments.push(fragment, &()); } @@ -1146,7 +1138,7 @@ impl Buffer { local_timestamp, lamport_timestamp, ); - new_ropes.insert(new_text); + new_ropes.push_str(new_text); new_fragments.push(fragment, &()); } @@ -1251,7 +1243,7 @@ impl Buffer { let first_split_id = insertion_splits.next().unwrap(); new_fragments = fragments_cursor.slice(&FragmentIdRef::new(first_split_id), SeekBias::Left, &()); - new_ropes.keep(new_fragments.summary().text); + new_ropes.push_tree(new_fragments.summary().text); loop { let mut fragment = fragments_cursor.item().unwrap().clone(); @@ -1259,21 +1251,14 @@ impl Buffer { fragment.visible = fragment.is_visible(&self.undo_map); fragment.max_undos.observe(undo.id); - if fragment.visible && !was_visible { - new_ropes.restore(fragment.len()); - } else if !fragment.visible && was_visible { - new_ropes.delete(fragment.len()); - } else { - new_ropes.keep(fragment.summary().text); - } - + new_ropes.push_fragment(&fragment, was_visible); new_fragments.push(fragment.clone(), &()); fragments_cursor.next(); if let Some(split_id) = insertion_splits.next() { let slice = fragments_cursor.slice(&FragmentIdRef::new(split_id), SeekBias::Left, &()); - new_ropes.keep(slice.summary().text); + new_ropes.push_tree(slice.summary().text); new_fragments.push_tree(slice, &()); } else { break; @@ -1285,31 +1270,22 @@ impl Buffer { SeekBias::Left, &(), ); - new_ropes.keep(new_fragments.summary().text); + new_ropes.push_tree(new_fragments.summary().text); while let Some(fragment) = fragments_cursor.item() { if fragment.id > end_fragment_id { break; } else { let mut fragment = fragment.clone(); + let fragment_was_visible = fragment.visible; if edit.version_in_range.observed(fragment.insertion.id) || fragment.insertion.id == undo.edit_id { - let was_visible = fragment.visible; fragment.visible = fragment.is_visible(&self.undo_map); fragment.max_undos.observe(undo.id); - - if fragment.visible && !was_visible { - new_ropes.restore(fragment.len()); - } else if !fragment.visible && was_visible { - new_ropes.delete(fragment.len()); - } else { - new_ropes.keep(fragment.summary().text); - } - } else { - new_ropes.keep(fragment.summary().text); } + new_ropes.push_fragment(&fragment, fragment_was_visible); new_fragments.push(fragment, &()); fragments_cursor.next(); } @@ -1416,7 +1392,7 @@ impl Buffer { let mut new_ropes = RopeBuilder::new(old_visible_text.cursor(0), old_deleted_text.cursor(0)); - new_ropes.keep(new_fragments.summary().text); + new_ropes.push_tree(new_fragments.summary().text); let mut start_id = None; let mut start_offset = None; @@ -1455,7 +1431,7 @@ impl Buffer { FragmentId::between(&new_fragments.last().unwrap().id, &fragment.id); fragment.range_in_insertion.start = prefix.range_in_insertion.end; - new_ropes.keep(prefix.summary().text); + new_ropes.push_fragment(&prefix, prefix.visible); new_fragments.push(prefix.clone(), &()); new_split_tree.push( InsertionSplit { @@ -1488,7 +1464,7 @@ impl Buffer { lamport_timestamp, ); - new_ropes.insert(&new_text); + new_ropes.push_str(&new_text); new_fragments.push(new_fragment, &()); } } @@ -1504,11 +1480,9 @@ impl Buffer { if prefix.visible { prefix.deletions.insert(local_timestamp); prefix.visible = false; - new_ropes.delete(prefix.len()); - } else { - new_ropes.keep(prefix.summary().text); } fragment.range_in_insertion.start = prefix.range_in_insertion.end; + new_ropes.push_fragment(&prefix, fragment_was_visible); new_fragments.push(prefix.clone(), &()); new_split_tree.push( InsertionSplit { @@ -1576,11 +1550,7 @@ impl Buffer { self.insertion_splits .insert(fragment.insertion.id, new_split_tree); - if fragment_was_visible && !fragment.visible { - new_ropes.delete(fragment.len()); - } else { - new_ropes.keep(fragment.summary().text); - } + new_ropes.push_fragment(&fragment, fragment_was_visible); new_fragments.push(fragment, &()); // Scan forward until we find a fragment that is not fully contained by the current splice. @@ -1588,6 +1558,7 @@ impl Buffer { if let Some(range) = cur_range.clone() { while let Some(fragment) = fragments_cursor.item() { let fragment_summary = fragments_cursor.item_summary().unwrap(); + let fragment_was_visible = fragment.visible; fragment_start = *fragments_cursor.start(); fragment_end = fragment_start + fragment.visible_len(); if range.start < fragment_start && range.end >= fragment_end { @@ -1596,10 +1567,9 @@ impl Buffer { if new_fragment.visible { new_fragment.deletions.insert(local_timestamp); new_fragment.visible = false; - new_ropes.delete(new_fragment.len()); - } else { - new_ropes.keep(new_fragment.summary().text); } + + new_ropes.push_fragment(&new_fragment, fragment_was_visible); new_fragments.push(new_fragment, &()); fragments_cursor.next(); @@ -1646,7 +1616,7 @@ impl Buffer { SeekBias::Right, &(), ); - new_ropes.keep(slice.summary().text); + new_ropes.push_tree(slice.summary().text); new_fragments.push_tree(slice, &()); } } @@ -1680,7 +1650,7 @@ impl Buffer { lamport_timestamp, ); - new_ropes.insert(&new_text); + new_ropes.push_str(&new_text); new_fragments.push(new_fragment, &()); } } @@ -1999,42 +1969,34 @@ impl<'a> RopeBuilder<'a> { } } - fn keep(&mut self, len: FragmentTextSummary) { - let visible_text = self - .old_visible_cursor - .slice(self.old_visible_cursor.offset() + len.visible); - let deleted_text = self - .old_deleted_cursor - .slice(self.old_deleted_cursor.offset() + len.deleted); - self.new_visible.append(visible_text); - self.new_deleted.append(deleted_text); + fn push_tree(&mut self, len: FragmentTextSummary) { + self.push(len.visible, true, true); + self.push(len.deleted, false, false); } - fn delete(&mut self, deleted_len: usize) { - let deleted = self - .old_visible_cursor - .slice(self.old_visible_cursor.offset() + deleted_len); - self.new_deleted.append(deleted); + fn push_fragment(&mut self, fragment: &Fragment, was_visible: bool) { + self.push(fragment.len(), was_visible, fragment.visible) } - fn restore(&mut self, restored_len: usize) { - let restored = self - .old_deleted_cursor - .slice(self.old_deleted_cursor.offset() + restored_len); - self.new_visible.append(restored); + fn push(&mut self, len: usize, was_visible: bool, is_visible: bool) { + let text = if was_visible { + self.old_visible_cursor + .slice(self.old_visible_cursor.offset() + len) + } else { + self.old_deleted_cursor + .slice(self.old_deleted_cursor.offset() + len) + }; + if is_visible { + self.new_visible.append(text); + } else { + self.new_deleted.append(text); + } } - fn insert(&mut self, text: &str) { + fn push_str(&mut self, text: &str) { self.new_visible.push(text); } - fn summary(&self) -> FragmentTextSummary { - FragmentTextSummary { - visible: self.new_visible.len(), - deleted: self.new_deleted.len(), - } - } - fn finish(mut self) -> (Rope, Rope) { self.new_visible.append(self.old_visible_cursor.suffix()); self.new_deleted.append(self.old_deleted_cursor.suffix()); From e860cacb9fa38a1c11afc318a3887d579d22c24c Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Fri, 14 May 2021 17:45:16 -0700 Subject: [PATCH 04/12] Get randomized tests passing in the presence of multibyte chars --- zed/src/editor/buffer/mod.rs | 31 ++++++++++++------ zed/src/editor/buffer/rope.rs | 44 +++++++++++++++----------- zed/src/editor/display_map/fold_map.rs | 7 ++-- zed/src/util.rs | 33 +++++++++++++++++-- 4 files changed, 83 insertions(+), 32 deletions(-) diff --git a/zed/src/editor/buffer/mod.rs b/zed/src/editor/buffer/mod.rs index 78c7c2f2af4cbe6ac77a736a0e18282803f1e67c..87ebf272cb8e1667b4fdaef34ef54c17550ffd01 100644 --- a/zed/src/editor/buffer/mod.rs +++ b/zed/src/editor/buffer/mod.rs @@ -2509,12 +2509,12 @@ mod tests { for _i in 0..10 { let (old_ranges, new_text, _) = buffer.randomly_mutate(rng, None); for old_range in old_ranges.iter().rev() { - reference_string = [ - &reference_string[0..old_range.start], - new_text.as_str(), - &reference_string[old_range.end..], - ] - .concat(); + reference_string = reference_string + .chars() + .take(old_range.start) + .chain(new_text.chars()) + .chain(reference_string.chars().skip(old_range.end)) + .collect(); } assert_eq!(buffer.text(), reference_string); @@ -2549,7 +2549,12 @@ mod tests { let range_sum = buffer.text_summary_for_range(start..end); assert_eq!(range_sum.rightmost_point.column, *longest_column); assert!(longest_rows.contains(&range_sum.rightmost_point.row)); - let range_text = &buffer.text()[start..end]; + let range_text = buffer + .text() + .chars() + .skip(start) + .take(end - start) + .collect::(); assert_eq!(range_sum.chars, range_text.chars().count()); assert_eq!(range_sum.bytes, range_text.len()); } @@ -3458,9 +3463,17 @@ mod tests { fn line_lengths_in_range(buffer: &Buffer, range: Range) -> BTreeMap> { let mut lengths = BTreeMap::new(); - for (row, line) in buffer.text()[range].lines().enumerate() { + for (row, line) in buffer + .text() + .chars() + .skip(range.start) + .take(range.len()) + .collect::() + .lines() + .enumerate() + { lengths - .entry(line.len() as u32) + .entry(line.chars().count() as u32) .or_insert(HashSet::default()) .insert(row as u32); } diff --git a/zed/src/editor/buffer/rope.rs b/zed/src/editor/buffer/rope.rs index 74a680286bc0ad2a0430094308d3040f8499f966..231aa59878bdedf5448e0e1c0ba8ef8ca9e921f1 100644 --- a/zed/src/editor/buffer/rope.rs +++ b/zed/src/editor/buffer/rope.rs @@ -1,12 +1,13 @@ use super::Point; use crate::sum_tree::{self, SeekBias, SumTree}; +use crate::util::byte_range_for_char_range; use anyhow::{anyhow, Result}; use arrayvec::ArrayString; use smallvec::SmallVec; -use std::{cmp, ops::Range, str}; +use std::{cmp, iter::Skip, ops::Range, str}; #[cfg(test)] -const CHUNK_BASE: usize = 2; +const CHUNK_BASE: usize = 6; #[cfg(not(test))] const CHUNK_BASE: usize = 16; @@ -55,10 +56,7 @@ impl Rope { if last_chunk.0.len() + first_new_chunk_ref.0.len() <= 2 * CHUNK_BASE { last_chunk.0.push_str(&first_new_chunk.take().unwrap().0); } else { - let mut text = ArrayString::<[_; 4 * CHUNK_BASE]>::new(); - text.push_str(&last_chunk.0); - text.push_str(&first_new_chunk_ref.0); - + let text = [last_chunk.0, first_new_chunk_ref.0].concat(); let mut midpoint = text.len() / 2; while !text.is_char_boundary(midpoint) { midpoint += 1; @@ -83,10 +81,11 @@ impl Rope { #[cfg(test)] { // Ensure all chunks except maybe the last one are not underflowing. + // Allow some wiggle room for multibyte characters at chunk boundaries. let mut chunks = self.chunks.cursor::<(), ()>().peekable(); while let Some(chunk) = chunks.next() { if chunks.peek().is_some() { - assert!(chunk.0.len() >= CHUNK_BASE); + assert!(chunk.0.len() + 3 >= CHUNK_BASE); } } } @@ -190,7 +189,8 @@ impl<'a> Cursor<'a> { if let Some(start_chunk) = self.chunks.item() { let start_ix = self.offset - self.chunks.start(); let end_ix = cmp::min(end_offset, self.chunks.end()) - self.chunks.start(); - slice.push(&start_chunk.0[start_ix..end_ix]); + let byte_range = byte_range_for_char_range(start_chunk.0, start_ix..end_ix); + slice.push(&start_chunk.0[byte_range]); } if end_offset > self.chunks.end() { @@ -199,7 +199,9 @@ impl<'a> Cursor<'a> { chunks: self.chunks.slice(&end_offset, SeekBias::Right, &()), }); if let Some(end_chunk) = self.chunks.item() { - slice.push(&end_chunk.0[..end_offset - self.chunks.start()]); + let end_ix = end_offset - self.chunks.start(); + let byte_range = byte_range_for_char_range(end_chunk.0, 0..end_ix); + slice.push(&end_chunk.0[byte_range]); } } @@ -217,7 +219,7 @@ impl<'a> Cursor<'a> { } #[derive(Clone, Debug, Default)] -struct Chunk(ArrayString<[u8; 2 * CHUNK_BASE]>); +struct Chunk(ArrayString<[u8; 4 * CHUNK_BASE]>); impl Chunk { fn to_point(&self, target: usize) -> Point { @@ -358,7 +360,7 @@ impl<'a> sum_tree::Dimension<'a, TextSummary> for Point { pub struct Chars<'a> { cursor: sum_tree::Cursor<'a, Chunk, usize, usize>, - chars: str::Chars<'a>, + chars: Skip>, } impl<'a> Chars<'a> { @@ -368,9 +370,9 @@ impl<'a> Chars<'a> { let chars = if let Some(chunk) = cursor.item() { let ix = start - cursor.start(); cursor.next(); - chunk.0[ix..].chars() + chunk.0.chars().skip(ix) } else { - "".chars() + "".chars().skip(0) }; Self { cursor, chars } @@ -384,7 +386,7 @@ impl<'a> Iterator for Chars<'a> { if let Some(ch) = self.chars.next() { Some(ch) } else if let Some(chunk) = self.cursor.item() { - self.chars = chunk.0.chars(); + self.chars = chunk.0.chars().skip(0); self.cursor.next(); Some(self.chars.next().unwrap()) } else { @@ -422,9 +424,9 @@ mod tests { let mut expected = String::new(); let mut actual = Rope::new(); for _ in 0..operations { - let end_ix = rng.gen_range(0..=expected.len()); + let end_ix = rng.gen_range(0..=expected.chars().count()); let start_ix = rng.gen_range(0..=end_ix); - let len = rng.gen_range(0..=20); + let len = rng.gen_range(0..=64); let new_text: String = RandomCharIter::new(&mut rng).take(len).collect(); let mut new_actual = Rope::new(); @@ -436,16 +438,20 @@ mod tests { actual = new_actual; let mut new_expected = String::new(); - new_expected.push_str(&expected[..start_ix]); + new_expected.extend(expected.chars().take(start_ix)); new_expected.push_str(&new_text); - new_expected.push_str(&expected[end_ix..]); + new_expected.extend(expected.chars().skip(end_ix)); expected = new_expected; assert_eq!(actual.text(), expected); + log::info!("text: {:?}", expected); for _ in 0..5 { let ix = rng.gen_range(0..=expected.len()); - assert_eq!(actual.chars_at(ix).collect::(), expected[ix..]); + assert_eq!( + actual.chars_at(ix).collect::(), + expected.chars().skip(ix).collect::() + ); } let mut point = Point::new(0, 0); diff --git a/zed/src/editor/display_map/fold_map.rs b/zed/src/editor/display_map/fold_map.rs index b404bcb3dc766fd1a8688c867ed9ae140eff2b98..89eb7b062e1e2ef8ec06df13bf303d8ec7d9b116 100644 --- a/zed/src/editor/display_map/fold_map.rs +++ b/zed/src/editor/display_map/fold_map.rs @@ -830,7 +830,7 @@ mod tests { #[gpui::test] fn test_random_folds(app: &mut gpui::MutableAppContext) { use crate::editor::ToPoint; - use crate::util::RandomCharIter; + use crate::util::{byte_range_for_char_range, RandomCharIter}; use rand::prelude::*; use std::env; @@ -905,7 +905,10 @@ mod tests { expected_buffer_rows.extend((fold_end.row + 1..=next_row).rev()); next_row = fold_start.row; - expected_text.replace_range(fold_range.start..fold_range.end, "…"); + expected_text.replace_range( + byte_range_for_char_range(&expected_text, fold_range.start..fold_range.end), + "…", + ); } expected_buffer_rows.extend((0..=next_row).rev()); expected_buffer_rows.reverse(); diff --git a/zed/src/util.rs b/zed/src/util.rs index 1ce914ddd46c272af2deb69c85ec7c0a8e5eca28..c5e968243f7fd16a4faf1327994328aadf50e6e3 100644 --- a/zed/src/util.rs +++ b/zed/src/util.rs @@ -1,5 +1,20 @@ use rand::prelude::*; -use std::cmp::Ordering; +use std::{cmp::Ordering, ops::Range}; + +pub fn byte_range_for_char_range(text: impl AsRef, char_range: Range) -> Range { + let text = text.as_ref(); + let mut result = text.len()..text.len(); + for (i, (offset, _)) in text.char_indices().enumerate() { + if i == char_range.start { + result.start = offset; + } + if i == char_range.end { + result.end = offset; + break; + } + } + result +} pub fn post_inc(value: &mut usize) -> usize { let prev = *value; @@ -44,7 +59,21 @@ impl Iterator for RandomCharIter { fn next(&mut self) -> Option { if self.0.gen_bool(1.0 / 5.0) { Some('\n') - } else { + } + // two-byte greek letters + else if self.0.gen_bool(1.0 / 8.0) { + Some(std::char::from_u32(self.0.gen_range(('α' as u32)..('ω' as u32 + 1))).unwrap()) + } + // three-byte characters + else if self.0.gen_bool(1.0 / 10.0) { + ['✋', '✅', '❌', '❎', '⭐'].choose(&mut self.0).cloned() + } + // four-byte characters + else if self.0.gen_bool(1.0 / 12.0) { + ['🍐', '🏀', '🍗', '🎉'].choose(&mut self.0).cloned() + } + // ascii letters + else { Some(self.0.gen_range(b'a'..b'z' + 1).into()) } } From 243b66a91d51efb57b72312c9d3b2503b935e012 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Fri, 14 May 2021 17:53:58 -0700 Subject: [PATCH 05/12] Add unit test for rope with all 4-byte chars --- zed/src/editor/buffer/rope.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/zed/src/editor/buffer/rope.rs b/zed/src/editor/buffer/rope.rs index 231aa59878bdedf5448e0e1c0ba8ef8ca9e921f1..3b85c04c7e479b3fa35b6a98ebaf4f0fba9ac29c 100644 --- a/zed/src/editor/buffer/rope.rs +++ b/zed/src/editor/buffer/rope.rs @@ -403,6 +403,14 @@ mod tests { use rand::prelude::*; use std::env; + #[test] + fn test_all_4_byte_chars() { + let mut rope = Rope::new(); + let text = "🏀".repeat(256); + rope.push(&text); + assert_eq!(rope.text(), text); + } + #[test] fn test_random() { let iterations = env::var("ITERATIONS") From 1190a87a052130f9ef6d8dd030c920fe4d5c9ab7 Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Sat, 15 May 2021 10:08:01 +0200 Subject: [PATCH 06/12] Avoid heap allocation when splitting an existing chunk --- zed/src/editor/buffer/rope.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/zed/src/editor/buffer/rope.rs b/zed/src/editor/buffer/rope.rs index 3b85c04c7e479b3fa35b6a98ebaf4f0fba9ac29c..9d902e6eee6ff59aa43cb9f4cb231207e66a237f 100644 --- a/zed/src/editor/buffer/rope.rs +++ b/zed/src/editor/buffer/rope.rs @@ -56,7 +56,9 @@ impl Rope { if last_chunk.0.len() + first_new_chunk_ref.0.len() <= 2 * CHUNK_BASE { last_chunk.0.push_str(&first_new_chunk.take().unwrap().0); } else { - let text = [last_chunk.0, first_new_chunk_ref.0].concat(); + let mut text = ArrayString::<[_; 4 * CHUNK_BASE]>::new(); + text.push_str(&last_chunk.0); + text.push_str(&first_new_chunk_ref.0); let mut midpoint = text.len() / 2; while !text.is_char_boundary(midpoint) { midpoint += 1; From def0aa98b204bccebc1333a7e61ad244fd05fa45 Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Sat, 15 May 2021 10:08:37 +0200 Subject: [PATCH 07/12] Maximize chunks occupation by splitting chunks appropriately --- zed/src/editor/buffer/rope.rs | 27 +++++++++++++++++++++------ 1 file changed, 21 insertions(+), 6 deletions(-) diff --git a/zed/src/editor/buffer/rope.rs b/zed/src/editor/buffer/rope.rs index 9d902e6eee6ff59aa43cb9f4cb231207e66a237f..cadf994c5d5bee5fc39b30507853abdc080de937 100644 --- a/zed/src/editor/buffer/rope.rs +++ b/zed/src/editor/buffer/rope.rs @@ -59,11 +59,7 @@ impl Rope { let mut text = ArrayString::<[_; 4 * CHUNK_BASE]>::new(); text.push_str(&last_chunk.0); text.push_str(&first_new_chunk_ref.0); - let mut midpoint = text.len() / 2; - while !text.is_char_boundary(midpoint) { - midpoint += 1; - } - let (left, right) = text.split_at(midpoint); + let (left, right) = text.split_at(find_split_ix(&text)); last_chunk.0.clear(); last_chunk.0.push_str(left); first_new_chunk_ref.0.clear(); @@ -221,7 +217,7 @@ impl<'a> Cursor<'a> { } #[derive(Clone, Debug, Default)] -struct Chunk(ArrayString<[u8; 4 * CHUNK_BASE]>); +struct Chunk(ArrayString<[u8; 2 * CHUNK_BASE]>); impl Chunk { fn to_point(&self, target: usize) -> Point { @@ -397,6 +393,25 @@ impl<'a> Iterator for Chars<'a> { } } +fn find_split_ix(text: &str) -> usize { + let mut ix = text.len() / 2; + while !text.is_char_boundary(ix) { + if ix < 2 * CHUNK_BASE { + ix += 1; + } else { + ix = (text.len() / 2) - 1; + break; + } + } + while !text.is_char_boundary(ix) { + ix -= 1; + } + + debug_assert!(ix <= 2 * CHUNK_BASE); + debug_assert!(text.len() - ix <= 2 * CHUNK_BASE); + ix +} + #[cfg(test)] mod tests { use crate::util::RandomCharIter; From 76a74e431eb66d1ceea1abc23964c96bfa9bb5b4 Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Sat, 15 May 2021 11:29:48 +0200 Subject: [PATCH 08/12] Introduce `rope::Cursor::summary` to avoid slicing only to get a summary This also deletes `Rope::slice`, as it's just a convenience method that can be easily re-implemented by using the cursor. --- zed/src/editor/buffer/mod.rs | 3 +- zed/src/editor/buffer/rope.rs | 70 ++++++++++++++++++++++++++--------- 2 files changed, 54 insertions(+), 19 deletions(-) diff --git a/zed/src/editor/buffer/mod.rs b/zed/src/editor/buffer/mod.rs index 87ebf272cb8e1667b4fdaef34ef54c17550ffd01..6bbebbbf268bc2b771db357408ad22d4b477ecc5 100644 --- a/zed/src/editor/buffer/mod.rs +++ b/zed/src/editor/buffer/mod.rs @@ -600,8 +600,7 @@ impl Buffer { } pub fn text_summary_for_range(&self, range: Range) -> TextSummary { - // TODO: Use a dedicated ::summarize method in Rope. - self.visible_text.slice(range).summary() + self.visible_text.cursor(range.start).summary(range.end) } pub fn len(&self) -> usize { diff --git a/zed/src/editor/buffer/rope.rs b/zed/src/editor/buffer/rope.rs index cadf994c5d5bee5fc39b30507853abdc080de937..994022deeeafb69000f2d5ab4556a953c4b6a3ea 100644 --- a/zed/src/editor/buffer/rope.rs +++ b/zed/src/editor/buffer/rope.rs @@ -4,7 +4,7 @@ use crate::util::byte_range_for_char_range; use anyhow::{anyhow, Result}; use arrayvec::ArrayString; use smallvec::SmallVec; -use std::{cmp, iter::Skip, ops::Range, str}; +use std::{cmp, iter::Skip, str}; #[cfg(test)] const CHUNK_BASE: usize = 6; @@ -89,10 +89,6 @@ impl Rope { } } - pub fn slice(&self, range: Range) -> Rope { - self.cursor(range.start).slice(range.end) - } - pub fn summary(&self) -> TextSummary { self.chunks.summary() } @@ -207,6 +203,30 @@ impl<'a> Cursor<'a> { slice } + pub fn summary(&mut self, end_offset: usize) -> TextSummary { + debug_assert!(end_offset >= self.offset); + + let mut summary = TextSummary::default(); + if let Some(start_chunk) = self.chunks.item() { + let start_ix = self.offset - self.chunks.start(); + let end_ix = cmp::min(end_offset, self.chunks.end()) - self.chunks.start(); + let byte_range = byte_range_for_char_range(start_chunk.0, start_ix..end_ix); + summary = TextSummary::from(&start_chunk.0[byte_range]); + } + + if end_offset > self.chunks.end() { + self.chunks.next(); + summary += &self.chunks.summary(&end_offset, SeekBias::Right, &()); + if let Some(end_chunk) = self.chunks.item() { + let end_ix = end_offset - self.chunks.start(); + let byte_range = byte_range_for_char_range(end_chunk.0, 0..end_ix); + summary += TextSummary::from(&end_chunk.0[byte_range]); + } + } + + summary + } + pub fn suffix(mut self) -> Rope { self.slice(self.rope.chunks.extent()) } @@ -263,12 +283,27 @@ impl sum_tree::Item for Chunk { type Summary = TextSummary; fn summary(&self) -> Self::Summary { + TextSummary::from(self.0.as_str()) + } +} + +#[derive(Clone, Debug, Default, Eq, PartialEq)] +pub struct TextSummary { + pub chars: usize, + pub bytes: usize, + pub lines: Point, + pub first_line_len: u32, + pub rightmost_point: Point, +} + +impl<'a> From<&'a str> for TextSummary { + fn from(text: &'a str) -> Self { let mut chars = 0; let mut bytes = 0; let mut lines = Point::new(0, 0); let mut first_line_len = 0; let mut rightmost_point = Point::new(0, 0); - for c in self.0.chars() { + for c in text.chars() { chars += 1; bytes += c.len_utf8(); if c == '\n' { @@ -295,15 +330,6 @@ impl sum_tree::Item for Chunk { } } -#[derive(Clone, Debug, Default, Eq, PartialEq)] -pub struct TextSummary { - pub chars: usize, - pub bytes: usize, - pub lines: Point, - pub first_line_len: u32, - pub rightmost_point: Point, -} - impl sum_tree::Summary for TextSummary { type Context = (); @@ -364,7 +390,7 @@ pub struct Chars<'a> { impl<'a> Chars<'a> { pub fn new(rope: &'a Rope, start: usize) -> Self { let mut cursor = rope.chunks.cursor::(); - cursor.slice(&start, SeekBias::Left, &()); + cursor.seek(&start, SeekBias::Left, &()); let chars = if let Some(chunk) = cursor.item() { let ix = start - cursor.start(); cursor.next(); @@ -472,7 +498,7 @@ mod tests { log::info!("text: {:?}", expected); for _ in 0..5 { - let ix = rng.gen_range(0..=expected.len()); + let ix = rng.gen_range(0..=expected.chars().count()); assert_eq!( actual.chars_at(ix).collect::(), expected.chars().skip(ix).collect::() @@ -492,6 +518,16 @@ mod tests { } offset += 1; } + + for _ in 0..5 { + let end_ix = rng.gen_range(0..=expected.chars().count()); + let start_ix = rng.gen_range(0..=end_ix); + let byte_range = byte_range_for_char_range(&expected, start_ix..end_ix); + assert_eq!( + actual.cursor(start_ix).summary(end_ix), + TextSummary::from(&expected[byte_range.start..byte_range.end]) + ); + } } } } From c9987f9488fde97c5f919cd1d1d9453793411e23 Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Sat, 15 May 2021 11:32:34 +0200 Subject: [PATCH 09/12] Optimize `Rope::append` by merging chunks only when they're underflowing --- zed/src/editor/buffer/rope.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/zed/src/editor/buffer/rope.rs b/zed/src/editor/buffer/rope.rs index 994022deeeafb69000f2d5ab4556a953c4b6a3ea..78ed1563087097b624914f5d4aae504dee88088b 100644 --- a/zed/src/editor/buffer/rope.rs +++ b/zed/src/editor/buffer/rope.rs @@ -26,8 +26,12 @@ impl Rope { let mut chunks = rope.chunks.cursor::<(), ()>(); chunks.next(); if let Some(chunk) = chunks.item() { - self.push(&chunk.0); - chunks.next(); + if self.chunks.last().map_or(false, |c| c.0.len() < CHUNK_BASE) + || chunk.0.len() < CHUNK_BASE + { + self.push(&chunk.0); + chunks.next(); + } } self.chunks.push_tree(chunks.suffix(&()), &()); From 5f93d7f755dbf6a86b1c735ce3343f15d7fd2939 Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Sat, 15 May 2021 11:43:29 +0200 Subject: [PATCH 10/12] Return error in `Rope::to_offset(point)` when the point doesn't exist --- zed/src/editor/buffer/rope.rs | 23 +++++++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/zed/src/editor/buffer/rope.rs b/zed/src/editor/buffer/rope.rs index 78ed1563087097b624914f5d4aae504dee88088b..0dec8fbc1464089b9812587d45b5fef0d6d1957e 100644 --- a/zed/src/editor/buffer/rope.rs +++ b/zed/src/editor/buffer/rope.rs @@ -136,12 +136,14 @@ impl Rope { } pub fn to_offset(&self, point: Point) -> Result { - // TODO: Verify the point actually exists. if point <= self.summary().lines { let mut cursor = self.chunks.cursor::(); cursor.seek(&point, SeekBias::Left, &()); let overshoot = point - cursor.start().lines; - Ok(cursor.start().chars + cursor.item().map_or(0, |chunk| chunk.to_offset(overshoot))) + Ok(cursor.start().chars + + cursor + .item() + .map_or(Ok(0), |chunk| chunk.to_offset(overshoot))?) } else { Err(anyhow!("offset out of bounds")) } @@ -263,7 +265,7 @@ impl Chunk { point } - fn to_offset(&self, target: Point) -> usize { + fn to_offset(&self, target: Point) -> Result { let mut offset = 0; let mut point = Point::new(0, 0); for ch in self.0.chars() { @@ -279,7 +281,12 @@ impl Chunk { } offset += 1; } - offset + + if point == target { + Ok(offset) + } else { + Err(anyhow!("point out of bounds")) + } } } @@ -515,6 +522,10 @@ mod tests { assert_eq!(actual.to_point(offset).unwrap(), point); assert_eq!(actual.to_offset(point).unwrap(), offset); if ch == '\n' { + assert!(actual + .to_offset(Point::new(point.row, point.column + 1)) + .is_err()); + point.row += 1; point.column = 0 } else { @@ -522,6 +533,10 @@ mod tests { } offset += 1; } + assert_eq!(actual.to_point(offset).unwrap(), point); + assert!(actual.to_point(offset + 1).is_err()); + assert_eq!(actual.to_offset(point).unwrap(), offset); + assert!(actual.to_offset(Point::new(point.row + 1, 0)).is_err()); for _ in 0..5 { let end_ix = rng.gen_range(0..=expected.chars().count()); From 81e162318f120f40ff865763d8f0ed98de075f52 Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Sat, 15 May 2021 11:52:49 +0200 Subject: [PATCH 11/12] :lipstick: --- zed/src/editor/buffer/rope.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/zed/src/editor/buffer/rope.rs b/zed/src/editor/buffer/rope.rs index 0dec8fbc1464089b9812587d45b5fef0d6d1957e..9924120cc0c1ea2fe72914bbbdbee1a45c36d6d7 100644 --- a/zed/src/editor/buffer/rope.rs +++ b/zed/src/editor/buffer/rope.rs @@ -544,7 +544,7 @@ mod tests { let byte_range = byte_range_for_char_range(&expected, start_ix..end_ix); assert_eq!( actual.cursor(start_ix).summary(end_ix), - TextSummary::from(&expected[byte_range.start..byte_range.end]) + TextSummary::from(&expected[byte_range]) ); } } From d234f63447a2b252c3415d000edc76419f4d96e3 Mon Sep 17 00:00:00 2001 From: Nathan Sobo Date: Mon, 17 May 2021 11:15:32 -0600 Subject: [PATCH 12/12] Update roadmap --- README.md | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/README.md b/README.md index e1a64b0dd810362ee160e0d33f9342fec832720a..dc576d604201dc6c1a2176d31d8801dba45bec68 100644 --- a/README.md +++ b/README.md @@ -22,21 +22,18 @@ Ship a minimal text editor to investors and other insiders. It should be extreme Establish basic infrastructure for building the app bundle and uploading an artifact. Once this is released, we should regularly distribute updates as features land. -### Minimal code editor for internal use +### Collaborative code editor for internal use [Tracking issue](https://github.com/zed-industries/zed/issues/6) -Turn the minimal text editor into a minimal *code* editor. We define "minimal" as the features that the Zed team needs to use Zed to build Zed without net loss in developer productivity. This includes productivity-critical features such as: +Turn the minimal text editor into a collaborative *code* editor. This will include the minimal features that the Zed team needs to collaborate in Zed to build Zed without net loss in developer productivity. This includes productivity-critical features such as: * Syntax highlighting and syntax-aware editing and navigation +* The ability to see and edit non-local working copies of a repository * Language server support for Rust code navigation, refactoring, diagnostics, etc. * Project browsing and project-wide search and replace -We don't need to implement everything, just anything stopping us from being productive. For example, maybe we don't implement soft wrap and continue to edit prose in another editor at first. - -### Minimal collaborative code editor for internal use - -Once we're using Zed every day, our next goal is to *collaborate* in Zed every day. What features do we need to stop pairing over Discord screen sharing, then stop using Discord screen sharing entirely, then spend increasingly less time talking about code in Discord, etc? How much team collaboration can take place inside of Zed with code as its focus? +We want to tackle collaboration fairly early so that the rest of the design of the product can flow around that assumption. We could probably produce a single-player code editor more quickly, but at the risk of having collaboration feel more "bolted on" when we eventually add it. ### Private alpha for Rust teams on macOS