Skip to content

Commit

Permalink
Fix multi-code point auto pairs
Browse files Browse the repository at this point in the history
The current code for auto pairs is counting offsets by summing the
length of the open and closing chars with char::len_utf8. Unfortunately,
this gives back bytes, and the offset needs to be in chars.

Additionally, it was discovered that there was a preexisting bug where
the selection was not computed correctly in the case that the cursor
was:

1. a single grapheme in width
2. this grapheme was more than one char
3. the direction of the cursor is backwards
4. a secondary range

In this case, the offset was not being added into the anchor. This was
fixed.
  • Loading branch information
dead10ck committed Sep 29, 2022
1 parent f114d2c commit 3dd7ccc
Show file tree
Hide file tree
Showing 2 changed files with 40 additions and 26 deletions.
56 changes: 30 additions & 26 deletions helix-core/src/auto_pairs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -147,13 +147,7 @@ fn prev_char(doc: &Rope, pos: usize) -> Option<char> {
}

/// calculate what the resulting range should be for an auto pair insertion
fn get_next_range(
doc: &Rope,
start_range: &Range,
offset: usize,
typed_char: char,
len_inserted: usize,
) -> Range {
fn get_next_range(doc: &Rope, start_range: &Range, offset: usize, len_inserted: usize) -> Range {
// When the character under the cursor changes due to complete pair
// insertion, we must look backward a grapheme and then add the length
// of the insertion to put the resulting cursor in the right place, e.g.
Expand All @@ -173,8 +167,8 @@ fn get_next_range(
// inserting at the very end of the document after the last newline
if start_range.head == doc.len_chars() && start_range.anchor == doc.len_chars() {
return Range::new(
start_range.anchor + offset + typed_char.len_utf8(),
start_range.head + offset + typed_char.len_utf8(),
start_range.anchor + offset + 1,
start_range.head + offset + 1,
);
}

Expand Down Expand Up @@ -204,21 +198,18 @@ fn get_next_range(
// trivial case: only inserted a single-char opener, just move the selection
if len_inserted == 1 {
let end_anchor = if single_grapheme || start_range.direction() == Direction::Backward {
start_range.anchor + offset + typed_char.len_utf8()
start_range.anchor + offset + 1
} else {
start_range.anchor + offset
};

return Range::new(
end_anchor,
start_range.head + offset + typed_char.len_utf8(),
);
return Range::new(end_anchor, start_range.head + offset + 1);
}

// If the head = 0, then we must be in insert mode with a backward
// cursor, which implies the head will just move
let end_head = if start_range.head == 0 || start_range.direction() == Direction::Backward {
start_range.head + offset + typed_char.len_utf8()
start_range.head + offset + 1
} else {
// We must have a forward cursor, which means we must move to the
// other end of the grapheme to get to where the new characters
Expand All @@ -244,8 +235,7 @@ fn get_next_range(

(_, Direction::Forward) => {
if single_grapheme {
graphemes::prev_grapheme_boundary(doc.slice(..), start_range.head)
+ typed_char.len_utf8()
graphemes::prev_grapheme_boundary(doc.slice(..), start_range.head) + 1

// if we are appending, the anchor stays where it is; only offset
// for multiple range insertions
Expand All @@ -259,7 +249,7 @@ fn get_next_range(
// if we're backward, then the head is at the first char
// of the typed char, so we need to add the length of
// the closing char
graphemes::prev_grapheme_boundary(doc.slice(..), start_range.anchor) + len_inserted
graphemes::prev_grapheme_boundary(doc.slice(..), start_range.anchor) + len_inserted + offset
} else {
// when we are inserting in front of a selection, we need to move
// the anchor over by however many characters were inserted overall
Expand All @@ -282,20 +272,20 @@ fn handle_open(doc: &Rope, selection: &Selection, pair: &Pair) -> Transaction {

let change = match next_char {
Some(_) if !pair.should_close(doc, start_range) => {
len_inserted = pair.open.len_utf8();
len_inserted = 1;
let mut tendril = Tendril::new();
tendril.push(pair.open);
(cursor, cursor, Some(tendril))
}
_ => {
// insert open & close
let pair_str = Tendril::from_iter([pair.open, pair.close]);
len_inserted = pair.open.len_utf8() + pair.close.len_utf8();
len_inserted = 2;
(cursor, cursor, Some(pair_str))
}
};

let next_range = get_next_range(doc, start_range, offs, pair.open, len_inserted);
let next_range = get_next_range(doc, start_range, offs, len_inserted);
end_ranges.push(next_range);
offs += len_inserted;

Expand All @@ -309,7 +299,6 @@ fn handle_open(doc: &Rope, selection: &Selection, pair: &Pair) -> Transaction {

fn handle_close(doc: &Rope, selection: &Selection, pair: &Pair) -> Transaction {
let mut end_ranges = SmallVec::with_capacity(selection.len());

let mut offs = 0;

let transaction = Transaction::change_by_selection(doc, selection, |start_range| {
Expand All @@ -321,13 +310,13 @@ fn handle_close(doc: &Rope, selection: &Selection, pair: &Pair) -> Transaction {
// return transaction that moves past close
(cursor, cursor, None) // no-op
} else {
len_inserted += pair.close.len_utf8();
len_inserted = 1;
let mut tendril = Tendril::new();
tendril.push(pair.close);
(cursor, cursor, Some(tendril))
};

let next_range = get_next_range(doc, start_range, offs, pair.close, len_inserted);
let next_range = get_next_range(doc, start_range, offs, len_inserted);
end_ranges.push(next_range);
offs += len_inserted;

Expand Down Expand Up @@ -363,11 +352,11 @@ fn handle_same(doc: &Rope, selection: &Selection, pair: &Pair) -> Transaction {
pair_str.push(pair.close);
}

len_inserted += pair_str.len();
len_inserted += pair_str.chars().count();
(cursor, cursor, Some(pair_str))
};

let next_range = get_next_range(doc, start_range, offs, pair.open, len_inserted);
let next_range = get_next_range(doc, start_range, offs, len_inserted);
end_ranges.push(next_range);
offs += len_inserted;

Expand Down Expand Up @@ -925,4 +914,19 @@ mod test {
&Selection::single(9, 5),
)
}

#[test]
fn test_multi_byte_pairs() {
// [NOTE] these are multi-byte Unicode characters
let test_pairs = &[('„', '“'), ('‚', '‘')];

test_hooks_with_pairs(
&Rope::from(LINE_END),
&Selection::single(1, 0),
test_pairs,
test_pairs,
|open, close| format!("{}{}{}", open, close, LINE_END),
&Selection::single(2, 1),
);
}
}
10 changes: 10 additions & 0 deletions helix-core/src/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,16 @@ mod test {
),
print("#[|he]#l#(|lo)#")
);
assert_eq!(
(
String::from("hello\r\nhello\r\nhello\r\n"),
Selection::new(
SmallVec::from_slice(&[Range::new(7, 5), Range::new(21, 19), Range::new(14, 12)]),
0
)
),
print("hello#[|\r\n]#hello#(|\r\n)#hello#(|\r\n)#")
);
}

#[test]
Expand Down

0 comments on commit 3dd7ccc

Please sign in to comment.