Detailed changes
@@ -322,7 +322,12 @@ impl BufferDiffSnapshot {
(new_id == old_id && new_version == old_version) || (new_empty && old_empty)
}
- pub fn row_to_base_text_row(&self, row: BufferRow, buffer: &text::BufferSnapshot) -> u32 {
+ pub fn row_to_base_text_row(
+ &self,
+ row: BufferRow,
+ bias: Bias,
+ buffer: &text::BufferSnapshot,
+ ) -> u32 {
// TODO(split-diff) expose a parameter to reuse a cursor to avoid repeatedly seeking from the start
// Find the last hunk that starts before this position.
@@ -339,15 +344,24 @@ impl BufferDiffSnapshot {
let unclipped_point = if let Some(hunk) = cursor.item()
&& hunk.buffer_range.start.cmp(&position, buffer).is_le()
{
- let mut unclipped_point = cursor
- .end()
- .diff_base_byte_range
- .end
- .to_point(self.base_text());
- if position.cmp(&cursor.end().buffer_range.end, buffer).is_ge() {
+ let unclipped_point = if position.cmp(&cursor.end().buffer_range.end, buffer).is_ge() {
+ let mut unclipped_point = cursor
+ .end()
+ .diff_base_byte_range
+ .end
+ .to_point(self.base_text());
unclipped_point +=
Point::new(row, 0) - cursor.end().buffer_range.end.to_point(buffer);
- }
+ unclipped_point
+ } else if bias == Bias::Right {
+ cursor
+ .end()
+ .diff_base_byte_range
+ .end
+ .to_point(self.base_text())
+ } else {
+ hunk.diff_base_byte_range.start.to_point(self.base_text())
+ };
// Move the cursor so that at the next step we can clip with the start of the next hunk.
cursor.next();
unclipped_point
@@ -868,7 +882,7 @@ fn compare_hunks(
start.get_or_insert(new_hunk.buffer_range.start);
base_text_start.get_or_insert(new_hunk.diff_base_byte_range.start);
end.replace(new_hunk.buffer_range.end);
- base_text_end.get_or_insert(new_hunk.diff_base_byte_range.end);
+ base_text_end.replace(new_hunk.diff_base_byte_range.end);
new_cursor.next();
}
Ordering::Equal => {
@@ -882,11 +896,16 @@ fn compare_hunks(
.is_ge()
{
end.replace(old_hunk.buffer_range.end);
- base_text_end.replace(old_hunk.diff_base_byte_range.end);
} else {
end.replace(new_hunk.buffer_range.end);
- base_text_end.replace(new_hunk.diff_base_byte_range.end);
}
+
+ base_text_end.replace(
+ old_hunk
+ .diff_base_byte_range
+ .end
+ .max(new_hunk.diff_base_byte_range.end),
+ );
}
new_cursor.next();
@@ -904,15 +923,17 @@ fn compare_hunks(
(Some(new_hunk), None) => {
start.get_or_insert(new_hunk.buffer_range.start);
base_text_start.get_or_insert(new_hunk.diff_base_byte_range.start);
+ // TODO(cole) it seems like this could move end backward?
end.replace(new_hunk.buffer_range.end);
- base_text_end.replace(new_hunk.diff_base_byte_range.end);
+ base_text_end = base_text_end.max(Some(new_hunk.diff_base_byte_range.end));
new_cursor.next();
}
(None, Some(old_hunk)) => {
start.get_or_insert(old_hunk.buffer_range.start);
base_text_start.get_or_insert(old_hunk.diff_base_byte_range.start);
+ // TODO(cole) it seems like this could move end backward?
end.replace(old_hunk.buffer_range.end);
- base_text_end.replace(old_hunk.diff_base_byte_range.end);
+ base_text_end = base_text_end.max(Some(old_hunk.diff_base_byte_range.end));
old_cursor.next();
}
(None, None) => break,
@@ -1550,7 +1571,7 @@ pub fn assert_hunks<ExpectedText, HunkIter>(
#[cfg(test)]
mod tests {
- use std::fmt::Write as _;
+ use std::{fmt::Write as _, sync::mpsc};
use super::*;
use gpui::TestAppContext;
@@ -2466,21 +2487,106 @@ mod tests {
let buffer_snapshot = buffer.snapshot();
let diff = BufferDiffSnapshot::new_sync(buffer_snapshot.clone(), base_text, cx);
let expected_results = [
- // don't format me
- (0, 0),
- (1, 2),
- (2, 2),
- (3, 5),
- (4, 5),
- (5, 7),
- (6, 9),
+ // main buffer row, base text row (right bias), base text row (left bias)
+ (0, 0, 0),
+ (1, 2, 1),
+ (2, 2, 2),
+ (3, 5, 3),
+ (4, 5, 5),
+ (5, 7, 7),
+ (6, 9, 9),
];
- for (buffer_row, expected) in expected_results {
+ for (buffer_row, expected_right, expected_left) in expected_results {
+ assert_eq!(
+ diff.row_to_base_text_row(buffer_row, Bias::Right, &buffer_snapshot),
+ expected_right,
+ "{buffer_row}"
+ );
assert_eq!(
- diff.row_to_base_text_row(buffer_row, &buffer_snapshot),
- expected,
+ diff.row_to_base_text_row(buffer_row, Bias::Left, &buffer_snapshot),
+ expected_left,
"{buffer_row}"
);
}
}
+
+ #[gpui::test]
+ async fn test_changed_ranges(cx: &mut gpui::TestAppContext) {
+ let base_text = "
+ one
+ two
+ three
+ four
+ five
+ six
+ "
+ .unindent();
+ let buffer_text = "
+ one
+ TWO
+ three
+ four
+ FIVE
+ six
+ "
+ .unindent();
+ let buffer = cx.new(|cx| language::Buffer::local(buffer_text, cx));
+ let diff = cx.new(|cx| {
+ BufferDiff::new_with_base_text(&base_text, &buffer.read(cx).text_snapshot(), cx)
+ });
+ let (tx, rx) = mpsc::channel();
+ let subscription =
+ cx.update(|cx| cx.subscribe(&diff, move |_, event, _| tx.send(event.clone()).unwrap()));
+
+ let snapshot = buffer.update(cx, |buffer, cx| {
+ buffer.set_text(
+ "
+ ONE
+ TWO
+ THREE
+ FOUR
+ FIVE
+ SIX
+ "
+ .unindent(),
+ cx,
+ );
+ buffer.text_snapshot()
+ });
+ let update = diff
+ .update(cx, |diff, cx| {
+ diff.update_diff(
+ snapshot.clone(),
+ Some(base_text.as_str().into()),
+ false,
+ None,
+ cx,
+ )
+ })
+ .await;
+ diff.update(cx, |diff, cx| {
+ diff.set_snapshot(update, &snapshot, false, cx)
+ });
+ cx.run_until_parked();
+ drop(subscription);
+ let events = rx.into_iter().collect::<Vec<_>>();
+ match events.as_slice() {
+ [
+ BufferDiffEvent::DiffChanged {
+ changed_range: _,
+ base_text_changed_range,
+ },
+ ] => {
+ // TODO(cole) this seems like it should pass but currently fails (see compare_hunks)
+ // assert_eq!(
+ // *changed_range,
+ // Some(Anchor::min_max_range_for_buffer(
+ // buffer.read_with(cx, |buffer, _| buffer.remote_id())
+ // ))
+ // );
+ assert_eq!(*base_text_changed_range, Some(0..base_text.len()));
+ }
+ _ => panic!("unexpected events: {:?}", events),
+ }
+ }
}
@@ -10,7 +10,7 @@ use language::{Buffer, Capability};
use multi_buffer::{Anchor, ExcerptId, ExcerptRange, ExpandExcerptDirection, MultiBuffer, PathKey};
use project::Project;
use rope::Point;
-use text::OffsetRangeExt as _;
+use text::{Bias, OffsetRangeExt as _};
use ui::{
App, Context, InteractiveElement as _, IntoElement as _, ParentElement as _, Render,
Styled as _, Window, div,
@@ -341,7 +341,7 @@ impl SplittableEditor {
#[cfg(test)]
impl SplittableEditor {
- fn check_invariants(&self, cx: &App) {
+ fn check_invariants(&self, quiesced: bool, cx: &App) {
use buffer_diff::DiffHunkStatusKind;
use collections::HashSet;
use multi_buffer::MultiBufferOffset;
@@ -410,50 +410,35 @@ impl SplittableEditor {
let secondary_excerpts = secondary.multibuffer.read(cx).excerpt_ids();
assert_eq!(primary_excerpts.len(), secondary_excerpts.len(),);
- let primary_diff_hunks = self
- .primary_multibuffer
- .read(cx)
- .snapshot(cx)
- .diff_hunks()
- .collect::<Vec<_>>();
- let secondary_diff_hunks = secondary
- .multibuffer
- .read(cx)
- .snapshot(cx)
- .diff_hunks()
- .collect::<Vec<_>>();
- assert_eq!(
- primary_diff_hunks.len(),
- secondary_diff_hunks.len(),
- "\n\nprimary: {primary_diff_hunks:#?}\nsecondary: {secondary_diff_hunks:#?}",
- );
-
- // self.primary_multibuffer.read(cx).check_invariants(cx);
- // secondary.multibuffer.read(cx).check_invariants(cx);
- // Assertions:...
- //
- // left.display_lines().filter(is_unmodified) == right.display_lines().filter(is_unmodified)
- //
- // left excerpts and right excerpts bijectivity
- //
- //
-
- // let primary_buffer_text = self
- // .primary_multibuffer
- // .read(cx)
- // .text_summary_for_range(Anchor::min()..Anchor::max());
- // let secondary_buffer_text = secondary
- // .multibuffer
- // .read(cx)
- // .text_summary_for_range(Anchor::min()..Anchor::max());
- // let primary_buffer_base_text = self
- // .primary_multibuffer
- // .read(cx)
- // .base_text_summary_for_range(Anchor::min()..Anchor::max());
- // let secondary_buffer_base_text = secondary
- // .multibuffer
- // .read(cx)
- // .base_text_summary_for_range(Anchor::min()..Anchor::max());
+ if quiesced {
+ let primary_snapshot = self.primary_multibuffer.read(cx).snapshot(cx);
+ let secondary_snapshot = secondary.multibuffer.read(cx).snapshot(cx);
+ let primary_diff_hunks = primary_snapshot
+ .diff_hunks()
+ .map(|hunk| hunk.diff_base_byte_range.clone())
+ .collect::<Vec<_>>();
+ let secondary_diff_hunks = secondary_snapshot
+ .diff_hunks()
+ .map(|hunk| hunk.diff_base_byte_range.clone())
+ .collect::<Vec<_>>();
+ pretty_assertions::assert_eq!(primary_diff_hunks, secondary_diff_hunks);
+
+ let primary_unmodified_rows = primary_snapshot
+ .text()
+ .split("\n")
+ .zip(primary_snapshot.row_infos(MultiBufferRow(0)))
+ .filter(|(_, row_info)| row_info.diff_status.is_none())
+ .map(|(line, _)| line.to_owned())
+ .collect::<Vec<_>>();
+ let secondary_unmodified_rows = secondary_snapshot
+ .text()
+ .split("\n")
+ .zip(secondary_snapshot.row_infos(MultiBufferRow(0)))
+ .filter(|(_, row_info)| row_info.diff_status.is_none())
+ .map(|(line, _)| line.to_owned())
+ .collect::<Vec<_>>();
+ pretty_assertions::assert_eq!(primary_unmodified_rows, secondary_unmodified_rows);
+ }
}
fn randomly_edit_excerpts(
@@ -535,62 +520,6 @@ impl SplittableEditor {
}
}
}
-
- fn randomly_mutate(
- &mut self,
- rng: &mut impl rand::Rng,
- mutation_count: usize,
- cx: &mut Context<Self>,
- ) {
- use rand::prelude::*;
-
- if rng.random_bool(0.7) {
- let buffers = self.primary_editor.read(cx).buffer().read(cx).all_buffers();
- let buffer = buffers.iter().choose(rng);
-
- if let Some(buffer) = buffer {
- buffer.update(cx, |buffer, cx| {
- if rng.random() {
- log::info!("randomly editing single buffer");
- buffer.randomly_edit(rng, mutation_count, cx);
- } else {
- log::info!("randomly undoing/redoing in single buffer");
- buffer.randomly_undo_redo(rng, cx);
- }
- });
- } else {
- log::info!("randomly editing multibuffer");
- self.primary_multibuffer.update(cx, |multibuffer, cx| {
- multibuffer.randomly_edit(rng, mutation_count, cx);
- });
- }
- } else if rng.random() {
- self.randomly_edit_excerpts(rng, mutation_count, cx);
- } else {
- log::info!("updating diffs and excerpts");
- for buffer in self.primary_multibuffer.read(cx).all_buffers() {
- let diff = self
- .primary_multibuffer
- .read(cx)
- .diff_for(buffer.read(cx).remote_id())
- .unwrap();
- let buffer_snapshot = buffer.read(cx).text_snapshot();
- diff.update(cx, |diff, cx| {
- diff.recalculate_diff_sync(&buffer_snapshot, cx);
- });
- // TODO(split-diff) might be a good idea to try to separate the diff recalculation from the excerpt recalculation
- let diff_snapshot = diff.read(cx).snapshot(cx);
- let ranges = diff_snapshot
- .hunks(&buffer_snapshot)
- .map(|hunk| hunk.range.clone())
- .collect::<Vec<_>>();
- let path = PathKey::for_buffer(&buffer, cx);
- self.set_excerpts_for_path(path, buffer, ranges, 2, diff, cx);
- }
- }
-
- self.check_invariants(cx);
- }
}
impl EventEmitter<EditorEvent> for SplittableEditor {}
@@ -655,9 +584,13 @@ impl SecondaryEditor {
.into_iter()
.map(|(_, excerpt_range)| {
let point_range_to_base_text_point_range = |range: Range<Point>| {
- let start_row =
- diff_snapshot.row_to_base_text_row(range.start.row, main_buffer);
- let end_row = diff_snapshot.row_to_base_text_row(range.end.row, main_buffer);
+ let start_row = diff_snapshot.row_to_base_text_row(
+ range.start.row,
+ Bias::Left,
+ main_buffer,
+ );
+ let end_row =
+ diff_snapshot.row_to_base_text_row(range.end.row, Bias::Right, main_buffer);
let end_column = diff_snapshot.base_text().line_len(end_row);
Point::new(start_row, 0)..Point::new(end_row, end_column)
};
@@ -692,7 +625,7 @@ mod tests {
use fs::FakeFs;
use gpui::AppContext as _;
use language::Capability;
- use multi_buffer::MultiBuffer;
+ use multi_buffer::{MultiBuffer, PathKey};
use project::Project;
use rand::rngs::StdRng;
use settings::SettingsStore;
@@ -712,6 +645,8 @@ mod tests {
#[gpui::test(iterations = 100)]
async fn test_random_split_editor(mut rng: StdRng, cx: &mut gpui::TestAppContext) {
+ use rand::prelude::*;
+
init_test(cx);
let project = Project::test(FakeFs::new(cx.executor()), [], cx).await;
let (workspace, cx) =
@@ -728,10 +663,84 @@ mod tests {
editor
});
- for _ in 0..10 {
+ let operations = std::env::var("OPERATIONS")
+ .map(|i| i.parse().expect("invalid `OPERATIONS` variable"))
+ .unwrap_or(20);
+ let rng = &mut rng;
+ for _ in 0..operations {
editor.update(cx, |editor, cx| {
- editor.randomly_mutate(&mut rng, 5, cx);
- })
+ let buffers = editor
+ .primary_editor
+ .read(cx)
+ .buffer()
+ .read(cx)
+ .all_buffers();
+
+ if buffers.is_empty() {
+ editor.randomly_edit_excerpts(rng, 2, cx);
+ editor.check_invariants(true, cx);
+ return;
+ }
+
+ let quiesced = match rng.random_range(0..100) {
+ 0..=69 if !buffers.is_empty() => {
+ let buffer = buffers.iter().choose(rng).unwrap();
+ buffer.update(cx, |buffer, cx| {
+ if rng.random() {
+ log::info!("randomly editing single buffer");
+ buffer.randomly_edit(rng, 5, cx);
+ } else {
+ log::info!("randomly undoing/redoing in single buffer");
+ buffer.randomly_undo_redo(rng, cx);
+ }
+ });
+ false
+ }
+ 70..=79 => {
+ log::info!("mutating excerpts");
+ editor.randomly_edit_excerpts(rng, 2, cx);
+ false
+ }
+ 80..=89 if !buffers.is_empty() => {
+ log::info!("recalculating buffer diff");
+ let buffer = buffers.iter().choose(rng).unwrap();
+ let diff = editor
+ .primary_multibuffer
+ .read(cx)
+ .diff_for(buffer.read(cx).remote_id())
+ .unwrap();
+ let buffer_snapshot = buffer.read(cx).text_snapshot();
+ diff.update(cx, |diff, cx| {
+ diff.recalculate_diff_sync(&buffer_snapshot, cx);
+ });
+ false
+ }
+ _ => {
+ log::info!("quiescing");
+ for buffer in buffers {
+ let buffer_snapshot = buffer.read(cx).text_snapshot();
+ let diff = editor
+ .primary_multibuffer
+ .read(cx)
+ .diff_for(buffer.read(cx).remote_id())
+ .unwrap();
+ diff.update(cx, |diff, cx| {
+ diff.recalculate_diff_sync(&buffer_snapshot, cx);
+ });
+ let diff_snapshot = diff.read(cx).snapshot(cx);
+ let ranges = diff_snapshot
+ .hunks(&buffer_snapshot)
+ .map(|hunk| hunk.range.clone())
+ .collect::<Vec<_>>();
+ let path = PathKey::for_buffer(&buffer, cx);
+ editor.set_excerpts_for_path(path, buffer, ranges, 2, diff, cx);
+ }
+ true
+ }
+ };
+
+ editor.check_invariants(quiesced, cx);
+ });
}
}
}
@@ -3143,6 +3143,11 @@ impl MultiBuffer {
inserted_hunk_info: Some(hunk),
..
}) => excerpts.item().is_some_and(|excerpt| {
+ if let Some(diff) = snapshot.diffs.get(&excerpt.buffer_id)
+ && let Some(main_buffer) = &diff.main_buffer
+ {
+ return hunk.hunk_start_anchor.is_valid(main_buffer);
+ }
hunk.hunk_start_anchor.is_valid(&excerpt.buffer)
}),
_ => true,
@@ -3259,6 +3264,7 @@ impl MultiBuffer {
log::trace!("skipping hunk that starts before excerpt");
continue;
}
+ hunk_buffer_range.end.to_point(&excerpt.buffer);
let hunk_excerpt_start = excerpt_start
+ hunk_buffer_range.start.saturating_sub(excerpt_buffer_start);
let hunk_excerpt_end = excerpt_end
@@ -3744,7 +3744,7 @@ async fn test_inverted_diff(cx: &mut TestAppContext) {
.new(|cx| BufferDiff::new_with_base_text(base_text, &buffer.read(cx).text_snapshot(), cx));
cx.run_until_parked();
- let base_text_buffer = diff.read_with(cx, |diff, cx| diff.base_text_buffer());
+ let base_text_buffer = diff.read_with(cx, |diff, _| diff.base_text_buffer());
let multibuffer = cx.new(|cx| {
let mut multibuffer = MultiBuffer::singleton(base_text_buffer.clone(), cx);
@@ -3824,6 +3824,43 @@ async fn test_inverted_diff(cx: &mut TestAppContext) {
},
);
+ buffer.update(cx, |buffer, cx| {
+ buffer.set_text("ZERO\nONE\nTWO\n", cx);
+ });
+ cx.run_until_parked();
+ let update = diff
+ .update(cx, |diff, cx| {
+ diff.update_diff(
+ buffer.read(cx).text_snapshot(),
+ Some(base_text.into()),
+ false,
+ None,
+ cx,
+ )
+ })
+ .await;
+ diff.update(cx, |diff, cx| {
+ diff.set_snapshot(update, &buffer.read(cx).text_snapshot(), false, cx);
+ });
+ cx.run_until_parked();
+
+ assert_new_snapshot(
+ &multibuffer,
+ &mut snapshot,
+ &mut subscription,
+ cx,
+ indoc! {
+ "
+ - one
+ - two
+ - three
+ - four
+ - five
+ - six
+ "
+ },
+ );
+
diff.update(cx, |diff, cx| {
diff.set_base_text(
Some("new base\n".into()),