From fd5a75deb0525b5d596212eb7008fe5a81e28cb0 Mon Sep 17 00:00:00 2001 From: Skyler Hawthorne Date: Mon, 6 Dec 2021 11:17:29 -0500 Subject: [PATCH 1/3] use auto pairs with selections Previously, the auto pairs code was converting the user selection into its cursor form, and setting the transaction's selection to that cursor. This has the effect of destroying the user's selection if they type a pair character that gets auto completed. This fixes the code to work with the user's selection, inserting auto pairs where appropriate, but either keeping or extending the user's selection. --- helix-core/src/auto_pairs.rs | 333 ++++++++++++++++++++++++++--------- helix-core/src/selection.rs | 21 +++ 2 files changed, 275 insertions(+), 79 deletions(-) diff --git a/helix-core/src/auto_pairs.rs b/helix-core/src/auto_pairs.rs index c037afef4d72..0d3273974a19 100644 --- a/helix-core/src/auto_pairs.rs +++ b/helix-core/src/auto_pairs.rs @@ -30,7 +30,6 @@ const CLOSE_BEFORE: &str = ")]}'\":;,> \n\r\u{000B}\u{000C}\u{0085}\u{2028}\u{20 // [TODO] // * delete implementation where it erases the whole bracket (|) -> | -// * do not reduce to cursors; use whole selections, and surround with pair // * 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 {% %} @@ -38,20 +37,18 @@ const CLOSE_BEFORE: &str = ")]}'\":;,> \n\r\u{000B}\u{000C}\u{0085}\u{2028}\u{20 pub fn hook(doc: &Rope, selection: &Selection, ch: char) -> Option { debug!("autopairs hook selection: {:#?}", selection); - let cursors = selection.clone().cursors(doc.slice(..)); - for &(open, close) in PAIRS { if open == ch { if open == close { - return Some(handle_same(doc, &cursors, open, CLOSE_BEFORE, OPEN_BEFORE)); + return Some(handle_same(doc, selection, open, CLOSE_BEFORE, OPEN_BEFORE)); } else { - return Some(handle_open(doc, &cursors, open, close, CLOSE_BEFORE)); + return Some(handle_open(doc, selection, open, close, CLOSE_BEFORE)); } } if close == ch { // && char_at pos == close - return Some(handle_close(doc, &cursors, open, close)); + return Some(handle_close(doc, selection, open, close)); } } @@ -66,6 +63,36 @@ fn prev_char(doc: &Rope, pos: usize) -> Option { doc.get_char(pos - 1) } +/// calculate what the resulting range should be for an auto pair insertion +fn get_next_range( + start_range: &Range, + offset: usize, + typed_char: char, + len_inserted: usize, +) -> Range { + let end_head = start_range.head + offset + typed_char.len_utf8(); + + let end_anchor = match (start_range.len(), start_range.is_forward()) { + // if we have a zero width cursor, it shifts to the same number + (0, _) => end_head, + + // if we are inserting for a regular one-width cursor, the anchor + // moves with the head + (1, true) => end_head - 1, + (1, false) => end_head + 1, + + // if we are appending, the anchor stays where it is; only offset + // for multiple range insertions + (_, true) => start_range.anchor + offset, + + // when we are inserting in front of a selection, we need to move + // the anchor over by however many characters were inserted overall + (_, false) => start_range.anchor + offset + len_inserted, + }; + + Range::new(end_anchor, end_head) +} + fn handle_open( doc: &Rope, selection: &Selection, @@ -74,36 +101,32 @@ fn handle_open( close_before: &str, ) -> 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 start_head = start_range.head; - - let next = doc.get_char(start_head); - let end_head = start_head + offs + open.len_utf8(); - - let end_anchor = if start_range.is_empty() { - end_head - } else { - start_range.anchor + offs - }; - - end_ranges.push(Range::new(end_anchor, end_head)); + let cursor = start_range.cursor(doc.slice(..)); + let next_char = doc.get_char(cursor); + let len_inserted; - match next { + let change = match next_char { Some(ch) if !close_before.contains(ch) => { - offs += open.len_utf8(); - (start_head, start_head, Some(Tendril::from_char(open))) + len_inserted = open.len_utf8(); + (cursor, cursor, Some(Tendril::from_char(open))) } // None | Some(ch) if close_before.contains(ch) => {} _ => { // insert open & close let pair = Tendril::from_iter([open, close]); - offs += open.len_utf8() + close.len_utf8(); - (start_head, start_head, Some(pair)) + len_inserted = open.len_utf8() + close.len_utf8(); + (cursor, cursor, Some(pair)) } - } + }; + + let next_range = get_next_range(start_range, offs, open, len_inserted); + end_ranges.push(next_range); + offs += len_inserted; + + change }); let t = transaction.with_selection(Selection::new(end_ranges, selection.primary_index())); @@ -117,28 +140,28 @@ fn handle_close(doc: &Rope, selection: &Selection, _open: char, close: char) -> let mut offs = 0; let transaction = Transaction::change_by_selection(doc, selection, |start_range| { - let start_head = start_range.head; - let next = doc.get_char(start_head); - let end_head = start_head + offs + close.len_utf8(); + let cursor = start_range.cursor(doc.slice(..)); + let next_char = doc.get_char(cursor); + let mut len_inserted = 0; - let end_anchor = if start_range.is_empty() { - end_head + let change = if next_char == Some(close) { + // return transaction that moves past close + (cursor, cursor, None) // no-op } else { - start_range.anchor + offs + len_inserted += close.len_utf8(); + (cursor, cursor, Some(Tendril::from_char(close))) }; - end_ranges.push(Range::new(end_anchor, end_head)); + let next_range = get_next_range(start_range, offs, close, len_inserted); + end_ranges.push(next_range); + offs += len_inserted; - if next == Some(close) { - // return transaction that moves past close - (start_head, start_head, None) // no-op - } else { - offs += close.len_utf8(); - (start_head, start_head, Some(Tendril::from_char(close))) - } + change }); - transaction.with_selection(Selection::new(end_ranges, selection.primary_index())) + let t = transaction.with_selection(Selection::new(end_ranges, selection.primary_index())); + debug!("auto pair transaction: {:#?}", t); + t } /// handle cases where open and close is the same, or in triples ("""docstring""") @@ -154,42 +177,41 @@ fn handle_same( let mut offs = 0; let transaction = Transaction::change_by_selection(doc, selection, |start_range| { - let start_head = start_range.head; - let end_head = start_head + offs + token.len_utf8(); + let cursor = start_range.cursor(doc.slice(..)); + let mut len_inserted = 0; - // if selection, retain anchor, if cursor, move over - let end_anchor = if start_range.is_empty() { - end_head - } else { - start_range.anchor + offs - }; + let next_char = doc.get_char(cursor); + let prev_char = prev_char(doc, cursor); - end_ranges.push(Range::new(end_anchor, end_head)); - - let next = doc.get_char(start_head); - let prev = prev_char(doc, start_head); - - if next == Some(token) { + let change = if next_char == Some(token) { // return transaction that moves past close - (start_head, start_head, None) // no-op + (cursor, cursor, None) // no-op } else { let mut pair = Tendril::with_capacity(2 * token.len_utf8() as u32); pair.push_char(token); // for equal pairs, don't insert both open and close if either // side has a non-pair char - if (next.is_none() || close_before.contains(next.unwrap())) - && (prev.is_none() || open_before.contains(prev.unwrap())) + if (next_char.is_none() || close_before.contains(next_char.unwrap())) + && (prev_char.is_none() || open_before.contains(prev_char.unwrap())) { pair.push_char(token); } - offs += pair.len(); - (start_head, start_head, Some(pair)) - } + len_inserted += pair.len(); + (cursor, cursor, Some(pair)) + }; + + let next_range = get_next_range(start_range, offs, token, len_inserted); + end_ranges.push(next_range); + offs += len_inserted; + + change }); - transaction.with_selection(Selection::new(end_ranges, selection.primary_index())) + let t = transaction.with_selection(Selection::new(end_ranges, selection.primary_index())); + debug!("auto pair transaction: {:#?}", t); + t } #[cfg(test)] @@ -252,7 +274,20 @@ mod test { &Selection::single(1, 0), PAIRS, |open, close| format!("{}{}", open, close), - &Selection::single(1, 1), + &Selection::single(2, 1), + ); + } + + /// [] -> 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("\n\n"), + &Selection::single(0, 2), + PAIRS, + |open, close| format!("\n{}{}\n", open, close), + &Selection::single(0, 3), ); } @@ -276,26 +311,50 @@ mod test { ) }, &Selection::new( - smallvec!(Range::point(1), Range::point(4), Range::point(7),), + smallvec!(Range::new(2, 1), Range::new(5, 4), Range::new(8, 7),), 0, ), ); } - // [TODO] broken until it works with selections /// fo[o] -> append ( -> fo[o(]) - #[ignore] #[test] fn test_append() { test_hooks_with_pairs( - &Rope::from("foo"), + &Rope::from("foo\n"), &Selection::single(2, 4), - PAIRS, - |open, close| format!("foo{}{}", open, close), + differing_pairs(), + |open, close| format!("foo{}{}\n", open, close), &Selection::single(2, 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(), + |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() { @@ -307,7 +366,23 @@ mod test { &Selection::single(2, 1), *close, &doc, - &Selection::point(2), + &Selection::single(3, 2), + ); + } + } + + /// [(]) -> append ) -> [()] + #[test] + fn test_append_close_inside_pair() { + for (open, close) in PAIRS { + let doc = Rope::from(format!("{}{}\n", open, close)); + + test_hooks( + &doc, + &Selection::single(0, 2), + *close, + &doc, + &Selection::single(0, 3), ); } } @@ -323,8 +398,33 @@ mod test { ); let expected_sel = Selection::new( - // smallvec!(Range::new(3, 2), Range::new(6, 5), Range::new(9, 8),), - smallvec!(Range::point(2), Range::point(5), Range::point(8),), + smallvec!(Range::new(3, 2), Range::new(6, 5), Range::new(9, 8),), + 0, + ); + + for (open, close) in PAIRS { + let doc = Rope::from(format!( + "{open}{close}\n{open}{close}\n{open}{close}\n", + open = open, + close = close + )); + + test_hooks(&doc, &sel, *close, &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, ); @@ -343,7 +443,7 @@ mod test { #[test] fn test_insert_open_inside_pair() { let sel = Selection::single(2, 1); - let expected_sel = Selection::point(2); + let expected_sel = Selection::single(3, 2); for (open, close) in differing_pairs() { let doc = Rope::from(format!("{}{}", open, close)); @@ -357,11 +457,49 @@ mod test { } } + /// [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, &expected_doc, &expected_sel); + } + } + /// ([]) -> insert " -> ("[]") #[test] fn test_insert_nested_open_inside_pair() { let sel = Selection::single(2, 1); - let expected_sel = Selection::point(2); + 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, &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,)); @@ -385,21 +523,44 @@ mod test { &Selection::single(1, 0), PAIRS, |open, _| format!("{}word", open), - &Selection::point(1), + &Selection::single(2, 1), ) } - // [TODO] broken until it works with selections /// [wor]d -> insert ( -> ([wor]d #[test] - #[ignore] fn test_insert_open_with_selection() { test_hooks_with_pairs( &Rope::from("word"), - &Selection::single(0, 4), + &Selection::single(3, 0), PAIRS, |open, _| format!("{}word", open), - &Selection::single(1, 5), + &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 PAIRS { + let doc = Rope::from("word"); + let expected_doc = Rope::from(format!("wor{}d", close)); + test_hooks(&doc, &sel, *close, &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(), + |open, close| format!("foo{}{} word", open, close), + &Selection::single(9, 4), ) } @@ -413,7 +574,7 @@ mod test { fn test_insert_open_after_non_pair() { let doc = Rope::from("word"); let sel = Selection::single(5, 4); - let expected_sel = Selection::point(5); + let expected_sel = Selection::single(6, 5); test_hooks_with_pairs( &doc, @@ -431,4 +592,18 @@ mod test { &expected_sel, ); } + + /// appending with only a cursor should stay a cursor + /// + /// [] -> append to end "foo -> "foo[]" + #[test] + fn test_append_single_cursor() { + test_hooks_with_pairs( + &Rope::from("\n"), + &Selection::single(0, 1), + PAIRS, + |open, close| format!("{}{}\n", open, close), + &Selection::single(1, 2), + ); + } } diff --git a/helix-core/src/selection.rs b/helix-core/src/selection.rs index 116a1c7c09b2..e2c8262a0e5b 100644 --- a/helix-core/src/selection.rs +++ b/helix-core/src/selection.rs @@ -82,6 +82,13 @@ impl Range { std::cmp::max(self.anchor, self.head) } + /// Total length of the range. + #[inline] + #[must_use] + pub fn len(&self) -> usize { + self.to() - self.from() + } + /// The (inclusive) range of lines that the range overlaps. #[inline] #[must_use] @@ -102,6 +109,20 @@ impl Range { self.anchor == self.head } + /// `true` when head > anchor. + #[inline] + #[must_use] + pub fn is_forward(&self) -> bool { + self.head > self.anchor + } + + /// `true` when head < anchor. + #[inline] + #[must_use] + pub fn is_backward(&self) -> bool { + self.head < self.anchor + } + /// Check two ranges for overlap. #[must_use] pub fn overlaps(&self, other: &Self) -> bool { From 3d66039a27b276a49eeb0688119f67257c652391 Mon Sep 17 00:00:00 2001 From: Skyler Hawthorne Date: Mon, 20 Dec 2021 19:10:58 -0500 Subject: [PATCH 2/3] use movement::Direction instead of bool --- helix-core/src/auto_pairs.rs | 15 +++++++++------ helix-core/src/selection.rs | 22 ++++++++++++---------- 2 files changed, 21 insertions(+), 16 deletions(-) diff --git a/helix-core/src/auto_pairs.rs b/helix-core/src/auto_pairs.rs index 0d3273974a19..d54e1ac038ef 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::{Range, Rope, Selection, Tendril, Transaction}; +use crate::{movement::Direction, Range, Rope, Selection, Tendril, Transaction}; use log::debug; use smallvec::SmallVec; @@ -72,22 +72,25 @@ fn get_next_range( ) -> Range { let end_head = start_range.head + offset + typed_char.len_utf8(); - let end_anchor = match (start_range.len(), start_range.is_forward()) { + let end_anchor = match (start_range.len(), start_range.direction()) { // if we have a zero width cursor, it shifts to the same number (0, _) => end_head, // if we are inserting for a regular one-width cursor, the anchor // moves with the head - (1, true) => end_head - 1, - (1, false) => end_head + 1, + (1, Some(Direction::Forward)) => end_head - 1, + (1, Some(Direction::Backward)) => end_head + 1, // if we are appending, the anchor stays where it is; only offset // for multiple range insertions - (_, true) => start_range.anchor + offset, + (_, Some(Direction::Forward)) => start_range.anchor + offset, // when we are inserting in front of a selection, we need to move // the anchor over by however many characters were inserted overall - (_, false) => start_range.anchor + offset + len_inserted, + (_, Some(Direction::Backward)) => start_range.anchor + offset + len_inserted, + + // None means 0 width + _ => unreachable!(), }; Range::new(end_anchor, end_head) diff --git a/helix-core/src/selection.rs b/helix-core/src/selection.rs index e2c8262a0e5b..aaa6d2655c50 100644 --- a/helix-core/src/selection.rs +++ b/helix-core/src/selection.rs @@ -7,6 +7,7 @@ use crate::{ ensure_grapheme_boundary_next, ensure_grapheme_boundary_prev, next_grapheme_boundary, prev_grapheme_boundary, }, + movement::Direction, Assoc, ChangeSet, RopeSlice, }; use smallvec::{smallvec, SmallVec}; @@ -109,18 +110,19 @@ impl Range { self.anchor == self.head } - /// `true` when head > anchor. + /// `Some(Direction::Forward)` when head > anchor. + /// `Some(Direction::Backward)` when head < anchor. + /// None otherwise. #[inline] #[must_use] - pub fn is_forward(&self) -> bool { - self.head > self.anchor - } - - /// `true` when head < anchor. - #[inline] - #[must_use] - pub fn is_backward(&self) -> bool { - self.head < self.anchor + pub fn direction(&self) -> Option { + if self.head < self.anchor { + Some(Direction::Backward) + } else if self.head > self.anchor { + Some(Direction::Forward) + } else { + None + } } /// Check two ranges for overlap. From 9b056d923d750904e345667c89f471ebb7c91d3c Mon Sep 17 00:00:00 2001 From: Skyler Hawthorne Date: Mon, 20 Dec 2021 23:51:23 -0500 Subject: [PATCH 3/3] assume 0-width cursor is forward --- helix-core/src/auto_pairs.rs | 11 ++++------- helix-core/src/selection.rs | 13 +++++-------- 2 files changed, 9 insertions(+), 15 deletions(-) diff --git a/helix-core/src/auto_pairs.rs b/helix-core/src/auto_pairs.rs index d54e1ac038ef..1b3de6ea08bd 100644 --- a/helix-core/src/auto_pairs.rs +++ b/helix-core/src/auto_pairs.rs @@ -78,19 +78,16 @@ fn get_next_range( // if we are inserting for a regular one-width cursor, the anchor // moves with the head - (1, Some(Direction::Forward)) => end_head - 1, - (1, Some(Direction::Backward)) => end_head + 1, + (1, Direction::Forward) => end_head - 1, + (1, Direction::Backward) => end_head + 1, // if we are appending, the anchor stays where it is; only offset // for multiple range insertions - (_, Some(Direction::Forward)) => start_range.anchor + offset, + (_, Direction::Forward) => start_range.anchor + offset, // when we are inserting in front of a selection, we need to move // the anchor over by however many characters were inserted overall - (_, Some(Direction::Backward)) => start_range.anchor + offset + len_inserted, - - // None means 0 width - _ => unreachable!(), + (_, Direction::Backward) => start_range.anchor + offset + len_inserted, }; Range::new(end_anchor, end_head) diff --git a/helix-core/src/selection.rs b/helix-core/src/selection.rs index aaa6d2655c50..884c98acf8c3 100644 --- a/helix-core/src/selection.rs +++ b/helix-core/src/selection.rs @@ -110,18 +110,15 @@ impl Range { self.anchor == self.head } - /// `Some(Direction::Forward)` when head > anchor. - /// `Some(Direction::Backward)` when head < anchor. - /// None otherwise. + /// `Direction::Backward` when head < anchor. + /// `Direction::Backward` otherwise. #[inline] #[must_use] - pub fn direction(&self) -> Option { + pub fn direction(&self) -> Direction { if self.head < self.anchor { - Some(Direction::Backward) - } else if self.head > self.anchor { - Some(Direction::Forward) + Direction::Backward } else { - None + Direction::Forward } }