From d3567e381cd944a172ed3e5d21f396017654bb0d Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Mon, 25 Jul 2022 09:53:51 +0200 Subject: [PATCH] Coalesce IME compositions into a single edit --- crates/editor/src/editor.rs | 87 +++++++++++++++++++++++++++++-- crates/editor/src/multi_buffer.rs | 51 ++++++++++++++++-- crates/language/src/buffer.rs | 4 ++ crates/text/src/tests.rs | 12 ++++- crates/text/src/text.rs | 29 +++++++++-- 5 files changed, 168 insertions(+), 15 deletions(-) diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index aa185ae09b..bf3dfc220e 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -406,6 +406,7 @@ pub struct Editor { autoclose_stack: InvalidationStack, snippet_stack: InvalidationStack, select_larger_syntax_node_stack: Vec]>>, + ime_transaction: Option, active_diagnostics: Option, 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, update: impl FnOnce(&mut Self, &mut ViewContext), - ) { + ) -> Option { 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) { @@ -5159,7 +5163,11 @@ impl Editor { } } - fn end_transaction_at(&mut self, now: Instant, cx: &mut ViewContext) { + fn end_transaction_at( + &mut self, + now: Instant, + cx: &mut ViewContext, + ) -> Option { 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.clear_text_highlights::(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::(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)); diff --git a/crates/editor/src/multi_buffer.rs b/crates/editor/src/multi_buffer.rs index d52f693f1a..abf1689343 100644 --- a/crates/editor/src/multi_buffer.rs +++ b/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, + ) { + 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], @@ -2701,9 +2715,8 @@ impl History { } fn group(&mut self) -> Option { - let mut new_len = self.undo_stack.len(); - let mut transactions = self.undo_stack.iter_mut(); - + let mut count = 0; + 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 { + 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"); }); } } diff --git a/crates/language/src/buffer.rs b/crates/language/src/buffer.rs index ee24539287..c413ef2de4 100644 --- a/crates/language/src/buffer.rs +++ b/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); } diff --git a/crates/text/src/tests.rs b/crates/text/src/tests.rs index 2f67579f47..5d5fba2be0 100644 --- a/crates/text/src/tests.rs +++ b/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] diff --git a/crates/text/src/text.rs b/crates/text/src/text.rs index 838b20f2ee..1e56439fcc 100644 --- a/crates/text/src/text.rs +++ b/crates/text/src/text.rs @@ -191,22 +191,39 @@ impl History { } fn group(&mut self) -> Option { - let mut new_len = self.undo_stack.len(); - let mut entries = self.undo_stack.iter_mut(); - + let mut count = 0; + 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 { + 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 { &self.history.base_text }