diff options
author | Mike Trinkala | 2024-03-07 17:20:07 +0000 |
---|---|---|
committer | GitHub | 2024-03-07 17:20:07 +0000 |
commit | cb01e52cd8b8021686ee98dd4d53dff8cdc826a9 (patch) | |
tree | 270c2ab4d42aa025fe16819b32dbde039c12cc08 | |
parent | b93fae9c8b955e11f427979134e3494294e8e2e0 (diff) |
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.
-rw-r--r-- | helix-term/src/commands.rs | 19 | ||||
-rw-r--r-- | 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 0b2ea0b8..4ac2496e 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 0873edbe..4ebaae85 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(()) } |