git.iko.soy/helix
on master help sign in
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>

commit6929a12f291fa5dee50cde9c89845b206b7333fd authorDaniel S Poulin <dan.poulin@digitalsurgeons.com> dateFri 10 Feb 2023, 14:52 (4 years ago) parentaf1157f
4 files changed+361−107
compare with master
unifiedsplit
helix-core/src/surround.rs+45−104 helix-core/src/textobject.rs+14−2 helix-term/src/commands.rs+1−1 helix-term/tests/test/movement.rs+301
helix-core/src/surround.rs+45−104
@@ -1,6 +1,6 @@
11use std::fmt::Display;
22
3−use crate::{search, Range, Selection};
3+use crate::{movement::Direction, search, Range, Selection};
44use ropey::RopeSlice;
55
66pub const PAIRS: &[(char, char)] = &[
@@ -55,15 +55,18 @@ pub fn get_pair(ch: char) -> (char, char) {
5555pub fn find_nth_closest_pairs_pos(
5656 text: RopeSlice,
5757 range: Range,
58− n: usize,
58+ mut skip: usize,
5959) -> Result<(usize, usize)> {
6060 let is_open_pair = |ch| PAIRS.iter().any(|(open, _)| *open == ch);
6161 let is_close_pair = |ch| PAIRS.iter().any(|(_, close)| *close == ch);
6262
6363 let mut stack = Vec::with_capacity(2);
64− let pos = range.cursor(text);
64+ let pos = range.from();
65+ let mut close_pos = pos.saturating_sub(1);
6566
6667 for ch in text.chars_at(pos) {
68+ close_pos += 1;
69+
6770 if is_open_pair(ch) {
6871 // Track open pairs encountered so that we can step over
6972 // the corresponding close pairs that will come up further
@@ -71,20 +74,46 @@ pub fn find_nth_closest_pairs_pos(
7174 // open pair is before the cursor position.
7275 stack.push(ch);
7376 continue;
74− } else if is_close_pair(ch) {
75− let (open, _) = get_pair(ch);
76− if stack.last() == Some(&open) {
77− stack.pop();
78− continue;
79− } else {
80− // In the ideal case the stack would be empty here and the
81− // current character would be the close pair that we are
82− // looking for. It could also be the case that the pairs
83− // are unbalanced and we encounter a close pair that doesn't
84− // close the last seen open pair. In either case use this
85− // char as the auto-detected closest pair.
86− return find_nth_pairs_pos(text, ch, range, n);
77+ }
78+
79+ if !is_close_pair(ch) {
80+ // We don't care if this character isn't a brace pair item,
81+ // so short circuit here.
82+ continue;
83+ }
84+
85+ let (open, close) = get_pair(ch);
86+
87+ if stack.last() == Some(&open) {
88+ // If we are encountering the closing pair for an opener
89+ // we just found while traversing, then its inside the
90+ // selection and should be skipped over.
91+ stack.pop();
92+ continue;
93+ }
94+
95+ match find_nth_open_pair(text, open, close, close_pos, 1) {
96+ // Before we accept this pair, we want to ensure that the
97+ // pair encloses the range rather than just the cursor.
98+ Some(open_pos)
99+ if open_pos <= pos.saturating_add(1)
100+ && close_pos >= range.to().saturating_sub(1) =>
101+ {
102+ // Since we have special conditions for when to
103+ // accept, we can't just pass the skip parameter on
104+ // through to the find_nth_*_pair methods, so we
105+ // track skips manually here.
106+ if skip > 1 {
107+ skip -= 1;
108+ continue;
109+ }
110+
111+ return match range.direction() {
112+ Direction::Forward => Ok((open_pos, close_pos)),
113+ Direction::Backward => Ok((close_pos, open_pos)),
114+ };
87115 }
116+ _ => continue,
88117 }
89118 }
90119
@@ -244,94 +273,6 @@ mod test {
244273 use ropey::Rope;
245274 use smallvec::SmallVec;
246275
247− #[allow(clippy::type_complexity)]
248− fn check_find_nth_pair_pos(
249− text: &str,
250− cases: Vec<(usize, char, usize, Result<(usize, usize)>)>,
251− ) {
252− let doc = Rope::from(text);
253− let slice = doc.slice(..);
254−
255− for (cursor_pos, ch, n, expected_range) in cases {
256− let range = find_nth_pairs_pos(slice, ch, (cursor_pos, cursor_pos + 1).into(), n);
257− assert_eq!(
258− range, expected_range,
259− "Expected {:?}, got {:?}",
260− expected_range, range
261− );
262− }
263− }
264−
265− #[test]
266− fn test_find_nth_pairs_pos() {
267− check_find_nth_pair_pos(
268− "some (text) here",
269− vec![
270− // cursor on [t]ext
271− (6, '(', 1, Ok((5, 10))),
272− (6, ')', 1, Ok((5, 10))),
273− // cursor on so[m]e
274− (2, '(', 1, Err(Error::PairNotFound)),
275− // cursor on bracket itself
276− (5, '(', 1, Ok((5, 10))),
277− (10, '(', 1, Ok((5, 10))),
278− ],
279− );
280− }
281−
282− #[test]
283− fn test_find_nth_pairs_pos_skip() {
284− check_find_nth_pair_pos(
285− "(so (many (good) text) here)",
286− vec![
287− // cursor on go[o]d
288− (13, '(', 1, Ok((10, 15))),
289− (13, '(', 2, Ok((4, 21))),
290− (13, '(', 3, Ok((0, 27))),
291− ],
292− );
293− }
294−
295− #[test]
296− fn test_find_nth_pairs_pos_same() {
297− check_find_nth_pair_pos(
298− "'so 'many 'good' text' here'",
299− vec![
300− // cursor on go[o]d
301− (13, '\'', 1, Ok((10, 15))),
302− (13, '\'', 2, Ok((4, 21))),
303− (13, '\'', 3, Ok((0, 27))),
304− // cursor on the quotes
305− (10, '\'', 1, Err(Error::CursorOnAmbiguousPair)),
306− ],
307− )
308− }
309−
310− #[test]
311− fn test_find_nth_pairs_pos_step() {
312− check_find_nth_pair_pos(
313− "((so)((many) good (text))(here))",
314− vec![
315− // cursor on go[o]d
316− (15, '(', 1, Ok((5, 24))),
317− (15, '(', 2, Ok((0, 31))),
318− ],
319− )
320− }
321−
322− #[test]
323− fn test_find_nth_pairs_pos_mixed() {
324− check_find_nth_pair_pos(
325− "(so [many {good} text] here)",
326− vec![
327− // cursor on go[o]d
328− (13, '{', 1, Ok((10, 15))),
329− (13, '[', 1, Ok((4, 21))),
330− (13, '(', 1, Ok((0, 27))),
331− ],
332− )
333− }
334−
335276 #[test]
336277 fn test_get_surround_pos() {
337278 let doc = Rope::from("(some) (chars)\n(newline)");
helix-core/src/textobject.rs+14−2
@@ -231,8 +231,20 @@ fn textobject_pair_surround_impl(
231231 };
232232 pair_pos
233233 .map(|(anchor, head)| match textobject {
234− TextObject::Inside => Range::new(next_grapheme_boundary(slice, anchor), head),
235− TextObject::Around => Range::new(anchor, next_grapheme_boundary(slice, head)),
234+ TextObject::Inside => {
235+ if anchor < head {
236+ Range::new(next_grapheme_boundary(slice, anchor), head)
237+ } else {
238+ Range::new(anchor, next_grapheme_boundary(slice, head))
239+ }
240+ }
241+ TextObject::Around => {
242+ if anchor < head {
243+ Range::new(anchor, next_grapheme_boundary(slice, head))
244+ } else {
245+ Range::new(next_grapheme_boundary(slice, anchor), head)
246+ }
247+ }
236248 TextObject::Movement => unreachable!(),
237249 })
238250 .unwrap_or(range)
helix-term/src/commands.rs+1−1
@@ -4791,7 +4791,7 @@ fn select_textobject(cx: &mut Context, objtype: textobject::TextObject) {
47914791 ("a", "Argument/parameter (tree-sitter)"),
47924792 ("c", "Comment (tree-sitter)"),
47934793 ("T", "Test (tree-sitter)"),
4794− ("m", "Closest surrounding pair to cursor"),
4794+ ("m", "Closest surrounding pair"),
47954795 (" ", "... or any character acting as a pair"),
47964796 ];
47974797
helix-term/tests/test/movement.rs+301
@@ -64,6 +64,307 @@ async fn insert_to_normal_mode_cursor_position() -> anyhow::Result<()> {
6464 Ok(())
6565}
6666
67+#[tokio::test(flavor = "multi_thread")]
68+async fn surround_by_character() -> anyhow::Result<()> {
69+ // Only pairs matching the passed character count
70+ test((
71+ "(so [many {go#[o|]#d} text] here)",
72+ "mi{",
73+ "(so [many {#[good|]#} text] here)",
74+ ))
75+ .await?;
76+ test((
77+ "(so [many {go#[o|]#d} text] here)",
78+ "mi[",
79+ "(so [#[many {good} text|]#] here)",
80+ ))
81+ .await?;
82+ test((
83+ "(so [many {go#[o|]#d} text] here)",
84+ "mi(",
85+ "(#[so [many {good} text] here|]#)",
86+ ))
87+ .await?;
88+
89+ // Works with characters that aren't pairs too
90+ test((
91+ "'so 'many 'go#[o|]#d' text' here'",
92+ "mi'",
93+ "'so 'many '#[good|]#' text' here'",
94+ ))
95+ .await?;
96+ test((
97+ "'so 'many 'go#[o|]#d' text' here'",
98+ "2mi'",
99+ "'so '#[many 'good' text|]#' here'",
100+ ))
101+ .await?;
102+ test((
103+ "'so \"many 'go#[o|]#d' text\" here'",
104+ "mi\"",
105+ "'so \"#[many 'good' text|]#\" here'",
106+ ))
107+ .await?;
108+
109+ Ok(())
110+}
111+
112+#[tokio::test(flavor = "multi_thread")]
113+async fn surround_inside_pair() -> anyhow::Result<()> {
114+ // Works at first character of buffer
115+ // TODO: Adjust test when opening pair failure is fixed
116+ test(("#[(|]#something)", "mim", "#[(|]#something)")).await?;
117+
118+ // Inside a valid pair selects pair
119+ test(("some (#[t|]#ext) here", "mim", "some (#[text|]#) here")).await?;
120+
121+ // On pair character selects pair
122+ // TODO: Opening pair character is a known failure case that needs addressing
123+ // test(("some #[(|]#text) here", "mim", "some (#[text|]#) here")).await?;
124+ test(("some (text#[)|]# here", "mim", "some (#[text|]#) here")).await?;
125+
126+ // No valid pair does nothing
127+ test(("so#[m|]#e (text) here", "mim", "so#[m|]#e (text) here")).await?;
128+
129+ // Count skips to outer pairs
130+ test((
131+ "(so (many (go#[o|]#d) text) here)",
132+ "1mim",
133+ "(so (many (#[good|]#) text) here)",
134+ ))
135+ .await?;
136+ test((
137+ "(so (many (go#[o|]#d) text) here)",
138+ "2mim",
139+ "(so (#[many (good) text|]#) here)",
140+ ))
141+ .await?;
142+ test((
143+ "(so (many (go#[o|]#d) text) here)",
144+ "3mim",
145+ "(#[so (many (good) text) here|]#)",
146+ ))
147+ .await?;
148+
149+ // Matching pairs outside selection don't match
150+ test((
151+ "((so)((many) go#[o|]#d (text))(here))",
152+ "mim",
153+ "((so)(#[(many) good (text)|]#)(here))",
154+ ))
155+ .await?;
156+ test((
157+ "((so)((many) go#[o|]#d (text))(here))",
158+ "2mim",
159+ "(#[(so)((many) good (text))(here)|]#)",
160+ ))
161+ .await?;
162+
163+ // Works with mixed braces
164+ test((
165+ "(so [many {go#[o|]#d} text] here)",
166+ "mim",
167+ "(so [many {#[good|]#} text] here)",
168+ ))
169+ .await?;
170+ test((
171+ "(so [many {go#[o|]#d} text] here)",
172+ "2mim",
173+ "(so [#[many {good} text|]#] here)",
174+ ))
175+ .await?;
176+ test((
177+ "(so [many {go#[o|]#d} text] here)",
178+ "3mim",
179+ "(#[so [many {good} text] here|]#)",
180+ ))
181+ .await?;
182+
183+ // Selection direction is preserved
184+ test((
185+ "(so [many {go#[|od]#} text] here)",
186+ "mim",
187+ "(so [many {#[|good]#} text] here)",
188+ ))
189+ .await?;
190+ test((
191+ "(so [many {go#[|od]#} text] here)",
192+ "2mim",
193+ "(so [#[|many {good} text]#] here)",
194+ ))
195+ .await?;
196+ test((
197+ "(so [many {go#[|od]#} text] here)",
198+ "3mim",
199+ "(#[|so [many {good} text] here]#)",
200+ ))
201+ .await?;
202+
203+ // Only pairs outside of full selection range are considered
204+ test((
205+ "(so (many (go#[od) |]#text) here)",
206+ "mim",
207+ "(so (#[many (good) text|]#) here)",
208+ ))
209+ .await?;
210+ test((
211+ "(so (many#[ (go|]#od) text) here)",
212+ "mim",
213+ "(so (#[many (good) text|]#) here)",
214+ ))
215+ .await?;
216+ test((
217+ "(so#[ (many (go|]#od) text) here)",
218+ "mim",
219+ "(#[so (many (good) text) here|]#)",
220+ ))
221+ .await?;
222+ test((
223+ "(so (many (go#[od) text) |]#here)",
224+ "mim",
225+ "(#[so (many (good) text) here|]#)",
226+ ))
227+ .await?;
228+
229+ // Works with multiple cursors
230+ test((
231+ "(so (many (good) text) #[he|]#re\nso (many (good) text) #(|he)#re)",
232+ "mim",
233+ "(#[so (many (good) text) here\nso (many (good) text) here|]#)",
234+ ))
235+ .await?;
236+
237+ Ok(())
238+}
239+
240+#[tokio::test(flavor = "multi_thread")]
241+async fn surround_around_pair() -> anyhow::Result<()> {
242+ // Works at first character of buffer
243+ // TODO: Adjust test when opening pair failure is fixed
244+ test(("#[(|]#something)", "mam", "#[(|]#something)")).await?;
245+
246+ // Inside a valid pair selects pair
247+ test(("some (#[t|]#ext) here", "mam", "some #[(text)|]# here")).await?;
248+
249+ // On pair character selects pair
250+ // TODO: Opening pair character is a known failure case that needs addressing
251+ // test(("some #[(|]#text) here", "mam", "some #[(text)|]# here")).await?;
252+ test(("some (text#[)|]# here", "mam", "some #[(text)|]# here")).await?;
253+
254+ // No valid pair does nothing
255+ test(("so#[m|]#e (text) here", "mam", "so#[m|]#e (text) here")).await?;
256+
257+ // Count skips to outer pairs
258+ test((
259+ "(so (many (go#[o|]#d) text) here)",
260+ "1mam",
261+ "(so (many #[(good)|]# text) here)",
262+ ))
263+ .await?;
264+ test((
265+ "(so (many (go#[o|]#d) text) here)",
266+ "2mam",
267+ "(so #[(many (good) text)|]# here)",
268+ ))
269+ .await?;
270+ test((
271+ "(so (many (go#[o|]#d) text) here)",
272+ "3mam",
273+ "#[(so (many (good) text) here)|]#",
274+ ))
275+ .await?;
276+
277+ // Matching pairs outside selection don't match
278+ test((
279+ "((so)((many) go#[o|]#d (text))(here))",
280+ "mam",
281+ "((so)#[((many) good (text))|]#(here))",
282+ ))
283+ .await?;
284+ test((
285+ "((so)((many) go#[o|]#d (text))(here))",
286+ "2mam",
287+ "#[((so)((many) good (text))(here))|]#",
288+ ))
289+ .await?;
290+
291+ // Works with mixed braces
292+ test((
293+ "(so [many {go#[o|]#d} text] here)",
294+ "mam",
295+ "(so [many #[{good}|]# text] here)",
296+ ))
297+ .await?;
298+ test((
299+ "(so [many {go#[o|]#d} text] here)",
300+ "2mam",
301+ "(so #[[many {good} text]|]# here)",
302+ ))
303+ .await?;
304+ test((
305+ "(so [many {go#[o|]#d} text] here)",
306+ "3mam",
307+ "#[(so [many {good} text] here)|]#",
308+ ))
309+ .await?;
310+
311+ // Selection direction is preserved
312+ test((
313+ "(so [many {go#[|od]#} text] here)",
314+ "mam",
315+ "(so [many #[|{good}]# text] here)",
316+ ))
317+ .await?;
318+ test((
319+ "(so [many {go#[|od]#} text] here)",
320+ "2mam",
321+ "(so #[|[many {good} text]]# here)",
322+ ))
323+ .await?;
324+ test((
325+ "(so [many {go#[|od]#} text] here)",
326+ "3mam",
327+ "#[|(so [many {good} text] here)]#",
328+ ))
329+ .await?;
330+
331+ // Only pairs outside of full selection range are considered
332+ test((
333+ "(so (many (go#[od) |]#text) here)",
334+ "mam",
335+ "(so #[(many (good) text)|]# here)",
336+ ))
337+ .await?;
338+ test((
339+ "(so (many#[ (go|]#od) text) here)",
340+ "mam",
341+ "(so #[(many (good) text)|]# here)",
342+ ))
343+ .await?;
344+ test((
345+ "(so#[ (many (go|]#od) text) here)",
346+ "mam",
347+ "#[(so (many (good) text) here)|]#",
348+ ))
349+ .await?;
350+ test((
351+ "(so (many (go#[od) text) |]#here)",
352+ "mam",
353+ "#[(so (many (good) text) here)|]#",
354+ ))
355+ .await?;
356+
357+ // Works with multiple cursors
358+ test((
359+ "(so (many (good) text) #[he|]#re\nso (many (good) text) #(|he)#re)",
360+ "mam",
361+ "#[(so (many (good) text) here\nso (many (good) text) here)|]#",
362+ ))
363+ .await?;
364+
365+ Ok(())
366+}
367+
67368/// Ensure the very initial cursor in an opened file is the width of
68369/// the first grapheme
69370#[tokio::test(flavor = "multi_thread")]