Coalesce IME compositions into a single edit

crates/editor/src/editor.rs

@@ -406,6 +406,7 @@ pub struct Editor {
     autoclose_stack: InvalidationStack<BracketPairState>,
     snippet_stack: InvalidationStack<SnippetState>,
     select_larger_syntax_node_stack: Vec<Box<[Selection<usize>]>>,
+    ime_transaction: Option<TransactionId>,
     active_diagnostics: Option<ActiveDiagnosticGroup>,
     scroll_position: Vector2F,
     scroll_top_anchor: Anchor,
@@ -993,6 +994,7 @@ impl Editor {
             autoclose_stack: Default::default(),
             snippet_stack: Default::default(),
             select_larger_syntax_node_stack: Vec::new(),
+            ime_transaction: Default::default(),
             active_diagnostics: None,
             soft_wrap_mode_override: None,
             get_field_editor_theme,
@@ -3616,6 +3618,7 @@ impl Editor {
                 });
             }
             self.request_autoscroll(Autoscroll::Fit, cx);
+            self.unmark_text(cx);
             cx.emit(Event::Edited);
         }
     }
@@ -3629,6 +3632,7 @@ impl Editor {
                 });
             }
             self.request_autoscroll(Autoscroll::Fit, cx);
+            self.unmark_text(cx);
             cx.emit(Event::Edited);
         }
     }
@@ -5142,10 +5146,10 @@ impl Editor {
         &mut self,
         cx: &mut ViewContext<Self>,
         update: impl FnOnce(&mut Self, &mut ViewContext<Self>),
-    ) {
+    ) -> Option<TransactionId> {
         self.start_transaction_at(Instant::now(), cx);
         update(self, cx);
-        self.end_transaction_at(Instant::now(), cx);
+        self.end_transaction_at(Instant::now(), cx)
     }
 
     fn start_transaction_at(&mut self, now: Instant, cx: &mut ViewContext<Self>) {
@@ -5159,7 +5163,11 @@ impl Editor {
         }
     }
 
-    fn end_transaction_at(&mut self, now: Instant, cx: &mut ViewContext<Self>) {
+    fn end_transaction_at(
+        &mut self,
+        now: Instant,
+        cx: &mut ViewContext<Self>,
+    ) -> Option<TransactionId> {
         if let Some(tx_id) = self
             .buffer
             .update(cx, |buffer, cx| buffer.end_transaction_at(now, cx))
@@ -5171,6 +5179,9 @@ impl Editor {
             }
 
             cx.emit(Event::Edited);
+            Some(tx_id)
+        } else {
+            None
         }
     }
 
@@ -5908,6 +5919,7 @@ impl View for Editor {
 
     fn unmark_text(&mut self, cx: &mut ViewContext<Self>) {
         self.clear_text_highlights::<InputComposition>(cx);
+        self.ime_transaction.take();
     }
 
     fn replace_text_in_range(
@@ -5928,8 +5940,15 @@ impl View for Editor {
                 });
             }
             this.handle_input(text, cx);
-            this.unmark_text(cx);
         });
+
+        if let Some(transaction) = self.ime_transaction {
+            self.buffer.update(cx, |buffer, cx| {
+                buffer.group_until_transaction(transaction, cx);
+            });
+        }
+
+        self.unmark_text(cx);
     }
 
     fn replace_and_mark_text_in_range(
@@ -5944,7 +5963,7 @@ impl View for Editor {
             return;
         }
 
-        self.transact(cx, |this, cx| {
+        let transaction = self.transact(cx, |this, cx| {
             let range_to_replace = if let Some(mut marked_range) = this.marked_text_range(cx) {
                 if let Some(relative_range_utf16) = range_utf16.as_ref() {
                     marked_range.end = marked_range.start + relative_range_utf16.end;
@@ -5994,6 +6013,17 @@ impl View for Editor {
                 });
             }
         });
+
+        self.ime_transaction = self.ime_transaction.or(transaction);
+        if let Some(transaction) = self.ime_transaction {
+            self.buffer.update(cx, |buffer, cx| {
+                buffer.group_until_transaction(transaction, cx);
+            });
+        }
+
+        if self.text_highlights::<InputComposition>(cx).is_none() {
+            self.ime_transaction.take();
+        }
     }
 }
 
@@ -6591,6 +6621,53 @@ mod tests {
         });
     }
 
+    #[gpui::test]
+    fn test_ime_composition(cx: &mut MutableAppContext) {
+        cx.set_global(Settings::test(cx));
+        let buffer = cx.add_model(|cx| {
+            let mut buffer = language::Buffer::new(0, "abcde", cx);
+            // Ensure automatic grouping doesn't occur.
+            buffer.set_group_interval(Duration::ZERO);
+            buffer
+        });
+
+        let buffer = cx.add_model(|cx| MultiBuffer::singleton(buffer, cx));
+        cx.add_window(Default::default(), |cx| {
+            let mut editor = build_editor(buffer.clone(), cx);
+
+            // Start a new IME composition.
+            editor.replace_and_mark_text_in_range(Some(0..1), "à", None, cx);
+            editor.replace_and_mark_text_in_range(Some(0..1), "á", None, cx);
+            editor.replace_and_mark_text_in_range(Some(0..1), "ä", None, cx);
+            assert_eq!(editor.text(cx), "äbcde");
+            assert_eq!(editor.marked_text_range(cx), Some(0..1));
+
+            // Finalize IME composition.
+            editor.replace_text_in_range(Some(0..1), "ā", cx);
+            assert_eq!(editor.text(cx), "ābcde");
+            assert_eq!(editor.marked_text_range(cx), None);
+
+            // IME composition edits are grouped and are undone/redone at once.
+            editor.undo(&Default::default(), cx);
+            assert_eq!(editor.text(cx), "abcde");
+            assert_eq!(editor.marked_text_range(cx), None);
+            editor.redo(&Default::default(), cx);
+            assert_eq!(editor.text(cx), "ābcde");
+            assert_eq!(editor.marked_text_range(cx), None);
+
+            // Start a new IME composition.
+            editor.replace_and_mark_text_in_range(Some(0..1), "à", None, cx);
+            assert_eq!(editor.marked_text_range(cx), Some(0..1));
+
+            // Undoing during an IME composition cancels it.
+            editor.undo(&Default::default(), cx);
+            assert_eq!(editor.text(cx), "ābcde");
+            assert_eq!(editor.marked_text_range(cx), None);
+
+            editor
+        });
+    }
+
     #[gpui::test]
     fn test_selection_with_mouse(cx: &mut gpui::MutableAppContext) {
         cx.set_global(Settings::test(cx));

crates/editor/src/multi_buffer.rs

@@ -558,6 +558,20 @@ impl MultiBuffer {
         self.history.finalize_last_transaction();
     }
 
+    pub fn group_until_transaction(
+        &mut self,
+        transaction_id: TransactionId,
+        cx: &mut ModelContext<Self>,
+    ) {
+        if let Some(buffer) = self.as_singleton() {
+            buffer.update(cx, |buffer, _| {
+                buffer.group_until_transaction(transaction_id)
+            });
+        } else {
+            self.history.group_until(transaction_id);
+        }
+    }
+
     pub fn set_active_selections(
         &mut self,
         selections: &[Selection<Anchor>],
@@ -2701,9 +2715,8 @@ impl History {
     }
 
     fn group(&mut self) -> Option<TransactionId> {
-        let mut new_len = self.undo_stack.len();
+        let mut count = 0;
-        let mut transactions = self.undo_stack.iter_mut();
+        let mut transactions = self.undo_stack.iter();
-
         if let Some(mut transaction) = transactions.next_back() {
             while let Some(prev_transaction) = transactions.next_back() {
                 if !prev_transaction.suppress_grouping
@@ -2711,13 +2724,31 @@ impl History {
                         <= self.group_interval
                 {
                     transaction = prev_transaction;
-                    new_len -= 1;
+                    count += 1;
                 } else {
                     break;
                 }
             }
         }
+        self.group_trailing(count)
+    }
 
+    fn group_until(&mut self, transaction_id: TransactionId) {
+        let mut count = 0;
+        for transaction in self.undo_stack.iter().rev() {
+            if transaction.id == transaction_id {
+                self.group_trailing(count);
+                break;
+            } else if transaction.suppress_grouping {
+                break;
+            } else {
+                count += 1;
+            }
+        }
+    }
+
+    fn group_trailing(&mut self, n: usize) -> Option<TransactionId> {
+        let new_len = self.undo_stack.len() - n;
         let (transactions_to_keep, transactions_to_merge) = self.undo_stack.split_at_mut(new_len);
         if let Some(last_transaction) = transactions_to_keep.last_mut() {
             if let Some(transaction) = transactions_to_merge.last() {
@@ -4143,7 +4174,7 @@ mod tests {
         let mut now = Instant::now();
 
         multibuffer.update(cx, |multibuffer, cx| {
-            multibuffer.start_transaction_at(now, cx);
+            let transaction_1 = multibuffer.start_transaction_at(now, cx).unwrap();
             multibuffer.edit(
                 [
                     (Point::new(0, 0)..Point::new(0, 0), "A"),
@@ -4226,6 +4257,16 @@ mod tests {
             assert_eq!(multibuffer.read(cx).text(), "ABCD1234\nAB5678");
             multibuffer.undo(cx);
             assert_eq!(multibuffer.read(cx).text(), "1234\n5678");
+
+            // Transactions can be grouped manually.
+            multibuffer.redo(cx);
+            multibuffer.redo(cx);
+            assert_eq!(multibuffer.read(cx).text(), "XABCD1234\nAB5678");
+            multibuffer.group_until_transaction(transaction_1, cx);
+            multibuffer.undo(cx);
+            assert_eq!(multibuffer.read(cx).text(), "1234\n5678");
+            multibuffer.redo(cx);
+            assert_eq!(multibuffer.read(cx).text(), "XABCD1234\nAB5678");
         });
     }
 }

crates/language/src/buffer.rs

@@ -1076,6 +1076,10 @@ impl Buffer {
         self.text.finalize_last_transaction()
     }
 
+    pub fn group_until_transaction(&mut self, transaction_id: TransactionId) {
+        self.text.group_until_transaction(transaction_id);
+    }
+
     pub fn forget_transaction(&mut self, transaction_id: TransactionId) {
         self.text.forget_transaction(transaction_id);
     }

crates/text/src/tests.rs

@@ -525,7 +525,7 @@ fn test_history() {
     let mut now = Instant::now();
     let mut buffer = Buffer::new(0, 0, "123456".into());
 
-    buffer.start_transaction_at(now);
+    let transaction_1 = buffer.start_transaction_at(now).unwrap();
     buffer.edit([(2..4, "cd")]);
     buffer.end_transaction_at(now);
     assert_eq!(buffer.text(), "12cd56");
@@ -574,6 +574,16 @@ fn test_history() {
     assert_eq!(buffer.text(), "12cde6");
     buffer.undo();
     assert_eq!(buffer.text(), "123456");
+
+    // Transactions can be grouped manually.
+    buffer.redo();
+    buffer.redo();
+    assert_eq!(buffer.text(), "X12cde6");
+    buffer.group_until_transaction(transaction_1);
+    buffer.undo();
+    assert_eq!(buffer.text(), "123456");
+    buffer.redo();
+    assert_eq!(buffer.text(), "X12cde6");
 }
 
 #[test]

crates/text/src/text.rs

@@ -191,22 +191,39 @@ impl History {
     }
 
     fn group(&mut self) -> Option<TransactionId> {
-        let mut new_len = self.undo_stack.len();
+        let mut count = 0;
-        let mut entries = self.undo_stack.iter_mut();
+        let mut entries = self.undo_stack.iter();
-
         if let Some(mut entry) = entries.next_back() {
             while let Some(prev_entry) = entries.next_back() {
                 if !prev_entry.suppress_grouping
                     && entry.first_edit_at - prev_entry.last_edit_at <= self.group_interval
                 {
                     entry = prev_entry;
-                    new_len -= 1;
+                    count += 1;
                 } else {
                     break;
                 }
             }
         }
+        self.group_trailing(count)
+    }
 
+    fn group_until(&mut self, transaction_id: TransactionId) {
+        let mut count = 0;
+        for entry in self.undo_stack.iter().rev() {
+            if entry.transaction_id() == transaction_id {
+                self.group_trailing(count);
+                break;
+            } else if entry.suppress_grouping {
+                break;
+            } else {
+                count += 1;
+            }
+        }
+    }
+
+    fn group_trailing(&mut self, n: usize) -> Option<TransactionId> {
+        let new_len = self.undo_stack.len() - n;
         let (entries_to_keep, entries_to_merge) = self.undo_stack.split_at_mut(new_len);
         if let Some(last_entry) = entries_to_keep.last_mut() {
             for entry in &*entries_to_merge {
@@ -1195,6 +1212,10 @@ impl Buffer {
         self.history.finalize_last_transaction()
     }
 
+    pub fn group_until_transaction(&mut self, transaction_id: TransactionId) {
+        self.history.group_until(transaction_id);
+    }
+
     pub fn base_text(&self) -> &Arc<str> {
         &self.history.base_text
     }