From 5bd007266a962a534bd722619821e998735b71e2 Mon Sep 17 00:00:00 2001 From: Mike Trinkala Date: Sat, 2 Mar 2024 06:05:58 -0800 Subject: [PATCH] Fix panic when using join_selections_space (#9783) Joining lines with Alt-J does not properly select the inserted spaces when the selection contains blank lines. In the worst case it panics with an out of bounds index. thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: Char index out of bounds: char index 11, Rope/RopeSlice char length 10' Steps to reproduce: * Create a new document ``` a b c d e ``` * % (Select all) * Alt-J (join and select the spaces) --- helix-term/src/commands.rs | 23 ++++++--- helix-term/tests/test/commands.rs | 83 +++++++++++++++++++++++++++++++ 2 files changed, 100 insertions(+), 6 deletions(-) diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs index bd0a60b7..0b2ea0b8 100644 --- a/helix-term/src/commands.rs +++ b/helix-term/src/commands.rs @@ -4372,16 +4372,27 @@ fn join_selections_impl(cx: &mut Context, select_space: bool) { // select inserted spaces let transaction = if select_space { + let mut offset: usize = 0; let ranges: SmallVec<_> = changes .iter() - .scan(0, |offset, change| { - let range = Range::point(change.0 - *offset); - *offset += change.1 - change.0 - 1; // -1 because cursor is 0-sized - Some(range) + .filter_map(|change| { + if change.2.is_some() { + let range = Range::point(change.0 - offset); + offset += change.1 - change.0 - 1; // -1 adjusts for the replacement of the range by a space + Some(range) + } else { + offset += change.1 - change.0; + None + } }) .collect(); - let selection = Selection::new(ranges, 0); - Transaction::change(text, changes.into_iter()).with_selection(selection) + let t = Transaction::change(text, changes.into_iter()); + if ranges.is_empty() { + t + } else { + let selection = Selection::new(ranges, 0); + t.with_selection(selection) + } } else { Transaction::change(text, changes.into_iter()) }; diff --git a/helix-term/tests/test/commands.rs b/helix-term/tests/test/commands.rs index e52b142c..1172a798 100644 --- a/helix-term/tests/test/commands.rs +++ b/helix-term/tests/test/commands.rs @@ -526,3 +526,86 @@ async fn test_join_selections() -> anyhow::Result<()> { Ok(()) } + +#[tokio::test(flavor = "multi_thread")] +async fn test_join_selections_space() -> anyhow::Result<()> { + // join with empty lines panic + test(( + platform_line(indoc! {"\ + #[a + + b + + c + + d + + e|]# + "}), + "", + platform_line(indoc! {"\ + a#[ |]#b#( |)#c#( |)#d#( |)#e + "}), + )) + .await?; + + // normal join + test(( + platform_line(indoc! {"\ + #[a|]#bc + def + "}), + "", + platform_line(indoc! {"\ + abc#[ |]#def + "}), + )) + .await?; + + // join with empty line + test(( + platform_line(indoc! {"\ + #[a|]#bc + + def + "}), + "", + platform_line(indoc! {"\ + #[a|]#bc + def + "}), + )) + .await?; + + // join with additional space in non-empty line + test(( + platform_line(indoc! {"\ + #[a|]#bc + + def + "}), + "", + platform_line(indoc! {"\ + abc#[ |]#def + "}), + )) + .await?; + + // join with retained trailing spaces + test(( + platform_line(indoc! {"\ + #[aaa + + bb + + c |]# + "}), + "", + platform_line(indoc! {"\ + aaa #[ |]#bb #( |)#c + "}), + )) + .await?; + + Ok(()) +}