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

@@ -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<Transaction>
-// problem is, we want to do this per range, so we can call default handler for some ranges
-// so maybe ret Vec<Option<Change>>
-// but we also need to be able to return transactions...
-//
-// to simplify, maybe return Option<Transaction> 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<Transaction> {
+pub fn hook(doc: &Rope, range: &Range, ch: char, pairs: &AutoPairs) -> Option<(Change, Range)> {
-    log::trace!("autopairs hook selection: {:#?}", selection);
+    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,9 +243,8 @@ 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 {
+fn handle_open(doc: &Rope, range: &Range, pair: &Pair) -> Option<(Change, Range)> {
-    let transaction = Transaction::change_by_and_with_selection(doc, selection, |start_range| {
+    let cursor = range.cursor(doc.slice(..));
-        let cursor = start_range.cursor(doc.slice(..));
     let next_char = doc.get_char(cursor);
     let len_inserted;
 
@@ -261,11 +252,8 @@ fn handle_open(doc: &Rope, selection: &Selection, pair: &Pair) -> Transaction {
     // 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) => {
+        Some(_) if !pair.should_close(doc, range) => {
-                len_inserted = 1;
+            return None;
-                let mut tendril = Tendril::new();
-                tendril.push(pair.open);
-                (cursor, cursor, Some(tendril))
         }
         _ => {
             // insert open & close
@@ -275,44 +263,36 @@ fn handle_open(doc: &Rope, selection: &Selection, pair: &Pair) -> Transaction {
         }
     };
 
-        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);
+    Some(result)
-    transaction
 }
 
-fn handle_close(doc: &Rope, selection: &Selection, pair: &Pair) -> Transaction {
+fn handle_close(doc: &Rope, range: &Range, pair: &Pair) -> Option<(Change, Range)> {
-    let transaction = Transaction::change_by_and_with_selection(doc, selection, |start_range| {
+    let cursor = range.cursor(doc.slice(..));
-        let cursor = start_range.cursor(doc.slice(..));
     let next_char = doc.get_char(cursor);
-        let mut len_inserted = 0;
 
     let change = if next_char == Some(pair.close) {
         // return transaction that moves past close
         (cursor, cursor, None) // no-op
     } else {
-            len_inserted = 1;
+        return None;
-            let mut tendril = Tendril::new();
-            tendril.push(pair.close);
-            (cursor, cursor, Some(tendril))
     };
 
-        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);
+    Some(result)
-    transaction
 }
 
 /// handle cases where open and close is the same, or in triples ("""docstring""")
-fn handle_same(doc: &Rope, selection: &Selection, pair: &Pair) -> Transaction {
+fn handle_same(doc: &Rope, range: &Range, pair: &Pair) -> Option<(Change, Range)> {
-    let transaction = Transaction::change_by_and_with_selection(doc, selection, |start_range| {
+    let cursor = range.cursor(doc.slice(..));
-        let cursor = start_range.cursor(doc.slice(..));
     let mut len_inserted = 0;
     let next_char = doc.get_char(cursor);
 
@@ -320,25 +300,19 @@ fn handle_same(doc: &Rope, selection: &Selection, pair: &Pair) -> Transaction {
         // return transaction that moves past close
         (cursor, cursor, None) // no-op
     } else {
-            let mut pair_str = Tendril::new();
+        if !pair.should_close(doc, range) {
-            pair_str.push(pair.open);
+            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);
         }
 
-            len_inserted += pair_str.chars().count();
+        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 transaction: {:#?}", transaction);
+    log::debug!("auto pair change: {:#?}", &result);
 
-    transaction
+    Some(result)
 }

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<I>(doc: &Rope, changes: I) -> Self
+    where
+        I: Iterator<Item = Change>,
+    {
+        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<Item = Change>,
     {
-        let len = doc.len_chars();
+        Self::from(ChangeSet::from_changes(doc, changes))
-
-        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)
     }
 
     /// 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
+    /// is returned by the function, that range is mapped through the change
-    /// selection.
+    /// as usual.
     pub fn change_by_and_with_selection<F>(doc: &Rope, selection: &Selection, mut f: F) -> Self
     where
         F: FnMut(&Range) -> (Change, Option<Range>),
@@ -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;

helix-term/src/commands.rs

@@ -3849,16 +3849,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<Transaction> {
-        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;
 
@@ -3868,15 +3858,25 @@ pub mod insert {
         let selection = doc.selection(view.id);
         let auto_pairs = doc.auto_pairs(cx.editor);
 
-        let transaction = auto_pairs
+        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 transaction = Transaction::change_by_and_with_selection(text, selection, |range| {
+            auto_pairs
                 .as_ref()
-            .and_then(|ap| auto_pairs::hook(text, selection, c, ap))
+                .and_then(|ap| {
-            .or_else(|| insert(text, selection, c));
+                    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 (view, doc) = current!(cx.editor);
+        let doc = doc_mut!(cx.editor, &doc.id());
-        if let Some(t) = transaction {
+        doc.apply(&transaction, view.id);
-            doc.apply(&t, view.id);
-        }
 
         helix_event::dispatch(PostInsertChar { c, cx });
     }