Trivernis
/
helix-plus
forked from Mirrors/helix
1
0
Fork 0
Code Issues Pull Requests 2 Packages Projects Releases Wiki Activity

fix(auto_pairs): fix auto pairs with crlf (#1470)

Browse Source 
Auto pairs were resulting in incorrect ranges in the resulting when the
line terminators are CRLF (i.e. Windows). It turns out this is because
when we were checking if the selection was a single-width cursor, it
incorrectly assumed that this would be a single char. This is not the
case, as a cursor can cover a multi-code point grapheme. Therefore,
we must instead explicitly work with and check graphemes to determine
if the cursor should move or extend the selection.

Fixes #1436
imgbot
Skyler Hawthorne 2 years ago committed by GitHub
parent 8ea5742b08
commit 56a9ce5d83
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
1 changed files with 214 additions and 41 deletions
Show Stats Download Patch File Download Diff File

255
helix-core/src/auto_pairs.rs
Unescape Escape View File

@ -1,7 +1,9 @@
//! When typing the opening character of one of the possible pairs defined below,
//! this module provides the functionality to insert the paired closing character.
use crate::{movement::Direction, Range, Rope, Selection, Tendril, Transaction};
use crate::{
graphemes, movement::Direction, Range, Rope, RopeGraphemes, Selection, Tendril, Transaction,
};
use log::debug;
use smallvec::SmallVec;
@ -63,31 +65,132 @@ fn prev_char(doc: &Rope, pos: usize) -> Option<char> {
doc.get_char(pos - 1)
}
fn is_single_grapheme(doc: &Rope, range: &Range) -> bool {
let mut graphemes = RopeGraphemes::new(doc.slice(range.from()..range.to()));
let first = graphemes.next();
let second = graphemes.next();
debug!("first: {:#?}, second: {:#?}", first, second);
first.is_some() && second.is_none()
}
/// 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 {
let end_head = start_range.head + offset + typed_char.len_utf8();
// 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.
//
// foo[\r\n] - anchor: 3, head: 5
// foo([)]\r\n - anchor: 4, head: 5
//
// foo[\r\n] - anchor: 3, head: 5
// foo'[\r\n] - anchor: 4, head: 6
//
// foo([)]\r\n - anchor: 4, head: 5
// foo()[\r\n] - anchor: 5, head: 7
//
// [foo]\r\n - anchor: 0, head: 3
// [foo(])\r\n - anchor: 0, head: 5
// 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(),
);
}
let single_grapheme = is_single_grapheme(doc, start_range);
let doc_slice = doc.slice(..);
// just skip over graphemes
if len_inserted == 0 {
let end_anchor = if single_grapheme {
graphemes::next_grapheme_boundary(doc_slice, start_range.anchor) + offset
// even for backward inserts with multiple grapheme selections,
// we want the anchor to stay where it is so that the relative
// selection does not change, e.g.:
//
// foo([) wor]d -> insert ) -> foo()[ wor]d
} else {
start_range.anchor + offset
};
return Range::new(
end_anchor,
graphemes::next_grapheme_boundary(doc_slice, start_range.head) + offset,
);
}
// 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()
} else {
start_range.anchor + offset
};
return Range::new(
end_anchor,
start_range.head + offset + typed_char.len_utf8(),
);
}
// 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()
} 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
// are inserted, then move the head to where it should be
let prev_bound = graphemes::prev_grapheme_boundary(doc_slice, start_range.head);
debug!(
"prev_bound: {}, offset: {}, len_inserted: {}",
prev_bound, offset, len_inserted
);
prev_bound + offset + len_inserted
};
let end_anchor = match (start_range.len(), start_range.direction()) {
// if we have a zero width cursor, it shifts to the same number
(0, _) => end_head,
// if we are inserting for a regular one-width cursor, the anchor
// moves with the head
// If we are inserting for a regular one-width cursor, the anchor
// moves with the head. This is the fast path for ASCII.
(1, Direction::Forward) => end_head - 1,
(1, Direction::Backward) => end_head + 1,
// if we are appending, the anchor stays where it is; only offset
// for multiple range insertions
(_, Direction::Forward) => start_range.anchor + offset,
(_, Direction::Forward) => {
if single_grapheme {
graphemes::prev_grapheme_boundary(doc.slice(..), start_range.head)
+ typed_char.len_utf8()
// when we are inserting in front of a selection, we need to move
// the anchor over by however many characters were inserted overall
(_, Direction::Backward) => start_range.anchor + offset + len_inserted,
// if we are appending, the anchor stays where it is; only offset
// for multiple range insertions
} else {
start_range.anchor + offset
}
}
(_, Direction::Backward) => {
if single_grapheme {
// 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
} else {
// when we are inserting in front of a selection, we need to move
// the anchor over by however many characters were inserted overall
start_range.anchor + offset + len_inserted
}
}
};
Range::new(end_anchor, end_head)
@ -122,7 +225,7 @@ fn handle_open(
}
};
let next_range = get_next_range(start_range, offs, open, len_inserted);
let next_range = get_next_range(doc, start_range, offs, open, len_inserted);
end_ranges.push(next_range);
offs += len_inserted;
@ -152,7 +255,7 @@ fn handle_close(doc: &Rope, selection: &Selection, _open: char, close: char) ->
(cursor, cursor, Some(Tendril::from_char(close)))
};
let next_range = get_next_range(start_range, offs, close, len_inserted);
let next_range = get_next_range(doc, start_range, offs, close, len_inserted);
end_ranges.push(next_range);
offs += len_inserted;
@ -202,7 +305,7 @@ fn handle_same(
(cursor, cursor, Some(pair))
};
let next_range = get_next_range(start_range, offs, token, len_inserted);
let next_range = get_next_range(doc, start_range, offs, token, len_inserted);
end_ranges.push(next_range);
offs += len_inserted;
@ -219,6 +322,8 @@ mod test {
use super::*;
use smallvec::smallvec;
const LINE_END: &'static str = crate::DEFAULT_LINE_ENDING.as_str();
fn differing_pairs() -> impl Iterator<Item = &'static (char, char)> {
PAIRS.iter().filter(|(open, close)| open != close)
}
@ -270,12 +375,59 @@ mod test {
#[test]
fn test_insert_blank() {
test_hooks_with_pairs(
&Rope::new(),
&Rope::from(LINE_END),
&Selection::single(1, 0),
PAIRS,
|open, close| format!("{}{}", open, close),
|open, close| format!("{}{}{}", open, close, LINE_END),
&Selection::single(2, 1),
);
let empty_doc = Rope::from(format!("{line_end}{line_end}", line_end = LINE_END));
test_hooks_with_pairs(
&empty_doc,
&Selection::single(empty_doc.len_chars(), LINE_END.len()),
PAIRS,
|open, close| {
format!(
"{line_end}{open}{close}{line_end}",
open = open,
close = close,
line_end = LINE_END
)
},
&Selection::single(LINE_END.len() + 2, LINE_END.len() + 1),
);
}
#[test]
fn test_insert_before_multi_code_point_graphemes() {
test_hooks_with_pairs(
&Rope::from(format!("hello 👨‍👩‍👧‍👦 goodbye{}", LINE_END)),
&Selection::single(13, 6),
PAIRS,
|open, _| format!("hello {}👨‍👩‍👧‍👦 goodbye{}", open, LINE_END),
&Selection::single(14, 7),
);
}
#[test]
fn test_insert_at_end_of_document() {
test_hooks_with_pairs(
&Rope::from(LINE_END),
&Selection::single(LINE_END.len(), LINE_END.len()),
PAIRS,
|open, close| format!("{}{}{}", LINE_END, open, close),
&Selection::single(LINE_END.len() + 1, LINE_END.len() + 1),
);
test_hooks_with_pairs(
&Rope::from(format!("foo{}", LINE_END)),
&Selection::single(3 + LINE_END.len(), 3 + LINE_END.len()),
PAIRS,
|open, close| format!("foo{}{}{}", LINE_END, open, close),
&Selection::single(LINE_END.len() + 4, LINE_END.len() + 4),
);
}
/// [] -> append ( -> ([])
@ -283,11 +435,20 @@ mod test {
fn test_append_blank() {
test_hooks_with_pairs(
// this is what happens when you have a totally blank document and then append
&Rope::from("\n\n"),
&Selection::single(0, 2),
&Rope::from(format!("{line_end}{line_end}", line_end = LINE_END)),
// before inserting the pair, the cursor covers all of both empty lines
&Selection::single(0, LINE_END.len() * 2),
PAIRS,
|open, close| format!("\n{}{}\n", open, close),
&Selection::single(0, 3),
|open, close| {
format!(
"{line_end}{open}{close}{line_end}",
line_end = LINE_END,
open = open,
close = close
)
},
// after inserting pair, the cursor covers the first new line and the open char
&Selection::single(0, LINE_END.len() + 2),
);
}
@ -329,6 +490,18 @@ mod test {
);
}
/// foo[] -> append to end of line ( -> foo([])
#[test]
fn test_append_single_cursor() {
test_hooks_with_pairs(
&Rope::from(format!("foo{}", LINE_END)),
&Selection::single(3, 3 + LINE_END.len()),
differing_pairs(),
|open, close| format!("foo{}{}{}", open, close, LINE_END),
&Selection::single(4, 5),
);
}
/// fo[o] fo[o(])
/// fo[o] -> append ( -> fo[o(])
/// fo[o] fo[o(])
@ -355,18 +528,18 @@ mod test {
);
}
/// ([]) -> insert ) -> ()[]
/// ([)] -> insert ) -> ()[]
#[test]
fn test_insert_close_inside_pair() {
for (open, close) in PAIRS {
let doc = Rope::from(format!("{}{}", open, close));
let doc = Rope::from(format!("{}{}{}", open, close, LINE_END));
test_hooks(
&doc,
&Selection::single(2, 1),
*close,
&doc,
&Selection::single(3, 2),
&Selection::single(2 + LINE_END.len(), 2),
);
}
}
@ -375,14 +548,14 @@ mod test {
#[test]
fn test_append_close_inside_pair() {
for (open, close) in PAIRS {
let doc = Rope::from(format!("{}{}\n", open, close));
let doc = Rope::from(format!("{}{}{}", open, close, LINE_END));
test_hooks(
&doc,
&Selection::single(0, 2),
*close,
&doc,
&Selection::single(0, 3),
&Selection::single(0, 2 + LINE_END.len()),
);
}
}
@ -564,6 +737,20 @@ mod test {
)
}
/// foo([) wor]d -> insert ) -> foo()[ wor]d
#[test]
fn test_insert_close_inside_pair_trailing_word_with_selection() {
for (open, close) in differing_pairs() {
test_hooks(
&Rope::from(format!("foo{}{} word{}", open, close, LINE_END)),
&Selection::single(9, 4),
*close,
&Rope::from(format!("foo{}{} word{}", open, close, LINE_END)),
&Selection::single(9, 5),
)
}
}
/// we want pairs that are *not* the same char to be inserted after
/// a non-pair char, for cases like functions, but for pairs that are
/// the same char, we want to *not* insert a pair to handle cases like "I'm"
@ -572,7 +759,7 @@ mod test {
/// word[] -> insert ' -> word'[]
#[test]
fn test_insert_open_after_non_pair() {
let doc = Rope::from("word");
let doc = Rope::from(format!("word{}", LINE_END));
let sel = Selection::single(5, 4);
let expected_sel = Selection::single(6, 5);
@ -580,7 +767,7 @@ mod test {
&doc,
&sel,
differing_pairs(),
|open, close| format!("word{}{}", open, close),
|open, close| format!("word{}{}{}", open, close, LINE_END),
&expected_sel,
);
@ -588,22 +775,8 @@ mod test {
&doc,
&sel,
matching_pairs(),
|open, _| format!("word{}", open),
|open, _| format!("word{}{}", open, LINE_END),
&expected_sel,
);
}
/// appending with only a cursor should stay a cursor
///
/// [] -> append to end "foo -> "foo[]"
#[test]
fn test_append_single_cursor() {
test_hooks_with_pairs(
&Rope::from("\n"),
&Selection::single(0, 1),
PAIRS,
|open, close| format!("{}{}\n", open, close),
&Selection::single(1, 2),
);
}
}

Write Preview
Loading…
Cancel
Save