From 63992e24313dc33f1ffd5d5e844cb44bad963c6a Mon Sep 17 00:00:00 2001 From: Skyler Hawthorne Date: Mon, 5 Jun 2023 19:07:08 -0400 Subject: [PATCH] account for overlapping deletes --- helix-core/src/auto_pairs.rs | 25 ++++- helix-core/src/transaction.rs | 17 ++- helix-term/tests/test/auto_pairs.rs | 159 ++++++++++++++++++++++------ 3 files changed, 161 insertions(+), 40 deletions(-) diff --git a/helix-core/src/auto_pairs.rs b/helix-core/src/auto_pairs.rs index 3bfadcc7b338..a279f233f983 100644 --- a/helix-core/src/auto_pairs.rs +++ b/helix-core/src/auto_pairs.rs @@ -136,6 +136,8 @@ pub fn hook_insert( #[must_use] pub fn hook_delete(doc: &Rope, range: &Range, pairs: &AutoPairs) -> Option<(Deletion, Range)> { + log::trace!("autopairs delete hook range: {:#?}", range); + let text = doc.slice(..); let cursor = range.cursor(text); @@ -175,12 +177,27 @@ pub fn handle_delete(doc: &Rope, range: &Range) -> Option<(Deletion, Range)> { let size_delete = end_next - end_prev; let next_head = graphemes::next_grapheme_boundary(text, range.head) - size_delete; - let next_range = match range.direction() { - Direction::Forward => Range::new(range.anchor, next_head), - Direction::Backward => Range::new(range.anchor - size_delete, next_head), + // if the range is a single grapheme cursor, we do not want to shrink the + // range, just move it, so we only subtract the size of the closing pair char + let next_anchor = match (range.direction(), range.is_single_grapheme(text)) { + // single grapheme forward needs to move, but only the width of the + // character under the cursor, which is the closer + (Direction::Forward, true) => range.anchor - (end_next - cursor), + (Direction::Backward, true) => range.anchor - (cursor - end_prev), + + (Direction::Forward, false) => range.anchor, + (Direction::Backward, false) => range.anchor - size_delete, }; - log::trace!("auto pair delete: {:?}, range: {:?}", delete, range,); + let next_range = Range::new(next_anchor, next_head); + + log::trace!( + "auto pair delete: {:?}, range: {:?}, next_range: {:?}, text len: {}", + delete, + range, + next_range, + text.len_chars() + ); Some((delete, next_range)) } diff --git a/helix-core/src/transaction.rs b/helix-core/src/transaction.rs index 2c33d283121a..f4d110be5940 100644 --- a/helix-core/src/transaction.rs +++ b/helix-core/src/transaction.rs @@ -792,10 +792,13 @@ impl Transaction { { let mut end_ranges = SmallVec::with_capacity(selection.len()); let mut offset = 0; + let mut last = 0; let transaction = Transaction::delete_by_selection(doc, selection, |start_range| { let ((from, to), end_range) = f(start_range); - let change_size = to - from; + + // must account for possibly overlapping deletes + let change_size = if last > from { to - last } else { to - from }; let new_range = if let Some(end_range) = end_range { end_range @@ -809,10 +812,18 @@ impl Transaction { new_range.head.saturating_sub(offset), ); + log::trace!( + "delete from: {}, to: {}, offset: {}, new_range: {:?}, offset_range: {:?}", + from, + to, + offset, + new_range, + offset_range + ); + end_ranges.push(offset_range); offset += change_size; - - log::trace!("delete from: {}, to: {}, offset: {}", from, to, offset); + last = to; (from, to) }); diff --git a/helix-term/tests/test/auto_pairs.rs b/helix-term/tests/test/auto_pairs.rs index 452292a1bfd8..a85b5ea69146 100644 --- a/helix-term/tests/test/auto_pairs.rs +++ b/helix-term/tests/test/auto_pairs.rs @@ -683,7 +683,7 @@ async fn delete_basic() -> anyhow::Result<()> { test(( format!("{}#[|{}]#{}", pair.0, pair.1, LINE_END), "i", - format!("#[{}|]#", LINE_END), + format!("#[|{}]#", LINE_END), )) .await?; } @@ -695,9 +695,21 @@ async fn delete_basic() -> anyhow::Result<()> { async fn delete_multi() -> anyhow::Result<()> { for pair in DEFAULT_PAIRS { test(( - format!("{}#[|{}]#{}", pair.0, pair.1, LINE_END), + format!( + indoc! {"\ + {open}#[|{close}]# + {open}#(|{close})# + {open}#(|{close})# + "}, + open = pair.0, + close = pair.1, + ), "i", - format!("#[{}|]#", LINE_END), + indoc! {"\ + #[|\n]# + #(|\n)# + #(|\n)# + "}, )) .await?; } @@ -711,7 +723,21 @@ async fn delete_whitespace() -> anyhow::Result<()> { test(( format!("{} #[| ]#{}", pair.0, pair.1), "i", - format!("{}#[{}|]#", pair.0, pair.1), + format!("{}#[|{}]#", pair.0, pair.1), + )) + .await?; + } + + Ok(()) +} + +#[tokio::test(flavor = "multi_thread")] +async fn delete_whitespace_after_word() -> anyhow::Result<()> { + for pair in DEFAULT_PAIRS { + test(( + format!("foo{} #[| ]#{}", pair.0, pair.1), + "i", + format!("foo{}#[|{}]#", pair.0, pair.1), )) .await?; } @@ -736,7 +762,7 @@ async fn delete_whitespace_multi() -> anyhow::Result<()> { "i", format!( indoc! {"\ - {open}#[{close}|]# + {open}#[|{close}]# {open}#(|{open})#{close}{close} {open}{open}#(|{close}{close})# foo#(|\n)# @@ -830,7 +856,7 @@ async fn delete_configured_multi_byte_chars() -> anyhow::Result<()> { ( format!("{}#[|{}]#{}", open, close, LINE_END), "i", - format!("#[{}|]#", LINE_END), + format!("#[|{}]#", LINE_END), ), ) .await?; @@ -843,9 +869,95 @@ async fn delete_configured_multi_byte_chars() -> anyhow::Result<()> { async fn delete_after_word() -> anyhow::Result<()> { for pair in DEFAULT_PAIRS { test(( - format!("foo{}#[|{}]#{}", pair.0, pair.1, LINE_END), + &format!("foo{}#[|{}]#", pair.0, pair.1), "i", - format!("foo#[{}|]#", LINE_END), + "foo#[|\n]#", + )) + .await?; + } + + Ok(()) +} + +#[tokio::test(flavor = "multi_thread")] +async fn insert_then_delete() -> anyhow::Result<()> { + for pair in differing_pairs() { + test(( + "#[\n|]#\n", + format!("ofoo{}", pair.0), + "\nfoo#[\n|]#\n", + )) + .await?; + } + + Ok(()) +} + +#[tokio::test(flavor = "multi_thread")] +async fn insert_then_delete_whitespace() -> anyhow::Result<()> { + for pair in differing_pairs() { + test(( + "foo#[\n|]#", + format!("i{}", pair.0), + "foo#[|\n]#", + )) + .await?; + } + + Ok(()) +} + +#[tokio::test(flavor = "multi_thread")] +async fn insert_then_delete_multi() -> anyhow::Result<()> { + for pair in differing_pairs() { + test(( + indoc! {"\ + through a day#[\n|]# + in and out of weeks#(\n|)# + over a year#(\n|)# + "}, + format!("i{}", pair.0), + indoc! {"\ + through a day#[|\n]# + in and out of weeks#(|\n)# + over a year#(|\n)# + "}, + )) + .await?; + } + + Ok(()) +} + +#[tokio::test(flavor = "multi_thread")] +async fn append_then_delete() -> anyhow::Result<()> { + for pair in differing_pairs() { + test(( + "fo#[o|]#", + format!("a{}", pair.0), + "fo#[o\n|]#", + )) + .await?; + } + + Ok(()) +} + +#[tokio::test(flavor = "multi_thread")] +async fn append_then_delete_multi() -> anyhow::Result<()> { + for pair in differing_pairs() { + test(( + indoc! {"\ + #[through a day|]# + #(in and out of weeks|)# + #(over a year|)# + "}, + format!("a{}", pair.0), + indoc! {"\ + #[through a day\n|]# + #(in and out of weeks\n|)# + #(over a year\n|)# + "}, )) .await?; } @@ -876,7 +988,7 @@ async fn delete_before_word() -> anyhow::Result<()> { test(( format!("{}#[|{}]#foo{}", pair.0, pair.1, LINE_END), "i", - format!("#[f|]#oo{}", LINE_END), + format!("#[|f]#oo{}", LINE_END), )) .await?; } @@ -940,7 +1052,7 @@ async fn delete_before_eol() -> anyhow::Result<()> { close = pair.1 ), "i", - format!("{0}#[{0}|]#", LINE_END), + format!("{0}#[|{0}]#", LINE_END), )) .await?; } @@ -971,25 +1083,6 @@ async fn delete_auto_pairs_disabled() -> anyhow::Result<()> { Ok(()) } -#[tokio::test(flavor = "multi_thread")] -async fn delete_multi_range() -> anyhow::Result<()> { - for pair in DEFAULT_PAIRS { - test(( - format!( - "{open}#[|{close}]#{eol}{open}#(|{close})#{eol}{open}#(|{close})#{eol}", - open = pair.0, - close = pair.1, - eol = LINE_END - ), - "i", - format!("#[{eol}|]##({eol}|)##({eol}|)#", eol = LINE_END), - )) - .await?; - } - - Ok(()) -} - #[tokio::test(flavor = "multi_thread")] async fn delete_before_multi_code_point_graphemes() -> anyhow::Result<()> { for pair in DEFAULT_PAIRS { @@ -1016,7 +1109,7 @@ async fn delete_before_multi_code_point_graphemes() -> anyhow::Result<()> { pair.0, pair.1, LINE_END ), "i", - format!("hello #[๐Ÿ‘จโ€๐Ÿ‘ฉโ€๐Ÿ‘งโ€๐Ÿ‘ฆ|]# goodbye{}", LINE_END), + format!("hello #[|๐Ÿ‘จโ€๐Ÿ‘ฉโ€๐Ÿ‘งโ€๐Ÿ‘ฆ]# goodbye{}", LINE_END), )) .await?; @@ -1072,7 +1165,7 @@ async fn delete_nested_open_inside_pair() -> anyhow::Result<()> { ), "i", format!( - "{open}#[{close}|]#{eol}", + "{open}#[|{close}]#{eol}", open = pair.0, close = pair.1, eol = LINE_END @@ -1103,7 +1196,7 @@ async fn delete_nested_open_inside_pair_multi() -> anyhow::Result<()> { ), "i", format!( - "{outer_open}#[{outer_close}|]#{eol}{outer_open}#({outer_close}|)#{eol}{outer_open}#({outer_close}|)#{eol}", + "{outer_open}#[|{outer_close}]#{eol}{outer_open}#(|{outer_close})#{eol}{outer_open}#(|{outer_close})#{eol}", outer_open = outer_pair.0, outer_close = outer_pair.1, eol = LINE_END @@ -1187,7 +1280,7 @@ async fn delete_mixed_dedent() -> anyhow::Result<()> { ), "i", indoc! {"\ - bar = #[\n|]# + bar = #[|\n]# #(|\n)# fo#(|\n)# "},