From d319dbfcc0d2546f8299c3d62f557e7855079b73 Mon Sep 17 00:00:00 2001 From: Skyler Hawthorne Date: Thu, 1 Jun 2023 14:34:52 -0400 Subject: [PATCH] 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 }); }