From 19dc22bdec8bc1fd6b47725f06b2f6e5d710aa7b Mon Sep 17 00:00:00 2001 From: Mike Trinkala 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 0b2ea0b8a52e..4ac2496ebd48 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 0873edbe503e..4ebaae854fdc 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(()) }