diff --git a/helix-core/src/surround.rs b/helix-core/src/surround.rs index a3de3cd17..64d48c13a 100644 --- a/helix-core/src/surround.rs +++ b/helix-core/src/surround.rs @@ -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)"); diff --git a/helix-core/src/textobject.rs b/helix-core/src/textobject.rs index 76c6d103e..972a80e78 100644 --- a/helix-core/src/textobject.rs +++ b/helix-core/src/textobject.rs @@ -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) diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs index 38a623642..f8a96074d 100644 --- a/helix-term/src/commands.rs +++ b/helix-term/src/commands.rs @@ -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"), ]; diff --git a/helix-term/tests/test/movement.rs b/helix-term/tests/test/movement.rs index fedf4b0e6..e6ea3f951 100644 --- a/helix-term/tests/test/movement.rs +++ b/helix-term/tests/test/movement.rs @@ -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")]