From 6a0b450f55675c76d67bfb026caa2df4b601153b Mon Sep 17 00:00:00 2001 From: Skyler Hawthorne Date: Thu, 20 Oct 2022 20:22:20 -0400 Subject: Fix multi byte auto pairs (#4024) * Fix test::print for Unicode The print function was not generating correct translations when the input has Unicode (non-ASCII) in it. This is due to its use of String::len, which gives the length in bytes, not chars. * Fix multi-code point auto pairs The current code for auto pairs is counting offsets by summing the length of the open and closing chars with char::len_utf8. Unfortunately, this gives back bytes, and the offset needs to be in chars. Additionally, it was discovered that there was a preexisting bug where the selection was not computed correctly in the case that the cursor was: 1. a single grapheme in width 2. this grapheme was more than one char 3. the direction of the cursor is backwards 4. a secondary range In this case, the offset was not being added into the anchor. This was fixed. * migrate auto pairs tests to integration * review comments--- helix-core/src/auto_pairs.rs | 594 ++----------------------------------------- 1 file changed, 20 insertions(+), 574 deletions(-) (limited to 'helix-core/src/auto_pairs.rs') diff --git a/helix-core/src/auto_pairs.rs b/helix-core/src/auto_pairs.rs index edc404ac..072c93d0 100644 --- a/helix-core/src/auto_pairs.rs +++ b/helix-core/src/auto_pairs.rs @@ -146,13 +146,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, - typed_char: char, - len_inserted: usize, -) -> Range { +fn get_next_range(doc: &Rope, start_range: &Range, offset: usize, 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. @@ -172,8 +166,8 @@ fn get_next_range( // 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 + typed_char.len_utf8(), - start_range.head + offset + typed_char.len_utf8(), + start_range.anchor + offset + 1, + start_range.head + offset + 1, ); } @@ -203,21 +197,18 @@ fn get_next_range( // 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 + typed_char.len_utf8() + start_range.anchor + offset + 1 } else { start_range.anchor + offset }; - return Range::new( - end_anchor, - start_range.head + offset + typed_char.len_utf8(), - ); + return Range::new(end_anchor, start_range.head + offset + 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 + typed_char.len_utf8() + start_range.head + offset + 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 @@ -243,8 +234,7 @@ fn get_next_range( (_, Direction::Forward) => { if single_grapheme { - graphemes::prev_grapheme_boundary(doc.slice(..), start_range.head) - + typed_char.len_utf8() + graphemes::prev_grapheme_boundary(doc.slice(..), start_range.head) + 1 // if we are appending, the anchor stays where it is; only offset // for multiple range insertions @@ -258,7 +248,9 @@ fn get_next_range( // 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 + graphemes::prev_grapheme_boundary(doc.slice(..), start_range.anchor) + + len_inserted + + offset } else { // when we are inserting in front of a selection, we need to move // the anchor over by however many characters were inserted overall @@ -279,9 +271,12 @@ fn handle_open(doc: &Rope, selection: &Selection, pair: &Pair) -> Transaction { 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 = pair.open.len_utf8(); + len_inserted = 1; let mut tendril = Tendril::new(); tendril.push(pair.open); (cursor, cursor, Some(tendril)) @@ -289,12 +284,12 @@ fn handle_open(doc: &Rope, selection: &Selection, pair: &Pair) -> Transaction { _ => { // insert open & close let pair_str = Tendril::from_iter([pair.open, pair.close]); - len_inserted = pair.open.len_utf8() + pair.close.len_utf8(); + len_inserted = 2; (cursor, cursor, Some(pair_str)) } }; - let next_range = get_next_range(doc, start_range, offs, pair.open, len_inserted); + let next_range = get_next_range(doc, start_range, offs, len_inserted); end_ranges.push(next_range); offs += len_inserted; @@ -308,7 +303,6 @@ fn handle_open(doc: &Rope, selection: &Selection, pair: &Pair) -> 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| { @@ -320,13 +314,13 @@ fn handle_close(doc: &Rope, selection: &Selection, pair: &Pair) -> Transaction { // return transaction that moves past close (cursor, cursor, None) // no-op } else { - len_inserted += pair.close.len_utf8(); + len_inserted = 1; let mut tendril = Tendril::new(); tendril.push(pair.close); (cursor, cursor, Some(tendril)) }; - let next_range = get_next_range(doc, start_range, offs, pair.close, len_inserted); + let next_range = get_next_range(doc, start_range, offs, len_inserted); end_ranges.push(next_range); offs += len_inserted; @@ -362,11 +356,11 @@ fn handle_same(doc: &Rope, selection: &Selection, pair: &Pair) -> Transaction { pair_str.push(pair.close); } - len_inserted += pair_str.len(); + len_inserted += pair_str.chars().count(); (cursor, cursor, Some(pair_str)) }; - let next_range = get_next_range(doc, start_range, offs, pair.open, len_inserted); + let next_range = get_next_range(doc, start_range, offs, len_inserted); end_ranges.push(next_range); offs += len_inserted; @@ -377,551 +371,3 @@ fn handle_same(doc: &Rope, selection: &Selection, pair: &Pair) -> Transaction { log::debug!("auto pair transaction: {:#?}", t); t } - -#[cfg(test)] -mod test { - use super::*; - use smallvec::smallvec; - - const LINE_END: &str = crate::DEFAULT_LINE_ENDING.as_str(); - - fn differing_pairs() -> impl Iterator { - DEFAULT_PAIRS.iter().filter(|(open, close)| open != close) - } - - fn matching_pairs() -> impl Iterator { - DEFAULT_PAIRS.iter().filter(|(open, close)| open == close) - } - - fn test_hooks( - in_doc: &Rope, - in_sel: &Selection, - ch: char, - pairs: &[(char, char)], - expected_doc: &Rope, - expected_sel: &Selection, - ) { - let pairs = AutoPairs::new(pairs.iter()); - let trans = hook(in_doc, in_sel, ch, &pairs).unwrap(); - let mut actual_doc = in_doc.clone(); - assert!(trans.apply(&mut actual_doc)); - assert_eq!(expected_doc, &actual_doc); - assert_eq!(expected_sel, trans.selection().unwrap()); - } - - fn test_hooks_with_pairs( - in_doc: &Rope, - in_sel: &Selection, - test_pairs: I, - pairs: &[(char, char)], - get_expected_doc: F, - actual_sel: &Selection, - ) where - I: IntoIterator, - F: Fn(char, char) -> R, - R: Into, - Rope: From, - { - test_pairs.into_iter().for_each(|(open, close)| { - test_hooks( - in_doc, - in_sel, - *open, - pairs, - &Rope::from(get_expected_doc(*open, *close)), - actual_sel, - ) - }); - } - - // [] indicates range - - /// [] -> insert ( -> ([]) - #[test] - fn test_insert_blank() { - test_hooks_with_pairs( - &Rope::from(LINE_END), - &Selection::single(1, 0), - DEFAULT_PAIRS, - DEFAULT_PAIRS, - |open, close| format!("{}{}{}", open, close, LINE_END), - &Selection::single(2, 1), - ); - - let empty_doc = Rope::from(format!("{line_end}{line_end}", line_end = LINE_END)); - - test_hooks_with_pairs( - &empty_doc, - &Selection::single(empty_doc.len_chars(), LINE_END.len()), - DEFAULT_PAIRS, - DEFAULT_PAIRS, - |open, close| { - format!( - "{line_end}{open}{close}{line_end}", - open = open, - close = close, - line_end = LINE_END - ) - }, - &Selection::single(LINE_END.len() + 2, LINE_END.len() + 1), - ); - } - - #[test] - fn test_insert_before_multi_code_point_graphemes() { - for (_, close) in differing_pairs() { - test_hooks( - &Rope::from(format!("hello ๐Ÿ‘จโ€๐Ÿ‘ฉโ€๐Ÿ‘งโ€๐Ÿ‘ฆ goodbye{}", LINE_END)), - &Selection::single(13, 6), - *close, - DEFAULT_PAIRS, - &Rope::from(format!("hello {}๐Ÿ‘จโ€๐Ÿ‘ฉโ€๐Ÿ‘งโ€๐Ÿ‘ฆ goodbye{}", close, LINE_END)), - &Selection::single(14, 7), - ); - } - } - - #[test] - fn test_insert_at_end_of_document() { - test_hooks_with_pairs( - &Rope::from(LINE_END), - &Selection::single(LINE_END.len(), LINE_END.len()), - DEFAULT_PAIRS, - DEFAULT_PAIRS, - |open, close| format!("{}{}{}", LINE_END, open, close), - &Selection::single(LINE_END.len() + 1, LINE_END.len() + 1), - ); - - test_hooks_with_pairs( - &Rope::from(format!("foo{}", LINE_END)), - &Selection::single(3 + LINE_END.len(), 3 + LINE_END.len()), - DEFAULT_PAIRS, - DEFAULT_PAIRS, - |open, close| format!("foo{}{}{}", LINE_END, open, close), - &Selection::single(LINE_END.len() + 4, LINE_END.len() + 4), - ); - } - - /// [] -> append ( -> ([]) - #[test] - fn test_append_blank() { - test_hooks_with_pairs( - // this is what happens when you have a totally blank document and then append - &Rope::from(format!("{line_end}{line_end}", line_end = LINE_END)), - // before inserting the pair, the cursor covers all of both empty lines - &Selection::single(0, LINE_END.len() * 2), - DEFAULT_PAIRS, - DEFAULT_PAIRS, - |open, close| { - format!( - "{line_end}{open}{close}{line_end}", - line_end = LINE_END, - open = open, - close = close - ) - }, - // after inserting pair, the cursor covers the first new line and the open char - &Selection::single(0, LINE_END.len() + 2), - ); - } - - /// [] ([]) - /// [] -> insert -> ([]) - /// [] ([]) - #[test] - fn test_insert_blank_multi_cursor() { - test_hooks_with_pairs( - &Rope::from("\n\n\n"), - &Selection::new( - smallvec!(Range::new(1, 0), Range::new(2, 1), Range::new(3, 2),), - 0, - ), - DEFAULT_PAIRS, - DEFAULT_PAIRS, - |open, close| { - format!( - "{open}{close}\n{open}{close}\n{open}{close}\n", - open = open, - close = close - ) - }, - &Selection::new( - smallvec!(Range::new(2, 1), Range::new(5, 4), Range::new(8, 7),), - 0, - ), - ); - } - - /// fo[o] -> append ( -> fo[o(]) - #[test] - fn test_append() { - test_hooks_with_pairs( - &Rope::from("foo\n"), - &Selection::single(2, 4), - differing_pairs(), - DEFAULT_PAIRS, - |open, close| format!("foo{}{}\n", open, close), - &Selection::single(2, 5), - ); - } - - /// foo[] -> append to end of line ( -> foo([]) - #[test] - fn test_append_single_cursor() { - test_hooks_with_pairs( - &Rope::from(format!("foo{}", LINE_END)), - &Selection::single(3, 3 + LINE_END.len()), - differing_pairs(), - DEFAULT_PAIRS, - |open, close| format!("foo{}{}{}", open, close, LINE_END), - &Selection::single(4, 5), - ); - } - - /// fo[o] fo[o(]) - /// fo[o] -> append ( -> fo[o(]) - /// fo[o] fo[o(]) - #[test] - fn test_append_multi() { - test_hooks_with_pairs( - &Rope::from("foo\nfoo\nfoo\n"), - &Selection::new( - smallvec!(Range::new(2, 4), Range::new(6, 8), Range::new(10, 12)), - 0, - ), - differing_pairs(), - DEFAULT_PAIRS, - |open, close| { - format!( - "foo{open}{close}\nfoo{open}{close}\nfoo{open}{close}\n", - open = open, - close = close - ) - }, - &Selection::new( - smallvec!(Range::new(2, 5), Range::new(8, 11), Range::new(14, 17)), - 0, - ), - ); - } - - /// ([)] -> insert ) -> ()[] - #[test] - fn test_insert_close_inside_pair() { - for (open, close) in DEFAULT_PAIRS { - let doc = Rope::from(format!("{}{}{}", open, close, LINE_END)); - - test_hooks( - &doc, - &Selection::single(2, 1), - *close, - DEFAULT_PAIRS, - &doc, - &Selection::single(2 + LINE_END.len(), 2), - ); - } - } - - /// [(]) -> append ) -> [()] - #[test] - fn test_append_close_inside_pair() { - for (open, close) in DEFAULT_PAIRS { - let doc = Rope::from(format!("{}{}{}", open, close, LINE_END)); - - test_hooks( - &doc, - &Selection::single(0, 2), - *close, - DEFAULT_PAIRS, - &doc, - &Selection::single(0, 2 + LINE_END.len()), - ); - } - } - - /// ([]) ()[] - /// ([]) -> insert ) -> ()[] - /// ([]) ()[] - #[test] - fn test_insert_close_inside_pair_multi_cursor() { - let sel = Selection::new( - smallvec!(Range::new(2, 1), Range::new(5, 4), Range::new(8, 7),), - 0, - ); - - let expected_sel = Selection::new( - smallvec!(Range::new(3, 2), Range::new(6, 5), Range::new(9, 8),), - 0, - ); - - for (open, close) in DEFAULT_PAIRS { - let doc = Rope::from(format!( - "{open}{close}\n{open}{close}\n{open}{close}\n", - open = open, - close = close - )); - - test_hooks(&doc, &sel, *close, DEFAULT_PAIRS, &doc, &expected_sel); - } - } - - /// [(]) [()] - /// [(]) -> append ) -> [()] - /// [(]) [()] - #[test] - fn test_append_close_inside_pair_multi_cursor() { - let sel = Selection::new( - smallvec!(Range::new(0, 2), Range::new(3, 5), Range::new(6, 8),), - 0, - ); - - let expected_sel = Selection::new( - smallvec!(Range::new(0, 3), Range::new(3, 6), Range::new(6, 9),), - 0, - ); - - for (open, close) in DEFAULT_PAIRS { - let doc = Rope::from(format!( - "{open}{close}\n{open}{close}\n{open}{close}\n", - open = open, - close = close - )); - - test_hooks(&doc, &sel, *close, DEFAULT_PAIRS, &doc, &expected_sel); - } - } - - /// ([]) -> insert ( -> (([])) - #[test] - fn test_insert_open_inside_pair() { - let sel = Selection::single(2, 1); - let expected_sel = Selection::single(3, 2); - - for (open, close) in differing_pairs() { - let doc = Rope::from(format!("{}{}", open, close)); - let expected_doc = Rope::from(format!( - "{open}{open}{close}{close}", - open = open, - close = close - )); - - test_hooks( - &doc, - &sel, - *open, - DEFAULT_PAIRS, - &expected_doc, - &expected_sel, - ); - } - } - - /// [word(]) -> append ( -> [word((])) - #[test] - fn test_append_open_inside_pair() { - let sel = Selection::single(0, 6); - let expected_sel = Selection::single(0, 7); - - for (open, close) in differing_pairs() { - let doc = Rope::from(format!("word{}{}", open, close)); - let expected_doc = Rope::from(format!( - "word{open}{open}{close}{close}", - open = open, - close = close - )); - - test_hooks( - &doc, - &sel, - *open, - DEFAULT_PAIRS, - &expected_doc, - &expected_sel, - ); - } - } - - /// ([]) -> insert " -> ("[]") - #[test] - fn test_insert_nested_open_inside_pair() { - let sel = Selection::single(2, 1); - let expected_sel = Selection::single(3, 2); - - for (outer_open, outer_close) in differing_pairs() { - let doc = Rope::from(format!("{}{}", outer_open, outer_close,)); - - for (inner_open, inner_close) in matching_pairs() { - let expected_doc = Rope::from(format!( - "{}{}{}{}", - outer_open, inner_open, inner_close, outer_close - )); - - test_hooks( - &doc, - &sel, - *inner_open, - DEFAULT_PAIRS, - &expected_doc, - &expected_sel, - ); - } - } - } - - /// [(]) -> append " -> [("]") - #[test] - fn test_append_nested_open_inside_pair() { - let sel = Selection::single(0, 2); - let expected_sel = Selection::single(0, 3); - - for (outer_open, outer_close) in differing_pairs() { - let doc = Rope::from(format!("{}{}", outer_open, outer_close,)); - - for (inner_open, inner_close) in matching_pairs() { - let expected_doc = Rope::from(format!( - "{}{}{}{}", - outer_open, inner_open, inner_close, outer_close - )); - - test_hooks( - &doc, - &sel, - *inner_open, - DEFAULT_PAIRS, - &expected_doc, - &expected_sel, - ); - } - } - } - - /// []word -> insert ( -> ([]word - #[test] - fn test_insert_open_before_non_pair() { - test_hooks_with_pairs( - &Rope::from("word"), - &Selection::single(1, 0), - DEFAULT_PAIRS, - DEFAULT_PAIRS, - |open, _| format!("{}word", open), - &Selection::single(2, 1), - ) - } - - /// [wor]d -> insert ( -> ([wor]d - #[test] - fn test_insert_open_with_selection() { - test_hooks_with_pairs( - &Rope::from("word"), - &Selection::single(3, 0), - DEFAULT_PAIRS, - DEFAULT_PAIRS, - |open, _| format!("{}word", open), - &Selection::single(4, 1), - ) - } - - /// [wor]d -> append ) -> [wor)]d - #[test] - fn test_append_close_inside_non_pair_with_selection() { - let sel = Selection::single(0, 4); - let expected_sel = Selection::single(0, 5); - - for (_, close) in DEFAULT_PAIRS { - let doc = Rope::from("word"); - let expected_doc = Rope::from(format!("wor{}d", close)); - test_hooks( - &doc, - &sel, - *close, - DEFAULT_PAIRS, - &expected_doc, - &expected_sel, - ); - } - } - - /// foo[ wor]d -> insert ( -> foo([) wor]d - #[test] - fn test_insert_open_trailing_word_with_selection() { - test_hooks_with_pairs( - &Rope::from("foo word"), - &Selection::single(7, 3), - differing_pairs(), - DEFAULT_PAIRS, - |open, close| format!("foo{}{} word", open, close), - &Selection::single(9, 4), - ) - } - - /// foo([) wor]d -> insert ) -> foo()[ wor]d - #[test] - fn test_insert_close_inside_pair_trailing_word_with_selection() { - for (open, close) in differing_pairs() { - test_hooks( - &Rope::from(format!("foo{}{} word{}", open, close, LINE_END)), - &Selection::single(9, 4), - *close, - DEFAULT_PAIRS, - &Rope::from(format!("foo{}{} word{}", open, close, LINE_END)), - &Selection::single(9, 5), - ) - } - } - - /// we want pairs that are *not* the same char to be inserted after - /// a non-pair char, for cases like functions, but for pairs that are - /// the same char, we want to *not* insert a pair to handle cases like "I'm" - /// - /// word[] -> insert ( -> word([]) - /// word[] -> insert ' -> word'[] - #[test] - fn test_insert_open_after_non_pair() { - let doc = Rope::from(format!("word{}", LINE_END)); - let sel = Selection::single(5, 4); - let expected_sel = Selection::single(6, 5); - - test_hooks_with_pairs( - &doc, - &sel, - differing_pairs(), - DEFAULT_PAIRS, - |open, close| format!("word{}{}{}", open, close, LINE_END), - &expected_sel, - ); - - test_hooks_with_pairs( - &doc, - &sel, - matching_pairs(), - DEFAULT_PAIRS, - |open, _| format!("word{}{}", open, LINE_END), - &expected_sel, - ); - } - - #[test] - fn test_configured_pairs() { - let test_pairs = &[('`', ':'), ('+', '-')]; - - test_hooks_with_pairs( - &Rope::from(LINE_END), - &Selection::single(1, 0), - test_pairs, - test_pairs, - |open, close| format!("{}{}{}", open, close, LINE_END), - &Selection::single(2, 1), - ); - - let doc = Rope::from(format!("foo`: word{}", LINE_END)); - - test_hooks( - &doc, - &Selection::single(9, 4), - ':', - test_pairs, - &doc, - &Selection::single(9, 5), - ) - } -} -- cgit v1.2.3-70-g09d2