From 4bd17e542ef4b6103b7b5821418c02fe3ab2ad62 Mon Sep 17 00:00:00 2001 From: Michael Davis Date: Wed, 15 Jan 2025 10:11:10 -0500 Subject: [PATCH] Fix offset tracking in `insert_newline` #12177 changed `insert_newline`'s behavior to trim any trailing whitespace on a line which came before a cursor. `insert_newline` would previously never delete text. Even the whitespace stripping behavior in #4854 worked by inserting text - a line ending at the beginning of the line. `global_offs`, a variable that tracks the number of characters inserted between iterations over the existing selection ranges, was not updated to also account for text deleted by the trimming behavior, causing cursors to be offset by the amount of trailing space deleted and causing panics in some cases. To fix this we need to subtract the number of trimmed whitespace characters from `global_offs`. `global_offs` must become an `isize` (was a `usize`) because it may become negative in cases where a lot of trailing whitespace is trimmed. Integration tests have been added for each of these cases. Fixes #12461 Fixes #12495 Fixes #12539 --- helix-term/src/commands.rs | 21 ++-- helix-term/tests/test/commands/insert.rs | 128 +++++++++++++++++++++++ 2 files changed, 140 insertions(+), 9 deletions(-) diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs index 755a7dc077bb..9ef1fe1533ba 100644 --- a/helix-term/src/commands.rs +++ b/helix-term/src/commands.rs @@ -3991,6 +3991,8 @@ pub mod insert { let mut global_offs = 0; let mut transaction = Transaction::change_by_selection(contents, &selection, |range| { + // Tracks the number of trailing whitespace characters deleted by this selection. + let mut chars_deleted = 0; let pos = range.cursor(text); let prev = if pos == 0 { @@ -4069,13 +4071,14 @@ pub mod insert { new_text.chars().count() }; + // Note that `first_trailing_whitespace_char` is at least `pos` so this unsigned + // subtraction cannot underflow. + chars_deleted = pos - first_trailing_whitespace_char; + ( first_trailing_whitespace_char, pos, - // Note that `first_trailing_whitespace_char` is at least `pos` so the - // unsigned subtraction (`pos - first_trailing_whitespace_char`) cannot - // underflow. - local_offs as isize - (pos - first_trailing_whitespace_char) as isize, + local_offs as isize - chars_deleted as isize, ) } else { // If the current line is all whitespace, insert a line ending at the beginning of @@ -4089,14 +4092,14 @@ pub mod insert { let new_range = if range.cursor(text) > range.anchor { // when appending, extend the range by local_offs Range::new( - range.anchor + global_offs, - (range.head as isize + local_offs) as usize + global_offs, + (range.anchor as isize + global_offs) as usize, + (range.head as isize + local_offs + global_offs) as usize, ) } else { // when inserting, slide the range by local_offs Range::new( - (range.anchor as isize + local_offs) as usize + global_offs, - (range.head as isize + local_offs) as usize + global_offs, + (range.anchor as isize + local_offs + global_offs) as usize, + (range.head as isize + local_offs + global_offs) as usize, ) }; @@ -4104,7 +4107,7 @@ pub mod insert { // range.replace(|range| range.is_empty(), head); -> fn extend if cond true, new head pos // can be used with cx.mode to do replace or extend on most changes ranges.push(new_range); - global_offs += new_text.chars().count(); + global_offs += new_text.chars().count() as isize - chars_deleted as isize; (from, to, Some(new_text.into())) }); diff --git a/helix-term/tests/test/commands/insert.rs b/helix-term/tests/test/commands/insert.rs index f23876dfbf8e..9352f737fa0a 100644 --- a/helix-term/tests/test/commands/insert.rs +++ b/helix-term/tests/test/commands/insert.rs @@ -1,5 +1,106 @@ use super::*; +#[tokio::test(flavor = "multi_thread")] +async fn insert_newline_many_selections() -> anyhow::Result<()> { + test(( + indoc! {"\ + #(|o)#ne + #(|t)#wo + #[|t]#hree + "}, + "i", + indoc! {"\ + \n#(|o)#ne + + #(|t)#wo + + #[|t]#hree + "}, + )) + .await?; + + // In this case the global offset that adjusts selections for inserted and deleted text + // should become negative because more text is deleted than is inserted. + test(( + indoc! {"\ + #[|🏴‍☠️]# #(|🏴‍☠️)# #(|🏴‍☠️)# + #(|🏴‍☠️)# #(|🏴‍☠️)# #(|🏴‍☠️)# + "}, + "i", + indoc! {"\ + \n#[|🏴‍☠️]# + #(|🏴‍☠️)# + #(|🏴‍☠️)# + + #(|🏴‍☠️)# + #(|🏴‍☠️)# + #(|🏴‍☠️)# + "}, + )) + .await?; + + // + test(( + indoc! {"\ + id #(|1)#,Item #(|1)#,cost #(|1)#,location #(|1)# + id #(|2)#,Item #(|2)#,cost #(|2)#,location #(|2)# + id #(|1)##(|0)#,Item #(|1)##(|0)#,cost #(|1)##(|0)#,location #(|1)##[|0]#"}, + "i", + indoc! {"\ + id + #(|1)#,Item + #(|1)#,cost + #(|1)#,location + #(|1)# + id + #(|2)#,Item + #(|2)#,cost + #(|2)#,location + #(|2)# + id + #(|1)# + #(|0)#,Item + #(|1)# + #(|0)#,cost + #(|1)# + #(|0)#,location + #(|1)# + #[|0]#"}, + )) + .await?; + + // + test(( + indoc! {"\ + real R〉 #(||)# 〈real R〉 @ 〈real R〉 + #(||)# 〈real R〉 + 〈ureal R〉 i #(||)# 〈real R〉 - 〈ureal R〉 i + #(||)# 〈real R〉 + i #(||)# 〈real R〉 - i #(||)# 〈real R〉 〈infnan〉 i + #(||)# + 〈ureal R〉 i #(||)# - 〈ureal R〉 i + #(||)# 〈infnan〉 i #(||)# + i #[||]# - i"}, + "i", + indoc! {"\ + real R〉 + #(||)# 〈real R〉 @ 〈real R〉 + + #(||)# 〈real R〉 + 〈ureal R〉 i + #(||)# 〈real R〉 - 〈ureal R〉 i + + #(||)# 〈real R〉 + i + #(||)# 〈real R〉 - i + #(||)# 〈real R〉 〈infnan〉 i + + #(||)# + 〈ureal R〉 i + #(||)# - 〈ureal R〉 i + + #(||)# 〈infnan〉 i + #(||)# + i + #[||]# - i"}, + )) + .await?; + + Ok(()) +} + #[tokio::test(flavor = "multi_thread")] async fn insert_newline_trim_trailing_whitespace() -> anyhow::Result<()> { // Trailing whitespace is trimmed. @@ -117,6 +218,33 @@ async fn insert_newline_continue_line_comment() -> anyhow::Result<()> { )) .await?; + // Comment continuation should work on multiple selections. + // + test(( + indoc! {"\ + ///·Docs#[|·]# + pub·struct·A; + + ///·Docs#(|·)# + pub·struct·B; + "} + .replace('·', " "), + ":lang rusti", + indoc! {"\ + ///·Docs + /// + ///·#[|·]# + pub·struct·A; + + ///·Docs + /// + ///·#(|·)# + pub·struct·B; + "} + .replace('·', " "), + )) + .await?; + Ok(()) }