diff --git a/helix-core/src/lib.rs b/helix-core/src/lib.rs index b67e2c8a..14abf016 100644 --- a/helix-core/src/lib.rs +++ b/helix-core/src/lib.rs @@ -67,4 +67,4 @@ pub use syntax::Syntax; pub use diagnostic::Diagnostic; pub use line_ending::{LineEnding, DEFAULT_LINE_ENDING}; -pub use transaction::{Assoc, Change, ChangeSet, Operation, Transaction}; +pub use transaction::{Assoc, Change, ChangeSet, Deletion, Operation, Transaction}; diff --git a/helix-core/src/transaction.rs b/helix-core/src/transaction.rs index d8e581aa..06efe259 100644 --- a/helix-core/src/transaction.rs +++ b/helix-core/src/transaction.rs @@ -5,6 +5,7 @@ use std::borrow::Cow; /// (from, to, replacement) pub type Change = (usize, usize, Option); +pub type Deletion = (usize, usize); // TODO: pub(crate) #[derive(Debug, Clone, PartialEq, Eq)] @@ -534,6 +535,41 @@ impl Transaction { Self::from(changeset) } + /// Generate a transaction from a set of potentially overlapping deletions + /// by merging overlapping deletions together. + pub fn delete(doc: &Rope, deletions: I) -> Self + where + I: Iterator, + { + let len = doc.len_chars(); + + let (lower, upper) = deletions.size_hint(); + let size = upper.unwrap_or(lower); + let mut changeset = ChangeSet::with_capacity(2 * size + 1); // rough estimate + + let mut last = 0; + for (mut from, to) in deletions { + if last > to { + continue; + } + if last > from { + from = last + } + debug_assert!( + from <= to, + "Edit end must end before it starts (should {from} <= {to})" + ); + // Retain from last "to" to current "from" + changeset.retain(from - last); + changeset.delete(to - from); + last = to; + } + + changeset.retain(len - last); + + Self::from(changeset) + } + /// Generate a transaction with a change per selection range. pub fn change_by_selection(doc: &Rope, selection: &Selection, f: F) -> Self where @@ -580,6 +616,16 @@ impl Transaction { ) } + /// Generate a transaction with a deletion per selection range. + /// Compared to using `change_by_selection` directly these ranges may overlap. + /// In that case they are merged + pub fn delete_by_selection(doc: &Rope, selection: &Selection, f: F) -> Self + where + F: FnMut(&Range) -> Deletion, + { + Self::delete(doc, selection.iter().map(f)) + } + /// Insert text at each selection head. pub fn insert(doc: &Rope, selection: &Selection, text: Tendril) -> Self { Self::change_by_selection(doc, selection, |range| { diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs index 80774cea..964d87ff 100644 --- a/helix-term/src/commands.rs +++ b/helix-term/src/commands.rs @@ -2315,9 +2315,8 @@ fn delete_selection_impl(cx: &mut Context, op: Operation) { }; // then delete - let transaction = Transaction::change_by_selection(doc.text(), selection, |range| { - (range.from(), range.to(), None) - }); + let transaction = + Transaction::delete_by_selection(doc.text(), selection, |range| (range.from(), range.to())); doc.apply(&transaction, view.id); match op { @@ -2333,9 +2332,8 @@ fn delete_selection_impl(cx: &mut Context, op: Operation) { #[inline] fn delete_selection_insert_mode(doc: &mut Document, view: &mut View, selection: &Selection) { - let transaction = Transaction::change_by_selection(doc.text(), selection, |range| { - (range.from(), range.to(), None) - }); + let transaction = + Transaction::delete_by_selection(doc.text(), selection, |range| (range.from(), range.to())); doc.apply(&transaction, view.id); } @@ -3422,10 +3420,10 @@ pub mod insert { let auto_pairs = doc.auto_pairs(cx.editor); let transaction = - Transaction::change_by_selection(doc.text(), doc.selection(view.id), |range| { + Transaction::delete_by_selection(doc.text(), doc.selection(view.id), |range| { let pos = range.cursor(text); if pos == 0 { - return (pos, pos, None); + return (pos, pos); } let line_start_pos = text.line_to_char(range.cursor_line(text)); // consider to delete by indent level if all characters before `pos` are indent units. @@ -3433,11 +3431,7 @@ pub mod insert { if !fragment.is_empty() && fragment.chars().all(|ch| ch == ' ' || ch == '\t') { if text.get_char(pos.saturating_sub(1)) == Some('\t') { // fast path, delete one char - ( - graphemes::nth_prev_grapheme_boundary(text, pos, 1), - pos, - None, - ) + (graphemes::nth_prev_grapheme_boundary(text, pos, 1), pos) } else { let width: usize = fragment .chars() @@ -3464,7 +3458,7 @@ pub mod insert { _ => break, } } - (start, pos, None) // delete! + (start, pos) // delete! } } else { match ( @@ -3482,17 +3476,12 @@ pub mod insert { ( graphemes::nth_prev_grapheme_boundary(text, pos, count), graphemes::nth_next_grapheme_boundary(text, pos, count), - None, ) } _ => // delete 1 char { - ( - graphemes::nth_prev_grapheme_boundary(text, pos, count), - pos, - None, - ) + (graphemes::nth_prev_grapheme_boundary(text, pos, count), pos) } } } @@ -3508,13 +3497,9 @@ pub mod insert { let (view, doc) = current!(cx.editor); let text = doc.text().slice(..); let transaction = - Transaction::change_by_selection(doc.text(), doc.selection(view.id), |range| { + Transaction::delete_by_selection(doc.text(), doc.selection(view.id), |range| { let pos = range.cursor(text); - ( - pos, - graphemes::nth_next_grapheme_boundary(text, pos, count), - None, - ) + (pos, graphemes::nth_next_grapheme_boundary(text, pos, count)) }); doc.apply(&transaction, view.id); diff --git a/helix-term/tests/test/commands.rs b/helix-term/tests/test/commands.rs index 342a849b..1efb204e 100644 --- a/helix-term/tests/test/commands.rs +++ b/helix-term/tests/test/commands.rs @@ -385,3 +385,47 @@ async fn test_character_info() -> anyhow::Result<()> { Ok(()) } + +#[tokio::test(flavor = "multi_thread")] +async fn test_delete_char_backward() -> anyhow::Result<()> { + // don't panic when deleting overlapping ranges + test(( + platform_line("#(x|)# #[x|]#").as_str(), + "c", + platform_line("#[\n|]#").as_str(), + )) + .await?; + test(( + platform_line("#( |)##( |)#a#( |)#axx#[x|]#a").as_str(), + "li", + platform_line("#(a|)##(|a)#xx#[|a]#").as_str(), + )) + .await?; + + Ok(()) +} + +#[tokio::test(flavor = "multi_thread")] +async fn test_delete_word_backward() -> anyhow::Result<()> { + // don't panic when deleting overlapping ranges + test(( + platform_line("fo#[o|]#ba#(r|)#").as_str(), + "a", + platform_line("#[\n|]#").as_str(), + )) + .await?; + Ok(()) +} + +#[tokio::test(flavor = "multi_thread")] +async fn test_delete_word_forward() -> anyhow::Result<()> { + // don't panic when deleting overlapping ranges + test(( + platform_line("fo#[o|]#b#(|ar)#").as_str(), + "i", + platform_line("fo#[\n|]#").as_str(), + )) + .await?; + Ok(()) +} +