From 32805063d49c41f21dda3dee3f5d771c3e78048f Mon Sep 17 00:00:00 2001 From: Skyler Hawthorne Date: Tue, 23 Nov 2021 00:37:42 -0500 Subject: [PATCH 1/6] Add auto pairs for same-char pairs * Add unit tests for all existing functionality * Add auto pairs for same-char pairs (quotes, etc). Account for apostrophe in prose by requiring both sides of the cursor to be non-pair chars or whitespace. This also incidentally will work for avoiding a double single quote in lifetime annotations, at least until <> is added * Slight factor of moving the cursor transform of the selection to inside the hooks. This will enable doing auto pairing with selections, and fixing the bug where auto pairs destroy the selection. Fixes #1014 --- helix-core/src/auto_pairs.rs | 414 ++++++++++++++++++++++++++++------ helix-core/src/transaction.rs | 2 +- helix-term/src/commands.rs | 7 +- 3 files changed, 356 insertions(+), 67 deletions(-) diff --git a/helix-core/src/auto_pairs.rs b/helix-core/src/auto_pairs.rs index cc9668529a6d..069f6af58dc5 100644 --- a/helix-core/src/auto_pairs.rs +++ b/helix-core/src/auto_pairs.rs @@ -2,6 +2,7 @@ //! this module provides the functionality to insert the paired closing character. use crate::{Range, Rope, Selection, Tendril, Transaction}; +use log::debug; use smallvec::SmallVec; // Heavily based on https://github.com/codemirror/closebrackets/ @@ -15,7 +16,9 @@ pub const PAIRS: &[(char, char)] = &[ ('`', '`'), ]; -const CLOSE_BEFORE: &str = ")]}'\":;> \n\r\u{000B}\u{000C}\u{0085}\u{2028}\u{2029}"; // includes space and newlines +// [TODO] build this dynamically in language config. see #992 +const OPEN_BEFORE: &str = "([{'\":;,> \n\r\u{000B}\u{000C}\u{0085}\u{2028}\u{2029}"; +const CLOSE_BEFORE: &str = ")]}'\":;,> \n\r\u{000B}\u{000C}\u{0085}\u{2028}\u{2029}"; // includes space and newlines // insert hook: // Fn(doc, selection, char) => Option @@ -25,40 +28,53 @@ const CLOSE_BEFORE: &str = ")]}'\":;> \n\r\u{000B}\u{000C}\u{0085}\u{2028}\u{202 // // to simplify, maybe return Option and just reimplement the default -// TODO: delete implementation where it erases the whole bracket (|) -> | +// [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 {% %} #[must_use] 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 handle_same(doc, selection, open); + return Some(handle_same(doc, &cursors, open, CLOSE_BEFORE, OPEN_BEFORE)); } else { - return Some(handle_open(doc, selection, open, close, CLOSE_BEFORE)); + return Some(handle_open(doc, &cursors, open, close, CLOSE_BEFORE)); } } if close == ch { // && char_at pos == close - return Some(handle_close(doc, selection, open, close)); + return Some(handle_close(doc, &cursors, open, close)); } } None } -// TODO: special handling for lifetimes in rust: if preceeded by & or < don't auto close ' -// for example "&'a mut", or "fn<'a>" - fn next_char(doc: &Rope, pos: usize) -> Option { if pos >= doc.len_chars() { return None; } Some(doc.char(pos)) } -// TODO: selections should be extended if range, moved if point. -// TODO: if not cursor but selection, wrap on both sides of selection (surround) +fn prev_char(doc: &Rope, mut pos: usize) -> Option { + if pos == 0 { + return None; + } + + pos -= 1; + + next_char(doc, pos) +} + fn handle_open( doc: &Rope, selection: &Selection, @@ -66,30 +82,29 @@ fn handle_open( close: char, close_before: &str, ) -> Transaction { - let mut ranges = SmallVec::with_capacity(selection.len()); + let mut end_ranges = SmallVec::with_capacity(selection.len()); let mut offs = 0; - let transaction = Transaction::change_by_selection(doc, selection, |range| { - let pos = range.head; - let next = next_char(doc, pos); + let transaction = Transaction::change_by_selection(doc, selection, |start_range| { + let start_head = start_range.head; - let head = pos + offs + open.len_utf8(); - // if selection, retain anchor, if cursor, move over - ranges.push(Range::new( - if range.is_empty() { - head - } else { - range.anchor + offs - }, - head, - )); + let next = next_char(doc, 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)); match next { Some(ch) if !close_before.contains(ch) => { offs += 1; // TODO: else return (use default handler that inserts open) - (pos, pos, Some(Tendril::from_char(open))) + (start_head, start_head, Some(Tendril::from_char(open))) } // None | Some(ch) if close_before.contains(ch) => {} _ => { @@ -100,64 +115,337 @@ fn handle_open( offs += 2; - (pos, pos, Some(pair)) + (start_head, start_head, Some(pair)) } } }); - transaction.with_selection(Selection::new(ranges, selection.primary_index())) + let t = transaction.with_selection(Selection::new(end_ranges, selection.primary_index())); + debug!("auto pair transaction: {:#?}", t); + t } fn handle_close(doc: &Rope, selection: &Selection, _open: char, close: char) -> Transaction { - let mut ranges = SmallVec::with_capacity(selection.len()); + let mut end_ranges = SmallVec::with_capacity(selection.len()); let mut offs = 0; - let transaction = Transaction::change_by_selection(doc, selection, |range| { - let pos = range.head; - let next = next_char(doc, pos); + let transaction = Transaction::change_by_selection(doc, selection, |start_range| { + let start_head = start_range.head; + let next = next_char(doc, start_head); + let end_head = start_head + offs + close.len_utf8(); - let head = pos + offs + close.len_utf8(); - // if selection, retain anchor, if cursor, move over - ranges.push(Range::new( - if range.is_empty() { - head - } else { - range.anchor + offs - }, - head, - )); + let end_anchor = if start_range.is_empty() { + end_head + } else { + start_range.anchor + offs + }; + + end_ranges.push(Range::new(end_anchor, end_head)); if next == Some(close) { - // return transaction that moves past close - (pos, pos, None) // no-op + // return transaction that moves past close + (start_head, start_head, None) // no-op } else { offs += close.len_utf8(); // TODO: else return (use default handler that inserts close) - (pos, pos, Some(Tendril::from_char(close))) + (start_head, start_head, Some(Tendril::from_char(close))) } }); - transaction.with_selection(Selection::new(ranges, selection.primary_index())) + transaction.with_selection(Selection::new(end_ranges, selection.primary_index())) } -// handle cases where open and close is the same, or in triples ("""docstring""") -fn handle_same(_doc: &Rope, _selection: &Selection, _token: char) -> Option { - // if not cursor but selection, wrap - // let next = next char - - // if next == bracket { - // // if start of syntax node, insert token twice (new pair because node is complete) - // // elseif colsedBracketAt - // // is_triple == allow triple && next 3 is equal - // // cursor jump over - // } - //} else if allow_triple && followed by triple { - //} - //} else if next != word char && prev != bracket && prev != word char { - // // condition checks for cases like I' where you don't want I'' (or I'm) - // insert pair ("") - //} - None +/// handle cases where open and close is the same, or in triples ("""docstring""") +fn handle_same( + doc: &Rope, + selection: &Selection, + token: char, + close_before: &str, + open_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 end_head = start_head + offs + token.len_utf8(); + + let end_anchor = if start_range.is_empty() { + end_head + } else { + start_range.anchor + offs + }; + + // if selection, retain anchor, if cursor, move over + end_ranges.push(Range::new(end_anchor, end_head)); + + let next = next_char(doc, start_head); + let prev = prev_char(doc, start_head); + + if next == Some(token) { + // return transaction that moves past close + (start_head, start_head, None) // no-op + } else { + let mut pair = Tendril::with_capacity(2); + 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())) + { + pair.push_char(token); + } + + offs += pair.len(); + + (start_head, start_head, Some(pair)) + } + }); + + transaction.with_selection(Selection::new(end_ranges, selection.primary_index())) +} + +#[cfg(test)] +mod test { + use super::*; + use smallvec::smallvec; + + fn differing_pairs() -> impl Iterator { + PAIRS.iter().filter(|(open, close)| open != close) + } + + fn matching_pairs() -> impl Iterator { + PAIRS.iter().filter(|(open, close)| open == close) + } + + fn test_hooks( + in_doc: &Rope, + in_sel: &Selection, + ch: char, + expected_doc: &Rope, + expected_sel: &Selection, + ) { + let trans = hook(&in_doc, &in_sel, ch).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, + pairs: I, + get_expected_doc: F, + actual_sel: &Selection, + ) where + I: IntoIterator, + F: Fn(char, char) -> R, + R: Into, + Rope: From, + { + pairs.into_iter().for_each(|(open, close)| { + test_hooks( + in_doc, + in_sel, + *open, + &Rope::from(get_expected_doc(*open, *close)), + actual_sel, + ) + }); + } + + // [] indicates range + + /// [] -> insert ( -> ([]) + #[test] + fn test_insert_blank() { + test_hooks_with_pairs( + &Rope::new(), + &Selection::single(1, 0), + PAIRS, + |open, close| format!("{}{}", open, close), + &Selection::single(1, 1), + ); + } + + /// [] ([]) + /// [] -> 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, + ), + PAIRS, + |open, close| { + format!( + "{open}{close}\n{open}{close}\n{open}{close}\n", + open = open, + close = close + ) + }, + &Selection::new( + smallvec!(Range::point(1), Range::point(4), Range::point(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"), + &Selection::single(2, 4), + PAIRS, + |open, close| format!("foo{}{}", open, close), + &Selection::single(2, 5), + ); + } + + /// ([]) -> insert ) -> ()[] + #[test] + fn test_insert_close_inside_pair() { + for (open, close) in PAIRS { + let doc = Rope::from(format!("{}{}", open, close)); + + test_hooks( + &doc, + &Selection::single(2, 1), + *close, + &doc, + &Selection::point(2), + ); + } + } + + /// ([]) ()[] + /// ([]) -> 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),), + smallvec!(Range::point(2), Range::point(5), Range::point(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); + } + } + + /// ([]) -> insert ( -> (([])) + #[test] + fn test_insert_open_inside_pair() { + let sel = Selection::single(2, 1); + let expected_sel = Selection::point(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, &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); + + 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); + } + } + } + + /// []word -> insert ( -> ([]word + #[test] + fn test_insert_open_before_non_pair() { + test_hooks_with_pairs( + &Rope::from("word"), + &Selection::single(1, 0), + PAIRS, + |open, _| format!("{}word", open), + &Selection::point(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), + PAIRS, + |open, _| format!("{}word", open), + &Selection::single(1, 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("word"); + let sel = Selection::single(5, 4); + let expected_sel = Selection::point(5); + + test_hooks_with_pairs( + &doc, + &sel, + differing_pairs(), + |open, close| format!("word{}{}", open, close), + &expected_sel, + ); + + test_hooks_with_pairs( + &doc, + &sel, + matching_pairs(), + |open, _| format!("word{}", open), + &expected_sel, + ); + } } diff --git a/helix-core/src/transaction.rs b/helix-core/src/transaction.rs index b62f4a9bd93b..d8d389f3b61a 100644 --- a/helix-core/src/transaction.rs +++ b/helix-core/src/transaction.rs @@ -409,7 +409,7 @@ impl ChangeSet { /// Transaction represents a single undoable unit of changes. Several changes can be grouped into /// a single transaction. -#[derive(Debug, Default, Clone)] +#[derive(Debug, Default, Clone, PartialEq, Eq)] pub struct Transaction { changes: ChangeSet, selection: Option, diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs index 87c5a63f6b79..3e5f60016147 100644 --- a/helix-term/src/commands.rs +++ b/helix-term/src/commands.rs @@ -4184,8 +4184,9 @@ 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 t = Tendril::from_char(ch); - let transaction = Transaction::insert(doc, selection, t); + let transaction = Transaction::insert(doc, &cursors, t); Some(transaction) } @@ -4200,11 +4201,11 @@ pub mod insert { }; let text = doc.text(); - let selection = doc.selection(view.id).clone().cursors(text.slice(..)); + let selection = doc.selection(view.id); // run through insert hooks, stopping on the first one that returns Some(t) for hook in hooks { - if let Some(transaction) = hook(text, &selection, c) { + if let Some(transaction) = hook(text, selection, c) { doc.apply(&transaction, view.id); break; } From ebb19d923a471eadf19e4c89dcbe22a199b8ea9c Mon Sep 17 00:00:00 2001 From: Skyler Hawthorne Date: Tue, 7 Dec 2021 23:43:14 -0500 Subject: [PATCH 2/6] move comment --- helix-core/src/auto_pairs.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/helix-core/src/auto_pairs.rs b/helix-core/src/auto_pairs.rs index 069f6af58dc5..1c536e994011 100644 --- a/helix-core/src/auto_pairs.rs +++ b/helix-core/src/auto_pairs.rs @@ -173,13 +173,13 @@ fn handle_same( let start_head = start_range.head; let end_head = start_head + offs + token.len_utf8(); + // if selection, retain anchor, if cursor, move over let end_anchor = if start_range.is_empty() { end_head } else { start_range.anchor + offs }; - // if selection, retain anchor, if cursor, move over end_ranges.push(Range::new(end_anchor, end_head)); let next = next_char(doc, start_head); From 22202a72679ecf61980363b291a45b8878d1f700 Mon Sep 17 00:00:00 2001 From: Skyler Hawthorne Date: Tue, 7 Dec 2021 23:48:52 -0500 Subject: [PATCH 3/6] use multiple of token utf8 length for initial Tendril capacity --- helix-core/src/auto_pairs.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/helix-core/src/auto_pairs.rs b/helix-core/src/auto_pairs.rs index 1c536e994011..8f6cd8a11077 100644 --- a/helix-core/src/auto_pairs.rs +++ b/helix-core/src/auto_pairs.rs @@ -189,7 +189,7 @@ fn handle_same( // return transaction that moves past close (start_head, start_head, None) // no-op } else { - let mut pair = Tendril::with_capacity(2); + 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 From ed4cffce954df4da6de9095f8a3e9eb6255ad4fb Mon Sep 17 00:00:00 2001 From: Skyler Hawthorne Date: Tue, 7 Dec 2021 23:57:36 -0500 Subject: [PATCH 4/6] use Rope::get_char instead of doing our own bounds checking --- helix-core/src/auto_pairs.rs | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-) diff --git a/helix-core/src/auto_pairs.rs b/helix-core/src/auto_pairs.rs index 8f6cd8a11077..3934c9c6197c 100644 --- a/helix-core/src/auto_pairs.rs +++ b/helix-core/src/auto_pairs.rs @@ -58,13 +58,6 @@ pub fn hook(doc: &Rope, selection: &Selection, ch: char) -> Option None } -fn next_char(doc: &Rope, pos: usize) -> Option { - if pos >= doc.len_chars() { - return None; - } - Some(doc.char(pos)) -} - fn prev_char(doc: &Rope, mut pos: usize) -> Option { if pos == 0 { return None; @@ -72,7 +65,7 @@ fn prev_char(doc: &Rope, mut pos: usize) -> Option { pos -= 1; - next_char(doc, pos) + doc.get_char(pos) } fn handle_open( @@ -89,7 +82,7 @@ fn handle_open( let transaction = Transaction::change_by_selection(doc, selection, |start_range| { let start_head = start_range.head; - let next = next_char(doc, start_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() { @@ -132,7 +125,7 @@ fn handle_close(doc: &Rope, selection: &Selection, _open: char, close: char) -> let transaction = Transaction::change_by_selection(doc, selection, |start_range| { let start_head = start_range.head; - let next = next_char(doc, start_head); + let next = doc.get_char(start_head); let end_head = start_head + offs + close.len_utf8(); let end_anchor = if start_range.is_empty() { @@ -182,7 +175,7 @@ fn handle_same( end_ranges.push(Range::new(end_anchor, end_head)); - let next = next_char(doc, start_head); + let next = doc.get_char(start_head); let prev = prev_char(doc, start_head); if next == Some(token) { From 2c62d561551ea2f3d5264a3db4e2d8e00627cac6 Mon Sep 17 00:00:00 2001 From: Skyler Hawthorne Date: Sun, 12 Dec 2021 15:42:22 -0500 Subject: [PATCH 5/6] consolidate logic, get rid of needless mut --- helix-core/src/auto_pairs.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/helix-core/src/auto_pairs.rs b/helix-core/src/auto_pairs.rs index 3934c9c6197c..fc7bb4b223dd 100644 --- a/helix-core/src/auto_pairs.rs +++ b/helix-core/src/auto_pairs.rs @@ -58,14 +58,12 @@ pub fn hook(doc: &Rope, selection: &Selection, ch: char) -> Option None } -fn prev_char(doc: &Rope, mut pos: usize) -> Option { +fn prev_char(doc: &Rope, pos: usize) -> Option { if pos == 0 { return None; } - pos -= 1; - - doc.get_char(pos) + doc.get_char(pos - 1) } fn handle_open( From 33f594118320a92a43aecac75e2f4ad3a6d0ab08 Mon Sep 17 00:00:00 2001 From: Skyler Hawthorne Date: Sun, 12 Dec 2021 15:51:27 -0500 Subject: [PATCH 6/6] use len_utf8 instead of assuming ascii --- helix-core/src/auto_pairs.rs | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-) diff --git a/helix-core/src/auto_pairs.rs b/helix-core/src/auto_pairs.rs index fc7bb4b223dd..c037afef4d72 100644 --- a/helix-core/src/auto_pairs.rs +++ b/helix-core/src/auto_pairs.rs @@ -93,19 +93,14 @@ fn handle_open( match next { Some(ch) if !close_before.contains(ch) => { - offs += 1; - // TODO: else return (use default handler that inserts open) + offs += open.len_utf8(); (start_head, start_head, Some(Tendril::from_char(open))) } // None | Some(ch) if close_before.contains(ch) => {} _ => { // insert open & close - let mut pair = Tendril::with_capacity(2); - pair.push_char(open); - pair.push_char(close); - - offs += 2; - + let pair = Tendril::from_iter([open, close]); + offs += open.len_utf8() + close.len_utf8(); (start_head, start_head, Some(pair)) } } @@ -139,8 +134,6 @@ fn handle_close(doc: &Rope, selection: &Selection, _open: char, close: char) -> (start_head, start_head, None) // no-op } else { offs += close.len_utf8(); - - // TODO: else return (use default handler that inserts close) (start_head, start_head, Some(Tendril::from_char(close))) } }); @@ -192,7 +185,6 @@ fn handle_same( } offs += pair.len(); - (start_head, start_head, Some(pair)) } });