From 00c75e68082875d7aaafc6cbe5d76d2249287088 Mon Sep 17 00:00:00 2001 From: Skyler Hawthorne Date: Sat, 3 Jun 2023 14:34:19 -0400 Subject: [PATCH] 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 | 32 ++++++- helix-core/src/transaction.rs | 44 ++++----- helix-term/src/commands.rs | 143 ++++++++++++++-------------- helix-term/tests/test/auto_pairs.rs | 20 ++-- 4 files changed, 133 insertions(+), 106 deletions(-) diff --git a/helix-core/src/auto_pairs.rs b/helix-core/src/auto_pairs.rs index 467d55d92..fb918a9a2 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, 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 { if pos == 0 { return None; diff --git a/helix-core/src/transaction.rs b/helix-core/src/transaction.rs index e0d6abc39..19b6ef26a 100644 --- a/helix-core/src/transaction.rs +++ b/helix-core/src/transaction.rs @@ -760,21 +760,19 @@ impl Transaction { change_size = -change_size; } - if let Some(end_range) = end_range { - let offset_range = Range::new( - (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); + let new_range = if let Some(end_range) = end_range { + end_range } else { let changeset = ChangeSet::from_change(doc, (from, to, replacement.clone())); - let end_range = start_range.map(&changeset); - end_ranges.push(end_range); - } + start_range.map(&changeset) + }; + + 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 offset_range = Range::new( - 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); + let new_range = if let Some(end_range) = end_range { + end_range } else { let changeset = ChangeSet::from_change(doc, (from, to, None)); - let end_range = start_range.map(&changeset); - end_ranges.push(end_range); - } + start_range.map(&changeset) + }; + + 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); diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs index 5c755c8c1..61c3db644 100644 --- a/helix-term/src/commands.rs +++ b/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 { + 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. - let fragment = Cow::from(text.slice(line_start_pos..pos)); - if !fragment.is_empty() && fragment.chars().all(|ch| ch == ' ' || ch == '\t') { - if text.get_char(pos.saturating_sub(1)) == Some('\t') { - // fast path, delete one char + + dedent(doc, range) + .map(|dedent| (dedent, None)) + .or_else(|| { + auto_pairs::hook_delete(doc.text(), range, doc.auto_pairs(cx.editor)?) + .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); } diff --git a/helix-term/tests/test/auto_pairs.rs b/helix-term/tests/test/auto_pairs.rs index 1768ec84f..04fec1c4e 100644 --- a/helix-term/tests/test/auto_pairs.rs +++ b/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", - 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", - 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", - 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", - 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", - format!("hello #[๐Ÿ‘จโ€๐Ÿ‘ฉโ€๐Ÿ‘งโ€๐Ÿ‘ฆ|]# goodbye{}", LINE_END), + format!("hello #[|๐Ÿ‘จโ€๐Ÿ‘ฉโ€๐Ÿ‘งโ€๐Ÿ‘ฆ]# goodbye{}", LINE_END), )) .await?; } @@ -988,7 +988,7 @@ async fn delete_mixed_dedent() -> anyhow::Result<()> { )), "i", 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", helpers::platform_line(indoc! {"\ - bar = #[woop|]# - #(|w)#ord - #(|fo)# + bar = #[woop\n|]# + #(w|)#ord + #(fo|)# "}), )) .await?;