From f262c88bc38005d9206634c7761fde15495243b1 Mon Sep 17 00:00:00 2001 From: Daniel Poulin Date: Fri, 5 Aug 2022 21:51:16 -0400 Subject: [PATCH 01/17] 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. --- helix-core/src/surround.rs | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/helix-core/src/surround.rs b/helix-core/src/surround.rs index a3de3cd17509..b62b161ecb92 100644 --- a/helix-core/src/surround.rs +++ b/helix-core/src/surround.rs @@ -61,7 +61,7 @@ pub fn find_nth_closest_pairs_pos( 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(); for ch in text.chars_at(pos) { if is_open_pair(ch) { @@ -83,7 +83,19 @@ pub fn find_nth_closest_pairs_pos( // 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); + match find_nth_pairs_pos(text, ch, range, n) { + // Before we accept this pair, we want to ensure that the + // pair encloses the range rather than just the cursor. + Ok(matching_pair) => { + let (_, close) = matching_pair; + if close >= range.to() { + return Ok(matching_pair); + } + } + Err(_) => continue, + } + + continue; } } } From e630dbf3d78ac4422048f78daca6161109ce5b71 Mon Sep 17 00:00:00 2001 From: Daniel S Poulin Date: Sat, 6 Aug 2022 10:14:10 -0400 Subject: [PATCH 02/17] Simplification of match code Co-authored-by: Michael Davis --- helix-core/src/surround.rs | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/helix-core/src/surround.rs b/helix-core/src/surround.rs index b62b161ecb92..340c7d37ccdd 100644 --- a/helix-core/src/surround.rs +++ b/helix-core/src/surround.rs @@ -86,16 +86,11 @@ pub fn find_nth_closest_pairs_pos( match find_nth_pairs_pos(text, ch, range, n) { // Before we accept this pair, we want to ensure that the // pair encloses the range rather than just the cursor. - Ok(matching_pair) => { - let (_, close) = matching_pair; - if close >= range.to() { - return Ok(matching_pair); - } + Ok(matching_pair) if matching_pair.1 >= range.to() => { + return Ok(matching_pair); } - Err(_) => continue, + _ => continue, } - - continue; } } } From 0085c669e5be05aba726b818f15624abab1826ab Mon Sep 17 00:00:00 2001 From: Daniel Poulin Date: Sat, 6 Aug 2022 12:32:35 -0400 Subject: [PATCH 03/17] 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. --- helix-core/src/surround.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/helix-core/src/surround.rs b/helix-core/src/surround.rs index 340c7d37ccdd..68eeb875ab8d 100644 --- a/helix-core/src/surround.rs +++ b/helix-core/src/surround.rs @@ -86,7 +86,10 @@ pub fn find_nth_closest_pairs_pos( match find_nth_pairs_pos(text, ch, range, n) { // Before we accept this pair, we want to ensure that the // pair encloses the range rather than just the cursor. - Ok(matching_pair) if matching_pair.1 >= range.to() => { + Ok(matching_pair) + if matching_pair.0 <= pos.saturating_add(1) + && matching_pair.1 >= range.to().saturating_sub(1) => + { return Ok(matching_pair); } _ => continue, From bc40f3856655782b621b92cfb47f6f2621206513 Mon Sep 17 00:00:00 2001 From: Daniel Poulin Date: Sat, 13 Aug 2022 15:09:19 -0400 Subject: [PATCH 04/17] Refactor to search for the open pair specifically to avoid edge cases --- helix-core/src/surround.rs | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/helix-core/src/surround.rs b/helix-core/src/surround.rs index 68eeb875ab8d..eadc367cd13d 100644 --- a/helix-core/src/surround.rs +++ b/helix-core/src/surround.rs @@ -62,8 +62,10 @@ pub fn find_nth_closest_pairs_pos( let mut stack = Vec::with_capacity(2); let pos = range.from(); + let mut close_pos = pos - 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 @@ -72,7 +74,7 @@ pub fn find_nth_closest_pairs_pos( stack.push(ch); continue; } else if is_close_pair(ch) { - let (open, _) = get_pair(ch); + let (open, close) = get_pair(ch); if stack.last() == Some(&open) { stack.pop(); continue; @@ -83,14 +85,14 @@ pub fn find_nth_closest_pairs_pos( // 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. - match find_nth_pairs_pos(text, ch, range, n) { + match find_nth_open_pair(text, open, close, close_pos, n) { // Before we accept this pair, we want to ensure that the // pair encloses the range rather than just the cursor. - Ok(matching_pair) - if matching_pair.0 <= pos.saturating_add(1) - && matching_pair.1 >= range.to().saturating_sub(1) => + Some(open_pos) + if open_pos <= pos.saturating_add(1) + && close_pos >= range.to().saturating_sub(1) => { - return Ok(matching_pair); + return Ok((open_pos, close_pos)); } _ => continue, } From b14a325a3821b8980c39e99200268d7221eccf3f Mon Sep 17 00:00:00 2001 From: Daniel Poulin Date: Sat, 13 Aug 2022 15:31:11 -0400 Subject: [PATCH 05/17] Adjust wording of autoinfo to reflect new functionality --- helix-term/src/commands.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs index d1dc92236cf0..ff98c479015a 100644 --- a/helix-term/src/commands.rs +++ b/helix-term/src/commands.rs @@ -2164,7 +2164,7 @@ fn extend_line_impl(cx: &mut Context, extend: Extend) { let start = text.line_to_char(start_line); let end = text.line_to_char( (end_line + 1) // newline of end_line - .min(text.len_lines()), + .min(text.len_lines()), ); // extend to previous/next line if current line is selected @@ -4784,7 +4784,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"), ]; @@ -5218,10 +5218,10 @@ fn increment_impl(cx: &mut Context, increment_direction: IncrementDirection) { let selected_text: Cow = range.fragment(text); let new_from = ((range.from() as i128) + cumulative_length_diff) as usize; let incremented = [increment::integer, increment::date_time] - .iter() + .iter() .find_map(|incrementor| incrementor(selected_text.as_ref(), amount)); - amount += increase_by; + amount += increase_by; match incremented { None => { @@ -5230,14 +5230,14 @@ fn increment_impl(cx: &mut Context, increment_direction: IncrementDirection) { (range.to() as i128 + cumulative_length_diff) as usize, ); new_selection_ranges.push(new_range); - } + } Some(new_text) => { let new_range = Range::new(new_from, new_from + new_text.len()); cumulative_length_diff += new_text.len() as i128 - selected_text.len() as i128; new_selection_ranges.push(new_range); changes.push((range.from(), range.to(), Some(new_text.into()))); + } } - } } if !changes.is_empty() { From ecbac264d1ee40585ca4383d9662d8c9e92c0de4 Mon Sep 17 00:00:00 2001 From: Daniel Poulin Date: Sun, 28 Aug 2022 14:09:46 -0400 Subject: [PATCH 06/17] Implement tests for surround functionality in new integration style --- helix-core/src/surround.rs | 88 ------------------------------- helix-term/tests/test/movement.rs | 51 ++++++++++++++++++ 2 files changed, 51 insertions(+), 88 deletions(-) diff --git a/helix-core/src/surround.rs b/helix-core/src/surround.rs index eadc367cd13d..7a7724c6768f 100644 --- a/helix-core/src/surround.rs +++ b/helix-core/src/surround.rs @@ -256,94 +256,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-term/tests/test/movement.rs b/helix-term/tests/test/movement.rs index fedf4b0e60e1..dcf43fed3b8f 100644 --- a/helix-term/tests/test/movement.rs +++ b/helix-term/tests/test/movement.rs @@ -64,6 +64,57 @@ async fn insert_to_normal_mode_cursor_position() -> anyhow::Result<()> { Ok(()) } +#[tokio::test] +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] +async fn surround_pair() -> anyhow::Result<()> { + // Inside a valid pair selects pair + test(("some (#[t|]#ext) here", "mim", "some (#[text|]#) here")).await?; + + // On pair character selects pair + // 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 paris 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?; + + // 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?; + + Ok(()) +} + /// Ensure the very initial cursor in an opened file is the width of /// the first grapheme #[tokio::test(flavor = "multi_thread")] From 3d3206cb4e91bafded06672e3c3fb78ca0f558aa Mon Sep 17 00:00:00 2001 From: Daniel Poulin Date: Sun, 18 Sep 2022 15:24:13 -0400 Subject: [PATCH 07/17] Fix handling of skip values --- helix-core/src/surround.rs | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/helix-core/src/surround.rs b/helix-core/src/surround.rs index 7a7724c6768f..ebd8533e488f 100644 --- a/helix-core/src/surround.rs +++ b/helix-core/src/surround.rs @@ -55,7 +55,7 @@ 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); @@ -85,13 +85,21 @@ pub fn find_nth_closest_pairs_pos( // 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. - match find_nth_open_pair(text, open, close, close_pos, n) { + 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 Ok((open_pos, close_pos)); } _ => continue, From 5d543db752ec3f282a541091c64fe02a41d3b25f Mon Sep 17 00:00:00 2001 From: Daniel Poulin Date: Sun, 18 Sep 2022 23:41:29 -0400 Subject: [PATCH 08/17] Fix out of bounds error --- helix-core/src/surround.rs | 2 +- helix-term/tests/test/movement.rs | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/helix-core/src/surround.rs b/helix-core/src/surround.rs index ebd8533e488f..701f36ea76be 100644 --- a/helix-core/src/surround.rs +++ b/helix-core/src/surround.rs @@ -62,7 +62,7 @@ pub fn find_nth_closest_pairs_pos( let mut stack = Vec::with_capacity(2); let pos = range.from(); - let mut close_pos = pos - 1; + let mut close_pos = pos.saturating_sub(1); for ch in text.chars_at(pos) { close_pos += 1; diff --git a/helix-term/tests/test/movement.rs b/helix-term/tests/test/movement.rs index dcf43fed3b8f..97da9ba74b89 100644 --- a/helix-term/tests/test/movement.rs +++ b/helix-term/tests/test/movement.rs @@ -81,6 +81,10 @@ async fn surround_by_character() -> anyhow::Result<()> { #[tokio::test] async fn surround_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?; From 5646318a1cc11317279a7e4137a92f1511668c79 Mon Sep 17 00:00:00 2001 From: Daniel Poulin Date: Mon, 10 Oct 2022 07:24:16 -0400 Subject: [PATCH 09/17] Add `ma` version of tests --- helix-term/tests/test/movement.rs | 42 ++++++++++++++++++++++++++++++- 1 file changed, 41 insertions(+), 1 deletion(-) diff --git a/helix-term/tests/test/movement.rs b/helix-term/tests/test/movement.rs index 97da9ba74b89..524ca2ad742e 100644 --- a/helix-term/tests/test/movement.rs +++ b/helix-term/tests/test/movement.rs @@ -80,7 +80,7 @@ async fn surround_by_character() -> anyhow::Result<()> { } #[tokio::test] -async fn surround_pair() -> anyhow::Result<()> { +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?; @@ -119,6 +119,46 @@ async fn surround_pair() -> anyhow::Result<()> { Ok(()) } +#[tokio::test] +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 + // 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 paris 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?; + + // 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?; + + Ok(()) +} + /// Ensure the very initial cursor in an opened file is the width of /// the first grapheme #[tokio::test(flavor = "multi_thread")] From b40a3ead65c2a91c7467f444b8d98bce8d7d8ec0 Mon Sep 17 00:00:00 2001 From: Daniel Poulin Date: Mon, 10 Oct 2022 07:34:13 -0400 Subject: [PATCH 10/17] Fix formatting of tests --- helix-term/tests/test/movement.rs | 210 +++++++++++++++++++++++++----- 1 file changed, 180 insertions(+), 30 deletions(-) diff --git a/helix-term/tests/test/movement.rs b/helix-term/tests/test/movement.rs index 524ca2ad742e..045f8998a1df 100644 --- a/helix-term/tests/test/movement.rs +++ b/helix-term/tests/test/movement.rs @@ -67,14 +67,44 @@ async fn insert_to_normal_mode_cursor_position() -> anyhow::Result<()> { #[tokio::test] 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?; + 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?; + 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(()) } @@ -97,24 +127,84 @@ async fn surround_inside_pair() -> anyhow::Result<()> { 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?; + 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 paris 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?; + 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?; + 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?; // 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?; + 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?; Ok(()) } @@ -137,24 +227,84 @@ async fn surround_around_pair() -> anyhow::Result<()> { 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?; + 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 paris 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?; + 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?; + 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?; // 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?; + 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?; Ok(()) } From 6e34717c2261bbfa81982783cc052622c7d0afa0 Mon Sep 17 00:00:00 2001 From: Daniel Poulin Date: Mon, 10 Oct 2022 07:45:39 -0400 Subject: [PATCH 11/17] Reduce indentation levels for readability, and update comments --- helix-core/src/surround.rs | 65 ++++++++++++++++++++------------------ 1 file changed, 35 insertions(+), 30 deletions(-) diff --git a/helix-core/src/surround.rs b/helix-core/src/surround.rs index 701f36ea76be..90be0b31d708 100644 --- a/helix-core/src/surround.rs +++ b/helix-core/src/surround.rs @@ -66,6 +66,7 @@ pub fn find_nth_closest_pairs_pos( 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 @@ -73,38 +74,42 @@ 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, close) = 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. - 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 Ok((open_pos, close_pos)); - } - _ => continue, + } + + 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 Ok((open_pos, close_pos)); } + _ => continue, } } From 1cf02dd593abe63bced5b2df73e6c48a78881df5 Mon Sep 17 00:00:00 2001 From: Daniel Poulin Date: Mon, 10 Oct 2022 08:27:04 -0400 Subject: [PATCH 12/17] Preserve each selection's direction with enclosing pair surround --- helix-core/src/surround.rs | 8 +++++-- helix-core/src/textobject.rs | 16 +++++++++++-- helix-term/tests/test/movement.rs | 40 +++++++++++++++++++++++++++++++ 3 files changed, 60 insertions(+), 4 deletions(-) diff --git a/helix-core/src/surround.rs b/helix-core/src/surround.rs index 90be0b31d708..64d48c13a918 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)] = &[ @@ -107,7 +107,11 @@ pub fn find_nth_closest_pairs_pos( skip -= 1; continue; } - return Ok((open_pos, close_pos)); + + return match range.direction() { + Direction::Forward => Ok((open_pos, close_pos)), + Direction::Backward => Ok((close_pos, open_pos)), + }; } _ => continue, } diff --git a/helix-core/src/textobject.rs b/helix-core/src/textobject.rs index 76c6d103e6c7..972a80e78a60 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/tests/test/movement.rs b/helix-term/tests/test/movement.rs index 045f8998a1df..6a3dd0f4bd39 100644 --- a/helix-term/tests/test/movement.rs +++ b/helix-term/tests/test/movement.rs @@ -180,6 +180,26 @@ async fn surround_inside_pair() -> anyhow::Result<()> { )) .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)", @@ -280,6 +300,26 @@ async fn surround_around_pair() -> anyhow::Result<()> { )) .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)", From 4806a621849c35a558ed046f8adbd541635c3692 Mon Sep 17 00:00:00 2001 From: Daniel Poulin Date: Mon, 10 Oct 2022 08:46:53 -0400 Subject: [PATCH 13/17] Add test case for multiple cursors resulting in overlap --- helix-term/tests/test/movement.rs | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/helix-term/tests/test/movement.rs b/helix-term/tests/test/movement.rs index 6a3dd0f4bd39..83c7c9d1a7f7 100644 --- a/helix-term/tests/test/movement.rs +++ b/helix-term/tests/test/movement.rs @@ -226,6 +226,14 @@ async fn surround_inside_pair() -> anyhow::Result<()> { )) .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(()) } @@ -346,6 +354,14 @@ async fn surround_around_pair() -> anyhow::Result<()> { )) .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(()) } From 581ea5458181068e8160c7d032e3380c4b45e824 Mon Sep 17 00:00:00 2001 From: Daniel Poulin Date: Mon, 10 Oct 2022 08:52:16 -0400 Subject: [PATCH 14/17] Mark known failures as TODO --- helix-term/tests/test/movement.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/helix-term/tests/test/movement.rs b/helix-term/tests/test/movement.rs index 83c7c9d1a7f7..9f4fa13bcce0 100644 --- a/helix-term/tests/test/movement.rs +++ b/helix-term/tests/test/movement.rs @@ -119,7 +119,7 @@ async fn surround_inside_pair() -> anyhow::Result<()> { test(("some (#[t|]#ext) here", "mim", "some (#[text|]#) here")).await?; // On pair character selects pair - // Opening pair character is a known failure case that needs addressing + // 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?; @@ -247,7 +247,7 @@ async fn surround_around_pair() -> anyhow::Result<()> { test(("some (#[t|]#ext) here", "mam", "some #[(text)|]# here")).await?; // On pair character selects pair - // Opening pair character is a known failure case that needs addressing + // 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?; From 71789c437e3e018b41c3a72cc82674e36f60a08d Mon Sep 17 00:00:00 2001 From: Daniel Poulin Date: Sun, 6 Nov 2022 22:55:28 -0500 Subject: [PATCH 15/17] Make tests multi-threaded or they fail --- helix-term/tests/test/movement.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/helix-term/tests/test/movement.rs b/helix-term/tests/test/movement.rs index 9f4fa13bcce0..b6f9cb5646bd 100644 --- a/helix-term/tests/test/movement.rs +++ b/helix-term/tests/test/movement.rs @@ -64,7 +64,7 @@ async fn insert_to_normal_mode_cursor_position() -> anyhow::Result<()> { Ok(()) } -#[tokio::test] +#[tokio::test(flavor = "multi_thread")] async fn surround_by_character() -> anyhow::Result<()> { // Only pairs matching the passed character count test(( @@ -109,7 +109,7 @@ async fn surround_by_character() -> anyhow::Result<()> { Ok(()) } -#[tokio::test] +#[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 @@ -237,7 +237,7 @@ async fn surround_inside_pair() -> anyhow::Result<()> { Ok(()) } -#[tokio::test] +#[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 From 5b680343973565beef1789fe37eda8b04756cf46 Mon Sep 17 00:00:00 2001 From: Daniel Poulin Date: Sat, 28 Jan 2023 15:02:36 -0500 Subject: [PATCH 16/17] Cargo fmt --- helix-term/src/commands.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs index ff98c479015a..5ac751ecfe6b 100644 --- a/helix-term/src/commands.rs +++ b/helix-term/src/commands.rs @@ -2164,7 +2164,7 @@ fn extend_line_impl(cx: &mut Context, extend: Extend) { let start = text.line_to_char(start_line); let end = text.line_to_char( (end_line + 1) // newline of end_line - .min(text.len_lines()), + .min(text.len_lines()), ); // extend to previous/next line if current line is selected @@ -5218,10 +5218,10 @@ fn increment_impl(cx: &mut Context, increment_direction: IncrementDirection) { let selected_text: Cow = range.fragment(text); let new_from = ((range.from() as i128) + cumulative_length_diff) as usize; let incremented = [increment::integer, increment::date_time] - .iter() + .iter() .find_map(|incrementor| incrementor(selected_text.as_ref(), amount)); - amount += increase_by; + amount += increase_by; match incremented { None => { @@ -5230,14 +5230,14 @@ fn increment_impl(cx: &mut Context, increment_direction: IncrementDirection) { (range.to() as i128 + cumulative_length_diff) as usize, ); new_selection_ranges.push(new_range); - } + } Some(new_text) => { let new_range = Range::new(new_from, new_from + new_text.len()); cumulative_length_diff += new_text.len() as i128 - selected_text.len() as i128; new_selection_ranges.push(new_range); changes.push((range.from(), range.to(), Some(new_text.into()))); - } } + } } if !changes.is_empty() { From 506fa15c6ab1fb66fce64fde78ef096cad16fb17 Mon Sep 17 00:00:00 2001 From: Michael Davis Date: Fri, 10 Feb 2023 13:34:22 -0600 Subject: [PATCH 17/17] Fix typos in integration test comments --- helix-term/tests/test/movement.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/helix-term/tests/test/movement.rs b/helix-term/tests/test/movement.rs index b6f9cb5646bd..e6ea3f951592 100644 --- a/helix-term/tests/test/movement.rs +++ b/helix-term/tests/test/movement.rs @@ -146,7 +146,7 @@ async fn surround_inside_pair() -> anyhow::Result<()> { )) .await?; - // Matching paris outside selection don't match + // Matching pairs outside selection don't match test(( "((so)((many) go#[o|]#d (text))(here))", "mim", @@ -274,7 +274,7 @@ async fn surround_around_pair() -> anyhow::Result<()> { )) .await?; - // Matching paris outside selection don't match + // Matching pairs outside selection don't match test(( "((so)((many) go#[o|]#d (text))(here))", "mam",