Skip to content

Commit

Permalink
fix: crash on surround delete when direction is backward
Browse files Browse the repository at this point in the history
`find_nth_closest_pairs_pos` produces unsorted tuple. Need to sort it before passing tuple to the delete transaction.

helix-editor#6626
  • Loading branch information
woojiq committed Sep 15, 2023
1 parent 13d4463 commit befcd35
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 4 deletions.
19 changes: 15 additions & 4 deletions helix-core/src/surround.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use std::fmt::Display;
use std::{cmp, fmt::Display};

use crate::{movement::Direction, search, Range, Selection};
use ropey::RopeSlice;
Expand Down Expand Up @@ -52,6 +52,9 @@ pub fn get_pair(ch: char) -> (char, char) {
.unwrap_or((ch, ch))
}

/// Find the position of surround pairs of any [`PAIRS`].
///
/// Be careful, the second position in return tuple can be to the left of the first position if the range is backward.
pub fn find_nth_closest_pairs_pos(
text: RopeSlice,
range: Range,
Expand Down Expand Up @@ -253,10 +256,18 @@ pub fn get_surround_pos(
let mut change_pos = Vec::new();

for &range in selection {
let (open_pos, close_pos) = match ch {
Some(ch) => find_nth_pairs_pos(text, ch, range, skip)?,
None => find_nth_closest_pairs_pos(text, range, skip)?,
let (open_pos, close_pos) = {
let (lhs, rhs) = match ch {
Some(ch) => find_nth_pairs_pos(text, ch, range, skip)?,
None => find_nth_closest_pairs_pos(text, range, skip)?,
};
// If the direction of the range is backward, the second value is to the left of the first.
(cmp::min(lhs, rhs), cmp::max(lhs, rhs))
};

// WARN this will not find overlaps when use `m` (None) as find character.
// E.g. "( < c ( > c ) )", c - cursor. Change_pos = [3, 11, 7, 15].
// Overlaps occurs but this condition didn't find it because pairs have different open/close chars.
if change_pos.contains(&open_pos) || change_pos.contains(&close_pos) {
return Err(Error::CursorOverlap);
}
Expand Down
9 changes: 9 additions & 0 deletions helix-term/tests/test/commands.rs
Original file line number Diff line number Diff line change
Expand Up @@ -480,3 +480,12 @@ fn bar() {#(\n|)#\

Ok(())
}

#[tokio::test(flavor = "multi_thread")]
async fn surround_delete() -> anyhow::Result<()> {
// Test `surround_delete` when head < anchor
test(("(#[| ]#)", "mdm", "#[| ]#")).await?;
test(("(#[| ]#)", "md(", "#[| ]#")).await?;

Ok(())
}

0 comments on commit befcd35

Please sign in to comment.