From 9267343830228490d379c90537ff1a6e4bba1260 Mon Sep 17 00:00:00 2001 From: Mike Trinkala Date: Sun, 3 Mar 2024 09:55:09 -0800 Subject: [PATCH] Fix panic when using surround_replace/delete (#9796) 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. --- helix-core/src/surround.rs | 3 +- helix-term/tests/test/movement.rs | 54 +++++++++++++++++++++++++++++++ 2 files changed, 56 insertions(+), 1 deletion(-) diff --git a/helix-core/src/surround.rs b/helix-core/src/surround.rs index b96cce5a0664..513f874931f3 100644 --- a/helix-core/src/surround.rs +++ b/helix-core/src/surround.rs @@ -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) } diff --git a/helix-term/tests/test/movement.rs b/helix-term/tests/test/movement.rs index e3c2668daa54..0873edbe503e 100644 --- a/helix-term/tests/test/movement.rs +++ b/helix-term/tests/test/movement.rs @@ -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(()) +}