Make paste a separate undo transaction from preceding edits (#52003)

Lukas Wirth created 1 week ago

When undoing a paste, it is really confusing when that actually also
removes what was type right before the paste if the paste happened fast
enough after.

Release Notes:

- Fixed undoing a paste sometimes also undoing edits right before the
paste

Change summary

crates/editor/src/editor.rs                   |  2 +
crates/editor/src/editor_tests.rs             | 30 +++++++++++++++++++++
crates/language/src/buffer.rs                 |  8 ++--
crates/multi_buffer/src/multi_buffer.rs       |  9 +++++
crates/multi_buffer/src/multi_buffer_tests.rs |  4 +-
crates/text/src/text.rs                       | 17 ++++++-----
6 files changed, 55 insertions(+), 15 deletions(-)

Detailed changes

crates/editor/src/editor.rs

@@ -14018,6 +14018,8 @@ impl Editor {
             return;
         }
 
+        self.finalize_last_transaction(cx);
+
         let clipboard_text = Cow::Borrowed(text.as_str());
 
         self.transact(window, cx, |this, window, cx| {

crates/editor/src/editor_tests.rs

@@ -8839,6 +8839,36 @@ async fn test_paste_multiline(cx: &mut TestAppContext) {
         )ˇ"});
 }
 
+#[gpui::test]
+async fn test_paste_undo_does_not_include_preceding_edits(cx: &mut TestAppContext) {
+    init_test(cx, |_| {});
+
+    let mut cx = EditorTestContext::new(cx).await;
+
+    cx.update_editor(|e, _, cx| {
+        e.buffer().update(cx, |buffer, cx| {
+            buffer.set_group_interval(Duration::from_secs(10), cx)
+        })
+    });
+    // Type some text
+    cx.set_state("ˇ");
+    cx.update_editor(|e, window, cx| e.insert("hello", window, cx));
+    // cx.assert_editor_state("helloˇ");
+
+    // Paste some text immediately after typing
+    cx.write_to_clipboard(ClipboardItem::new_string(" world".into()));
+    cx.update_editor(|e, window, cx| e.paste(&Paste, window, cx));
+    cx.assert_editor_state("hello worldˇ");
+
+    // Undo should only undo the paste, not the preceding typing
+    cx.update_editor(|e, window, cx| e.undo(&Undo, window, cx));
+    cx.assert_editor_state("helloˇ");
+
+    // Undo again should undo the typing
+    cx.update_editor(|e, window, cx| e.undo(&Undo, window, cx));
+    cx.assert_editor_state("ˇ");
+}
+
 #[gpui::test]
 async fn test_paste_content_from_other_app(cx: &mut TestAppContext) {
     init_test(cx, |_| {});

crates/language/src/buffer.rs

@@ -3274,6 +3274,10 @@ impl Buffer {
     pub fn preserve_preview(&self) -> bool {
         !self.has_edits_since(&self.preview_version)
     }
+
+    pub fn set_group_interval(&mut self, group_interval: Duration) {
+        self.text.set_group_interval(group_interval);
+    }
 }
 
 #[doc(hidden)]
@@ -3289,10 +3293,6 @@ impl Buffer {
         self.edit(edits, autoindent_mode, cx);
     }
 
-    pub fn set_group_interval(&mut self, group_interval: Duration) {
-        self.text.set_group_interval(group_interval);
-    }
-
     pub fn randomly_edit<T>(&mut self, rng: &mut T, old_range_count: usize, cx: &mut Context<Self>)
     where
         T: rand::Rng,

crates/multi_buffer/src/multi_buffer.rs

@@ -1234,8 +1234,15 @@ impl MultiBuffer {
         }
     }
 
-    pub fn set_group_interval(&mut self, group_interval: Duration) {
+    pub fn set_group_interval(&mut self, group_interval: Duration, cx: &mut Context<Self>) {
         self.history.set_group_interval(group_interval);
+        if self.singleton {
+            for BufferState { buffer, .. } in self.buffers.values() {
+                buffer.update(cx, |buffer, _| {
+                    buffer.set_group_interval(group_interval);
+                });
+            }
+        }
     }
 
     pub fn with_title(mut self, title: String) -> Self {

crates/multi_buffer/src/multi_buffer_tests.rs

@@ -3513,8 +3513,8 @@ fn test_history(cx: &mut App) {
         buf
     });
     let multibuffer = cx.new(|_| MultiBuffer::new(Capability::ReadWrite));
-    multibuffer.update(cx, |this, _| {
-        this.set_group_interval(group_interval);
+    multibuffer.update(cx, |this, cx| {
+        this.set_group_interval(group_interval, cx);
     });
     multibuffer.update(cx, |multibuffer, cx| {
         multibuffer.set_excerpts_for_path(

crates/text/src/text.rs

@@ -223,10 +223,11 @@ impl History {
             redo_stack: Vec::new(),
             transaction_depth: 0,
             // Don't group transactions in tests unless we opt in, because it's a footgun.
-            #[cfg(any(test, feature = "test-support"))]
-            group_interval: Duration::ZERO,
-            #[cfg(not(any(test, feature = "test-support")))]
-            group_interval: Duration::from_millis(300),
+            group_interval: if cfg!(any(test, feature = "test-support")) {
+                Duration::ZERO
+            } else {
+                Duration::from_millis(300)
+            },
         }
     }
 
@@ -1825,6 +1826,10 @@ impl Buffer {
             tx.try_send(()).ok();
         }
     }
+
+    pub fn set_group_interval(&mut self, group_interval: Duration) {
+        self.history.group_interval = group_interval;
+    }
 }
 
 #[cfg(any(test, feature = "test-support"))]
@@ -1929,10 +1934,6 @@ impl Buffer {
         assert!(!self.text().contains("\r\n"));
     }
 
-    pub fn set_group_interval(&mut self, group_interval: Duration) {
-        self.history.group_interval = group_interval;
-    }
-
     pub fn random_byte_range(&self, start_offset: usize, rng: &mut impl rand::Rng) -> Range<usize> {
         let end = self.clip_offset(rng.random_range(start_offset..=self.len()), Bias::Right);
         let start = self.clip_offset(rng.random_range(start_offset..=end), Bias::Right);