From 213aa36e1c03e50ad083a1f58605b544d9d90782 Mon Sep 17 00:00:00 2001 From: Nathan Sobo Date: Wed, 17 Nov 2021 19:45:56 -0700 Subject: [PATCH] WIP: Track down bugs with longest_row on wrap map Co-Authored-By: Max Brunsfeld --- crates/buffer/src/rope.rs | 36 +++++- crates/editor/src/display_map/block_map.rs | 9 +- crates/editor/src/display_map/tab_map.rs | 54 +++++--- crates/editor/src/display_map/wrap_map.rs | 141 ++++++++++++++++++++- crates/editor/src/test.rs | 1 + 5 files changed, 216 insertions(+), 25 deletions(-) diff --git a/crates/buffer/src/rope.rs b/crates/buffer/src/rope.rs index bb61b29b74..71a906fe5d 100644 --- a/crates/buffer/src/rope.rs +++ b/crates/buffer/src/rope.rs @@ -767,7 +767,7 @@ mod tests { use super::*; use crate::random_char_iter::RandomCharIter; use rand::prelude::*; - use std::env; + use std::{cmp::Ordering, env}; use Bias::{Left, Right}; #[test] @@ -814,7 +814,7 @@ mod tests { } #[gpui::test(iterations = 100)] - fn test_random(mut rng: StdRng) { + fn test_random_rope(mut rng: StdRng) { let operations = env::var("OPERATIONS") .map(|i| i.parse().expect("invalid `OPERATIONS` variable")) .unwrap_or(10); @@ -898,6 +898,38 @@ mod tests { TextSummary::from(&expected[start_ix..end_ix]) ); } + + let mut expected_longest_rows = Vec::new(); + let mut longest_line_len = -1_isize; + for (row, line) in expected.split('\n').enumerate() { + let row = row as u32; + assert_eq!( + actual.line_len(row), + line.len() as u32, + "invalid line len for row {}", + row + ); + + let line_char_count = line.chars().count() as isize; + match line_char_count.cmp(&longest_line_len) { + Ordering::Less => {} + Ordering::Equal => expected_longest_rows.push(row), + Ordering::Greater => { + longest_line_len = line_char_count; + expected_longest_rows.clear(); + expected_longest_rows.push(row); + } + } + } + + let longest_row = actual.summary().longest_row; + assert!( + expected_longest_rows.contains(&longest_row), + "incorrect longest row {}. expected {:?} with length {}", + longest_row, + expected_longest_rows, + longest_line_len, + ); } } diff --git a/crates/editor/src/display_map/block_map.rs b/crates/editor/src/display_map/block_map.rs index 2d02bc40f0..ef7e262136 100644 --- a/crates/editor/src/display_map/block_map.rs +++ b/crates/editor/src/display_map/block_map.rs @@ -1330,23 +1330,26 @@ mod tests { row ); - match (line.len() as isize).cmp(&longest_line_len) { + let line_char_count = line.chars().count() as isize; + match line_char_count.cmp(&longest_line_len) { Ordering::Less => {} Ordering::Equal => expected_longest_rows.push(row), Ordering::Greater => { - longest_line_len = line.len() as isize; + longest_line_len = line_char_count; expected_longest_rows.clear(); expected_longest_rows.push(row); } } } + log::info!("getting longest row >>>>>>>>>>>>>>>>>>>>>>>>"); let longest_row = blocks_snapshot.longest_row(); assert!( expected_longest_rows.contains(&longest_row), - "incorrect longest row {}. expected {:?}", + "incorrect longest row {}. expected {:?} with length {}", longest_row, expected_longest_rows, + longest_line_len, ); for row in 0..=blocks_snapshot.wrap_snapshot.max_point().row() { diff --git a/crates/editor/src/display_map/tab_map.rs b/crates/editor/src/display_map/tab_map.rs index e0a7f2f846..a5b614e506 100644 --- a/crates/editor/src/display_map/tab_map.rs +++ b/crates/editor/src/display_map/tab_map.rs @@ -110,34 +110,31 @@ impl Snapshot { .text_summary_for_range(input_start..input_end); let mut first_line_chars = 0; - let mut first_line_bytes = 0; + let line_end = if range.start.row() == range.end.row() { + range.end + } else { + self.max_point() + }; for c in self - .chunks(range.start..self.max_point(), false) + .chunks(range.start..line_end, false) .flat_map(|chunk| chunk.text.chars()) { - if c == '\n' - || (range.start.row() == range.end.row() && first_line_bytes == range.end.column()) - { + if c == '\n' { break; } first_line_chars += 1; - first_line_bytes += c.len_utf8() as u32; } let mut last_line_chars = 0; - let mut last_line_bytes = 0; - for c in self - .chunks( - TabPoint::new(range.end.row(), 0).max(range.start)..self.max_point(), - false, - ) - .flat_map(|chunk| chunk.text.chars()) - { - if last_line_bytes == range.end.column() { - break; + if range.start.row() == range.end.row() { + last_line_chars = first_line_chars; + } else { + for _ in self + .chunks(TabPoint::new(range.end.row(), 0)..self.max_point(), false) + .flat_map(|chunk| chunk.text.chars()) + { + last_line_chars += 1; } - last_line_chars += 1; - last_line_bytes += c.len_utf8() as u32; } TextSummary { @@ -427,6 +424,12 @@ impl<'a> Iterator for Chunks<'a> { #[cfg(test)] mod tests { + use buffer::RandomCharIter; + use language::Buffer; + use rand::{prelude::StdRng, Rng}; + + use crate::display_map::fold_map::FoldMap; + use super::*; #[test] @@ -435,4 +438,19 @@ mod tests { assert_eq!(Snapshot::expand_tabs("\t".chars(), 1, 4), 4); assert_eq!(Snapshot::expand_tabs("\ta".chars(), 2, 4), 5); } + + #[gpui::test(iterations = 100)] + fn test_text_summary_for_range(cx: &mut gpui::MutableAppContext, mut rng: StdRng) { + let tab_size = rng.gen_range(1..=4); + let buffer = cx.add_model(|cx| { + let len = rng.gen_range(0..30); + let text = RandomCharIter::new(&mut rng).take(len).collect::(); + Buffer::new(0, text, cx) + }); + let (_, folds_snapshot) = FoldMap::new(buffer.clone(), cx); + let (_, tabs_snapshot) = TabMap::new(folds_snapshot.clone(), tab_size); + + println!("{:?}", tabs_snapshot.text()); + // TODO: Test text_summary_for_range with random ranges + } } diff --git a/crates/editor/src/display_map/wrap_map.rs b/crates/editor/src/display_map/wrap_map.rs index 25623b47c9..b2ad957882 100644 --- a/crates/editor/src/display_map/wrap_map.rs +++ b/crates/editor/src/display_map/wrap_map.rs @@ -299,11 +299,20 @@ impl Snapshot { } fn interpolate(&mut self, new_tab_snapshot: TabSnapshot, tab_edits: &[TabEdit]) -> Patch { + log::info!("INTERPOLATING"); + + log::info!("updating transforms... old transforms are:"); + for transform in self.transforms.items(&()) { + log::info!(" - i {:?}", transform.summary.input); + log::info!(" o {:?}", transform.summary.output); + } + let mut new_transforms; if tab_edits.is_empty() { new_transforms = self.transforms.clone(); } else { let mut old_cursor = self.transforms.cursor::(); + let mut tab_edits_iter = tab_edits.iter().peekable(); new_transforms = old_cursor.slice( &tab_edits_iter.peek().unwrap().old_lines.start, @@ -311,12 +320,26 @@ impl Snapshot { &(), ); + log::info!("sliced, new_transforms are:"); + for transform in new_transforms.items(&()) { + log::info!(" - i {:?}", transform.summary.input); + log::info!(" o {:?}", transform.summary.output); + } + while let Some(edit) = tab_edits_iter.next() { + log::info!("processing edit {:?}", edit); + if edit.new_lines.start > TabPoint::from(new_transforms.summary().input.lines) { let summary = new_tab_snapshot.text_summary_for_range( TabPoint::from(new_transforms.summary().input.lines)..edit.new_lines.start, ); + log::info!("pushing prefix before edit: {:?}", summary); new_transforms.push_or_extend(Transform::isomorphic(summary)); + log::info!("new transforms are now:"); + for transform in new_transforms.items(&()) { + log::info!(" - i {:?}", transform.summary.input); + log::info!(" o {:?}", transform.summary.output); + } } if !edit.new_lines.is_empty() { @@ -325,6 +348,12 @@ impl Snapshot { )); } + log::info!("pushed summary within edit new range; new transforms now:"); + for transform in new_transforms.items(&()) { + log::info!(" - i {:?}", transform.summary.input); + log::info!(" o {:?}", transform.summary.output); + } + old_cursor.seek_forward(&edit.old_lines.end, Bias::Right, &()); if let Some(next_edit) = tab_edits_iter.peek() { if next_edit.old_lines.start > old_cursor.end(&()) { @@ -334,21 +363,46 @@ impl Snapshot { .text_summary_for_range(edit.old_lines.end..old_cursor.end(&())); new_transforms.push_or_extend(Transform::isomorphic(summary)); } + + log::info!("pushed transform suffix after edit; new transforms now:"); + for transform in new_transforms.items(&()) { + log::info!(" - i {:?}", transform.summary.input); + log::info!(" o {:?}", transform.summary.output); + } + old_cursor.next(&()); new_transforms.push_tree( old_cursor.slice(&next_edit.old_lines.start, Bias::Right, &()), &(), ); + + log::info!("pushed tree suffix after edit; new transforms now:"); + for transform in new_transforms.items(&()) { + log::info!(" - i {:?}", transform.summary.input); + log::info!(" o {:?}", transform.summary.output); + } } } else { + log::info!("no more edits"); if old_cursor.end(&()) > edit.old_lines.end { let summary = self .tab_snapshot .text_summary_for_range(edit.old_lines.end..old_cursor.end(&())); new_transforms.push_or_extend(Transform::isomorphic(summary)); + + log::info!("pushed transform suffix after edit; new transforms now:"); + for transform in new_transforms.items(&()) { + log::info!(" - i {:?}", transform.summary.input); + log::info!(" o {:?}", transform.summary.output); + } } old_cursor.next(&()); new_transforms.push_tree(old_cursor.suffix(&()), &()); + log::info!("pushed suffix:"); + for transform in new_transforms.items(&()) { + log::info!(" - i {:?}", transform.summary.input); + log::info!(" o {:?}", transform.summary.output); + } } } } @@ -378,6 +432,12 @@ impl Snapshot { new_rows: Range, } + log::info!("updating transforms... old transforms are:"); + for transform in self.transforms.items(&()) { + log::info!(" - i {:?}", transform.summary.input); + log::info!(" o {:?}", transform.summary.output); + } + let mut tab_edits_iter = tab_edits.into_iter().peekable(); let mut row_edits = Vec::new(); while let Some(edit) = tab_edits_iter.next() { @@ -399,6 +459,11 @@ impl Snapshot { row_edits.push(row_edit); } + log::info!("row edits are:"); + for edit in &row_edits { + log::info!(" {:?}", edit); + } + let mut new_transforms; if row_edits.is_empty() { new_transforms = self.transforms.clone(); @@ -412,6 +477,12 @@ impl Snapshot { &(), ); + log::info!("sliced a prefix:"); + for transform in new_transforms.items(&()) { + log::info!(" - i {:?}", transform.summary.input); + log::info!(" o {:?}", transform.summary.output); + } + while let Some(edit) = row_edits.next() { if edit.new_rows.start > new_transforms.summary().input.lines.row { let summary = new_tab_snapshot.text_summary_for_range( @@ -465,9 +536,15 @@ impl Snapshot { } let mut edit_transforms = edit_transforms.into_iter(); + log::info!("extending tree with edit transforms"); if let Some(transform) = edit_transforms.next() { + log::info!( + "push or extend with first transform: {:?}", + transform.summary.output + ); new_transforms.push_or_extend(transform); } + log::info!("extending with remaining transforms"); new_transforms.extend(edit_transforms, &()); old_cursor.seek_forward(&TabPoint::new(edit.old_rows.end, 0), Bias::Right, &()); @@ -859,10 +936,17 @@ impl sum_tree::Item for Transform { } fn push_isomorphic(transforms: &mut Vec, summary: TextSummary) { + log::info!("push_isomorphic: {:?}", summary); if let Some(last_transform) = transforms.last_mut() { if last_transform.is_isomorphic() { last_transform.summary.input += &summary; last_transform.summary.output += &summary; + + log::info!( + " extended previous isomorphic: {:?}", + last_transform.summary.output + ); + return; } } @@ -879,15 +963,28 @@ impl SumTreeExt for SumTree { self.update_last( |last_transform| { if last_transform.is_isomorphic() && transform.as_ref().unwrap().is_isomorphic() { + // log::info!("extending last transform in tree"); + // log::info!( + // " extending with: {:?}", + // transform.as_ref().map(|t| t.summary.output.clone()), + // ); + // log::info!(" last transform was: {:?}", last_transform.summary.output); + let transform = transform.take().unwrap(); last_transform.summary.input += &transform.summary.input; last_transform.summary.output += &transform.summary.output; + + // log::info!( + // " last transform is now {:?}", + // last_transform.summary.output, + // ) } }, &(), ); if let Some(transform) = transform { + log::info!("!!!!!!!!!!! push transform: {:?}", transform.summary.output,); self.push(transform, &()); } } @@ -1011,7 +1108,7 @@ mod tests { let unwrapped_text = tabs_snapshot.text(); let expected_text = wrap_text(&unwrapped_text, wrap_width, &mut line_wrapper); - let (wrap_map, initial_snapshot) = + let (wrap_map, _) = cx.update(|cx| WrapMap::new(tabs_snapshot.clone(), font_id, font_size, wrap_width, cx)); let (_observer, notifications) = Observer::new(&wrap_map, &mut cx); @@ -1019,6 +1116,11 @@ mod tests { notifications.recv().await.unwrap(); } + let (initial_snapshot, _) = wrap_map.update(&mut cx, |map, cx| { + assert!(!map.is_rewrapping()); + map.sync(tabs_snapshot.clone(), Vec::new(), cx) + }); + let actual_text = initial_snapshot.text(); assert_eq!( actual_text, expected_text, @@ -1029,6 +1131,8 @@ mod tests { let mut edits = Vec::new(); for _i in 0..operations { + log::info!("{} ==============================================", _i); + match rng.gen_range(0..=100) { 0..=19 => { wrap_width = if rng.gen_bool(0.2) { @@ -1088,15 +1192,48 @@ mod tests { let (mut wrapped_snapshot, wrap_edits) = wrap_map.update(&mut cx, |map, cx| map.sync(tabs_snapshot, Vec::new(), cx)); let actual_text = wrapped_snapshot.text(); + let actual_longest_row = wrapped_snapshot.longest_row(); log::info!("Wrapping finished: {:?}", actual_text); wrapped_snapshot.check_invariants(); wrapped_snapshot.verify_chunks(&mut rng); - edits.push((wrapped_snapshot, wrap_edits)); + edits.push((wrapped_snapshot.clone(), wrap_edits)); assert_eq!( actual_text, expected_text, "unwrapped text is: {:?}", unwrapped_text ); + + let mut summary = TextSummary::default(); + for (ix, item) in wrapped_snapshot + .transforms + .items(&()) + .into_iter() + .enumerate() + { + summary += &item.summary.output; + log::info!("{} summary: {:?}", ix, item.summary.output,); + } + + let mut expected_longest_rows = Vec::new(); + let mut longest_line_len = -1; + for (row, line) in expected_text.split('\n').enumerate() { + let line_char_count = line.chars().count() as isize; + if line_char_count > longest_line_len { + expected_longest_rows.clear(); + longest_line_len = line_char_count; + } + if line_char_count >= longest_line_len { + expected_longest_rows.push(row as u32); + } + } + + assert!( + expected_longest_rows.contains(&actual_longest_row), + "incorrect longest row {}. expected {:?} with length {}", + actual_longest_row, + expected_longest_rows, + longest_line_len, + ) } } diff --git a/crates/editor/src/test.rs b/crates/editor/src/test.rs index 33fafc3f54..26f29364fd 100644 --- a/crates/editor/src/test.rs +++ b/crates/editor/src/test.rs @@ -5,6 +5,7 @@ use std::marker::PhantomData; #[cfg(test)] #[ctor::ctor] fn init_logger() { + // std::env::set_var("RUST_LOG", "info"); env_logger::init(); }