From 6627bff8c7bd8d8f9e5431e673f9149cc56be132 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 ea65024e971b..46fa87ab5f92 100644 --- a/helix-term/tests/test/auto_pairs.rs +++ b/helix-term/tests/test/auto_pairs.rs @@ -656,7 +656,7 @@ async fn delete_basic() -> anyhow::Result<()> { test(( format!("{}#[|{}]#{}", pair.0, pair.1, LINE_END), "i", - format!("#[{}|]#", LINE_END), + format!("#[|{}]#", LINE_END), )) .await?; } @@ -668,9 +668,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), + helpers::platform_line(&format!( + indoc! {"\ + {open}#[|{close}]# + {open}#(|{close})# + {open}#(|{close})# + "}, + open = pair.0, + close = pair.1, + )), "i", - format!("#[{}|]#", LINE_END), + helpers::platform_line(indoc! {"\ + #[|\n]# + #(|\n)# + #(|\n)# + "}), )) .await?; } @@ -684,7 +696,21 @@ async fn delete_whitespace() -> anyhow::Result<()> { test(( helpers::platform_line(&format!("{} #[| ]#{}", pair.0, pair.1)), "i", - helpers::platform_line(&format!("{}#[{}|]#", pair.0, pair.1)), + helpers::platform_line(&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(( + helpers::platform_line(&format!("foo{} #[| ]#{}", pair.0, pair.1)), + "i", + helpers::platform_line(&format!("foo{}#[|{}]#", pair.0, pair.1)), )) .await?; } @@ -709,7 +735,7 @@ async fn delete_whitespace_multi() -> anyhow::Result<()> { "i", helpers::platform_line(&format!( indoc! {"\ - {open}#[{close}|]# + {open}#[|{close}]# {open}#(|{open})#{close}{close} {open}{open}#(|{close}{close})# foo#(|\n)# @@ -803,7 +829,7 @@ async fn delete_configured_multi_byte_chars() -> anyhow::Result<()> { ( format!("{}#[|{}]#{}", open, close, LINE_END), "i", - format!("#[{}|]#", LINE_END), + format!("#[|{}]#", LINE_END), ), ) .await?; @@ -816,9 +842,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), + helpers::platform_line(&format!("foo{}#[|{}]#", pair.0, pair.1)), "i", - format!("foo#[{}|]#", LINE_END), + helpers::platform_line("foo#[|\n]#"), + )) + .await?; + } + + Ok(()) +} + +#[tokio::test(flavor = "multi_thread")] +async fn insert_then_delete() -> anyhow::Result<()> { + for pair in differing_pairs() { + test(( + helpers::platform_line("#[\n|]#\n"), + format!("ofoo{}", pair.0), + helpers::platform_line("\nfoo#[\n|]#\n"), + )) + .await?; + } + + Ok(()) +} + +#[tokio::test(flavor = "multi_thread")] +async fn insert_then_delete_whitespace() -> anyhow::Result<()> { + for pair in differing_pairs() { + test(( + helpers::platform_line("foo#[\n|]#"), + format!("i{}", pair.0), + helpers::platform_line("foo#[|\n]#"), + )) + .await?; + } + + Ok(()) +} + +#[tokio::test(flavor = "multi_thread")] +async fn insert_then_delete_multi() -> anyhow::Result<()> { + for pair in differing_pairs() { + test(( + helpers::platform_line(indoc! {"\ + through a day#[\n|]# + in and out of weeks#(\n|)# + over a year#(\n|)# + "}), + format!("i{}", pair.0), + helpers::platform_line(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(( + helpers::platform_line("fo#[o|]#"), + format!("a{}", pair.0), + helpers::platform_line("fo#[o\n|]#"), + )) + .await?; + } + + Ok(()) +} + +#[tokio::test(flavor = "multi_thread")] +async fn append_then_delete_multi() -> anyhow::Result<()> { + for pair in differing_pairs() { + test(( + helpers::platform_line(indoc! {"\ + #[through a day|]# + #(in and out of weeks|)# + #(over a year|)# + "}), + format!("a{}", pair.0), + helpers::platform_line(indoc! {"\ + #[through a day\n|]# + #(in and out of weeks\n|)# + #(over a year\n|)# + "}), )) .await?; } @@ -849,7 +961,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?; } @@ -913,7 +1025,7 @@ async fn delete_before_eol() -> anyhow::Result<()> { close = pair.1 ), "i", - format!("{0}#[{0}|]#", LINE_END), + format!("{0}#[|{0}]#", LINE_END), )) .await?; } @@ -944,25 +1056,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 { @@ -989,7 +1082,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?; @@ -1043,7 +1136,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 @@ -1074,7 +1167,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 @@ -1158,7 +1251,7 @@ async fn delete_mixed_dedent() -> anyhow::Result<()> { )), "i", helpers::platform_line(indoc! {"\ - bar = #[\n|]# + bar = #[|\n]# #(|\n)# fo#(|\n)# "}),