Skip to content

Commit

Permalink
Fix panic when using join_selections_space (helix-editor#9783)
Browse files Browse the repository at this point in the history
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)
  • Loading branch information
trink authored and mtoohey31 committed Jun 2, 2024
1 parent 030242a commit 8cfcf88
Show file tree
Hide file tree
Showing 2 changed files with 100 additions and 6 deletions.
23 changes: 17 additions & 6 deletions helix-term/src/commands.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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())
};
Expand Down
83 changes: 83 additions & 0 deletions helix-term/tests/test/commands.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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|]#
"}),
"<A-J>",
platform_line(indoc! {"\
a#[ |]#b#( |)#c#( |)#d#( |)#e
"}),
))
.await?;

// normal join
test((
platform_line(indoc! {"\
#[a|]#bc
def
"}),
"<A-J>",
platform_line(indoc! {"\
abc#[ |]#def
"}),
))
.await?;

// join with empty line
test((
platform_line(indoc! {"\
#[a|]#bc
def
"}),
"<A-J>",
platform_line(indoc! {"\
#[a|]#bc
def
"}),
))
.await?;

// join with additional space in non-empty line
test((
platform_line(indoc! {"\
#[a|]#bc
def
"}),
"<A-J><A-J>",
platform_line(indoc! {"\
abc#[ |]#def
"}),
))
.await?;

// join with retained trailing spaces
test((
platform_line(indoc! {"\
#[aaa
bb
c |]#
"}),
"<A-J>",
platform_line(indoc! {"\
aaa #[ |]#bb #( |)#c
"}),
))
.await?;

Ok(())
}

0 comments on commit 8cfcf88

Please sign in to comment.