Make m textobject look for pairs enclosing selections (#3344)

* Make `m` textobject look for pairs enclosing selections

Right now, this textobject only looks for pairs that surround the
cursor. This ensures that the pair found encloses each selection, which
is likely to be intuitively what is expected of this textobject.

* Simplification of match code

Co-authored-by: Michael Davis <mcarsondavis@gmail.com>

* Adjust logic for ensuring surround range encloses selection

Prior, it was missing the case where the start of the selection came
before the opening brace. We also had an off-by-one error where if the
end of the selection was on the closing brace it would not work.

* Refactor to search for the open pair specifically to avoid edge cases

* Adjust wording of autoinfo to reflect new functionality

* Implement tests for surround functionality in new integration style

* Fix handling of skip values

* Fix out of bounds error

* Add `ma` version of tests

* Fix formatting of tests

* Reduce indentation levels for readability, and update comments

* Preserve each selection's direction with enclosing pair surround

* Add test case for multiple cursors resulting in overlap

* Mark known failures as TODO

* Make tests multi-threaded or they fail

* Cargo fmt

* Fix typos in integration test comments

---------

Co-authored-by: Michael Davis <mcarsondavis@gmail.com>
This commit is contained in:
Daniel S Poulin 2023-02-10 14:52:57 -05:00 committed by GitHub
parent af1157f37c
commit 6929a12f29
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 361 additions and 107 deletions

View File

@ -1,6 +1,6 @@
use std::fmt::Display;
use crate::{search, Range, Selection};
use crate::{movement::Direction, search, Range, Selection};
use ropey::RopeSlice;
pub const PAIRS: &[(char, char)] = &[
@ -55,15 +55,18 @@ pub fn get_pair(ch: char) -> (char, char) {
pub fn find_nth_closest_pairs_pos(
text: RopeSlice,
range: Range,
n: usize,
mut skip: usize,
) -> Result<(usize, usize)> {
let is_open_pair = |ch| PAIRS.iter().any(|(open, _)| *open == ch);
let is_close_pair = |ch| PAIRS.iter().any(|(_, close)| *close == ch);
let mut stack = Vec::with_capacity(2);
let pos = range.cursor(text);
let pos = range.from();
let mut close_pos = pos.saturating_sub(1);
for ch in text.chars_at(pos) {
close_pos += 1;
if is_open_pair(ch) {
// Track open pairs encountered so that we can step over
// the corresponding close pairs that will come up further
@ -71,20 +74,46 @@ pub fn find_nth_closest_pairs_pos(
// open pair is before the cursor position.
stack.push(ch);
continue;
} else if is_close_pair(ch) {
let (open, _) = get_pair(ch);
if stack.last() == Some(&open) {
stack.pop();
continue;
} else {
// In the ideal case the stack would be empty here and the
// current character would be the close pair that we are
// looking for. It could also be the case that the pairs
// are unbalanced and we encounter a close pair that doesn't
// close the last seen open pair. In either case use this
// char as the auto-detected closest pair.
return find_nth_pairs_pos(text, ch, range, n);
}
if !is_close_pair(ch) {
// We don't care if this character isn't a brace pair item,
// so short circuit here.
continue;
}
let (open, close) = get_pair(ch);
if stack.last() == Some(&open) {
// If we are encountering the closing pair for an opener
// we just found while traversing, then its inside the
// selection and should be skipped over.
stack.pop();
continue;
}
match find_nth_open_pair(text, open, close, close_pos, 1) {
// Before we accept this pair, we want to ensure that the
// pair encloses the range rather than just the cursor.
Some(open_pos)
if open_pos <= pos.saturating_add(1)
&& close_pos >= range.to().saturating_sub(1) =>
{
// Since we have special conditions for when to
// accept, we can't just pass the skip parameter on
// through to the find_nth_*_pair methods, so we
// track skips manually here.
if skip > 1 {
skip -= 1;
continue;
}
return match range.direction() {
Direction::Forward => Ok((open_pos, close_pos)),
Direction::Backward => Ok((close_pos, open_pos)),
};
}
_ => continue,
}
}
@ -244,94 +273,6 @@ mod test {
use ropey::Rope;
use smallvec::SmallVec;
#[allow(clippy::type_complexity)]
fn check_find_nth_pair_pos(
text: &str,
cases: Vec<(usize, char, usize, Result<(usize, usize)>)>,
) {
let doc = Rope::from(text);
let slice = doc.slice(..);
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() {
check_find_nth_pair_pos(
"some (text) here",
vec![
// cursor on [t]ext
(6, '(', 1, Ok((5, 10))),
(6, ')', 1, Ok((5, 10))),
// cursor on so[m]e
(2, '(', 1, Err(Error::PairNotFound)),
// cursor on bracket itself
(5, '(', 1, Ok((5, 10))),
(10, '(', 1, Ok((5, 10))),
],
);
}
#[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, Ok((10, 15))),
(13, '(', 2, Ok((4, 21))),
(13, '(', 3, Ok((0, 27))),
],
);
}
#[test]
fn test_find_nth_pairs_pos_same() {
check_find_nth_pair_pos(
"'so 'many 'good' text' here'",
vec![
// cursor on go[o]d
(13, '\'', 1, Ok((10, 15))),
(13, '\'', 2, Ok((4, 21))),
(13, '\'', 3, Ok((0, 27))),
// cursor on the quotes
(10, '\'', 1, Err(Error::CursorOnAmbiguousPair)),
],
)
}
#[test]
fn test_find_nth_pairs_pos_step() {
check_find_nth_pair_pos(
"((so)((many) good (text))(here))",
vec![
// cursor on go[o]d
(15, '(', 1, Ok((5, 24))),
(15, '(', 2, Ok((0, 31))),
],
)
}
#[test]
fn test_find_nth_pairs_pos_mixed() {
check_find_nth_pair_pos(
"(so [many {good} text] here)",
vec![
// cursor on go[o]d
(13, '{', 1, Ok((10, 15))),
(13, '[', 1, Ok((4, 21))),
(13, '(', 1, Ok((0, 27))),
],
)
}
#[test]
fn test_get_surround_pos() {
let doc = Rope::from("(some) (chars)\n(newline)");

View File

@ -231,8 +231,20 @@ fn textobject_pair_surround_impl(
};
pair_pos
.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)),
TextObject::Inside => {
if anchor < head {
Range::new(next_grapheme_boundary(slice, anchor), head)
} else {
Range::new(anchor, next_grapheme_boundary(slice, head))
}
}
TextObject::Around => {
if anchor < head {
Range::new(anchor, next_grapheme_boundary(slice, head))
} else {
Range::new(next_grapheme_boundary(slice, anchor), head)
}
}
TextObject::Movement => unreachable!(),
})
.unwrap_or(range)

View File

@ -4791,7 +4791,7 @@ fn select_textobject(cx: &mut Context, objtype: textobject::TextObject) {
("a", "Argument/parameter (tree-sitter)"),
("c", "Comment (tree-sitter)"),
("T", "Test (tree-sitter)"),
("m", "Closest surrounding pair to cursor"),
("m", "Closest surrounding pair"),
(" ", "... or any character acting as a pair"),
];

View File

@ -64,6 +64,307 @@ async fn insert_to_normal_mode_cursor_position() -> anyhow::Result<()> {
Ok(())
}
#[tokio::test(flavor = "multi_thread")]
async fn surround_by_character() -> anyhow::Result<()> {
// Only pairs matching the passed character count
test((
"(so [many {go#[o|]#d} text] here)",
"mi{",
"(so [many {#[good|]#} text] here)",
))
.await?;
test((
"(so [many {go#[o|]#d} text] here)",
"mi[",
"(so [#[many {good} text|]#] here)",
))
.await?;
test((
"(so [many {go#[o|]#d} text] here)",
"mi(",
"(#[so [many {good} text] here|]#)",
))
.await?;
// Works with characters that aren't pairs too
test((
"'so 'many 'go#[o|]#d' text' here'",
"mi'",
"'so 'many '#[good|]#' text' here'",
))
.await?;
test((
"'so 'many 'go#[o|]#d' text' here'",
"2mi'",
"'so '#[many 'good' text|]#' here'",
))
.await?;
test((
"'so \"many 'go#[o|]#d' text\" here'",
"mi\"",
"'so \"#[many 'good' text|]#\" here'",
))
.await?;
Ok(())
}
#[tokio::test(flavor = "multi_thread")]
async fn surround_inside_pair() -> anyhow::Result<()> {
// Works at first character of buffer
// TODO: Adjust test when opening pair failure is fixed
test(("#[(|]#something)", "mim", "#[(|]#something)")).await?;
// Inside a valid pair selects pair
test(("some (#[t|]#ext) here", "mim", "some (#[text|]#) here")).await?;
// On pair character selects pair
// TODO: Opening pair character is a known failure case that needs addressing
// test(("some #[(|]#text) here", "mim", "some (#[text|]#) here")).await?;
test(("some (text#[)|]# here", "mim", "some (#[text|]#) here")).await?;
// No valid pair does nothing
test(("so#[m|]#e (text) here", "mim", "so#[m|]#e (text) here")).await?;
// Count skips to outer pairs
test((
"(so (many (go#[o|]#d) text) here)",
"1mim",
"(so (many (#[good|]#) text) here)",
))
.await?;
test((
"(so (many (go#[o|]#d) text) here)",
"2mim",
"(so (#[many (good) text|]#) here)",
))
.await?;
test((
"(so (many (go#[o|]#d) text) here)",
"3mim",
"(#[so (many (good) text) here|]#)",
))
.await?;
// Matching pairs outside selection don't match
test((
"((so)((many) go#[o|]#d (text))(here))",
"mim",
"((so)(#[(many) good (text)|]#)(here))",
))
.await?;
test((
"((so)((many) go#[o|]#d (text))(here))",
"2mim",
"(#[(so)((many) good (text))(here)|]#)",
))
.await?;
// Works with mixed braces
test((
"(so [many {go#[o|]#d} text] here)",
"mim",
"(so [many {#[good|]#} text] here)",
))
.await?;
test((
"(so [many {go#[o|]#d} text] here)",
"2mim",
"(so [#[many {good} text|]#] here)",
))
.await?;
test((
"(so [many {go#[o|]#d} text] here)",
"3mim",
"(#[so [many {good} text] here|]#)",
))
.await?;
// Selection direction is preserved
test((
"(so [many {go#[|od]#} text] here)",
"mim",
"(so [many {#[|good]#} text] here)",
))
.await?;
test((
"(so [many {go#[|od]#} text] here)",
"2mim",
"(so [#[|many {good} text]#] here)",
))
.await?;
test((
"(so [many {go#[|od]#} text] here)",
"3mim",
"(#[|so [many {good} text] here]#)",
))
.await?;
// Only pairs outside of full selection range are considered
test((
"(so (many (go#[od) |]#text) here)",
"mim",
"(so (#[many (good) text|]#) here)",
))
.await?;
test((
"(so (many#[ (go|]#od) text) here)",
"mim",
"(so (#[many (good) text|]#) here)",
))
.await?;
test((
"(so#[ (many (go|]#od) text) here)",
"mim",
"(#[so (many (good) text) here|]#)",
))
.await?;
test((
"(so (many (go#[od) text) |]#here)",
"mim",
"(#[so (many (good) text) here|]#)",
))
.await?;
// Works with multiple cursors
test((
"(so (many (good) text) #[he|]#re\nso (many (good) text) #(|he)#re)",
"mim",
"(#[so (many (good) text) here\nso (many (good) text) here|]#)",
))
.await?;
Ok(())
}
#[tokio::test(flavor = "multi_thread")]
async fn surround_around_pair() -> anyhow::Result<()> {
// Works at first character of buffer
// TODO: Adjust test when opening pair failure is fixed
test(("#[(|]#something)", "mam", "#[(|]#something)")).await?;
// Inside a valid pair selects pair
test(("some (#[t|]#ext) here", "mam", "some #[(text)|]# here")).await?;
// On pair character selects pair
// TODO: Opening pair character is a known failure case that needs addressing
// test(("some #[(|]#text) here", "mam", "some #[(text)|]# here")).await?;
test(("some (text#[)|]# here", "mam", "some #[(text)|]# here")).await?;
// No valid pair does nothing
test(("so#[m|]#e (text) here", "mam", "so#[m|]#e (text) here")).await?;
// Count skips to outer pairs
test((
"(so (many (go#[o|]#d) text) here)",
"1mam",
"(so (many #[(good)|]# text) here)",
))
.await?;
test((
"(so (many (go#[o|]#d) text) here)",
"2mam",
"(so #[(many (good) text)|]# here)",
))
.await?;
test((
"(so (many (go#[o|]#d) text) here)",
"3mam",
"#[(so (many (good) text) here)|]#",
))
.await?;
// Matching pairs outside selection don't match
test((
"((so)((many) go#[o|]#d (text))(here))",
"mam",
"((so)#[((many) good (text))|]#(here))",
))
.await?;
test((
"((so)((many) go#[o|]#d (text))(here))",
"2mam",
"#[((so)((many) good (text))(here))|]#",
))
.await?;
// Works with mixed braces
test((
"(so [many {go#[o|]#d} text] here)",
"mam",
"(so [many #[{good}|]# text] here)",
))
.await?;
test((
"(so [many {go#[o|]#d} text] here)",
"2mam",
"(so #[[many {good} text]|]# here)",
))
.await?;
test((
"(so [many {go#[o|]#d} text] here)",
"3mam",
"#[(so [many {good} text] here)|]#",
))
.await?;
// Selection direction is preserved
test((
"(so [many {go#[|od]#} text] here)",
"mam",
"(so [many #[|{good}]# text] here)",
))
.await?;
test((
"(so [many {go#[|od]#} text] here)",
"2mam",
"(so #[|[many {good} text]]# here)",
))
.await?;
test((
"(so [many {go#[|od]#} text] here)",
"3mam",
"#[|(so [many {good} text] here)]#",
))
.await?;
// Only pairs outside of full selection range are considered
test((
"(so (many (go#[od) |]#text) here)",
"mam",
"(so #[(many (good) text)|]# here)",
))
.await?;
test((
"(so (many#[ (go|]#od) text) here)",
"mam",
"(so #[(many (good) text)|]# here)",
))
.await?;
test((
"(so#[ (many (go|]#od) text) here)",
"mam",
"#[(so (many (good) text) here)|]#",
))
.await?;
test((
"(so (many (go#[od) text) |]#here)",
"mam",
"#[(so (many (good) text) here)|]#",
))
.await?;
// Works with multiple cursors
test((
"(so (many (good) text) #[he|]#re\nso (many (good) text) #(|he)#re)",
"mam",
"#[(so (many (good) text) here\nso (many (good) text) here)|]#",
))
.await?;
Ok(())
}
/// Ensure the very initial cursor in an opened file is the width of
/// the first grapheme
#[tokio::test(flavor = "multi_thread")]