Skip to content

Commit a80c708

Browse files
the-mikedavisTrivernis
authored andcommitted
Fix offset tracking in insert_newline
helix-editor#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 helix-editor#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 helix-editor#12461 Fixes helix-editor#12495 Fixes helix-editor#12539
1 parent d119279 commit a80c708

File tree

2 files changed

+140
-9
lines changed

2 files changed

+140
-9
lines changed

helix-term/src/commands.rs

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -3991,6 +3991,8 @@ pub mod insert {
39913991
let mut global_offs = 0;
39923992

39933993
let mut transaction = Transaction::change_by_selection(contents, &selection, |range| {
3994+
// Tracks the number of trailing whitespace characters deleted by this selection.
3995+
let mut chars_deleted = 0;
39943996
let pos = range.cursor(text);
39953997

39963998
let prev = if pos == 0 {
@@ -4069,13 +4071,14 @@ pub mod insert {
40694071
new_text.chars().count()
40704072
};
40714073

4074+
// Note that `first_trailing_whitespace_char` is at least `pos` so this unsigned
4075+
// subtraction cannot underflow.
4076+
chars_deleted = pos - first_trailing_whitespace_char;
4077+
40724078
(
40734079
first_trailing_whitespace_char,
40744080
pos,
4075-
// Note that `first_trailing_whitespace_char` is at least `pos` so the
4076-
// unsigned subtraction (`pos - first_trailing_whitespace_char`) cannot
4077-
// underflow.
4078-
local_offs as isize - (pos - first_trailing_whitespace_char) as isize,
4081+
local_offs as isize - chars_deleted as isize,
40794082
)
40804083
} else {
40814084
// If the current line is all whitespace, insert a line ending at the beginning of
@@ -4089,22 +4092,22 @@ pub mod insert {
40894092
let new_range = if range.cursor(text) > range.anchor {
40904093
// when appending, extend the range by local_offs
40914094
Range::new(
4092-
range.anchor + global_offs,
4093-
(range.head as isize + local_offs) as usize + global_offs,
4095+
(range.anchor as isize + global_offs) as usize,
4096+
(range.head as isize + local_offs + global_offs) as usize,
40944097
)
40954098
} else {
40964099
// when inserting, slide the range by local_offs
40974100
Range::new(
4098-
(range.anchor as isize + local_offs) as usize + global_offs,
4099-
(range.head as isize + local_offs) as usize + global_offs,
4101+
(range.anchor as isize + local_offs + global_offs) as usize,
4102+
(range.head as isize + local_offs + global_offs) as usize,
41004103
)
41014104
};
41024105

41034106
// TODO: range replace or extend
41044107
// range.replace(|range| range.is_empty(), head); -> fn extend if cond true, new head pos
41054108
// can be used with cx.mode to do replace or extend on most changes
41064109
ranges.push(new_range);
4107-
global_offs += new_text.chars().count();
4110+
global_offs += new_text.chars().count() as isize - chars_deleted as isize;
41084111

41094112
(from, to, Some(new_text.into()))
41104113
});

helix-term/tests/test/commands/insert.rs

Lines changed: 128 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,106 @@
11
use super::*;
22

3+
#[tokio::test(flavor = "multi_thread")]
4+
async fn insert_newline_many_selections() -> anyhow::Result<()> {
5+
test((
6+
indoc! {"\
7+
#(|o)#ne
8+
#(|t)#wo
9+
#[|t]#hree
10+
"},
11+
"i<ret>",
12+
indoc! {"\
13+
\n#(|o)#ne
14+
15+
#(|t)#wo
16+
17+
#[|t]#hree
18+
"},
19+
))
20+
.await?;
21+
22+
// In this case the global offset that adjusts selections for inserted and deleted text
23+
// should become negative because more text is deleted than is inserted.
24+
test((
25+
indoc! {"\
26+
#[|🏴‍☠️]# #(|🏴‍☠️)# #(|🏴‍☠️)#
27+
#(|🏴‍☠️)# #(|🏴‍☠️)# #(|🏴‍☠️)#
28+
"},
29+
"i<ret>",
30+
indoc! {"\
31+
\n#[|🏴‍☠️]#
32+
#(|🏴‍☠️)#
33+
#(|🏴‍☠️)#
34+
35+
#(|🏴‍☠️)#
36+
#(|🏴‍☠️)#
37+
#(|🏴‍☠️)#
38+
"},
39+
))
40+
.await?;
41+
42+
// <https://github.com/helix-editor/helix/issues/12495>
43+
test((
44+
indoc! {"\
45+
id #(|1)#,Item #(|1)#,cost #(|1)#,location #(|1)#
46+
id #(|2)#,Item #(|2)#,cost #(|2)#,location #(|2)#
47+
id #(|1)##(|0)#,Item #(|1)##(|0)#,cost #(|1)##(|0)#,location #(|1)##[|0]#"},
48+
"i<ret>",
49+
indoc! {"\
50+
id
51+
#(|1)#,Item
52+
#(|1)#,cost
53+
#(|1)#,location
54+
#(|1)#
55+
id
56+
#(|2)#,Item
57+
#(|2)#,cost
58+
#(|2)#,location
59+
#(|2)#
60+
id
61+
#(|1)#
62+
#(|0)#,Item
63+
#(|1)#
64+
#(|0)#,cost
65+
#(|1)#
66+
#(|0)#,location
67+
#(|1)#
68+
#[|0]#"},
69+
))
70+
.await?;
71+
72+
// <https://github.com/helix-editor/helix/issues/12461>
73+
test((
74+
indoc! {"\
75+
real R〉 #(||)# 〈real R〉 @ 〈real R〉
76+
#(||)# 〈real R〉 + 〈ureal R〉 i #(||)# 〈real R〉 - 〈ureal R〉 i
77+
#(||)# 〈real R〉 + i #(||)# 〈real R〉 - i #(||)# 〈real R〉 〈infnan〉 i
78+
#(||)# + 〈ureal R〉 i #(||)# - 〈ureal R〉 i
79+
#(||)# 〈infnan〉 i #(||)# + i #[||]# - i"},
80+
"i<ret>",
81+
indoc! {"\
82+
real R〉
83+
#(||)# 〈real R〉 @ 〈real R〉
84+
85+
#(||)# 〈real R〉 + 〈ureal R〉 i
86+
#(||)# 〈real R〉 - 〈ureal R〉 i
87+
88+
#(||)# 〈real R〉 + i
89+
#(||)# 〈real R〉 - i
90+
#(||)# 〈real R〉 〈infnan〉 i
91+
92+
#(||)# + 〈ureal R〉 i
93+
#(||)# - 〈ureal R〉 i
94+
95+
#(||)# 〈infnan〉 i
96+
#(||)# + i
97+
#[||]# - i"},
98+
))
99+
.await?;
100+
101+
Ok(())
102+
}
103+
3104
#[tokio::test(flavor = "multi_thread")]
4105
async fn insert_newline_trim_trailing_whitespace() -> anyhow::Result<()> {
5106
// Trailing whitespace is trimmed.
@@ -117,6 +218,33 @@ async fn insert_newline_continue_line_comment() -> anyhow::Result<()> {
117218
))
118219
.await?;
119220

221+
// Comment continuation should work on multiple selections.
222+
// <https://github.com/helix-editor/helix/issues/12539>
223+
test((
224+
indoc! {"\
225+
///·Docs#[|·]#
226+
pub·struct·A;
227+
228+
///·Docs#(|·)#
229+
pub·struct·B;
230+
"}
231+
.replace('·', " "),
232+
":lang rust<ret>i<ret><ret>",
233+
indoc! {"\
234+
///·Docs
235+
///
236+
///·#[|·]#
237+
pub·struct·A;
238+
239+
///·Docs
240+
///
241+
///·#(|·)#
242+
pub·struct·B;
243+
"}
244+
.replace('·', " "),
245+
))
246+
.await?;
247+
120248
Ok(())
121249
}
122250

0 commit comments

Comments
 (0)