Delete pairs with multi-char-range selections

This completes auto pair deletions. Currently, auto pairs only get
deleted when the range is a single grapheme wide, since otherwise,
the selection would get computed incorrectly through the normal change
mapping process. Now auto pairs get deleted even with larger ranges, and
the resulting selection is correct.

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, Change, Range, Rope, Tendril};
+use crate::{graphemes, movement::Direction, Change, Deletion, Range, Rope, Tendril};
 use std::collections::HashMap;
 
 // Heavily based on https://github.com/codemirror/closebrackets/
@@ -132,6 +132,36 @@ pub fn hook_insert(
     None
 }
 
+#[must_use]
+pub fn hook_delete(doc: &Rope, range: &Range, pairs: &AutoPairs) -> Option<(Deletion, Range)> {
+    let text = doc.slice(..);
+    let cursor = range.cursor(text);
+
+    let cur = doc.get_char(cursor)?;
+    let prev = prev_char(doc, cursor)?;
+    let pair = pairs.get(cur)?;
+
+    if pair.open != prev {
+        return None;
+    }
+
+    let end_next = graphemes::next_grapheme_boundary(text, cursor);
+    let end_prev = graphemes::prev_grapheme_boundary(text, cursor);
+
+    let delete = (end_prev, end_next);
+    let size_delete = end_next - end_prev;
+    let next_head = graphemes::next_grapheme_boundary(text, range.head) - size_delete;
+
+    let next_range = match range.direction() {
+        Direction::Forward => Range::new(range.anchor, next_head),
+        Direction::Backward => Range::new(range.anchor - size_delete, next_head),
+    };
+
+    log::trace!("auto pair delete: {:?}, range: {:?}", delete, range,);
+
+    Some((delete, next_range))
+}
+
 fn prev_char(doc: &Rope, pos: usize) -> Option<char> {
     if pos == 0 {
         return None;

helix-core/src/transaction.rs

@@ -760,21 +760,19 @@ impl Transaction {
                 change_size = -change_size;
             }
 
-            if let Some(end_range) = end_range {
+            let new_range = if let Some(end_range) = end_range {
-                let offset_range = Range::new(
+                end_range
-                    (end_range.anchor as isize + offset) as usize,
-                    (end_range.head as isize + offset) as usize,
-                );
-
-                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);
+                start_range.map(&changeset)
-                end_ranges.push(end_range);
+            };
-            }
+
+            let offset_range = Range::new(
+                (new_range.anchor as isize + offset) as usize,
+                (new_range.head as isize + offset) as usize,
+            );
 
+            end_ranges.push(offset_range);
             offset += change_size;
 
             log::trace!(
@@ -820,21 +818,19 @@ impl Transaction {
             let ((from, to), end_range) = f(start_range);
             let change_size = to - from;
 
-            if let Some(end_range) = end_range {
+            let new_range = if let Some(end_range) = end_range {
-                let offset_range = Range::new(
+                end_range
-                    end_range.anchor.saturating_sub(offset),
-                    end_range.head.saturating_sub(offset),
-                );
-
-                log::trace!("end range {:?} offset to: {:?}", end_range, offset_range);
-
-                end_ranges.push(offset_range);
             } else {
                 let changeset = ChangeSet::from_change(doc, (from, to, None));
-                let end_range = start_range.map(&changeset);
+                start_range.map(&changeset)
-                end_ranges.push(end_range);
+            };
-            }
+
+            let offset_range = Range::new(
+                new_range.anchor.saturating_sub(offset),
+                new_range.head.saturating_sub(offset),
+            );
 
+            end_ranges.push(offset_range);
             offset += change_size;
 
             log::trace!("delete from: {}, to: {}, offset: {}", from, to, offset);

helix-term/src/commands.rs

@@ -3870,7 +3870,7 @@ pub mod insert {
                 .and_then(|ap| {
                     auto_pairs::hook_insert(text, range, c, ap)
                         .map(|(change, range)| (change, Some(range)))
-                        .or(Some(insert_char(*range, c)))
+                        .or_else(|| Some(insert_char(*range, c)))
                 })
                 .unwrap_or_else(|| insert_char(*range, c))
         });
@@ -4047,95 +4047,96 @@ pub mod insert {
         doc.apply(&transaction, view.id);
     }
 
+    fn dedent(doc: &Document, range: &Range) -> Option<Deletion> {
+        let text = doc.text().slice(..);
+        let pos = range.cursor(text);
+        let line_start_pos = text.line_to_char(range.cursor_line(text));
+
+        // consider to delete by indent level if all characters before `pos` are indent units.
+        let fragment = Cow::from(text.slice(line_start_pos..pos));
+
+        if fragment.is_empty() || !fragment.chars().all(|ch| ch == ' ' || ch == '\t') {
+            return None;
+        }
+
+        if text.get_char(pos.saturating_sub(1)) == Some('\t') {
+            // fast path, delete one char
+            return Some((graphemes::nth_prev_grapheme_boundary(text, pos, 1), pos));
+        }
+
+        let tab_width = doc.tab_width();
+        let indent_width = doc.indent_width();
+
+        let width: usize = fragment
+            .chars()
+            .map(|ch| {
+                if ch == '\t' {
+                    tab_width
+                } else {
+                    // it can be none if it still meet control characters other than '\t'
+                    // here just set the width to 1 (or some value better?).
+                    ch.width().unwrap_or(1)
+                }
+            })
+            .sum();
+
+        // round down to nearest unit
+        let mut drop = width % indent_width;
+
+        // if it's already at a unit, consume a whole unit
+        if drop == 0 {
+            drop = indent_width
+        };
+
+        let mut chars = fragment.chars().rev();
+        let mut start = pos;
+
+        for _ in 0..drop {
+            // delete up to `drop` spaces
+            match chars.next() {
+                Some(' ') => start -= 1,
+                _ => break,
+            }
+        }
+
+        Some((start, pos)) // delete!
+    }
+
     pub fn delete_char_backward(cx: &mut Context) {
         let count = cx.count();
         let (view, doc) = current_ref!(cx.editor);
         let text = doc.text().slice(..);
-        let tab_width = doc.tab_width();
-        let indent_width = doc.indent_width();
-        let auto_pairs = doc.auto_pairs(cx.editor);
 
         let transaction = Transaction::delete_by_and_with_selection(
             doc.text(),
             doc.selection(view.id),
             |range| {
                 let pos = range.cursor(text);
+
+                log::debug!("cursor: {}, len: {}", pos, text.len_chars());
+
                 if pos == 0 {
                     return ((pos, pos), None);
                 }
-                let line_start_pos = text.line_to_char(range.cursor_line(text));
+
-                // consider to delete by indent level if all characters before `pos` are indent units.
+                dedent(doc, range)
-                let fragment = Cow::from(text.slice(line_start_pos..pos));
+                    .map(|dedent| (dedent, None))
-                if !fragment.is_empty() && fragment.chars().all(|ch| ch == ' ' || ch == '\t') {
+                    .or_else(|| {
-                    if text.get_char(pos.saturating_sub(1)) == Some('\t') {
+                        auto_pairs::hook_delete(doc.text(), range, doc.auto_pairs(cx.editor)?)
-                        // fast path, delete one char
+                            .map(|(delete, new_range)| (delete, Some(new_range)))
+                    })
+                    .unwrap_or_else(|| {
                         (
-                            (graphemes::nth_prev_grapheme_boundary(text, pos, 1), pos),
+                            (graphemes::nth_prev_grapheme_boundary(text, pos, count), pos),
                             None,
                         )
-                    } else {
+                    })
-                        let width: usize = fragment
-                            .chars()
-                            .map(|ch| {
-                                if ch == '\t' {
-                                    tab_width
-                                } else {
-                                    // it can be none if it still meet control characters other than '\t'
-                                    // here just set the width to 1 (or some value better?).
-                                    ch.width().unwrap_or(1)
-                                }
-                            })
-                            .sum();
-                        let mut drop = width % indent_width; // round down to nearest unit
-                        if drop == 0 {
-                            drop = indent_width
-                        }; // if it's already at a unit, consume a whole unit
-                        let mut chars = fragment.chars().rev();
-                        let mut start = pos;
-                        for _ in 0..drop {
-                            // delete up to `drop` spaces
-                            match chars.next() {
-                                Some(' ') => start -= 1,
-                                _ => break,
-                            }
-                        }
-                        ((start, pos), None) // delete!
-                    }
-                } else {
-                    match (
-                        text.get_char(pos.saturating_sub(1)),
-                        text.get_char(pos),
-                        auto_pairs,
-                    ) {
-                        (Some(_x), Some(_y), Some(ap))
-                            if range.is_single_grapheme(text)
-                                && ap.get(_x).is_some()
-                                && ap.get(_x).unwrap().open == _x
-                                && ap.get(_x).unwrap().close == _y =>
-                        // delete both autopaired characters
-                        {
-                            (
-                                (
-                                    graphemes::nth_prev_grapheme_boundary(text, pos, count),
-                                    graphemes::nth_next_grapheme_boundary(text, pos, count),
-                                ),
-                                None,
-                            )
-                        }
-                        _ =>
-                        // delete 1 char
-                        {
-                            (
-                                (graphemes::nth_prev_grapheme_boundary(text, pos, count), pos),
-                                None,
-                            )
-                        }
-                    }
-                }
             },
         );
 
-        let (view, doc) = current!(cx.editor);
+        log::debug!("delete_char_backward transaction: {:?}", transaction);
+
+        let doc = doc_mut!(cx.editor, &doc.id());
         doc.apply(&transaction, view.id);
     }
 

helix-term/tests/test/auto_pairs.rs

@@ -676,7 +676,7 @@ async fn delete_before_word_selection() -> anyhow::Result<()> {
         test((
             format!("{}#[|foo]#{}", pair.0, LINE_END),
             "i<backspace>",
-            format!("#[foo|]#{}", LINE_END),
+            format!("#[|foo]#{}", LINE_END),
         ))
         .await?;
 
@@ -684,7 +684,7 @@ async fn delete_before_word_selection() -> anyhow::Result<()> {
         test((
             format!("{}{}#[|foo]#{}", pair.0, pair.1, LINE_END),
             "i<backspace>",
-            format!("{}#[foo|]#{}", pair.0, LINE_END),
+            format!("{}#[|foo]#{}", pair.0, LINE_END),
         ))
         .await?;
 
@@ -692,7 +692,7 @@ async fn delete_before_word_selection() -> anyhow::Result<()> {
         test((
             format!("{}#[|{}foo]#{}", pair.0, pair.1, LINE_END),
             "i<backspace>",
-            format!("#[foo|]#{}", LINE_END),
+            format!("#[|foo]#{}", LINE_END),
         ))
         .await?;
     }
@@ -706,7 +706,7 @@ async fn delete_before_word_selection_trailing_word() -> anyhow::Result<()> {
         test((
             format!("foo{}#[|{} wor]#{}", pair.0, pair.1, LINE_END),
             "i<backspace>",
-            format!("foo#[ wor|]#{}", LINE_END),
+            format!("foo#[| wor]#{}", LINE_END),
         ))
         .await?;
     }
@@ -811,7 +811,7 @@ async fn delete_before_multi_code_point_graphemes() -> anyhow::Result<()> {
                 pair.0, pair.1, LINE_END
             ),
             "i<backspace>",
-            format!("hello #[👨‍👩‍👧‍👦|]# goodbye{}", LINE_END),
+            format!("hello #[|👨‍👩‍👧‍👦]# goodbye{}", LINE_END),
         ))
         .await?;
     }
@@ -988,7 +988,7 @@ async fn delete_mixed_dedent() -> anyhow::Result<()> {
             )),
             "i<backspace>",
             helpers::platform_line(indoc! {"\
-                bar = #[woop|]#
+                bar = #[|woop]#
                 #(|word)#
                 f#(|o)#
             "}),
@@ -1000,16 +1000,16 @@ async fn delete_mixed_dedent() -> anyhow::Result<()> {
             helpers::platform_line(&format!(
                 indoc! {"\
                     bar = #[|woop{}]#{}
-                    #(|   )# word
+                    #(|    )#word
                     #(|fo)#o
                 "},
                 pair.0, pair.1,
             )),
             "a<backspace>",
             helpers::platform_line(indoc! {"\
-                bar = #[woop|]#
+                bar = #[woop\n|]#
-                #(|w)#ord
+                #(w|)#ord
-                #(|fo)#
+                #(fo|)#
             "}),
         ))
         .await?;