Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix surround cursor position calculation #1183

Merged
merged 1 commit into from
Nov 29, 2021
Merged

Fix surround cursor position calculation #1183

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions helix-core/src/selection.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -308,10 +308,10 @@ impl Range {
}

impl From<(usize, usize)> for Range {
fn from(tuple: (usize, usize)) -> Self {
fn from((anchor, head): (usize, usize)) -> Self {
Self {
anchor: tuple.0,
head: tuple.1,
anchor,
head,
horiz: None,
}
}
Expand Down
148 changes: 83 additions & 65 deletions helix-core/src/surround.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use crate::{search, Selection};
use crate::{search, Range, Selection};
use ropey::RopeSlice;

pub const PAIRS: &[(char, char)] = &[
Expand Down Expand Up @@ -35,33 +35,27 @@ pub fn get_pair(ch: char) -> (char, char) {
pub fn find_nth_pairs_pos(
text: RopeSlice,
ch: char,
pos: usize,
range: Range,
n: usize,
) -> Option<(usize, usize)> {
let (open, close) = get_pair(ch);

if text.len_chars() < 2 || pos >= text.len_chars() {
if text.len_chars() < 2 || range.to() >= text.len_chars() {
return None;
}

let (open, close) = get_pair(ch);
let pos = range.cursor(text);

if open == close {
if Some(open) == text.get_char(pos) {
// Special case: cursor is directly on a matching char.
match pos {
0 => Some((pos, search::find_nth_next(text, close, pos + 1, n)?)),
_ if (pos + 1) == text.len_chars() => {
Some((search::find_nth_prev(text, open, pos, n)?, pos))
}
// We return no match because there's no way to know which
// side of the char we should be searching on.
_ => None,
}
} else {
Some((
search::find_nth_prev(text, open, pos, n)?,
search::find_nth_next(text, close, pos, n)?,
))
// Cursor is directly on match char. We return no match
// because there's no way to know which side of the char
// we should be searching on.
return None;
}
Some((
search::find_nth_prev(text, open, pos, n)?,
search::find_nth_next(text, close, pos, n)?,
))
} else {
Some((
find_nth_open_pair(text, open, close, pos, n)?,
Expand Down Expand Up @@ -160,8 +154,8 @@ pub fn get_surround_pos(
) -> Option<Vec<usize>> {
let mut change_pos = Vec::new();

for range in selection {
let (open_pos, close_pos) = find_nth_pairs_pos(text, ch, range.head, skip)?;
for &range in selection {
let (open_pos, close_pos) = find_nth_pairs_pos(text, ch, range, skip)?;
if change_pos.contains(&open_pos) || change_pos.contains(&close_pos) {
return None;
}
Expand All @@ -178,67 +172,91 @@ mod test {
use ropey::Rope;
use smallvec::SmallVec;

#[test]
fn test_find_nth_pairs_pos() {
let doc = Rope::from("some (text) here");
fn check_find_nth_pair_pos(
text: &str,
cases: Vec<(usize, char, usize, Option<(usize, usize)>)>,
) {
let doc = Rope::from(text);
let slice = doc.slice(..);

// cursor on [t]ext
assert_eq!(find_nth_pairs_pos(slice, '(', 6, 1), Some((5, 10)));
assert_eq!(find_nth_pairs_pos(slice, ')', 6, 1), Some((5, 10)));
// cursor on so[m]e
assert_eq!(find_nth_pairs_pos(slice, '(', 2, 1), None);
// cursor on bracket itself
assert_eq!(find_nth_pairs_pos(slice, '(', 5, 1), Some((5, 10)));
assert_eq!(find_nth_pairs_pos(slice, '(', 10, 1), Some((5, 10)));
for (cursor_pos, ch, n, expected_range) in cases {
let range = find_nth_pairs_pos(slice, ch, (cursor_pos, cursor_pos + 1).into(), n);
assert_eq!(
range, expected_range,
"Expected {:?}, got {:?}",
expected_range, range
);
}
}

#[test]
fn test_find_nth_pairs_pos_skip() {
let doc = Rope::from("(so (many (good) text) here)");
let slice = doc.slice(..);
fn test_find_nth_pairs_pos() {
check_find_nth_pair_pos(
"some (text) here",
vec![
// cursor on [t]ext
(6, '(', 1, Some((5, 10))),
(6, ')', 1, Some((5, 10))),
// cursor on so[m]e
(2, '(', 1, None),
// cursor on bracket itself
(5, '(', 1, Some((5, 10))),
(10, '(', 1, Some((5, 10))),
],
);
}

// cursor on go[o]d
assert_eq!(find_nth_pairs_pos(slice, '(', 13, 1), Some((10, 15)));
assert_eq!(find_nth_pairs_pos(slice, '(', 13, 2), Some((4, 21)));
assert_eq!(find_nth_pairs_pos(slice, '(', 13, 3), Some((0, 27)));
#[test]
fn test_find_nth_pairs_pos_skip() {
check_find_nth_pair_pos(
"(so (many (good) text) here)",
vec![
// cursor on go[o]d
(13, '(', 1, Some((10, 15))),
(13, '(', 2, Some((4, 21))),
(13, '(', 3, Some((0, 27))),
],
);
}

#[test]
fn test_find_nth_pairs_pos_same() {
let doc = Rope::from("'so 'many 'good' text' here'");
let slice = doc.slice(..);

// cursor on go[o]d
assert_eq!(find_nth_pairs_pos(slice, '\'', 13, 1), Some((10, 15)));
assert_eq!(find_nth_pairs_pos(slice, '\'', 13, 2), Some((4, 21)));
assert_eq!(find_nth_pairs_pos(slice, '\'', 13, 3), Some((0, 27)));
// cursor on the quotes
assert_eq!(find_nth_pairs_pos(slice, '\'', 10, 1), None);
// this is the best we can do since opening and closing pairs are same
assert_eq!(find_nth_pairs_pos(slice, '\'', 0, 1), Some((0, 4)));
assert_eq!(find_nth_pairs_pos(slice, '\'', 27, 1), Some((21, 27)));
check_find_nth_pair_pos(
"'so 'many 'good' text' here'",
vec![
// cursor on go[o]d
(13, '\'', 1, Some((10, 15))),
(13, '\'', 2, Some((4, 21))),
(13, '\'', 3, Some((0, 27))),
// cursor on the quotes
(10, '\'', 1, None),
],
)
}

#[test]
fn test_find_nth_pairs_pos_step() {
let doc = Rope::from("((so)((many) good (text))(here))");
let slice = doc.slice(..);

// cursor on go[o]d
assert_eq!(find_nth_pairs_pos(slice, '(', 15, 1), Some((5, 24)));
assert_eq!(find_nth_pairs_pos(slice, '(', 15, 2), Some((0, 31)));
check_find_nth_pair_pos(
"((so)((many) good (text))(here))",
vec![
// cursor on go[o]d
(15, '(', 1, Some((5, 24))),
(15, '(', 2, Some((0, 31))),
],
)
}

#[test]
fn test_find_nth_pairs_pos_mixed() {
let doc = Rope::from("(so [many {good} text] here)");
let slice = doc.slice(..);

// cursor on go[o]d
assert_eq!(find_nth_pairs_pos(slice, '{', 13, 1), Some((10, 15)));
assert_eq!(find_nth_pairs_pos(slice, '[', 13, 1), Some((4, 21)));
assert_eq!(find_nth_pairs_pos(slice, '(', 13, 1), Some((0, 27)));
check_find_nth_pair_pos(
"(so [many {good} text] here)",
vec![
// cursor on go[o]d
(13, '{', 1, Some((10, 15))),
(13, '[', 1, Some((4, 21))),
(13, '(', 1, Some((0, 27))),
],
)
}

#[test]
Expand Down
10 changes: 6 additions & 4 deletions helix-core/src/textobject.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ pub fn textobject_surround(
ch: char,
count: usize,
) -> Range {
surround::find_nth_pairs_pos(slice, ch, range.head, count)
surround::find_nth_pairs_pos(slice, ch, range, count)
.map(|(anchor, head)| match textobject {
TextObject::Inside => Range::new(next_grapheme_boundary(slice, anchor), head),
TextObject::Around => Range::new(anchor, next_grapheme_boundary(slice, head)),
Expand Down Expand Up @@ -170,7 +170,7 @@ mod test {

#[test]
fn test_textobject_word() {
// (text, [(cursor position, textobject, final range), ...])
// (text, [(char position, textobject, final range), ...])
let tests = &[
(
"cursor at beginning of doc",
Expand Down Expand Up @@ -269,7 +269,9 @@ mod test {
let slice = doc.slice(..);
for &case in scenario {
let (pos, objtype, expected_range) = case;
let result = textobject_word(slice, Range::point(pos), objtype, 1, false);
// cursor is a single width selection
let range = Range::new(pos, pos + 1);
let result = textobject_word(slice, range, objtype, 1, false);
assert_eq!(
result,
expected_range.into(),
Expand All @@ -283,7 +285,7 @@ mod test {

#[test]
fn test_textobject_surround() {
// (text, [(cursor position, textobject, final range, count), ...])
// (text, [(cursor position, textobject, final range, surround char, count), ...])
let tests = &[
(
"simple (single) surround pairs",
Expand Down