Fix bugs with applying hunks from branch buffers (zed-industries#18721)

Release Notes:

- N/A

---------

Co-authored-by: Marshall <marshall@zed.dev>

crates/clock/src/clock.rs

@@ -216,10 +216,11 @@ impl fmt::Debug for Global {
  if timestamp.replica_id > 0 {
  write!(f, ", ")?;
  }
- write!(f, "{}: {}", timestamp.replica_id, timestamp.value)?;
- }
- if self.local_branch_value > 0 {
- write!(f, "<branch>: {}", self.local_branch_value)?;
+ if timestamp.replica_id == LOCAL_BRANCH_REPLICA_ID {
+ write!(f, "<branch>: {}", timestamp.value)?;
+ } else {
+ write!(f, "{}: {}", timestamp.replica_id, timestamp.value)?;
+ }
  }
  write!(f, "}}")
  }

crates/language/src/buffer.rs

@@ -18,6 +18,7 @@ use crate::{
 };
 use anyhow::{anyhow, Context, Result};
 use async_watch as watch;
+use clock::Lamport;
 pub use clock::ReplicaId;
 use futures::channel::oneshot;
 use gpui::{
@@ -90,7 +91,7 @@ enum BufferDiffBase {
  PastBufferVersion {
  buffer: Model<Buffer>,
  rope: Rope,
- operations_to_ignore: Vec<clock::Lamport>,
+ merged_operations: Vec<Lamport>,
  },
 }
 
@@ -802,7 +803,7 @@ impl Buffer {
  diff_base: Some(BufferDiffBase::PastBufferVersion {
  buffer: this.clone(),
  rope: self.as_rope().clone(),
- operations_to_ignore: Vec::new(),
+ merged_operations: Default::default(),
  }),
  language: self.language.clone(),
  has_conflict: self.has_conflict,
@@ -834,34 +835,32 @@ impl Buffer {
  return;
  };
 
- base_buffer.update(cx, |base_buffer, cx| {
- let edits = self
- .edits_since::<usize>(&base_buffer.version)
- .filter_map(|edit| {
- if range
- .as_ref()
- .map_or(true, |range| range.overlaps(&edit.new))
- {
- Some((edit.old, self.text_for_range(edit.new).collect::<String>()))
- } else {
- None
- }
- })
- .collect::<Vec<_>>();
+ let mut edits = Vec::new();
+ for edit in self.edits_since::<usize>(&base_buffer.read(cx).version()) {
+ if let Some(range) = &range {
+ if range.start > edit.new.end || edit.new.start > range.end {
+ continue;
+ }
+ }
+ edits.push((
+ edit.old.clone(),
+ self.text_for_range(edit.new.clone()).collect::<String>(),
+ ));
+ }
 
- let operation = base_buffer.edit(edits, None, cx);
+ let operation = base_buffer.update(cx, |base_buffer, cx| {
+ cx.emit(BufferEvent::DiffBaseChanged);
+ base_buffer.edit(edits, None, cx)
+ });
 
-  // Prevent this operation from being reapplied to the branch.
+ if let Some(operation) = operation {
  if let Some(BufferDiffBase::PastBufferVersion {
- operations_to_ignore,
- ..
+ merged_operations, ..
  }) = &mut self.diff_base
  {
- operations_to_ignore.extend(operation);
+ merged_operations.push(operation);
  }
-
- cx.emit(BufferEvent::DiffBaseChanged);
- });
+ }
  }
 
  fn on_base_buffer_event(
@@ -870,31 +869,34 @@ impl Buffer {
  event: &BufferEvent,
  cx: &mut ModelContext<Self>,
  ) {
- if let BufferEvent::Operation { operation, .. } = event {
- if let Some(BufferDiffBase::PastBufferVersion {
- operations_to_ignore,
- ..
- }) = &mut self.diff_base
- {
- let mut is_ignored = false;
- if let Operation::Buffer(text::Operation::Edit(buffer_operation)) = &operation {
- operations_to_ignore.retain(|operation_to_ignore| {
- match buffer_operation.timestamp.cmp(&operation_to_ignore) {
- Ordering::Less => true,
- Ordering::Equal => {
- is_ignored = true;
- false
- }
- Ordering::Greater => false,
- }
- });
- }
- if !is_ignored {
- self.apply_ops([operation.clone()], cx);
- self.diff_base_version += 1;
- }
+ let BufferEvent::Operation { operation, .. } = event else {
+ return;
+ };
+ let Some(BufferDiffBase::PastBufferVersion {
+ merged_operations, ..
+ }) = &mut self.diff_base
+ else {
+ return;
+ };
+
+ let mut operation_to_undo = None;
+ if let Operation::Buffer(text::Operation::Edit(operation)) = &operation {
+ if let Ok(ix) = merged_operations.binary_search(&operation.timestamp) {
+ merged_operations.remove(ix);
+ operation_to_undo = Some(operation.timestamp);
  }
  }
+
+ self.apply_ops([operation.clone()], cx);
+
+ if let Some(timestamp) = operation_to_undo {
+ let operation = self
+ .text
+ .undo_operations([(timestamp, u32::MAX)].into_iter().collect());
+ self.send_operation(Operation::Buffer(operation), true, cx);
+ }
+
+ self.diff_base_version += 1;
  }
 
  #[cfg(test)]

crates/language/src/buffer_tests.rs

@@ -2485,15 +2485,73 @@ fn test_branch_and_merge(cx: &mut TestAppContext) {
 }
 
 #[gpui::test]
-fn test_merge_into_base(cx: &mut AppContext) {
- init_settings(cx, |_| {});
+fn test_merge_into_base(cx: &mut TestAppContext) {
+ cx.update(|cx| init_settings(cx, |_| {}));
+
  let base = cx.new_model(|cx| Buffer::local("abcdefghijk", cx));
  let branch = base.update(cx, |buffer, cx| buffer.branch(cx));
+
+ // Make 3 edits, merge one into the base.
  branch.update(cx, |branch, cx| {
- branch.edit([(0..3, "ABC"), (7..9, "HI")], None, cx);
+ branch.edit([(0..3, "ABC"), (7..9, "HI"), (11..11, "LMN")], None, cx);
  branch.merge_into_base(Some(5..8), cx);
  });
- assert_eq!(base.read(cx).text(), "abcdefgHIjk");
+
+ branch.read_with(cx, |branch, _| assert_eq!(branch.text(), "ABCdefgHIjkLMN"));
+ base.read_with(cx, |base, _| assert_eq!(base.text(), "abcdefgHIjk"));
+
+ // Undo the one already-merged edit. Merge that into the base.
+ branch.update(cx, |branch, cx| {
+ branch.edit([(7..9, "hi")], None, cx);
+ branch.merge_into_base(Some(5..8), cx);
+ });
+ base.read_with(cx, |base, _| assert_eq!(base.text(), "abcdefghijk"));
+
+ // Merge an insertion into the base.
+ branch.update(cx, |branch, cx| {
+ branch.merge_into_base(Some(11..11), cx);
+ });
+
+ branch.read_with(cx, |branch, _| assert_eq!(branch.text(), "ABCdefghijkLMN"));
+ base.read_with(cx, |base, _| assert_eq!(base.text(), "abcdefghijkLMN"));
+
+ // Deleted the inserted text and merge that into the base.
+ branch.update(cx, |branch, cx| {
+ branch.edit([(11..14, "")], None, cx);
+ branch.merge_into_base(Some(10..11), cx);
+ });
+
+ base.read_with(cx, |base, _| assert_eq!(base.text(), "abcdefghijk"));
+}
+
+#[gpui::test]
+fn test_undo_after_merge_into_base(cx: &mut TestAppContext) {
+ cx.update(|cx| init_settings(cx, |_| {}));
+
+ let base = cx.new_model(|cx| Buffer::local("abcdefghijk", cx));
+ let branch = base.update(cx, |buffer, cx| buffer.branch(cx));
+
+ // Make 2 edits, merge one into the base.
+ branch.update(cx, |branch, cx| {
+ branch.edit([(0..3, "ABC"), (7..9, "HI")], None, cx);
+ branch.merge_into_base(Some(7..7), cx);
+ });
+ base.read_with(cx, |base, _| assert_eq!(base.text(), "abcdefgHIjk"));
+ branch.read_with(cx, |branch, _| assert_eq!(branch.text(), "ABCdefgHIjk"));
+
+ // Undo the merge in the base buffer.
+ base.update(cx, |base, cx| {
+ base.undo(cx);
+ });
+ base.read_with(cx, |base, _| assert_eq!(base.text(), "abcdefghijk"));
+ branch.read_with(cx, |branch, _| assert_eq!(branch.text(), "ABCdefgHIjk"));
+
+ // Merge that operation into the base again.
+ branch.update(cx, |branch, cx| {
+ branch.merge_into_base(Some(7..7), cx);
+ });
+ base.read_with(cx, |base, _| assert_eq!(base.text(), "abcdefgHIjk"));
+ branch.read_with(cx, |branch, _| assert_eq!(branch.text(), "ABCdefgHIjk"));
 }
 
 fn start_recalculating_diff(buffer: &Model<Buffer>, cx: &mut TestAppContext) {

crates/text/src/text.rs

@@ -1430,16 +1430,22 @@ impl Buffer {
  counts.insert(edit_id, self.undo_map.undo_count(edit_id) + 1);
  }
 
+ let operation = self.undo_operations(counts);
+ self.history.push(operation.clone());
+ operation
+ }
+
+ pub fn undo_operations(&mut self, counts: HashMap<clock::Lamport, u32>) -> Operation {
+ let timestamp = self.lamport_clock.tick();
+ let version = self.version();
+ self.snapshot.version.observe(timestamp);
  let undo = UndoOperation {
- timestamp: self.lamport_clock.tick(),
- version: self.version(),
+ timestamp,
+ version,
  counts,
  };
  self.apply_undo(&undo);
- self.snapshot.version.observe(undo.timestamp);
- let operation = Operation::Undo(undo);
- self.history.push(operation.clone());
- operation
+ Operation::Undo(undo)
  }
 
  pub fn push_transaction(&mut self, transaction: Transaction, now: Instant) {