Skip to content

Commit

Permalink
Fix offset tracking in insert_newline
Browse files Browse the repository at this point in the history
#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
  • Loading branch information
the-mikedavis committed Jan 15, 2025
1 parent 99d33c7 commit 4bd17e5
Show file tree
Hide file tree
Showing 2 changed files with 140 additions and 9 deletions.
21 changes: 12 additions & 9 deletions helix-term/src/commands.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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
Expand All @@ -4089,22 +4092,22 @@ 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,
)
};

// TODO: range replace or extend
// 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()))
});
Expand Down
128 changes: 128 additions & 0 deletions helix-term/tests/test/commands/insert.rs
Original file line number Diff line number Diff line change
@@ -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<ret>",
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<ret>",
indoc! {"\
\n#[|🏴‍☠️]#
#(|🏴‍☠️)#
#(|🏴‍☠️)#
#(|🏴‍☠️)#
#(|🏴‍☠️)#
#(|🏴‍☠️)#
"},
))
.await?;

// <https://github.com/helix-editor/helix/issues/12495>
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<ret>",
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?;

// <https://github.com/helix-editor/helix/issues/12461>
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<ret>",
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.
Expand Down Expand Up @@ -117,6 +218,33 @@ async fn insert_newline_continue_line_comment() -> anyhow::Result<()> {
))
.await?;

// Comment continuation should work on multiple selections.
// <https://github.com/helix-editor/helix/issues/12539>
test((
indoc! {"\
///·Docs#[|·]#
pub·struct·A;
///·Docs#(|·)#
pub·struct·B;
"}
.replace('·', " "),
":lang rust<ret>i<ret><ret>",
indoc! {"\
///·Docs
///
///·#[|·]#
pub·struct·A;
///·Docs
///
///·#(|·)#
pub·struct·B;
"}
.replace('·', " "),
))
.await?;

Ok(())
}

Expand Down

0 comments on commit 4bd17e5

Please sign in to comment.