Skip to content

Commit

Permalink
Fix panic when using surround_replace/delete (helix-editor#9796)
Browse files Browse the repository at this point in the history
1. Create a document containing `{A}`
1. C-w v # vsplit
1. gl    # goto_line_end
1. b     # move_prev_word_start
1. `     # switch_to_lowercase
1. mrm(  # surround replace
1. C-w v # vsplit

In the debug build surround_replace/delete will immedately assert with
`assertion failed: last <= from', transaction.rs:597:13`. The splits and
lowercase conversion are not needed to trigger the bug.

In the release build the surround becomes `)a(` and the last vsplit
causes the transaction to panic.
`internal error: entered unreachable code:
(Some(Retain(18446744073709551573)))', transaction.rs:185:46`

Since the selection direction is backwards get_surround_pos returns the
pairs reversed but the downstream code assumes they are in the forward
direction.
  • Loading branch information
trink authored and Schuyler Mortimer committed Jul 10, 2024
1 parent cdb019e commit cdf377c
Show file tree
Hide file tree
Showing 2 changed files with 56 additions and 1 deletion.
3 changes: 2 additions & 1 deletion helix-core/src/surround.rs
Original file line number Diff line number Diff line change
Expand Up @@ -260,7 +260,8 @@ pub fn get_surround_pos(
if change_pos.contains(&open_pos) || change_pos.contains(&close_pos) {
return Err(Error::CursorOverlap);
}
change_pos.extend_from_slice(&[open_pos, close_pos]);
// ensure the positions are always paired in the forward direction
change_pos.extend_from_slice(&[open_pos.min(close_pos), close_pos.max(open_pos)]);
}
Ok(change_pos)
}
Expand Down
54 changes: 54 additions & 0 deletions helix-term/tests/test/movement.rs
Original file line number Diff line number Diff line change
Expand Up @@ -552,3 +552,57 @@ async fn find_char_line_ending() -> anyhow::Result<()> {

Ok(())
}

#[tokio::test(flavor = "multi_thread")]
async fn test_surround_replace() -> anyhow::Result<()> {
test((
platform_line(indoc! {"\
(#[|a]#)
"}),
"mrm{",
platform_line(indoc! {"\
{#[|a]#}
"}),
))
.await?;

test((
platform_line(indoc! {"\
(#[a|]#)
"}),
"mrm{",
platform_line(indoc! {"\
{#[a|]#}
"}),
))
.await?;

Ok(())
}

#[tokio::test(flavor = "multi_thread")]
async fn test_surround_delete() -> anyhow::Result<()> {
test((
platform_line(indoc! {"\
(#[|a]#)
"}),
"mdm",
platform_line(indoc! {"\
#[|a]#
"}),
))
.await?;

test((
platform_line(indoc! {"\
(#[a|]#)
"}),
"mdm",
platform_line(indoc! {"\
#[a|]#
"}),
))
.await?;

Ok(())
}

0 comments on commit cdf377c

Please sign in to comment.