From cb01e52cd8b8021686ee98dd4d53dff8cdc826a9 Mon Sep 17 00:00:00 2001
From: Mike Trinkala <trink@acm.org>
Date: Thu, 7 Mar 2024 09:20:07 -0800
Subject: [PATCH] Fix panic in surround_replace/delete nested multi-cursor
 (#9815)

Test Document
-------------
```
{{

}
}

```

Steps To Reproduce
------------------
1. 2j  # move_visual_line_down
1. C   # copy_selection_on_next_line
1. mdm # surround_delete

Debug
-----
`assertion failed: last <= from', transaction.rs:597:13`

Release
-------
`called `Result::unwrap()` on an `Err` value: Char range out of bounds:
char range 18446744073709551614..18446744073709551615,
Rope/RopeSlice char length 7', ropey-1.6.1/src/rope.rs:546:37`

Description
-----------

Processing the surrounding pairs in order violates the assertion the
ranges are ordered. To handle nested surrounds all positions have to
be sorted. Also surround_replace has to track the proper replacement
character for each position.
---
 helix-term/src/commands.rs        | 19 ++++++++++++++-----
 helix-term/tests/test/movement.rs | 29 +++++++++++++++++++++++++++++
 2 files changed, 43 insertions(+), 5 deletions(-)

diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs
index 0b2ea0b8a..4ac2496eb 100644
--- a/helix-term/src/commands.rs
+++ b/helix-term/src/commands.rs
@@ -5282,12 +5282,21 @@ fn surround_replace(cx: &mut Context) {
                 None => return doc.set_selection(view.id, selection),
             };
             let (open, close) = surround::get_pair(to);
+
+            // the changeset has to be sorted to allow nested surrounds
+            let mut sorted_pos: Vec<(usize, char)> = Vec::new();
+            for p in change_pos.chunks(2) {
+                sorted_pos.push((p[0], open));
+                sorted_pos.push((p[1], close));
+            }
+            sorted_pos.sort_unstable();
+
             let transaction = Transaction::change(
                 doc.text(),
-                change_pos.iter().enumerate().map(|(i, &pos)| {
+                sorted_pos.iter().map(|&pos| {
                     let mut t = Tendril::new();
-                    t.push(if i % 2 == 0 { open } else { close });
-                    (pos, pos + 1, Some(t))
+                    t.push(pos.1);
+                    (pos.0, pos.0 + 1, Some(t))
                 }),
             );
             doc.set_selection(view.id, selection);
@@ -5309,14 +5318,14 @@ fn surround_delete(cx: &mut Context) {
         let text = doc.text().slice(..);
         let selection = doc.selection(view.id);
 
-        let change_pos = match surround::get_surround_pos(text, selection, surround_ch, count) {
+        let mut change_pos = match surround::get_surround_pos(text, selection, surround_ch, count) {
             Ok(c) => c,
             Err(err) => {
                 cx.editor.set_error(err.to_string());
                 return;
             }
         };
-
+        change_pos.sort_unstable(); // the changeset has to be sorted to allow nested surrounds
         let transaction =
             Transaction::change(doc.text(), change_pos.into_iter().map(|p| (p, p + 1, None)));
         doc.apply(&transaction, view.id);
diff --git a/helix-term/tests/test/movement.rs b/helix-term/tests/test/movement.rs
index 0873edbe5..4ebaae854 100644
--- a/helix-term/tests/test/movement.rs
+++ b/helix-term/tests/test/movement.rs
@@ -577,6 +577,23 @@ async fn test_surround_replace() -> anyhow::Result<()> {
     ))
     .await?;
 
+    test((
+        platform_line(indoc! {"\
+            {{
+
+            #(}|)#
+            #[}|]#
+            "}),
+        "mrm)",
+        platform_line(indoc! {"\
+            ((
+
+            #()|)#
+            #[)|]#
+            "}),
+    ))
+    .await?;
+
     Ok(())
 }
 
@@ -604,5 +621,17 @@ async fn test_surround_delete() -> anyhow::Result<()> {
     ))
     .await?;
 
+    test((
+        platform_line(indoc! {"\
+            {{
+
+            #(}|)#
+            #[}|]#
+            "}),
+        "mdm",
+        platform_line("\n\n#(\n|)##[\n|]#"),
+    ))
+    .await?;
+
     Ok(())
 }