From d29ca3d4a557b0f1ddb48c2aae1ea4a4b2a2fd8f Mon Sep 17 00:00:00 2001 From: Skyler Hawthorne Date: Mon, 13 Mar 2023 00:09:46 -0400 Subject: [PATCH 1/8] Add Transaction::change_by_and_with_selection Adds `Transaction::change_by_and_with_selection` which centralizes logic for producing change sets with a potentially new selection that is applied incrementally, rather than all at once at the end with `with_selection`. It also centralizes the offset tracking logic so that the caller can construct a new selection with ranges as if they were operating on the text as-is. --- helix-core/src/auto_pairs.rs | 91 ++++++++++++----------------------- helix-core/src/transaction.rs | 49 +++++++++++++++++++ 2 files changed, 80 insertions(+), 60 deletions(-) diff --git a/helix-core/src/auto_pairs.rs b/helix-core/src/auto_pairs.rs index 853290404..e319b76a5 100644 --- a/helix-core/src/auto_pairs.rs +++ b/helix-core/src/auto_pairs.rs @@ -4,8 +4,6 @@ use crate::{graphemes, movement::Direction, Range, Rope, Selection, Tendril, Transaction}; use std::collections::HashMap; -use smallvec::SmallVec; - // Heavily based on https://github.com/codemirror/closebrackets/ pub const DEFAULT_PAIRS: &[(char, char)] = &[ ('(', ')'), @@ -146,7 +144,7 @@ fn prev_char(doc: &Rope, pos: usize) -> Option { } /// calculate what the resulting range should be for an auto pair insertion -fn get_next_range(doc: &Rope, start_range: &Range, offset: usize, len_inserted: usize) -> Range { +fn get_next_range(doc: &Rope, start_range: &Range, len_inserted: usize) -> Range { // When the character under the cursor changes due to complete pair // insertion, we must look backward a grapheme and then add the length // of the insertion to put the resulting cursor in the right place, e.g. @@ -165,10 +163,7 @@ fn get_next_range(doc: &Rope, start_range: &Range, offset: usize, len_inserted: // inserting at the very end of the document after the last newline if start_range.head == doc.len_chars() && start_range.anchor == doc.len_chars() { - return Range::new( - start_range.anchor + offset + 1, - start_range.head + offset + 1, - ); + return Range::new(start_range.anchor + 1, start_range.head + 1); } let doc_slice = doc.slice(..); @@ -177,7 +172,7 @@ fn get_next_range(doc: &Rope, start_range: &Range, offset: usize, len_inserted: // just skip over graphemes if len_inserted == 0 { let end_anchor = if single_grapheme { - graphemes::next_grapheme_boundary(doc_slice, start_range.anchor) + offset + graphemes::next_grapheme_boundary(doc_slice, start_range.anchor) // even for backward inserts with multiple grapheme selections, // we want the anchor to stay where it is so that the relative @@ -185,42 +180,38 @@ fn get_next_range(doc: &Rope, start_range: &Range, offset: usize, len_inserted: // // foo([) wor]d -> insert ) -> foo()[ wor]d } else { - start_range.anchor + offset + start_range.anchor }; return Range::new( end_anchor, - graphemes::next_grapheme_boundary(doc_slice, start_range.head) + offset, + graphemes::next_grapheme_boundary(doc_slice, start_range.head), ); } // trivial case: only inserted a single-char opener, just move the selection if len_inserted == 1 { let end_anchor = if single_grapheme || start_range.direction() == Direction::Backward { - start_range.anchor + offset + 1 + start_range.anchor + 1 } else { - start_range.anchor + offset + start_range.anchor }; - return Range::new(end_anchor, start_range.head + offset + 1); + return Range::new(end_anchor, start_range.head + 1); } // If the head = 0, then we must be in insert mode with a backward // cursor, which implies the head will just move let end_head = if start_range.head == 0 || start_range.direction() == Direction::Backward { - start_range.head + offset + 1 + start_range.head + 1 } else { // We must have a forward cursor, which means we must move to the // other end of the grapheme to get to where the new characters // are inserted, then move the head to where it should be let prev_bound = graphemes::prev_grapheme_boundary(doc_slice, start_range.head); - log::trace!( - "prev_bound: {}, offset: {}, len_inserted: {}", - prev_bound, - offset, - len_inserted - ); - prev_bound + offset + len_inserted + log::trace!("prev_bound: {}, len_inserted: {}", prev_bound, len_inserted); + + prev_bound + len_inserted }; let end_anchor = match (start_range.len(), start_range.direction()) { @@ -239,7 +230,7 @@ fn get_next_range(doc: &Rope, start_range: &Range, offset: usize, len_inserted: // if we are appending, the anchor stays where it is; only offset // for multiple range insertions } else { - start_range.anchor + offset + start_range.anchor } } @@ -248,13 +239,11 @@ fn get_next_range(doc: &Rope, start_range: &Range, offset: usize, len_inserted: // if we're backward, then the head is at the first char // of the typed char, so we need to add the length of // the closing char - graphemes::prev_grapheme_boundary(doc.slice(..), start_range.anchor) - + len_inserted - + offset + graphemes::prev_grapheme_boundary(doc.slice(..), start_range.anchor) + len_inserted } else { // when we are inserting in front of a selection, we need to move // the anchor over by however many characters were inserted overall - start_range.anchor + offset + len_inserted + start_range.anchor + len_inserted } } }; @@ -263,10 +252,7 @@ fn get_next_range(doc: &Rope, start_range: &Range, offset: usize, len_inserted: } fn handle_open(doc: &Rope, selection: &Selection, pair: &Pair) -> Transaction { - let mut end_ranges = SmallVec::with_capacity(selection.len()); - let mut offs = 0; - - let transaction = Transaction::change_by_selection(doc, selection, |start_range| { + let transaction = Transaction::change_by_and_with_selection(doc, selection, |start_range| { let cursor = start_range.cursor(doc.slice(..)); let next_char = doc.get_char(cursor); let len_inserted; @@ -289,23 +275,17 @@ fn handle_open(doc: &Rope, selection: &Selection, pair: &Pair) -> Transaction { } }; - let next_range = get_next_range(doc, start_range, offs, len_inserted); - end_ranges.push(next_range); - offs += len_inserted; + let next_range = get_next_range(doc, start_range, len_inserted); - change + (change, Some(next_range)) }); - let t = transaction.with_selection(Selection::new(end_ranges, selection.primary_index())); - log::debug!("auto pair transaction: {:#?}", t); - t + log::debug!("auto pair transaction: {:#?}", transaction); + transaction } fn handle_close(doc: &Rope, selection: &Selection, pair: &Pair) -> Transaction { - let mut end_ranges = SmallVec::with_capacity(selection.len()); - let mut offs = 0; - - let transaction = Transaction::change_by_selection(doc, selection, |start_range| { + let transaction = Transaction::change_by_and_with_selection(doc, selection, |start_range| { let cursor = start_range.cursor(doc.slice(..)); let next_char = doc.get_char(cursor); let mut len_inserted = 0; @@ -320,25 +300,18 @@ fn handle_close(doc: &Rope, selection: &Selection, pair: &Pair) -> Transaction { (cursor, cursor, Some(tendril)) }; - let next_range = get_next_range(doc, start_range, offs, len_inserted); - end_ranges.push(next_range); - offs += len_inserted; + let next_range = get_next_range(doc, start_range, len_inserted); - change + (change, Some(next_range)) }); - let t = transaction.with_selection(Selection::new(end_ranges, selection.primary_index())); - log::debug!("auto pair transaction: {:#?}", t); - t + log::debug!("auto pair transaction: {:#?}", transaction); + transaction } /// handle cases where open and close is the same, or in triples ("""docstring""") fn handle_same(doc: &Rope, selection: &Selection, pair: &Pair) -> Transaction { - let mut end_ranges = SmallVec::with_capacity(selection.len()); - - let mut offs = 0; - - let transaction = Transaction::change_by_selection(doc, selection, |start_range| { + let transaction = Transaction::change_by_and_with_selection(doc, selection, |start_range| { let cursor = start_range.cursor(doc.slice(..)); let mut len_inserted = 0; let next_char = doc.get_char(cursor); @@ -360,14 +333,12 @@ fn handle_same(doc: &Rope, selection: &Selection, pair: &Pair) -> Transaction { (cursor, cursor, Some(pair_str)) }; - let next_range = get_next_range(doc, start_range, offs, len_inserted); - end_ranges.push(next_range); - offs += len_inserted; + let next_range = get_next_range(doc, start_range, len_inserted); - change + (change, Some(next_range)) }); - let t = transaction.with_selection(Selection::new(end_ranges, selection.primary_index())); - log::debug!("auto pair transaction: {:#?}", t); - t + log::debug!("auto pair transaction: {:#?}", transaction); + + transaction } diff --git a/helix-core/src/transaction.rs b/helix-core/src/transaction.rs index 450b47365..5aae801cd 100644 --- a/helix-core/src/transaction.rs +++ b/helix-core/src/transaction.rs @@ -736,6 +736,55 @@ impl Transaction { Self::delete(doc, selection.iter().map(f)) } + /// Generate a transaction with a change per selection range, which + /// generates a new selection as well. Each range is operated upon by + /// the given function and can optionally produce a new range. If none + /// is returned by the function, that range is dropped from the resulting + /// selection. + pub fn change_by_and_with_selection(doc: &Rope, selection: &Selection, mut f: F) -> Self + where + F: FnMut(&Range) -> (Change, Option), + { + let mut end_ranges = SmallVec::with_capacity(selection.len()); + let mut offset = 0; + + let transaction = Transaction::change_by_selection(doc, selection, |start_range| { + let ((from, to, replacement), end_range) = f(start_range); + let mut change_size = to as isize - from as isize; + + if let Some(ref text) = replacement { + change_size = text.chars().count() as isize - change_size; + } else { + change_size = -change_size; + } + + if let Some(end_range) = end_range { + let offset_range = Range::new( + (end_range.anchor as isize + offset) as usize, + (end_range.head as isize + offset) as usize, + ); + + log::trace!("end range {:?} offset to: {:?}", end_range, offset_range); + + end_ranges.push(offset_range); + } + + offset += change_size; + + log::trace!( + "from: {}, to: {}, replacement: {:?}, offset: {}", + from, + to, + replacement, + offset + ); + + (from, to, replacement) + }); + + transaction.with_selection(Selection::new(end_ranges, selection.primary_index())) + } + /// Insert text at each selection head. pub fn insert(doc: &Rope, selection: &Selection, text: Tendril) -> Self { Self::change_by_selection(doc, selection, |range| { From 0bafcdb10590c6af2f59e1010d8d6becb49617bd Mon Sep 17 00:00:00 2001 From: Skyler Hawthorne Date: Mon, 29 May 2023 00:49:49 -0400 Subject: [PATCH 2/8] backfill auto pair delete tests --- helix-term/tests/test/auto_pairs.rs | 549 ++++++++++++++++++++++++++++ 1 file changed, 549 insertions(+) diff --git a/helix-term/tests/test/auto_pairs.rs b/helix-term/tests/test/auto_pairs.rs index c921e2ae7..1768ec84f 100644 --- a/helix-term/tests/test/auto_pairs.rs +++ b/helix-term/tests/test/auto_pairs.rs @@ -567,3 +567,552 @@ async fn append_inside_nested_pair_multi() -> anyhow::Result<()> { Ok(()) } + +#[tokio::test(flavor = "multi_thread")] +async fn delete_basic() -> anyhow::Result<()> { + for pair in DEFAULT_PAIRS { + test(( + format!("{}#[|{}]#{}", pair.0, pair.1, LINE_END), + "i", + format!("#[{}|]#", LINE_END), + )) + .await?; + } + + Ok(()) +} + +#[tokio::test(flavor = "multi_thread")] +async fn delete_multi() -> anyhow::Result<()> { + for pair in DEFAULT_PAIRS { + test(( + format!("{}#[|{}]#{}", pair.0, pair.1, LINE_END), + "i", + format!("#[{}|]#", LINE_END), + )) + .await?; + } + + Ok(()) +} + +#[tokio::test(flavor = "multi_thread")] +async fn delete_configured_multi_byte_chars() -> anyhow::Result<()> { + // NOTE: these are multi-byte Unicode characters + let pairs = hashmap!('„' => '“', '‚' => '‘', '「' => '」'); + + let config = Config { + editor: helix_view::editor::Config { + auto_pairs: AutoPairConfig::Pairs(pairs.clone()), + ..Default::default() + }, + ..Default::default() + }; + + for (open, close) in pairs.iter() { + test_with_config( + AppBuilder::new().with_config(config.clone()), + ( + format!("{}#[|{}]#{}", open, close, LINE_END), + "i", + format!("#[{}|]#", LINE_END), + ), + ) + .await?; + } + + Ok(()) +} + +#[tokio::test(flavor = "multi_thread")] +async fn delete_after_word() -> anyhow::Result<()> { + for pair in DEFAULT_PAIRS { + test(( + format!("foo{}#[|{}]#{}", pair.0, pair.1, LINE_END), + "i", + format!("foo#[{}|]#", LINE_END), + )) + .await?; + } + + Ok(()) +} + +#[tokio::test(flavor = "multi_thread")] +async fn delete_before_word() -> anyhow::Result<()> { + for pair in DEFAULT_PAIRS { + // sanity check unclosed pair delete + test(( + format!("{}#[|f]#oo{}", pair.0, LINE_END), + "i", + format!("#[|f]#oo{}", LINE_END), + )) + .await?; + + // deleting the closing pair should NOT delete the whole pair + test(( + format!("{}{}#[|f]#oo{}", pair.0, pair.1, LINE_END), + "i", + format!("{}#[|f]#oo{}", pair.0, LINE_END), + )) + .await?; + + // deleting whole pair before word + test(( + format!("{}#[|{}]#foo{}", pair.0, pair.1, LINE_END), + "i", + format!("#[f|]#oo{}", LINE_END), + )) + .await?; + } + + Ok(()) +} + +#[tokio::test(flavor = "multi_thread")] +async fn delete_before_word_selection() -> anyhow::Result<()> { + for pair in DEFAULT_PAIRS { + // sanity check unclosed pair delete + test(( + format!("{}#[|foo]#{}", pair.0, LINE_END), + "i", + format!("#[foo|]#{}", LINE_END), + )) + .await?; + + // deleting the closing pair should NOT delete the whole pair + test(( + format!("{}{}#[|foo]#{}", pair.0, pair.1, LINE_END), + "i", + format!("{}#[foo|]#{}", pair.0, LINE_END), + )) + .await?; + + // deleting whole pair before word + test(( + format!("{}#[|{}foo]#{}", pair.0, pair.1, LINE_END), + "i", + format!("#[foo|]#{}", LINE_END), + )) + .await?; + } + + Ok(()) +} + +#[tokio::test(flavor = "multi_thread")] +async fn delete_before_word_selection_trailing_word() -> anyhow::Result<()> { + for pair in DEFAULT_PAIRS { + test(( + format!("foo{}#[|{} wor]#{}", pair.0, pair.1, LINE_END), + "i", + format!("foo#[ wor|]#{}", LINE_END), + )) + .await?; + } + + Ok(()) +} + +#[tokio::test(flavor = "multi_thread")] +async fn delete_before_eol() -> anyhow::Result<()> { + for pair in DEFAULT_PAIRS { + test(( + format!( + "{eol}{open}#[|{close}]#{eol}", + eol = LINE_END, + open = pair.0, + close = pair.1 + ), + "i", + format!("{0}#[{0}|]#", LINE_END), + )) + .await?; + } + + Ok(()) +} + +#[tokio::test(flavor = "multi_thread")] +async fn delete_auto_pairs_disabled() -> anyhow::Result<()> { + for pair in DEFAULT_PAIRS { + test_with_config( + AppBuilder::new().with_config(Config { + editor: helix_view::editor::Config { + auto_pairs: AutoPairConfig::Enable(false), + ..Default::default() + }, + ..Default::default() + }), + ( + format!("{}#[|{}]#{}", pair.0, pair.1, LINE_END), + "i", + format!("#[|{}]#{}", pair.1, LINE_END), + ), + ) + .await?; + } + + 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 { + test(( + format!("hello {}#[|👨‍👩‍👧‍👦]# goodbye{}", pair.1, LINE_END), + "i", + format!("hello #[|👨‍👩‍👧‍👦]# goodbye{}", LINE_END), + )) + .await?; + + test(( + format!( + "hello {}{}#[|👨‍👩‍👧‍👦]# goodbye{}", + pair.0, pair.1, LINE_END + ), + "i", + format!("hello {}#[|👨‍👩‍👧‍👦]# goodbye{}", pair.0, LINE_END), + )) + .await?; + + test(( + format!( + "hello {}#[|{}]#👨‍👩‍👧‍👦 goodbye{}", + pair.0, pair.1, LINE_END + ), + "i", + format!("hello #[👨‍👩‍👧‍👦|]# goodbye{}", LINE_END), + )) + .await?; + + test(( + format!( + "hello {}#[|{}👨‍👩‍👧‍👦]# goodbye{}", + pair.0, pair.1, LINE_END + ), + "i", + format!("hello #[👨‍👩‍👧‍👦|]# goodbye{}", LINE_END), + )) + .await?; + } + Ok(()) +} + +#[tokio::test(flavor = "multi_thread")] +async fn delete_at_end_of_document() -> anyhow::Result<()> { + for pair in DEFAULT_PAIRS { + test(TestCase { + in_text: format!("{}{}{}", LINE_END, pair.0, pair.1), + in_selection: Selection::single(LINE_END.len() + 1, LINE_END.len() + 2), + in_keys: String::from("i"), + out_text: String::from(LINE_END), + out_selection: Selection::single(LINE_END.len(), LINE_END.len()), + }) + .await?; + + test(TestCase { + in_text: format!("foo{}{}{}", LINE_END, pair.0, pair.1), + in_selection: Selection::single(LINE_END.len() + 4, LINE_END.len() + 5), + in_keys: String::from("i"), + out_text: format!("foo{}", LINE_END), + out_selection: Selection::single(3 + LINE_END.len(), 3 + LINE_END.len()), + }) + .await?; + } + + Ok(()) +} + +#[tokio::test(flavor = "multi_thread")] +async fn delete_nested_open_inside_pair() -> anyhow::Result<()> { + for pair in differing_pairs() { + test(( + format!( + "{open}{open}#[|{close}]#{close}{eol}", + open = pair.0, + close = pair.1, + eol = LINE_END + ), + "i", + format!( + "{open}#[{close}|]#{eol}", + open = pair.0, + close = pair.1, + eol = LINE_END + ), + )) + .await?; + } + + Ok(()) +} + +#[tokio::test(flavor = "multi_thread")] +async fn delete_nested_open_inside_pair_multi() -> anyhow::Result<()> { + for outer_pair in DEFAULT_PAIRS { + for inner_pair in DEFAULT_PAIRS { + if inner_pair.0 == outer_pair.0 { + continue; + } + + test(( + format!( + "{outer_open}{inner_open}#[|{inner_close}]#{outer_close}{eol}{outer_open}{inner_open}#(|{inner_close})#{outer_close}{eol}{outer_open}{inner_open}#(|{inner_close})#{outer_close}{eol}", + outer_open = outer_pair.0, + outer_close = outer_pair.1, + inner_open = inner_pair.0, + inner_close = inner_pair.1, + eol = LINE_END + ), + "i", + format!( + "{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 + ), + )) + .await?; + } + } + + Ok(()) +} + +#[tokio::test(flavor = "multi_thread")] +async fn delete_append_basic() -> anyhow::Result<()> { + for pair in DEFAULT_PAIRS { + test(( + format!( + "#[{eol}{open}|]#{close}{eol}", + open = pair.0, + close = pair.1, + eol = LINE_END + ), + "a", + format!("#[{eol}{eol}|]#", eol = LINE_END), + )) + .await?; + } + + Ok(()) +} + +#[tokio::test(flavor = "multi_thread")] +async fn delete_append_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 + ), + "a", + format!("#[ {eol}|]##( {eol}|)##( {eol}|)#", eol = LINE_END), + )) + .await?; + } + + Ok(()) +} + +#[tokio::test(flavor = "multi_thread")] +async fn delete_append_end_of_word() -> anyhow::Result<()> { + for pair in DEFAULT_PAIRS { + test(( + format!( + "fo#[o{open}|]#{close}{eol}", + open = pair.0, + close = pair.1, + eol = LINE_END + ), + "a", + format!("fo#[o{}|]#", LINE_END), + )) + .await?; + } + + Ok(()) +} + +#[tokio::test(flavor = "multi_thread")] +async fn delete_mixed_dedent() -> anyhow::Result<()> { + for pair in DEFAULT_PAIRS { + test(( + helpers::platform_line(&format!( + indoc! {"\ + bar = {}#[|{}]# + #(|\n)# + foo#(|\n)# + "}, + pair.0, pair.1, + )), + "i", + helpers::platform_line(indoc! {"\ + bar = #[\n|]# + #(|\n)# + fo#(|\n)# + "}), + )) + .await?; + + test(( + helpers::platform_line(&format!( + indoc! {"\ + bar = {}#[|{}woop]# + #(|word)# + fo#(|o)# + "}, + pair.0, pair.1, + )), + "i", + helpers::platform_line(indoc! {"\ + bar = #[woop|]# + #(|word)# + f#(|o)# + "}), + )) + .await?; + + // delete from the right with append + test(( + helpers::platform_line(&format!( + indoc! {"\ + bar = #[|woop{}]#{} + #(| )# word + #(|fo)#o + "}, + pair.0, pair.1, + )), + "a", + helpers::platform_line(indoc! {"\ + bar = #[woop|]# + #(|w)#ord + #(|fo)# + "}), + )) + .await?; + } + + Ok(()) +} + +#[tokio::test(flavor = "multi_thread")] +async fn delete_append_end_of_word_multi() -> anyhow::Result<()> { + for pair in DEFAULT_PAIRS { + test(( + format!( + "fo#[o{open}|]#{close}{eol}fo#(o{open}|)#{close}{eol}fo#(o{open}|)#{close}{eol}", + open = pair.0, + close = pair.1, + eol = LINE_END + ), + "a", + format!("fo#[o{eol}|]#fo#(o{eol}|)#fo#(o{eol}|)#", eol = LINE_END), + )) + .await?; + } + + Ok(()) +} + +#[tokio::test(flavor = "multi_thread")] +async fn delete_append_inside_nested_pair() -> anyhow::Result<()> { + for pair in DEFAULT_PAIRS { + test(( + format!( + "f#[oo{open}{open}|]#{close}{close}{eol}", + open = pair.0, + close = pair.1, + eol = LINE_END + ), + "a", + format!( + "f#[oo{open}{close}|]#{eol}", + open = pair.0, + close = pair.1, + eol = LINE_END + ), + )) + .await?; + } + + Ok(()) +} + +#[tokio::test(flavor = "multi_thread")] +async fn delete_append_middle_of_word() -> anyhow::Result<()> { + for pair in DEFAULT_PAIRS { + test(( + format!( + "f#[oo{open}{open}|]#{close}{close}{eol}", + open = pair.0, + close = pair.1, + eol = LINE_END + ), + "a", + format!( + "f#[oo{open}{close}|]#{eol}", + open = pair.0, + close = pair.1, + eol = LINE_END + ), + )) + .await?; + } + + Ok(()) +} + +#[tokio::test(flavor = "multi_thread")] +async fn delete_append_inside_nested_pair_multi() -> anyhow::Result<()> { + for outer_pair in DEFAULT_PAIRS { + for inner_pair in DEFAULT_PAIRS { + if inner_pair.0 == outer_pair.0 { + continue; + } + + test(( + format!( + "f#[oo{outer_open}{inner_open}|]#{inner_close}{outer_close}{eol}f#(oo{outer_open}{inner_open}|)#{inner_close}{outer_close}{eol}f#(oo{outer_open}{inner_open}|)#{inner_close}{outer_close}{eol}", + outer_open = outer_pair.0, + outer_close = outer_pair.1, + inner_open = inner_pair.0, + inner_close = inner_pair.1, + eol = LINE_END + ), + "a", + format!( + "f#[oo{outer_open}{outer_close}|]#{eol}f#(oo{outer_open}{outer_close}|)#{eol}f#(oo{outer_open}{outer_close}|)#{eol}", + outer_open = outer_pair.0, + outer_close = outer_pair.1, + eol = LINE_END + ), + )) + .await?; + } + } + + Ok(()) +} From d319dbfcc0d2546f8299c3d62f557e7855079b73 Mon Sep 17 00:00:00 2001 From: Skyler Hawthorne Date: Thu, 1 Jun 2023 14:34:52 -0400 Subject: [PATCH 3/8] Change auto pair hook to operate on single changes Change the auto pair hook to operate on single ranges to allow transactions that mix auto pair changes with other operations, such as inserting or deleting a single char, and denendting. --- helix-core/src/auto_pairs.rs | 146 ++++++++++++++-------------------- helix-core/src/transaction.rs | 84 +++++++++++-------- helix-term/src/commands.rs | 36 ++++----- 3 files changed, 128 insertions(+), 138 deletions(-) diff --git a/helix-core/src/auto_pairs.rs b/helix-core/src/auto_pairs.rs index e319b76a5..2fd373ded 100644 --- a/helix-core/src/auto_pairs.rs +++ b/helix-core/src/auto_pairs.rs @@ -1,7 +1,7 @@ //! When typing the opening character of one of the possible pairs defined below, //! this module provides the functionality to insert the paired closing character. -use crate::{graphemes, movement::Direction, Range, Rope, Selection, Tendril, Transaction}; +use crate::{graphemes, movement::Direction, Change, Range, Rope, Tendril}; use std::collections::HashMap; // Heavily based on https://github.com/codemirror/closebrackets/ @@ -104,31 +104,23 @@ impl Default for AutoPairs { } } -// insert hook: -// Fn(doc, selection, char) => Option -// problem is, we want to do this per range, so we can call default handler for some ranges -// so maybe ret Vec> -// but we also need to be able to return transactions... -// -// to simplify, maybe return Option and just reimplement the default - // [TODO] // * delete implementation where it erases the whole bracket (|) -> | // * change to multi character pairs to handle cases like placing the cursor in the // middle of triple quotes, and more exotic pairs like Jinja's {% %} #[must_use] -pub fn hook(doc: &Rope, selection: &Selection, ch: char, pairs: &AutoPairs) -> Option { - log::trace!("autopairs hook selection: {:#?}", selection); +pub fn hook(doc: &Rope, range: &Range, ch: char, pairs: &AutoPairs) -> Option<(Change, Range)> { + log::trace!("autopairs hook range: {:#?}", range); if let Some(pair) = pairs.get(ch) { if pair.same() { - return Some(handle_same(doc, selection, pair)); + return handle_same(doc, range, pair); } else if pair.open == ch { - return Some(handle_open(doc, selection, pair)); + return handle_open(doc, range, pair); } else if pair.close == ch { // && char_at pos == close - return Some(handle_close(doc, selection, pair)); + return handle_close(doc, range, pair); } } @@ -251,94 +243,76 @@ fn get_next_range(doc: &Rope, start_range: &Range, len_inserted: usize) -> Range Range::new(end_anchor, end_head) } -fn handle_open(doc: &Rope, selection: &Selection, pair: &Pair) -> Transaction { - let transaction = Transaction::change_by_and_with_selection(doc, selection, |start_range| { - let cursor = start_range.cursor(doc.slice(..)); - let next_char = doc.get_char(cursor); - let len_inserted; +fn handle_open(doc: &Rope, range: &Range, pair: &Pair) -> Option<(Change, Range)> { + let cursor = range.cursor(doc.slice(..)); + let next_char = doc.get_char(cursor); + let len_inserted; - // Since auto pairs are currently limited to single chars, we're either - // inserting exactly one or two chars. When arbitrary length pairs are - // added, these will need to be changed. - let change = match next_char { - Some(_) if !pair.should_close(doc, start_range) => { - len_inserted = 1; - let mut tendril = Tendril::new(); - tendril.push(pair.open); - (cursor, cursor, Some(tendril)) - } - _ => { - // insert open & close - let pair_str = Tendril::from_iter([pair.open, pair.close]); - len_inserted = 2; - (cursor, cursor, Some(pair_str)) - } - }; + // Since auto pairs are currently limited to single chars, we're either + // inserting exactly one or two chars. When arbitrary length pairs are + // added, these will need to be changed. + let change = match next_char { + Some(_) if !pair.should_close(doc, range) => { + return None; + } + _ => { + // insert open & close + let pair_str = Tendril::from_iter([pair.open, pair.close]); + len_inserted = 2; + (cursor, cursor, Some(pair_str)) + } + }; - let next_range = get_next_range(doc, start_range, len_inserted); + let next_range = get_next_range(doc, range, len_inserted); + let result = (change, next_range); - (change, Some(next_range)) - }); + log::debug!("auto pair change: {:#?}", &result); - log::debug!("auto pair transaction: {:#?}", transaction); - transaction + Some(result) } -fn handle_close(doc: &Rope, selection: &Selection, pair: &Pair) -> Transaction { - let transaction = Transaction::change_by_and_with_selection(doc, selection, |start_range| { - let cursor = start_range.cursor(doc.slice(..)); - let next_char = doc.get_char(cursor); - let mut len_inserted = 0; +fn handle_close(doc: &Rope, range: &Range, pair: &Pair) -> Option<(Change, Range)> { + let cursor = range.cursor(doc.slice(..)); + let next_char = doc.get_char(cursor); - let change = if next_char == Some(pair.close) { - // return transaction that moves past close - (cursor, cursor, None) // no-op - } else { - len_inserted = 1; - let mut tendril = Tendril::new(); - tendril.push(pair.close); - (cursor, cursor, Some(tendril)) - }; + let change = if next_char == Some(pair.close) { + // return transaction that moves past close + (cursor, cursor, None) // no-op + } else { + return None; + }; - let next_range = get_next_range(doc, start_range, len_inserted); + let next_range = get_next_range(doc, range, 0); + let result = (change, next_range); - (change, Some(next_range)) - }); + log::debug!("auto pair change: {:#?}", &result); - log::debug!("auto pair transaction: {:#?}", transaction); - transaction + Some(result) } /// handle cases where open and close is the same, or in triples ("""docstring""") -fn handle_same(doc: &Rope, selection: &Selection, pair: &Pair) -> Transaction { - let transaction = Transaction::change_by_and_with_selection(doc, selection, |start_range| { - let cursor = start_range.cursor(doc.slice(..)); - let mut len_inserted = 0; - let next_char = doc.get_char(cursor); +fn handle_same(doc: &Rope, range: &Range, pair: &Pair) -> Option<(Change, Range)> { + let cursor = range.cursor(doc.slice(..)); + let mut len_inserted = 0; + let next_char = doc.get_char(cursor); - let change = if next_char == Some(pair.open) { - // return transaction that moves past close - (cursor, cursor, None) // no-op - } else { - let mut pair_str = Tendril::new(); - pair_str.push(pair.open); + let change = if next_char == Some(pair.open) { + // return transaction that moves past close + (cursor, cursor, None) // no-op + } else { + if !pair.should_close(doc, range) { + return None; + } - // for equal pairs, don't insert both open and close if either - // side has a non-pair char - if pair.should_close(doc, start_range) { - pair_str.push(pair.close); - } + let pair_str = Tendril::from_iter([pair.open, pair.close]); + len_inserted = 2; + (cursor, cursor, Some(pair_str)) + }; - len_inserted += pair_str.chars().count(); - (cursor, cursor, Some(pair_str)) - }; + let next_range = get_next_range(doc, range, len_inserted); + let result = (change, next_range); - let next_range = get_next_range(doc, start_range, len_inserted); + log::debug!("auto pair change: {:#?}", &result); - (change, Some(next_range)) - }); - - log::debug!("auto pair transaction: {:#?}", transaction); - - transaction + Some(result) } diff --git a/helix-core/src/transaction.rs b/helix-core/src/transaction.rs index 5aae801cd..41402beee 100644 --- a/helix-core/src/transaction.rs +++ b/helix-core/src/transaction.rs @@ -513,6 +513,49 @@ impl ChangeSet { pub fn changes_iter(&self) -> ChangeIterator { ChangeIterator::new(self) } + + pub fn from_change(doc: &Rope, change: Change) -> Self { + Self::from_changes(doc, std::iter::once(change)) + } + + /// Generate a ChangeSet from a set of changes. + pub fn from_changes(doc: &Rope, changes: I) -> Self + where + I: Iterator, + { + let len = doc.len_chars(); + + let (lower, upper) = changes.size_hint(); + let size = upper.unwrap_or(lower); + let mut changeset = ChangeSet::with_capacity(2 * size + 1); // rough estimate + + let mut last = 0; + for (from, to, tendril) in changes { + // Verify ranges are ordered and not overlapping + debug_assert!(last <= from); + // Verify ranges are correct + 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); + let span = to - from; + match tendril { + Some(text) => { + changeset.insert(text); + changeset.delete(span); + } + None => changeset.delete(span), + } + last = to; + } + + changeset.retain(len - last); + + changeset + } } /// Transaction represents a single undoable unit of changes. Several changes can be grouped into @@ -606,38 +649,7 @@ impl Transaction { where I: Iterator, { - let len = doc.len_chars(); - - let (lower, upper) = changes.size_hint(); - let size = upper.unwrap_or(lower); - let mut changeset = ChangeSet::with_capacity(2 * size + 1); // rough estimate - - let mut last = 0; - for (from, to, tendril) in changes { - // Verify ranges are ordered and not overlapping - debug_assert!(last <= from); - // Verify ranges are correct - 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); - let span = to - from; - match tendril { - Some(text) => { - changeset.insert(text); - changeset.delete(span); - } - None => changeset.delete(span), - } - last = to; - } - - changeset.retain(len - last); - - Self::from(changeset) + Self::from(ChangeSet::from_changes(doc, changes)) } /// Generate a transaction from a set of potentially overlapping deletions @@ -739,8 +751,8 @@ impl Transaction { /// Generate a transaction with a change per selection range, which /// generates a new selection as well. Each range is operated upon by /// the given function and can optionally produce a new range. If none - /// is returned by the function, that range is dropped from the resulting - /// selection. + /// is returned by the function, that range is mapped through the change + /// as usual. pub fn change_by_and_with_selection(doc: &Rope, selection: &Selection, mut f: F) -> Self where F: FnMut(&Range) -> (Change, Option), @@ -767,6 +779,10 @@ impl Transaction { log::trace!("end range {:?} offset to: {:?}", end_range, offset_range); end_ranges.push(offset_range); + } else { + let changeset = ChangeSet::from_change(doc, (from, to, replacement.clone())); + let end_range = start_range.map(&changeset); + end_ranges.push(end_range); } offset += change_size; diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs index 10b7b1662..62111b6bf 100644 --- a/helix-term/src/commands.rs +++ b/helix-term/src/commands.rs @@ -4108,16 +4108,6 @@ pub mod insert { } } - // The default insert hook: simply insert the character - #[allow(clippy::unnecessary_wraps)] // need to use Option<> because of the Hook signature - fn insert(doc: &Rope, selection: &Selection, ch: char) -> Option { - let cursors = selection.clone().cursors(doc.slice(..)); - let mut t = Tendril::new(); - t.push(ch); - let transaction = Transaction::insert(doc, &cursors, t); - Some(transaction) - } - use helix_core::auto_pairs; use helix_view::editor::SmartTabConfig; @@ -4127,15 +4117,25 @@ pub mod insert { let selection = doc.selection(view.id); let auto_pairs = doc.auto_pairs(cx.editor); - let transaction = auto_pairs - .as_ref() - .and_then(|ap| auto_pairs::hook(text, selection, c, ap)) - .or_else(|| insert(text, selection, c)); + let insert_char = |range: Range, ch: char| { + let cursor = range.cursor(text.slice(..)); + let t = Tendril::from_iter([ch]); + ((cursor, cursor, Some(t)), None) + }; - let (view, doc) = current!(cx.editor); - if let Some(t) = transaction { - doc.apply(&t, view.id); - } + let transaction = Transaction::change_by_and_with_selection(text, selection, |range| { + auto_pairs + .as_ref() + .and_then(|ap| { + auto_pairs::hook(text, range, c, ap) + .map(|(change, range)| (change, Some(range))) + .or(Some(insert_char(*range, c))) + }) + .unwrap_or_else(|| insert_char(*range, c)) + }); + + let doc = doc_mut!(cx.editor, &doc.id()); + doc.apply(&transaction, view.id); helix_event::dispatch(PostInsertChar { c, cx }); } From e046bb75a6b92a65732c51fe0af2a5a0bd8424f9 Mon Sep 17 00:00:00 2001 From: Skyler Hawthorne Date: Sat, 3 Jun 2023 11:06:31 -0400 Subject: [PATCH 4/8] add delete_by_and_with_selection --- helix-core/src/auto_pairs.rs | 19 +++++++---- helix-core/src/transaction.rs | 64 +++++++++++++++++++++++++++++------ helix-term/src/commands.rs | 33 ++++++++++++------ 3 files changed, 89 insertions(+), 27 deletions(-) diff --git a/helix-core/src/auto_pairs.rs b/helix-core/src/auto_pairs.rs index 2fd373ded..467d55d92 100644 --- a/helix-core/src/auto_pairs.rs +++ b/helix-core/src/auto_pairs.rs @@ -110,17 +110,22 @@ impl Default for AutoPairs { // middle of triple quotes, and more exotic pairs like Jinja's {% %} #[must_use] -pub fn hook(doc: &Rope, range: &Range, ch: char, pairs: &AutoPairs) -> Option<(Change, Range)> { +pub fn hook_insert( + doc: &Rope, + range: &Range, + ch: char, + pairs: &AutoPairs, +) -> Option<(Change, Range)> { log::trace!("autopairs hook range: {:#?}", range); if let Some(pair) = pairs.get(ch) { if pair.same() { - return handle_same(doc, range, pair); + return handle_insert_same(doc, range, pair); } else if pair.open == ch { - return handle_open(doc, range, pair); + return handle_insert_open(doc, range, pair); } else if pair.close == ch { // && char_at pos == close - return handle_close(doc, range, pair); + return handle_insert_close(doc, range, pair); } } @@ -243,7 +248,7 @@ fn get_next_range(doc: &Rope, start_range: &Range, len_inserted: usize) -> Range Range::new(end_anchor, end_head) } -fn handle_open(doc: &Rope, range: &Range, pair: &Pair) -> Option<(Change, Range)> { +fn handle_insert_open(doc: &Rope, range: &Range, pair: &Pair) -> Option<(Change, Range)> { let cursor = range.cursor(doc.slice(..)); let next_char = doc.get_char(cursor); let len_inserted; @@ -271,7 +276,7 @@ fn handle_open(doc: &Rope, range: &Range, pair: &Pair) -> Option<(Change, Range) Some(result) } -fn handle_close(doc: &Rope, range: &Range, pair: &Pair) -> Option<(Change, Range)> { +fn handle_insert_close(doc: &Rope, range: &Range, pair: &Pair) -> Option<(Change, Range)> { let cursor = range.cursor(doc.slice(..)); let next_char = doc.get_char(cursor); @@ -291,7 +296,7 @@ fn handle_close(doc: &Rope, range: &Range, pair: &Pair) -> Option<(Change, Range } /// handle cases where open and close is the same, or in triples ("""docstring""") -fn handle_same(doc: &Rope, range: &Range, pair: &Pair) -> Option<(Change, Range)> { +fn handle_insert_same(doc: &Rope, range: &Range, pair: &Pair) -> Option<(Change, Range)> { let cursor = range.cursor(doc.slice(..)); let mut len_inserted = 0; let next_char = doc.get_char(cursor); diff --git a/helix-core/src/transaction.rs b/helix-core/src/transaction.rs index 41402beee..1a86e72aa 100644 --- a/helix-core/src/transaction.rs +++ b/helix-core/src/transaction.rs @@ -738,16 +738,6 @@ 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)) - } - /// Generate a transaction with a change per selection range, which /// generates a new selection as well. Each range is operated upon by /// the given function and can optionally produce a new range. If none @@ -801,6 +791,60 @@ impl Transaction { transaction.with_selection(Selection::new(end_ranges, selection.primary_index())) } + /// 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)) + } + + /// Generate a transaction with a delete per selection range, which + /// generates a new selection as well. Each range is operated upon by + /// the given function and can optionally produce a new range. If none + /// is returned by the function, that range is mapped through the change + /// as usual. + /// + /// Compared to using `change_by_and_with_selection` directly these ranges + /// may overlap. In that case they are merged. + pub fn delete_by_and_with_selection(doc: &Rope, selection: &Selection, mut f: F) -> Self + where + F: FnMut(&Range) -> (Deletion, Option), + { + let mut end_ranges = SmallVec::with_capacity(selection.len()); + let mut offset = 0; + + let transaction = Transaction::delete_by_selection(doc, selection, |start_range| { + let ((from, to), end_range) = f(start_range); + let change_size = to - from; + + if let Some(end_range) = end_range { + let offset_range = Range::new( + end_range.anchor.saturating_sub(offset), + end_range.head.saturating_sub(offset), + ); + + log::trace!("end range {:?} offset to: {:?}", end_range, offset_range); + + end_ranges.push(offset_range); + } else { + let changeset = ChangeSet::from_change(doc, (from, to, None)); + let end_range = start_range.map(&changeset); + end_ranges.push(end_range); + } + + offset += change_size; + + log::trace!("delete from: {}, to: {}, offset: {}", from, to, offset); + + (from, to) + }); + + transaction.with_selection(Selection::new(end_ranges, selection.primary_index())) + } + /// 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 62111b6bf..b0e16d66e 100644 --- a/helix-term/src/commands.rs +++ b/helix-term/src/commands.rs @@ -4127,7 +4127,7 @@ pub mod insert { auto_pairs .as_ref() .and_then(|ap| { - auto_pairs::hook(text, range, c, ap) + auto_pairs::hook_insert(text, range, c, ap) .map(|(change, range)| (change, Some(range))) .or(Some(insert_char(*range, c))) }) @@ -4338,11 +4338,13 @@ pub mod insert { let indent_width = doc.indent_width(); let auto_pairs = doc.auto_pairs(cx.editor); - let transaction = - Transaction::delete_by_selection(doc.text(), doc.selection(view.id), |range| { + let transaction = Transaction::delete_by_and_with_selection( + doc.text(), + doc.selection(view.id), + |range| { let pos = range.cursor(text); if pos == 0 { - return (pos, pos); + return ((pos, pos), None); } 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. @@ -4350,7 +4352,10 @@ 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) + ( + (graphemes::nth_prev_grapheme_boundary(text, pos, 1), pos), + None, + ) } else { let width: usize = fragment .chars() @@ -4377,7 +4382,7 @@ pub mod insert { _ => break, } } - (start, pos) // delete! + ((start, pos), None) // delete! } } else { match ( @@ -4393,18 +4398,26 @@ pub mod insert { // delete both autopaired characters { ( - graphemes::nth_prev_grapheme_boundary(text, pos, count), - graphemes::nth_next_grapheme_boundary(text, pos, count), + ( + 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) + ( + (graphemes::nth_prev_grapheme_boundary(text, pos, count), pos), + None, + ) } } } - }); + }, + ); + let (view, doc) = current!(cx.editor); doc.apply(&transaction, view.id); } From 62b2af7e852bb1540dccd4df1a85054619e981a6 Mon Sep 17 00:00:00 2001 From: Skyler Hawthorne Date: Sat, 3 Jun 2023 14:34:19 -0400 Subject: [PATCH 5/8] Delete pairs with multi-char-range selections This completes auto pair deletions. Currently, auto pairs only get deleted when the range is a single grapheme wide, since otherwise, the selection would get computed incorrectly through the normal change mapping process. Now auto pairs get deleted even with larger ranges, and the resulting selection is correct. --- helix-core/src/auto_pairs.rs | 32 ++++++- helix-core/src/transaction.rs | 44 ++++----- helix-term/src/commands.rs | 143 ++++++++++++++-------------- helix-term/tests/test/auto_pairs.rs | 20 ++-- 4 files changed, 133 insertions(+), 106 deletions(-) diff --git a/helix-core/src/auto_pairs.rs b/helix-core/src/auto_pairs.rs index 467d55d92..fb918a9a2 100644 --- a/helix-core/src/auto_pairs.rs +++ b/helix-core/src/auto_pairs.rs @@ -1,7 +1,7 @@ //! When typing the opening character of one of the possible pairs defined below, //! this module provides the functionality to insert the paired closing character. -use crate::{graphemes, movement::Direction, Change, Range, Rope, Tendril}; +use crate::{graphemes, movement::Direction, Change, Deletion, Range, Rope, Tendril}; use std::collections::HashMap; // Heavily based on https://github.com/codemirror/closebrackets/ @@ -132,6 +132,36 @@ pub fn hook_insert( None } +#[must_use] +pub fn hook_delete(doc: &Rope, range: &Range, pairs: &AutoPairs) -> Option<(Deletion, Range)> { + let text = doc.slice(..); + let cursor = range.cursor(text); + + let cur = doc.get_char(cursor)?; + let prev = prev_char(doc, cursor)?; + let pair = pairs.get(cur)?; + + if pair.open != prev { + return None; + } + + let end_next = graphemes::next_grapheme_boundary(text, cursor); + let end_prev = graphemes::prev_grapheme_boundary(text, cursor); + + let delete = (end_prev, end_next); + 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), + }; + + log::trace!("auto pair delete: {:?}, range: {:?}", delete, range,); + + Some((delete, next_range)) +} + fn prev_char(doc: &Rope, pos: usize) -> Option { if pos == 0 { return None; diff --git a/helix-core/src/transaction.rs b/helix-core/src/transaction.rs index 1a86e72aa..c9dd438a3 100644 --- a/helix-core/src/transaction.rs +++ b/helix-core/src/transaction.rs @@ -760,21 +760,19 @@ impl Transaction { change_size = -change_size; } - if let Some(end_range) = end_range { - let offset_range = Range::new( - (end_range.anchor as isize + offset) as usize, - (end_range.head as isize + offset) as usize, - ); - - log::trace!("end range {:?} offset to: {:?}", end_range, offset_range); - - end_ranges.push(offset_range); + let new_range = if let Some(end_range) = end_range { + end_range } else { let changeset = ChangeSet::from_change(doc, (from, to, replacement.clone())); - let end_range = start_range.map(&changeset); - end_ranges.push(end_range); - } + start_range.map(&changeset) + }; + let offset_range = Range::new( + (new_range.anchor as isize + offset) as usize, + (new_range.head as isize + offset) as usize, + ); + + end_ranges.push(offset_range); offset += change_size; log::trace!( @@ -820,21 +818,19 @@ impl Transaction { let ((from, to), end_range) = f(start_range); let change_size = to - from; - if let Some(end_range) = end_range { - let offset_range = Range::new( - end_range.anchor.saturating_sub(offset), - end_range.head.saturating_sub(offset), - ); - - log::trace!("end range {:?} offset to: {:?}", end_range, offset_range); - - end_ranges.push(offset_range); + let new_range = if let Some(end_range) = end_range { + end_range } else { let changeset = ChangeSet::from_change(doc, (from, to, None)); - let end_range = start_range.map(&changeset); - end_ranges.push(end_range); - } + start_range.map(&changeset) + }; + let offset_range = Range::new( + new_range.anchor.saturating_sub(offset), + new_range.head.saturating_sub(offset), + ); + + end_ranges.push(offset_range); offset += change_size; log::trace!("delete from: {}, to: {}, offset: {}", from, to, offset); diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs index b0e16d66e..9f729fa90 100644 --- a/helix-term/src/commands.rs +++ b/helix-term/src/commands.rs @@ -4129,7 +4129,7 @@ pub mod insert { .and_then(|ap| { auto_pairs::hook_insert(text, range, c, ap) .map(|(change, range)| (change, Some(range))) - .or(Some(insert_char(*range, c))) + .or_else(|| Some(insert_char(*range, c))) }) .unwrap_or_else(|| insert_char(*range, c)) }); @@ -4330,95 +4330,96 @@ pub mod insert { doc.apply(&transaction, view.id); } + fn dedent(doc: &Document, range: &Range) -> Option { + let text = doc.text().slice(..); + let pos = range.cursor(text); + 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. + let fragment = Cow::from(text.slice(line_start_pos..pos)); + + if fragment.is_empty() || !fragment.chars().all(|ch| ch == ' ' || ch == '\t') { + return None; + } + + if text.get_char(pos.saturating_sub(1)) == Some('\t') { + // fast path, delete one char + return Some((graphemes::nth_prev_grapheme_boundary(text, pos, 1), pos)); + } + + let tab_width = doc.tab_width(); + let indent_width = doc.indent_width(); + + let width: usize = fragment + .chars() + .map(|ch| { + if ch == '\t' { + tab_width + } else { + // it can be none if it still meet control characters other than '\t' + // here just set the width to 1 (or some value better?). + ch.width().unwrap_or(1) + } + }) + .sum(); + + // round down to nearest unit + let mut drop = width % indent_width; + + // if it's already at a unit, consume a whole unit + if drop == 0 { + drop = indent_width + }; + + let mut chars = fragment.chars().rev(); + let mut start = pos; + + for _ in 0..drop { + // delete up to `drop` spaces + match chars.next() { + Some(' ') => start -= 1, + _ => break, + } + } + + Some((start, pos)) // delete! + } + pub fn delete_char_backward(cx: &mut Context) { let count = cx.count(); let (view, doc) = current_ref!(cx.editor); let text = doc.text().slice(..); - let tab_width = doc.tab_width(); - let indent_width = doc.indent_width(); - let auto_pairs = doc.auto_pairs(cx.editor); let transaction = Transaction::delete_by_and_with_selection( doc.text(), doc.selection(view.id), |range| { let pos = range.cursor(text); + + log::debug!("cursor: {}, len: {}", pos, text.len_chars()); + if pos == 0 { return ((pos, pos), None); } - 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. - let fragment = Cow::from(text.slice(line_start_pos..pos)); - 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 + + dedent(doc, range) + .map(|dedent| (dedent, None)) + .or_else(|| { + auto_pairs::hook_delete(doc.text(), range, doc.auto_pairs(cx.editor)?) + .map(|(delete, new_range)| (delete, Some(new_range))) + }) + .unwrap_or_else(|| { ( - (graphemes::nth_prev_grapheme_boundary(text, pos, 1), pos), + (graphemes::nth_prev_grapheme_boundary(text, pos, count), pos), None, ) - } else { - let width: usize = fragment - .chars() - .map(|ch| { - if ch == '\t' { - tab_width - } else { - // it can be none if it still meet control characters other than '\t' - // here just set the width to 1 (or some value better?). - ch.width().unwrap_or(1) - } - }) - .sum(); - let mut drop = width % indent_width; // round down to nearest unit - if drop == 0 { - drop = indent_width - }; // if it's already at a unit, consume a whole unit - let mut chars = fragment.chars().rev(); - let mut start = pos; - for _ in 0..drop { - // delete up to `drop` spaces - match chars.next() { - Some(' ') => start -= 1, - _ => break, - } - } - ((start, pos), None) // delete! - } - } else { - match ( - text.get_char(pos.saturating_sub(1)), - text.get_char(pos), - auto_pairs, - ) { - (Some(_x), Some(_y), Some(ap)) - if range.is_single_grapheme(text) - && ap.get(_x).is_some() - && ap.get(_x).unwrap().open == _x - && ap.get(_x).unwrap().close == _y => - // delete both autopaired characters - { - ( - ( - 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, - ) - } - } - } + }) }, ); - let (view, doc) = current!(cx.editor); + log::debug!("delete_char_backward transaction: {:?}", transaction); + + let doc = doc_mut!(cx.editor, &doc.id()); doc.apply(&transaction, view.id); } diff --git a/helix-term/tests/test/auto_pairs.rs b/helix-term/tests/test/auto_pairs.rs index 1768ec84f..04fec1c4e 100644 --- a/helix-term/tests/test/auto_pairs.rs +++ b/helix-term/tests/test/auto_pairs.rs @@ -676,7 +676,7 @@ async fn delete_before_word_selection() -> anyhow::Result<()> { test(( format!("{}#[|foo]#{}", pair.0, LINE_END), "i", - format!("#[foo|]#{}", LINE_END), + format!("#[|foo]#{}", LINE_END), )) .await?; @@ -684,7 +684,7 @@ async fn delete_before_word_selection() -> anyhow::Result<()> { test(( format!("{}{}#[|foo]#{}", pair.0, pair.1, LINE_END), "i", - format!("{}#[foo|]#{}", pair.0, LINE_END), + format!("{}#[|foo]#{}", pair.0, LINE_END), )) .await?; @@ -692,7 +692,7 @@ async fn delete_before_word_selection() -> anyhow::Result<()> { test(( format!("{}#[|{}foo]#{}", pair.0, pair.1, LINE_END), "i", - format!("#[foo|]#{}", LINE_END), + format!("#[|foo]#{}", LINE_END), )) .await?; } @@ -706,7 +706,7 @@ async fn delete_before_word_selection_trailing_word() -> anyhow::Result<()> { test(( format!("foo{}#[|{} wor]#{}", pair.0, pair.1, LINE_END), "i", - format!("foo#[ wor|]#{}", LINE_END), + format!("foo#[| wor]#{}", LINE_END), )) .await?; } @@ -811,7 +811,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?; } @@ -988,7 +988,7 @@ async fn delete_mixed_dedent() -> anyhow::Result<()> { )), "i", helpers::platform_line(indoc! {"\ - bar = #[woop|]# + bar = #[|woop]# #(|word)# f#(|o)# "}), @@ -1000,16 +1000,16 @@ async fn delete_mixed_dedent() -> anyhow::Result<()> { helpers::platform_line(&format!( indoc! {"\ bar = #[|woop{}]#{} - #(| )# word + #(| )#word #(|fo)#o "}, pair.0, pair.1, )), "a", helpers::platform_line(indoc! {"\ - bar = #[woop|]# - #(|w)#ord - #(|fo)# + bar = #[woop\n|]# + #(w|)#ord + #(fo|)# "}), )) .await?; From d17bfea990333d608eaf18624197e4407e54b532 Mon Sep 17 00:00:00 2001 From: Skyler Hawthorne Date: Mon, 5 Jun 2023 01:16:05 -0400 Subject: [PATCH 6/8] insert double whitespace inside pair --- helix-core/src/auto_pairs.rs | 49 +++++- helix-term/tests/test/auto_pairs.rs | 247 ++++++++++++++++++++++++++-- 2 files changed, 280 insertions(+), 16 deletions(-) diff --git a/helix-core/src/auto_pairs.rs b/helix-core/src/auto_pairs.rs index fb918a9a2..37d3a2ddb 100644 --- a/helix-core/src/auto_pairs.rs +++ b/helix-core/src/auto_pairs.rs @@ -127,6 +127,8 @@ pub fn hook_insert( // && char_at pos == close return handle_insert_close(doc, range, pair); } + } else if ch.is_whitespace() { + return handle_insert_whitespace(doc, range, ch, pairs); } None @@ -139,12 +141,33 @@ pub fn hook_delete(doc: &Rope, range: &Range, pairs: &AutoPairs) -> Option<(Dele let cur = doc.get_char(cursor)?; let prev = prev_char(doc, cursor)?; + + // check for whitespace surrounding a pair + if doc.len_chars() >= 4 && prev.is_whitespace() && cur.is_whitespace() { + let second_prev = doc.get_char(graphemes::nth_prev_grapheme_boundary(text, cursor, 2))?; + let second_next = doc.get_char(graphemes::next_grapheme_boundary(text, cursor))?; + log::debug!("second_prev: {}, second_next: {}", second_prev, second_next); + + if let Some(pair) = pairs.get(second_prev) { + if pair.open == second_prev && pair.close == second_next { + return handle_delete(doc, range); + } + } + } + let pair = pairs.get(cur)?; - if pair.open != prev { + if pair.open != prev || pair.close != cur { return None; } + handle_delete(doc, range) +} + +pub fn handle_delete(doc: &Rope, range: &Range) -> Option<(Deletion, Range)> { + let text = doc.slice(..); + let cursor = range.cursor(text); + let end_next = graphemes::next_grapheme_boundary(text, cursor); let end_prev = graphemes::prev_grapheme_boundary(text, cursor); @@ -162,6 +185,30 @@ pub fn hook_delete(doc: &Rope, range: &Range, pairs: &AutoPairs) -> Option<(Dele Some((delete, next_range)) } +fn handle_insert_whitespace( + doc: &Rope, + range: &Range, + ch: char, + pairs: &AutoPairs, +) -> Option<(Change, Range)> { + let text = doc.slice(..); + let cursor = range.cursor(text); + let cur = doc.get_char(cursor)?; + let prev = prev_char(doc, cursor)?; + let pair = pairs.get(cur)?; + + if pair.open != prev || pair.close != cur { + return None; + } + + let whitespace_pair = Pair { + open: ch, + close: ch, + }; + + handle_insert_same(doc, range, &whitespace_pair) +} + fn prev_char(doc: &Rope, pos: usize) -> Option { if pos == 0 { return None; diff --git a/helix-term/tests/test/auto_pairs.rs b/helix-term/tests/test/auto_pairs.rs index 04fec1c4e..452292a1b 100644 --- a/helix-term/tests/test/auto_pairs.rs +++ b/helix-term/tests/test/auto_pairs.rs @@ -16,10 +16,119 @@ fn matching_pairs() -> impl Iterator { async fn insert_basic() -> anyhow::Result<()> { for pair in DEFAULT_PAIRS { test(( - format!("#[{}|]#", LINE_END), + "#[\n|]#", format!("i{}", pair.0), - format!("{}#[|{}]#{}", pair.0, pair.1, LINE_END), - LineFeedHandling::AsIs, + format!("{}#[|{}]#", pair.0, pair.1), + )) + .await?; + } + + Ok(()) +} + +#[tokio::test(flavor = "multi_thread")] +async fn insert_whitespace() -> anyhow::Result<()> { + for pair in DEFAULT_PAIRS { + test(( + format!("{}#[|{}]#", pair.0, pair.1), + "i ", + format!("{} #[| ]#{}", pair.0, pair.1), + )) + .await?; + } + + Ok(()) +} + +#[tokio::test(flavor = "multi_thread")] +async fn insert_whitespace_multi() -> anyhow::Result<()> { + for pair in differing_pairs() { + test(( + format!( + indoc! {"\ + {open}#[|{close}]# + {open}#(|{open})#{close}{close} + {open}{open}#(|{close}{close})# + foo#(|\n)# + "}, + open = pair.0, + close = pair.1, + ), + "i ", + format!( + indoc! {"\ + {open} #[| ]#{close} + {open} #(|{open})#{close}{close} + {open}{open} #(| {close}{close})# + foo #(|\n)# + "}, + open = pair.0, + close = pair.1, + ), + )) + .await?; + } + + Ok(()) +} + +#[tokio::test(flavor = "multi_thread")] +async fn append_whitespace_multi() -> anyhow::Result<()> { + for pair in differing_pairs() { + test(( + format!( + indoc! {"\ + #[|{open}]#{close} + #(|{open})#{open}{close}{close} + #(|{open}{open})#{close}{close} + #(|foo)# + "}, + open = pair.0, + close = pair.1, + ), + "a ", + format!( + indoc! {"\ + #[{open} |]#{close} + #({open} {open}|)#{close}{close} + #({open}{open} |)#{close}{close} + #(foo \n|)# + "}, + open = pair.0, + close = pair.1, + ), + )) + .await?; + } + + Ok(()) +} + +#[tokio::test(flavor = "multi_thread")] +async fn insert_whitespace_no_pair() -> anyhow::Result<()> { + for pair in DEFAULT_PAIRS { + // sanity check - do not insert extra whitespace unless immediately + // surrounded by a pair + test(( + format!("{} #[|{}]#", pair.0, pair.1), + "i ", + format!("{} #[|{}]#", pair.0, pair.1), + )) + .await?; + } + + Ok(()) +} + +#[tokio::test(flavor = "multi_thread")] +async fn insert_whitespace_no_matching_pair() -> anyhow::Result<()> { + for pair in differing_pairs() { + // sanity check - verify whitespace does not insert unless both pairs + // are matches, i.e. no two different openers + test(( + format!("{}#[|{}]#", pair.0, pair.0), + "i ", + format!("{} #[|{}]#", pair.0, pair.0), )) .await?; } @@ -596,6 +705,112 @@ async fn delete_multi() -> anyhow::Result<()> { Ok(()) } +#[tokio::test(flavor = "multi_thread")] +async fn delete_whitespace() -> anyhow::Result<()> { + for pair in DEFAULT_PAIRS { + test(( + format!("{} #[| ]#{}", pair.0, pair.1), + "i", + format!("{}#[{}|]#", pair.0, pair.1), + )) + .await?; + } + + Ok(()) +} + +#[tokio::test(flavor = "multi_thread")] +async fn delete_whitespace_multi() -> anyhow::Result<()> { + for pair in DEFAULT_PAIRS { + test(( + format!( + indoc! {"\ + {open} #[| ]#{close} + {open} #(|{open})#{close}{close} + {open}{open} #(| {close}{close})# + foo #(|\n)# + "}, + open = pair.0, + close = pair.1, + ), + "i", + format!( + indoc! {"\ + {open}#[{close}|]# + {open}#(|{open})#{close}{close} + {open}{open}#(|{close}{close})# + foo#(|\n)# + "}, + open = pair.0, + close = pair.1, + ), + )) + .await?; + } + + Ok(()) +} + +#[tokio::test(flavor = "multi_thread")] +async fn delete_append_whitespace_multi() -> anyhow::Result<()> { + for pair in DEFAULT_PAIRS { + test(( + format!( + indoc! {"\ + #[{open} |]# {close} + #({open} |)#{open}{close}{close} + #({open}{open} |)# {close}{close} + #(foo |)# + "}, + open = pair.0, + close = pair.1, + ), + "a", + format!( + indoc! {"\ + #[{open}{close}|]# + #({open}{open}|)#{close}{close} + #({open}{open}{close}|)#{close} + #(foo\n|)# + "}, + open = pair.0, + close = pair.1, + ), + )) + .await?; + } + + Ok(()) +} + +#[tokio::test(flavor = "multi_thread")] +async fn delete_whitespace_no_pair() -> anyhow::Result<()> { + for pair in DEFAULT_PAIRS { + test(( + format!("{} #[|{}]#", pair.0, pair.1), + "i", + format!("{} #[|{}]#", pair.0, pair.1), + )) + .await?; + } + + Ok(()) +} + +#[tokio::test(flavor = "multi_thread")] +async fn delete_whitespace_no_matching_pair() -> anyhow::Result<()> { + for pair in differing_pairs() { + test(( + format!("{} #[|{}]#", pair.0, pair.0), + "i", + format!("{}#[|{}]#", pair.0, pair.0), + )) + .await?; + } + + Ok(()) +} + #[tokio::test(flavor = "multi_thread")] async fn delete_configured_multi_byte_chars() -> anyhow::Result<()> { // NOTE: these are multi-byte Unicode characters @@ -827,6 +1042,7 @@ async fn delete_at_end_of_document() -> anyhow::Result<()> { in_keys: String::from("i"), out_text: String::from(LINE_END), out_selection: Selection::single(LINE_END.len(), LINE_END.len()), + line_feed_handling: LineFeedHandling::AsIs, }) .await?; @@ -836,6 +1052,7 @@ async fn delete_at_end_of_document() -> anyhow::Result<()> { in_keys: String::from("i"), out_text: format!("foo{}", LINE_END), out_selection: Selection::single(3 + LINE_END.len(), 3 + LINE_END.len()), + line_feed_handling: LineFeedHandling::AsIs, }) .await?; } @@ -960,57 +1177,57 @@ async fn delete_append_end_of_word() -> anyhow::Result<()> { async fn delete_mixed_dedent() -> anyhow::Result<()> { for pair in DEFAULT_PAIRS { test(( - helpers::platform_line(&format!( + format!( indoc! {"\ bar = {}#[|{}]# #(|\n)# foo#(|\n)# "}, pair.0, pair.1, - )), + ), "i", - helpers::platform_line(indoc! {"\ + indoc! {"\ bar = #[\n|]# #(|\n)# fo#(|\n)# - "}), + "}, )) .await?; test(( - helpers::platform_line(&format!( + format!( indoc! {"\ bar = {}#[|{}woop]# #(|word)# fo#(|o)# "}, pair.0, pair.1, - )), + ), "i", - helpers::platform_line(indoc! {"\ + indoc! {"\ bar = #[|woop]# #(|word)# f#(|o)# - "}), + "}, )) .await?; // delete from the right with append test(( - helpers::platform_line(&format!( + format!( indoc! {"\ bar = #[|woop{}]#{} #(| )#word #(|fo)#o "}, pair.0, pair.1, - )), + ), "a", - helpers::platform_line(indoc! {"\ + indoc! {"\ bar = #[woop\n|]# #(w|)#ord #(fo|)# - "}), + "}, )) .await?; } From 1ee34d5bb9022b41dc99013dd647fdcb2a391df4 Mon Sep 17 00:00:00 2001 From: Skyler Hawthorne Date: Mon, 5 Jun 2023 19:07:08 -0400 Subject: [PATCH 7/8] 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 37d3a2ddb..5811186cd 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 c9dd438a3..10dc6a3ee 100644 --- a/helix-core/src/transaction.rs +++ b/helix-core/src/transaction.rs @@ -813,10 +813,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 @@ -830,10 +833,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 452292a1b..a85b5ea69 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)# "}, From fb5b557957583cd164aa9026f8aec21ab5e2b88c Mon Sep 17 00:00:00 2001 From: Skyler Hawthorne Date: Sun, 7 Apr 2024 16:08:28 -0400 Subject: [PATCH 8/8] fix as-is tests --- helix-term/tests/test/auto_pairs.rs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/helix-term/tests/test/auto_pairs.rs b/helix-term/tests/test/auto_pairs.rs index a85b5ea69..8048d97f1 100644 --- a/helix-term/tests/test/auto_pairs.rs +++ b/helix-term/tests/test/auto_pairs.rs @@ -684,6 +684,7 @@ async fn delete_basic() -> anyhow::Result<()> { format!("{}#[|{}]#{}", pair.0, pair.1, LINE_END), "i", format!("#[|{}]#", LINE_END), + LineFeedHandling::AsIs, )) .await?; } @@ -857,6 +858,7 @@ async fn delete_configured_multi_byte_chars() -> anyhow::Result<()> { format!("{}#[|{}]#{}", open, close, LINE_END), "i", format!("#[|{}]#", LINE_END), + LineFeedHandling::AsIs, ), ) .await?; @@ -1053,6 +1055,7 @@ async fn delete_before_eol() -> anyhow::Result<()> { ), "i", format!("{0}#[|{0}]#", LINE_END), + LineFeedHandling::AsIs, )) .await?; } @@ -1221,6 +1224,7 @@ async fn delete_append_basic() -> anyhow::Result<()> { ), "a", format!("#[{eol}{eol}|]#", eol = LINE_END), + LineFeedHandling::AsIs, )) .await?; } @@ -1240,6 +1244,7 @@ async fn delete_append_multi_range() -> anyhow::Result<()> { ), "a", format!("#[ {eol}|]##( {eol}|)##( {eol}|)#", eol = LINE_END), + LineFeedHandling::AsIs, )) .await?; } @@ -1259,6 +1264,7 @@ async fn delete_append_end_of_word() -> anyhow::Result<()> { ), "a", format!("fo#[o{}|]#", LINE_END), + LineFeedHandling::AsIs, )) .await?; } @@ -1340,6 +1346,7 @@ async fn delete_append_end_of_word_multi() -> anyhow::Result<()> { ), "a", format!("fo#[o{eol}|]#fo#(o{eol}|)#fo#(o{eol}|)#", eol = LINE_END), + LineFeedHandling::AsIs, )) .await?; }