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.
This commit is contained in:
Mike Trinkala 2024-03-07 09:20:07 -08:00 committed by GitHub
parent b93fae9c8b
commit cb01e52cd8
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 43 additions and 5 deletions

View file

@ -5282,12 +5282,21 @@ fn surround_replace(cx: &mut Context) {
None => return doc.set_selection(view.id, selection), None => return doc.set_selection(view.id, selection),
}; };
let (open, close) = surround::get_pair(to); 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( let transaction = Transaction::change(
doc.text(), doc.text(),
change_pos.iter().enumerate().map(|(i, &pos)| { sorted_pos.iter().map(|&pos| {
let mut t = Tendril::new(); let mut t = Tendril::new();
t.push(if i % 2 == 0 { open } else { close }); t.push(pos.1);
(pos, pos + 1, Some(t)) (pos.0, pos.0 + 1, Some(t))
}), }),
); );
doc.set_selection(view.id, selection); doc.set_selection(view.id, selection);
@ -5309,14 +5318,14 @@ fn surround_delete(cx: &mut Context) {
let text = doc.text().slice(..); let text = doc.text().slice(..);
let selection = doc.selection(view.id); 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, Ok(c) => c,
Err(err) => { Err(err) => {
cx.editor.set_error(err.to_string()); cx.editor.set_error(err.to_string());
return; return;
} }
}; };
change_pos.sort_unstable(); // the changeset has to be sorted to allow nested surrounds
let transaction = let transaction =
Transaction::change(doc.text(), change_pos.into_iter().map(|p| (p, p + 1, None))); Transaction::change(doc.text(), change_pos.into_iter().map(|p| (p, p + 1, None)));
doc.apply(&transaction, view.id); doc.apply(&transaction, view.id);

View file

@ -577,6 +577,23 @@ async fn test_surround_replace() -> anyhow::Result<()> {
)) ))
.await?; .await?;
test((
platform_line(indoc! {"\
{{
#(}|)#
#[}|]#
"}),
"mrm)",
platform_line(indoc! {"\
((
#()|)#
#[)|]#
"}),
))
.await?;
Ok(()) Ok(())
} }
@ -604,5 +621,17 @@ async fn test_surround_delete() -> anyhow::Result<()> {
)) ))
.await?; .await?;
test((
platform_line(indoc! {"\
{{
#(}|)#
#[}|]#
"}),
"mdm",
platform_line("\n\n#(\n|)##[\n|]#"),
))
.await?;
Ok(()) Ok(())
} }