From bc8ee85cbb8fbf7b64e5c0c27cb17423be06bee2 Mon Sep 17 00:00:00 2001 From: Skyler Hawthorne Date: Wed, 1 Mar 2023 00:30:32 -0500 Subject: [PATCH 1/6] backfill expand/shrink tests --- helix-term/tests/test/commands/movement.rs | 112 +++++++++++++++++++++ 1 file changed, 112 insertions(+) diff --git a/helix-term/tests/test/commands/movement.rs b/helix-term/tests/test/commands/movement.rs index 5868fa49433c..93218be826a6 100644 --- a/helix-term/tests/test/commands/movement.rs +++ b/helix-term/tests/test/commands/movement.rs @@ -948,3 +948,115 @@ async fn match_bracket() -> anyhow::Result<()> { Ok(()) } + +#[tokio::test(flavor = "multi_thread")] +async fn expand_shrink_selection() -> anyhow::Result<()> { + let tests = vec![ + // single range + ( + indoc! {r##" + Some(#[thing|]#) + "##}, + "", + indoc! {r##" + #[Some(thing)|]# + "##}, + ), + // multi range + ( + indoc! {r##" + Some(#[thing|]#) + Some(#(other_thing|)#) + "##}, + "", + indoc! {r##" + Some#[(thing)|]# + Some#((other_thing)|)# + "##}, + ), + // multi range collision merges + ( + indoc! {r##" + ( + Some(#[thing|]#), + Some(#(other_thing|)#), + ) + "##}, + "", + indoc! {r##" + #[( + Some(thing), + Some(other_thing), + )|]# + "##}, + ), + // multi range collision merges, then shrinks back to original + ( + indoc! {r##" + ( + Some(#[thing|]#), + Some(#(other_thing|)#), + ) + "##}, + "", + indoc! {r##" + ( + #[Some(thing)|]#, + #(Some(other_thing)|)#, + ) + "##}, + ), + ( + indoc! {r##" + ( + Some(#[thing|]#), + Some(#(other_thing|)#), + ) + "##}, + "", + indoc! {r##" + ( + Some#[(thing)|]#, + Some#((other_thing)|)#, + ) + "##}, + ), + ( + indoc! {r##" + ( + Some(#[thing|]#), + Some(#(other_thing|)#), + ) + "##}, + "", + indoc! {r##" + ( + Some(#[thing|]#), + Some(#(other_thing|)#), + ) + "##}, + ), + // shrink with no expansion history defaults to first child + ( + indoc! {r##" + ( + #[Some(thing)|]#, + Some(other_thing), + ) + "##}, + "", + indoc! {r##" + ( + #[Some|]#(thing), + Some(other_thing), + ) + "##}, + ), + ]; + + for test in tests { + test_with_config(AppBuilder::new().with_file("foo.rs", None), test).await?; + } + + Ok(()) +} From 5e1b730d7476c7df8a06a633ac86b2829a7aabd7 Mon Sep 17 00:00:00 2001 From: Skyler Hawthorne Date: Sun, 7 Apr 2024 14:26:52 -0400 Subject: [PATCH 2/6] Add TreeRecursiveWalker Adds a method of breadth-first recursive descent for TreeCursors --- helix-core/src/syntax/tree_cursor.rs | 66 +++++++++++++++++++++++++--- 1 file changed, 59 insertions(+), 7 deletions(-) diff --git a/helix-core/src/syntax/tree_cursor.rs b/helix-core/src/syntax/tree_cursor.rs index d82ea74dbfff..6c645f2f3188 100644 --- a/helix-core/src/syntax/tree_cursor.rs +++ b/helix-core/src/syntax/tree_cursor.rs @@ -1,7 +1,8 @@ -use std::{cmp::Reverse, ops::Range}; +use std::{cmp::Reverse, collections::VecDeque, ops::Range}; use super::{LanguageLayer, LayerId}; +use ropey::RopeSlice; use slotmap::HopSlotMap; use tree_sitter::Node; @@ -18,18 +19,18 @@ struct InjectionRange { depth: u32, } -pub struct TreeCursor<'a> { - layers: &'a HopSlotMap, +pub struct TreeCursor<'n> { + layers: &'n HopSlotMap, root: LayerId, current: LayerId, injection_ranges: Vec, // TODO: Ideally this would be a `tree_sitter::TreeCursor<'a>` but // that returns very surprising results in testing. - cursor: Node<'a>, + cursor: Node<'n>, } -impl<'a> TreeCursor<'a> { - pub(super) fn new(layers: &'a HopSlotMap, root: LayerId) -> Self { +impl<'n> TreeCursor<'n> { + pub(super) fn new(layers: &'n HopSlotMap, root: LayerId) -> Self { let mut injection_ranges = Vec::new(); for (layer_id, layer) in layers.iter() { @@ -61,7 +62,7 @@ impl<'a> TreeCursor<'a> { } } - pub fn node(&self) -> Node<'a> { + pub fn node(&self) -> Node<'n> { self.cursor } @@ -262,3 +263,54 @@ impl<'n> Iterator for ChildIter<'n> { } } } + +impl<'n> IntoIterator for &'n mut TreeCursor<'n> { + type Item = Node<'n>; + type IntoIter = TreeRecursiveWalker<'n>; + + fn into_iter(self) -> Self::IntoIter { + let mut queue = VecDeque::new(); + let root = self.node(); + queue.push_back(root); + + TreeRecursiveWalker { + cursor: self, + queue, + root, + } + } +} + +pub struct TreeRecursiveWalker<'n> { + cursor: &'n mut TreeCursor<'n>, + queue: VecDeque>, + root: Node<'n>, +} + +impl<'n> Iterator for TreeRecursiveWalker<'n> { + type Item = Node<'n>; + + fn next(&mut self) -> Option { + let current = self.cursor.node(); + log::debug!("recursive walk -- current: {current:?}"); + + if current != self.root && self.cursor.goto_next_named_sibling() { + self.queue.push_back(current); + log::debug!("recursive walk -- sibling: {:?}", self.cursor.node()); + return Some(self.cursor.node()); + } + + while let Some(queued) = self.queue.pop_front() { + self.cursor.cursor = queued; + + if !self.cursor.goto_first_named_child() { + continue; + } + + log::debug!("recursive walk -- child: {:?}", self.cursor.node()); + return Some(self.cursor.node()); + } + + None + } +} From b43a808473ec0e913e1d3d113d6515f9cdf233d2 Mon Sep 17 00:00:00 2001 From: Skyler Hawthorne Date: Sun, 26 Feb 2023 00:53:43 -0500 Subject: [PATCH 3/6] shrink_selection without history selects first contained child This changes the behavior of `shrink_selection` to iterate through child nodes until it finds one that is contained within the selection, with at least one of the ends of the selection being exclusively inside the starting selection (though not necessarily both ends). This produces more intuitive behavior for selecting the "first logical thing" inside the selection. --- helix-core/src/object.rs | 104 ++++++++------------- helix-core/src/syntax/tree_cursor.rs | 27 +++++- helix-term/tests/test/commands/movement.rs | 21 ++++- 3 files changed, 83 insertions(+), 69 deletions(-) diff --git a/helix-core/src/object.rs b/helix-core/src/object.rs index 17a393caf277..7ef5611b17ea 100644 --- a/helix-core/src/object.rs +++ b/helix-core/src/object.rs @@ -25,31 +25,49 @@ pub fn expand_selection(syntax: &Syntax, text: RopeSlice, selection: Selection) } pub fn shrink_selection(syntax: &Syntax, text: RopeSlice, selection: Selection) -> Selection { - select_node_impl( - syntax, - text, - selection, - |cursor| { - cursor.goto_first_child(); - }, - None, - ) + selection.transform(move |range| { + let (from, to) = range.into_byte_range(text); + let mut cursor = syntax.walk(); + cursor.reset_to_byte_range(from, to); + + if let Some(node) = cursor.first_contained_child(&range, text) { + return Range::from_node(node, text, range.direction()); + } + + range + }) } pub fn select_next_sibling(syntax: &Syntax, text: RopeSlice, selection: Selection) -> Selection { - select_node_impl( - syntax, - text, - selection, - |cursor| { - while !cursor.goto_next_sibling() { - if !cursor.goto_parent() { - break; - } + selection.transform(move |range| { + let (from, to) = range.into_byte_range(text); + let mut cursor = syntax.walk(); + cursor.reset_to_byte_range(from, to); + + while !cursor.goto_next_sibling() { + if !cursor.goto_parent() { + return range; + } + } + + Range::from_node(cursor.node(), text, Direction::Forward) + }) +} + +pub fn select_prev_sibling(syntax: &Syntax, text: RopeSlice, selection: Selection) -> Selection { + selection.transform(move |range| { + let (from, to) = range.into_byte_range(text); + let mut cursor = syntax.walk(); + cursor.reset_to_byte_range(from, to); + + while !cursor.goto_prev_sibling() { + if !cursor.goto_parent() { + return range; } - }, - Some(Direction::Forward), - ) + } + + Range::from_node(cursor.node(), text, Direction::Backward) + }) } pub fn select_all_siblings(syntax: &Syntax, text: RopeSlice, selection: Selection) -> Selection { @@ -91,47 +109,3 @@ fn select_children<'n>( vec![range] } } - -pub fn select_prev_sibling(syntax: &Syntax, text: RopeSlice, selection: Selection) -> Selection { - select_node_impl( - syntax, - text, - selection, - |cursor| { - while !cursor.goto_prev_sibling() { - if !cursor.goto_parent() { - break; - } - } - }, - Some(Direction::Backward), - ) -} - -fn select_node_impl( - syntax: &Syntax, - text: RopeSlice, - selection: Selection, - motion: F, - direction: Option, -) -> Selection -where - F: Fn(&mut TreeCursor), -{ - let cursor = &mut syntax.walk(); - - selection.transform(|range| { - let from = text.char_to_byte(range.from()); - let to = text.char_to_byte(range.to()); - - cursor.reset_to_byte_range(from, to); - - motion(cursor); - - let node = cursor.node(); - let from = text.byte_to_char(node.start_byte()); - let to = text.byte_to_char(node.end_byte()); - - Range::new(from, to).with_direction(direction.unwrap_or_else(|| range.direction())) - }) -} diff --git a/helix-core/src/syntax/tree_cursor.rs b/helix-core/src/syntax/tree_cursor.rs index 6c645f2f3188..74d1de3e4ab1 100644 --- a/helix-core/src/syntax/tree_cursor.rs +++ b/helix-core/src/syntax/tree_cursor.rs @@ -166,6 +166,29 @@ impl<'n> TreeCursor<'n> { } } + /// Finds the first child node that is contained "inside" the given input + /// range, i.e. either start_new > start_old and end_new <= end old OR + /// start_new >= start_old and end_new < end_old + pub fn goto_first_contained_child(&'n mut self, range: &crate::Range, text: RopeSlice) -> bool { + self.first_contained_child(range, text).is_some() + } + + /// Finds the first child node that is contained "inside" the given input + /// range, i.e. either start_new > start_old and end_new <= end old OR + /// start_new >= start_old and end_new < end_old + pub fn first_contained_child( + &'n mut self, + range: &crate::Range, + text: RopeSlice, + ) -> Option> { + let (from, to) = range.into_byte_range(text); + + self.into_iter().find(|&node| { + (node.start_byte() > from && node.end_byte() <= to) + || (node.start_byte() >= from && node.end_byte() < to) + }) + } + pub fn goto_next_sibling(&mut self) -> bool { self.goto_next_sibling_impl(false) } @@ -218,7 +241,7 @@ impl<'n> TreeCursor<'n> { /// Returns an iterator over the children of the node the TreeCursor is on /// at the time this is called. - pub fn children(&'a mut self) -> ChildIter<'a> { + pub fn children(&'n mut self) -> ChildIter<'n> { let parent = self.node(); ChildIter { @@ -230,7 +253,7 @@ impl<'n> TreeCursor<'n> { /// Returns an iterator over the named children of the node the TreeCursor is on /// at the time this is called. - pub fn named_children(&'a mut self) -> ChildIter<'a> { + pub fn named_children(&'n mut self) -> ChildIter<'n> { let parent = self.node(); ChildIter { diff --git a/helix-term/tests/test/commands/movement.rs b/helix-term/tests/test/commands/movement.rs index 93218be826a6..dd5a3beef576 100644 --- a/helix-term/tests/test/commands/movement.rs +++ b/helix-term/tests/test/commands/movement.rs @@ -1038,16 +1038,33 @@ async fn expand_shrink_selection() -> anyhow::Result<()> { ), // shrink with no expansion history defaults to first child ( + indoc! {r##" + #[( + Some(thing), + Some(other_thing), + )|]# + "##}, + "", indoc! {r##" ( #[Some(thing)|]#, Some(other_thing), ) "##}, - "", + ), + // any movement cancels selection history and falls back to first child + ( + indoc! {r##" + ( + Some(#[thing|]#), + Some(#(other_thing|)#), + ) + + "##}, + "jkvkkk", indoc! {r##" ( - #[Some|]#(thing), + #[|Some(thing)]#, Some(other_thing), ) "##}, From f44b51e6790dba3958612107a88032a8bf7e7709 Mon Sep 17 00:00:00 2001 From: Skyler Hawthorne Date: Fri, 24 Feb 2023 00:06:08 -0500 Subject: [PATCH 4/6] Add Selection::{overlaps,without} and fix contains Adds two helper functions to `Selection`: * `overlaps`: tests whether two `Selection`s contain any ranges which overlap with each other * `without`: Computes a new `Selection` that is the set difference of two `Selection`s, i.e. everything in the first `Selection` with everything that overlaps in the second `Selection` removed, potentially leaving holes in the original ranges. It also fixes a bug with `Selection::contains`: it assumes that if the second `Selection` has a greater number of ranges than the first, then the first cannot contain the second; but this is false, since one range from the first could contain multiple smaller ranges in the second. --- helix-core/src/selection.rs | 449 +++++++++++++++++++++++++++++++++++- 1 file changed, 444 insertions(+), 5 deletions(-) diff --git a/helix-core/src/selection.rs b/helix-core/src/selection.rs index 76de63628d1e..27d6d6e7d47e 100644 --- a/helix-core/src/selection.rs +++ b/helix-core/src/selection.rs @@ -701,22 +701,161 @@ impl Selection { pub fn contains(&self, other: &Selection) -> bool { is_subset::(self.range_bounds(), other.range_bounds()) } + + /// returns true if self has at least one range that overlaps with at least one range from other + pub fn overlaps(&self, other: &Selection) -> bool { + let (mut iter_self, mut iter_other) = (self.iter(), other.iter()); + let (mut ele_self, mut ele_other) = (iter_self.next(), iter_other.next()); + + loop { + match (ele_self, ele_other) { + (Some(ra), Some(rb)) => { + if ra.overlaps(rb) { + return true; + } else if ra.from() > rb.from() { + ele_self = iter_self.next(); + } else { + ele_other = iter_other.next(); + } + } + _ => return false, + } + } + } + + /// Returns the given selection with the overlapping portions of `other` + /// cut out. If one range from this selection is equal to one from `other`, + /// this range is removed. If this results in an entirely empty selection, + /// `None` is returned. + pub fn without(self, other: &Selection) -> Option { + if other.contains(&self) { + return None; + } + + let mut primary_index = self.primary_index; + let mut ranges = smallvec!(); + let (mut iter_self, mut iter_other) = (self.into_iter(), other.iter()); + let (mut ele_self, mut ele_other) = (iter_self.next(), iter_other.next()); + let mut cur_index = 0; + + loop { + match (ele_self, ele_other) { + (Some(ra), Some(rb)) => { + if !ra.overlaps(rb) { + // there's no overlap and it's on the left of rb + if ra.to() <= rb.from() { + ranges.push(ra); + ele_self = iter_self.next(); + cur_index += 1; + + // otherwise it must be on the right, so move rb forward + } else { + ele_other = iter_other.next(); + } + + continue; + } + + // otherwise there is overlap, so truncate or split + if rb.contains_range(&ra) { + ele_self = iter_self.next(); + cur_index += 1; + continue; + } + + // [ ra ] + // [ rb ] + if ra.from() <= rb.from() && ra.to() <= rb.to() && ra.to() >= rb.from() { + let new = if ra.direction() == Direction::Backward { + Range::new(rb.from(), ra.head) + } else { + Range::new(ra.anchor, rb.from()) + }; + + ranges.push(new); + ele_self = iter_self.next(); + cur_index += 1; + + // [ ra ] + // [ rb ] + } else if ra.from() >= rb.from() && ra.to() >= rb.to() && ra.from() <= rb.to() { + let new = if ra.direction() == Direction::Backward { + Range::new(ra.anchor, rb.to() + 1) + } else { + Range::new(rb.to(), ra.head) + }; + + // don't settle on the new range yet because the next + // rb could chop off the other end of ra + ele_self = Some(new); + ele_other = iter_other.next(); + + // [ ra ] + // [ rb ] + } else if ra.from() < rb.from() && ra.to() > rb.to() { + // we must split the range into two + let left = if ra.direction() == Direction::Backward { + Range::new(rb.from(), ra.head) + } else { + Range::new(ra.anchor, rb.from()) + }; + + let right = if ra.direction() == Direction::Backward { + Range::new(ra.anchor, rb.to()) + } else { + Range::new(rb.to(), ra.head) + }; + + // We do NOT push right onto the results right away. + // We must put it back into the iterator and check it + // again in case a further range splits it again. + ranges.push(left); + ele_other = iter_other.next(); + + // offset the primary index whenever we split + if cur_index < primary_index { + primary_index += 1; + } + + cur_index += 1; + ele_self = Some(right); + } + } + // the rest just get included as is + (Some(range), None) => { + ranges.push(range); + ele_self = iter_self.next(); + cur_index += 1; + } + // exhausted `self`, nothing left to do + (None, _) => { + break; + } + } + } + + if ranges.is_empty() { + return None; + } + + Some(Selection::new(ranges, primary_index)) + } } impl<'a> IntoIterator for &'a Selection { type Item = &'a Range; type IntoIter = std::slice::Iter<'a, Range>; - fn into_iter(self) -> std::slice::Iter<'a, Range> { + fn into_iter(self) -> Self::IntoIter { self.ranges().iter() } } impl IntoIterator for Selection { type Item = Range; - type IntoIter = smallvec::IntoIter<[Range; 1]>; + type IntoIter = smallvec::IntoIter<[Self::Item; 1]>; - fn into_iter(self) -> smallvec::IntoIter<[Range; 1]> { + fn into_iter(self) -> Self::IntoIter { self.ranges.into_iter() } } @@ -877,6 +1016,7 @@ pub fn split_on_matches(text: RopeSlice, selection: &Selection, regex: &rope::Re #[cfg(test)] mod test { use super::*; + use crate::test; use crate::Rope; #[test] @@ -967,7 +1107,7 @@ mod test { } #[test] - fn test_overlaps() { + fn test_range_overlaps() { fn overlaps(a: (usize, usize), b: (usize, usize)) -> bool { Range::new(a.0, a.1).overlaps(&Range::new(b.0, b.1)) } @@ -1017,6 +1157,160 @@ mod test { assert!(overlaps((1, 1), (1, 1))); } + #[test] + fn test_selection_overlaps() { + fn overlaps(a: &[(usize, usize)], b: &[(usize, usize)]) -> bool { + let a = Selection::new( + a.iter() + .map(|(anchor, head)| Range::new(*anchor, *head)) + .collect(), + 0, + ); + let b = Selection::new( + b.iter() + .map(|(anchor, head)| Range::new(*anchor, *head)) + .collect(), + 0, + ); + a.overlaps(&b) + } + + // Two non-zero-width ranges, no overlap. + assert!(!overlaps(&[(0, 3)], &[(3, 6)])); + assert!(!overlaps(&[(0, 3)], &[(6, 3)])); + assert!(!overlaps(&[(3, 0)], &[(3, 6)])); + assert!(!overlaps(&[(3, 0)], &[(6, 3)])); + assert!(!overlaps(&[(3, 6)], &[(0, 3)])); + assert!(!overlaps(&[(3, 6)], &[(3, 0)])); + assert!(!overlaps(&[(6, 3)], &[(0, 3)])); + assert!(!overlaps(&[(6, 3)], &[(3, 0)])); + + // more ranges in one or the other, no overlap + assert!(!overlaps(&[(6, 3), (9, 6)], &[(3, 0)])); + assert!(!overlaps(&[(6, 3), (6, 9)], &[(3, 0)])); + assert!(!overlaps(&[(3, 6), (9, 6)], &[(3, 0)])); + assert!(!overlaps(&[(3, 6), (6, 9)], &[(3, 0)])); + assert!(!overlaps(&[(6, 3), (9, 6)], &[(0, 3)])); + assert!(!overlaps(&[(6, 3), (6, 9)], &[(0, 3)])); + assert!(!overlaps(&[(3, 6), (9, 6)], &[(0, 3)])); + assert!(!overlaps(&[(3, 6), (6, 9)], &[(0, 3)])); + + assert!(!overlaps(&[(6, 3)], &[(3, 0), (9, 6)])); + assert!(!overlaps(&[(6, 3)], &[(3, 0), (6, 9)])); + assert!(!overlaps(&[(3, 6)], &[(3, 0), (9, 6)])); + assert!(!overlaps(&[(3, 6)], &[(3, 0), (6, 9)])); + assert!(!overlaps(&[(6, 3)], &[(0, 3), (9, 6)])); + assert!(!overlaps(&[(6, 3)], &[(0, 3), (6, 9)])); + assert!(!overlaps(&[(3, 6)], &[(0, 3), (9, 6)])); + assert!(!overlaps(&[(3, 6)], &[(0, 3), (6, 9)])); + + // Two non-zero-width ranges, overlap. + assert!(overlaps(&[(0, 4)], &[(3, 6)])); + assert!(overlaps(&[(0, 4)], &[(6, 3)])); + assert!(overlaps(&[(4, 0)], &[(3, 6)])); + assert!(overlaps(&[(4, 0)], &[(6, 3)])); + assert!(overlaps(&[(3, 6)], &[(0, 4)])); + assert!(overlaps(&[(3, 6)], &[(4, 0)])); + assert!(overlaps(&[(6, 3)], &[(0, 4)])); + assert!(overlaps(&[(6, 3)], &[(4, 0)])); + + // Two non-zero-width ranges, overlap, extra from one or the other + assert!(overlaps(&[(0, 4), (7, 10)], &[(3, 6)])); + assert!(overlaps(&[(0, 4), (7, 10)], &[(6, 3)])); + assert!(overlaps(&[(4, 0), (7, 10)], &[(3, 6)])); + assert!(overlaps(&[(4, 0), (7, 10)], &[(6, 3)])); + assert!(overlaps(&[(3, 6), (7, 10)], &[(0, 4)])); + assert!(overlaps(&[(3, 6), (7, 10)], &[(4, 0)])); + assert!(overlaps(&[(6, 3), (7, 10)], &[(0, 4)])); + assert!(overlaps(&[(6, 3), (7, 10)], &[(4, 0)])); + assert!(overlaps(&[(0, 4), (10, 7)], &[(3, 6)])); + assert!(overlaps(&[(0, 4), (10, 7)], &[(6, 3)])); + assert!(overlaps(&[(4, 0), (10, 7)], &[(3, 6)])); + assert!(overlaps(&[(4, 0), (10, 7)], &[(6, 3)])); + assert!(overlaps(&[(3, 6), (10, 7)], &[(0, 4)])); + assert!(overlaps(&[(3, 6), (10, 7)], &[(4, 0)])); + assert!(overlaps(&[(6, 3), (10, 7)], &[(0, 4)])); + assert!(overlaps(&[(6, 3), (10, 7)], &[(4, 0)])); + + assert!(overlaps(&[(0, 4)], &[(3, 6), (7, 10)])); + assert!(overlaps(&[(0, 4)], &[(6, 3), (7, 10)])); + assert!(overlaps(&[(4, 0)], &[(3, 6), (7, 10)])); + assert!(overlaps(&[(4, 0)], &[(6, 3), (7, 10)])); + assert!(overlaps(&[(3, 6)], &[(0, 4), (7, 10)])); + assert!(overlaps(&[(3, 6)], &[(4, 0), (7, 10)])); + assert!(overlaps(&[(6, 3)], &[(0, 4), (7, 10)])); + assert!(overlaps(&[(6, 3)], &[(4, 0), (7, 10)])); + assert!(overlaps(&[(0, 4)], &[(3, 6), (10, 7)])); + assert!(overlaps(&[(0, 4)], &[(6, 3), (10, 7)])); + assert!(overlaps(&[(4, 0)], &[(3, 6), (10, 7)])); + assert!(overlaps(&[(4, 0)], &[(6, 3), (10, 7)])); + assert!(overlaps(&[(3, 6)], &[(0, 4), (10, 7)])); + assert!(overlaps(&[(3, 6)], &[(4, 0), (10, 7)])); + assert!(overlaps(&[(6, 3)], &[(0, 4), (10, 7)])); + assert!(overlaps(&[(6, 3)], &[(4, 0), (10, 7)])); + + // Zero-width and non-zero-width range, no overlap. + assert!(!overlaps(&[(0, 3)], &[(3, 3)])); + assert!(!overlaps(&[(3, 0)], &[(3, 3)])); + assert!(!overlaps(&[(3, 3)], &[(0, 3)])); + assert!(!overlaps(&[(3, 3)], &[(3, 0)])); + + assert!(!overlaps(&[(0, 3), (7, 10)], &[(3, 3)])); + assert!(!overlaps(&[(3, 0), (7, 10)], &[(3, 3)])); + assert!(!overlaps(&[(3, 3), (7, 10)], &[(0, 3)])); + assert!(!overlaps(&[(3, 3), (7, 10)], &[(3, 0)])); + + assert!(!overlaps(&[(0, 3)], &[(3, 3), (7, 10)])); + assert!(!overlaps(&[(3, 0)], &[(3, 3), (7, 10)])); + assert!(!overlaps(&[(3, 3)], &[(0, 3), (7, 10)])); + assert!(!overlaps(&[(3, 3)], &[(3, 0), (7, 10)])); + + // Zero-width and non-zero-width range, overlap. + assert!(overlaps(&[(1, 4)], &[(1, 1)])); + assert!(overlaps(&[(4, 1)], &[(1, 1)])); + assert!(overlaps(&[(1, 1)], &[(1, 4)])); + assert!(overlaps(&[(1, 1)], &[(4, 1)])); + + assert!(overlaps(&[(1, 4)], &[(3, 3)])); + assert!(overlaps(&[(4, 1)], &[(3, 3)])); + assert!(overlaps(&[(3, 3)], &[(1, 4)])); + assert!(overlaps(&[(3, 3)], &[(4, 1)])); + + assert!(overlaps(&[(1, 4), (7, 10)], &[(1, 1)])); + assert!(overlaps(&[(4, 1), (7, 10)], &[(1, 1)])); + assert!(overlaps(&[(1, 1), (7, 10)], &[(1, 4)])); + assert!(overlaps(&[(1, 1), (7, 10)], &[(4, 1)])); + + assert!(overlaps(&[(1, 4), (7, 10)], &[(3, 3)])); + assert!(overlaps(&[(4, 1), (7, 10)], &[(3, 3)])); + assert!(overlaps(&[(3, 3), (7, 10)], &[(1, 4)])); + assert!(overlaps(&[(3, 3), (7, 10)], &[(4, 1)])); + + assert!(overlaps(&[(1, 4)], &[(1, 1), (7, 10)])); + assert!(overlaps(&[(4, 1)], &[(1, 1), (7, 10)])); + assert!(overlaps(&[(1, 1)], &[(1, 4), (7, 10)])); + assert!(overlaps(&[(1, 1)], &[(4, 1), (7, 10)])); + + assert!(overlaps(&[(1, 4)], &[(3, 3), (7, 10)])); + assert!(overlaps(&[(4, 1)], &[(3, 3), (7, 10)])); + assert!(overlaps(&[(3, 3)], &[(1, 4), (7, 10)])); + assert!(overlaps(&[(3, 3)], &[(4, 1), (7, 10)])); + + // Two zero-width ranges, no overlap. + assert!(!overlaps(&[(0, 0)], &[(1, 1)])); + assert!(!overlaps(&[(1, 1)], &[(0, 0)])); + + assert!(!overlaps(&[(0, 0), (2, 2)], &[(1, 1)])); + assert!(!overlaps(&[(0, 0), (2, 2)], &[(1, 1)])); + assert!(!overlaps(&[(1, 1)], &[(0, 0), (2, 2)])); + assert!(!overlaps(&[(1, 1)], &[(0, 0), (2, 2)])); + + // Two zero-width ranges, overlap. + assert!(overlaps(&[(1, 1)], &[(1, 1)])); + assert!(overlaps(&[(1, 1), (2, 2)], &[(1, 1)])); + assert!(overlaps(&[(1, 1)], &[(1, 1), (2, 2)])); + } + #[test] fn test_grapheme_aligned() { let r = Rope::from_str("\r\nHi\r\n"); @@ -1373,9 +1667,15 @@ mod test { // multiple matches assert!(contains(vec!((1, 1), (2, 2)), vec!((1, 1), (2, 2)))); - // smaller set can't contain bigger + // extra items out of range assert!(!contains(vec!((1, 1)), vec!((1, 1), (2, 2)))); + // one big range can contain multiple smaller ranges + assert!(contains( + vec!((1, 10)), + vec!((1, 1), (2, 2), (3, 3), (3, 5), (7, 10)) + )); + assert!(contains( vec!((1, 1), (2, 4), (5, 6), (7, 9), (10, 13)), vec!((3, 4), (7, 9)) @@ -1388,4 +1688,143 @@ mod test { vec!((1, 2), (3, 4), (7, 9)) )); } + + #[test] + fn test_selection_without() { + let without = |one, two, expected: Option<_>| { + println!("one: {:?}", one); + println!("two: {:?}", two); + println!("expected: {:?}", expected); + + let (one_text, one_sel) = test::print(one); + let (two_text, two_sel) = test::print(two); + assert_eq!(one_text, two_text); // sanity + let actual_sel = one_sel.without(&two_sel); + + let expected_sel = expected.map(|exp| { + let (expected_text, expected_sel) = test::print(exp); + assert_eq!(two_text, expected_text); + expected_sel + }); + let actual = actual_sel + .as_ref() + .map(|sel| test::plain(two_text.to_string(), sel)); + + println!("actual: {:?}\n\n", actual); + + assert_eq!( + expected_sel, + actual_sel, + "expected: {:?}, got: {:?}", + expected_sel + .as_ref() + .map(|sel| test::plain(two_text.to_string(), sel)), + actual, + ); + }; + + without( + "#[foo bar baz|]#", + "foo #[bar|]# baz", + Some("#[foo |]#bar#( baz|)#"), + ); + + without("#[foo bar baz|]#", "#[foo bar baz|]#", None); + without("#[foo bar|]# baz", "#[foo bar baz|]#", None); + without("foo #[bar|]# baz", "#[foo bar baz|]#", None); + + // direction is preserved + without( + "#[|foo bar baz]#", + "foo #[|bar]# baz", + Some("#[|foo ]#bar#(| baz)#"), + ); + + // preference for direction is given to the left + without( + "#[|foo bar baz]#", + "foo #[bar|]# baz", + Some("#[|foo ]#bar#(| baz)#"), + ); + + // disjoint ranges on the right are ignored + without( + "#[foo bar|]# baz", + "foo bar #[baz|]#", + Some("#[foo bar|]# baz"), + ); + without( + "#[foo bar|]# baz", + "foo bar#[ baz|]#", + Some("#[foo bar|]# baz"), + ); + without( + "#(foo|)# #[bar|]# baz", + "foo#[ b|]#ar ba#(z|)#", + Some("#(foo|)# b#[ar|]# baz"), + ); + + // ranges contained by those one on the right are removed + without( + "#[foo bar|]# #(b|)#az", + "foo bar#[ b|]#a#(z|)#", + Some("#[foo bar|]# baz"), + ); + without( + "#[foo|]# bar #(baz|)#", + "foo bar#[ b|]#a#(z|)#", + Some("#[foo|]# bar b#(a|)#z"), + ); + without( + "#[foo bar|]# #(b|)#az", + "foo bar #[b|]#a#(z|)#", + Some("#[foo bar|]# baz"), + ); + without( + "#[foo bar|]# #(b|)#a#(z|)#", + "foo bar #[b|]#a#(z|)#", + Some("#[foo bar|]# baz"), + ); + without( + "#[foo bar|]# #(b|)#a#(z|)#", + "foo bar #[b|]#a#(z|)#", + Some("#[foo bar|]# baz"), + ); + + // more than one range intersected by a single range on the right + without( + "#[foo bar|]# #(baz|)#", + "foo b#[ar ba|]#z", + Some("#[foo b|]#ar ba#(z|)#"), + ); + + // partial overlap + without( + "#[foo bar|]# baz", + "foo #[bar baz|]#", + Some("#[foo |]#bar baz"), + ); + without( + "#[foo bar|]# baz", + "foo#[ bar baz|]#", + Some("#[foo|]# bar baz"), + ); + without( + "#[foo bar|]# baz", + "foo ba#[r baz|]#", + Some("#[foo ba|]#r baz"), + ); + without( + "foo ba#[r baz|]#", + "#[foo bar|]# baz", + Some("foo bar#[ baz|]#"), + ); + + // primary selection is moved - preference given to the left of a split + without( + "#(|foo)# #(|bar baz)# #[|quux]#", + "f#(o|)#o ba#[r b|]#az q#(uu|)#x", + Some("#(|f)#o#(|o)# #(|ba)#r b#(|az)# #[|q]#uu#(|x)#"), + ); + } } From 9e473974780dee81a753c758c6f08e4ba091e8b4 Mon Sep 17 00:00:00 2001 From: Skyler Hawthorne Date: Sun, 9 Oct 2022 18:04:09 -0400 Subject: [PATCH 5/6] Generalize View::object_selections into map This allows using multiple distinct state histories. By default, all history is also cleared any time a view's selection is set, unless explicitly told to save the state. This way, we can have control over when selection history is saved. They are also cleared on any text edit, since an edit could invalidate the previous selection, potentially causing a panic. Additionally, the object selections have been moved into `Document` so that it is easier to manipulate them when changes to the document happen. They have been put into a wrapper struct named `ViewData`, where the intention is that any further fields that we want to add in the future that must be associated with a view, but are more convenient to store in a document, can be added here, instead of further polluting the core `Document` type. --- helix-term/src/commands.rs | 49 ++++++++++++++++++++++++-------------- helix-view/src/document.rs | 35 +++++++++++++++++++++++---- helix-view/src/view.rs | 3 --- 3 files changed, 61 insertions(+), 26 deletions(-) diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs index a93fa445ee80..3e75a8734ea2 100644 --- a/helix-term/src/commands.rs +++ b/helix-term/src/commands.rs @@ -3714,6 +3714,7 @@ fn goto_first_diag(cx: &mut Context) { Some(diag) => Selection::single(diag.range.start, diag.range.end), None => return, }; + doc.set_selection(view.id, selection); view.diagnostics_handler .immediately_show_diagnostic(doc, view.id); @@ -3725,6 +3726,7 @@ fn goto_last_diag(cx: &mut Context) { Some(diag) => Selection::single(diag.range.start, diag.range.end), None => return, }; + doc.set_selection(view.id, selection); view.diagnostics_handler .immediately_show_diagnostic(doc, view.id); @@ -3783,6 +3785,7 @@ fn goto_prev_diag(cx: &mut Context) { view.diagnostics_handler .immediately_show_diagnostic(doc, view.id); }; + cx.editor.apply_motion(motion) } @@ -5123,6 +5126,8 @@ fn reverse_selection_contents(cx: &mut Context) { // tree sitter node selection +const EXPAND_KEY: &str = "expand"; + fn expand_selection(cx: &mut Context) { let motion = |editor: &mut Editor| { let (view, doc) = current!(editor); @@ -5130,42 +5135,52 @@ fn expand_selection(cx: &mut Context) { if let Some(syntax) = doc.syntax() { let text = doc.text().slice(..); - let current_selection = doc.selection(view.id); + let current_selection = doc.selection(view.id).clone(); let selection = object::expand_selection(syntax, text, current_selection.clone()); // check if selection is different from the last one - if *current_selection != selection { - // save current selection so it can be restored using shrink_selection - view.object_selections.push(current_selection.clone()); + if current_selection != selection { + let prev_selections = doc + .view_data_mut(view.id) + .object_selections + .entry(EXPAND_KEY) + .or_default(); - doc.set_selection(view.id, selection); + // save current selection so it can be restored using shrink_selection + prev_selections.push(current_selection); + doc.set_selection_clear(view.id, selection, false); } } }; + cx.editor.apply_motion(motion); } fn shrink_selection(cx: &mut Context) { let motion = |editor: &mut Editor| { let (view, doc) = current!(editor); - let current_selection = doc.selection(view.id); + let current_selection = doc.selection(view.id).clone(); + let prev_expansions = doc + .view_data_mut(view.id) + .object_selections + .entry(EXPAND_KEY) + .or_default(); + // try to restore previous selection - if let Some(prev_selection) = view.object_selections.pop() { - if current_selection.contains(&prev_selection) { - doc.set_selection(view.id, prev_selection); - return; - } else { - // clear existing selection as they can't be shrunk to anyway - view.object_selections.clear(); - } + if let Some(prev_selection) = prev_expansions.pop() { + // allow shrinking the selection only if current selection contains the previous object selection + doc.set_selection_clear(view.id, prev_selection, false); + return; } + // if not previous selection, shrink to first child if let Some(syntax) = doc.syntax() { let text = doc.text().slice(..); - let selection = object::shrink_selection(syntax, text, current_selection.clone()); - doc.set_selection(view.id, selection); + let selection = object::shrink_selection(syntax, text, current_selection); + doc.set_selection_clear(view.id, selection, false); } }; + cx.editor.apply_motion(motion); } @@ -5284,8 +5299,6 @@ fn match_brackets(cx: &mut Context) { doc.set_selection(view.id, selection); } -// - fn jump_forward(cx: &mut Context) { let count = cx.count(); let config = cx.editor.config(); diff --git a/helix-view/src/document.rs b/helix-view/src/document.rs index 29fd736ab1f5..13b9b5c212ea 100644 --- a/helix-view/src/document.rs +++ b/helix-view/src/document.rs @@ -1231,15 +1231,27 @@ impl Document { Ok(()) } - /// Select text within the [`Document`]. - pub fn set_selection(&mut self, view_id: ViewId, selection: Selection) { + /// Select text within the [`Document`], optionally clearing the previous selection state. + pub fn set_selection_clear(&mut self, view_id: ViewId, selection: Selection, clear_prev: bool) { // TODO: use a transaction? self.selections .insert(view_id, selection.ensure_invariants(self.text().slice(..))); + helix_event::dispatch(SelectionDidChange { doc: self, view: view_id, - }) + }); + + if clear_prev { + self.view_data + .entry(view_id) + .and_modify(|view_data| view_data.object_selections.clear()); + } + } + + /// Select text within the [`Document`]. + pub fn set_selection(&mut self, view_id: ViewId, selection: Selection) { + self.set_selection_clear(view_id, selection, true); } /// Find the origin selection of the text in a document, i.e. where @@ -1426,6 +1438,12 @@ impl Document { apply_inlay_hint_changes(padding_after_inlay_hints); } + // clear out all associated view object selections, as they are no + // longer valid + self.view_data + .values_mut() + .for_each(|view_data| view_data.object_selections.clear()); + helix_event::dispatch(DocumentDidChange { doc: self, view: view_id, @@ -1860,13 +1878,13 @@ impl Document { &self.selections } - fn view_data(&self, view_id: ViewId) -> &ViewData { + pub fn view_data(&self, view_id: ViewId) -> &ViewData { self.view_data .get(&view_id) .expect("This should only be called after ensure_view_init") } - fn view_data_mut(&mut self, view_id: ViewId) -> &mut ViewData { + pub fn view_data_mut(&mut self, view_id: ViewId) -> &mut ViewData { self.view_data.entry(view_id).or_default() } @@ -2173,9 +2191,13 @@ impl Document { } } +/// Stores data needed for views that are tied to this specific Document. #[derive(Debug, Default)] pub struct ViewData { view_position: ViewPosition, + + /// used to store previous selections of tree-sitter objects + pub object_selections: HashMap<&'static str, Vec>, } #[derive(Clone, Debug)] @@ -2227,6 +2249,7 @@ mod test { Arc::new(ArcSwap::new(Arc::new(Config::default()))), ); let view = ViewId::default(); + doc.ensure_view_init(view); doc.set_selection(view, Selection::single(0, 0)); let transaction = @@ -2264,7 +2287,9 @@ mod test { None, Arc::new(ArcSwap::new(Arc::new(Config::default()))), ); + let view = ViewId::default(); + doc.ensure_view_init(view); doc.set_selection(view, Selection::single(5, 5)); // insert diff --git a/helix-view/src/view.rs b/helix-view/src/view.rs index a229f01ea66a..fbd766607dbd 100644 --- a/helix-view/src/view.rs +++ b/helix-view/src/view.rs @@ -138,8 +138,6 @@ pub struct View { // uses two docs because we want to be able to swap between the // two last modified docs which we need to manually keep track of pub last_modified_docs: [Option; 2], - /// used to store previous selections of tree-sitter objects - pub object_selections: Vec, /// all gutter-related configuration settings, used primarily for gutter rendering pub gutters: GutterConfig, /// A mapping between documents and the last history revision the view was updated at. @@ -176,7 +174,6 @@ impl View { jumps: JumpList::new((doc, Selection::point(0))), // TODO: use actual sel docs_access_history: Vec::new(), last_modified_docs: [None, None], - object_selections: Vec::new(), gutters, doc_revisions: HashMap::new(), diagnostics_handler: DiagnosticsHandler::new(), From c76d4a43791abd3c07c8244d189c88fb3b3a388f Mon Sep 17 00:00:00 2001 From: Skyler Hawthorne Date: Sun, 9 Oct 2022 18:04:09 -0400 Subject: [PATCH 6/6] feat(command): expand_selection_around Introduces a new command `expand_selection_around` that expands the selection to the parent node, like `expand_selection`, except it splits on the selection you start with and continues expansion around this initial selection. --- helix-term/src/commands.rs | 106 +++++++++++++++++++++ helix-term/src/keymap/default.rs | 1 + helix-term/tests/test/commands/movement.rs | 66 +++++++++++++ 3 files changed, 173 insertions(+) diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs index 3e75a8734ea2..0034f760a30c 100644 --- a/helix-term/src/commands.rs +++ b/helix-term/src/commands.rs @@ -507,6 +507,7 @@ impl MappableCommand { select_prev_sibling, "Select previous sibling the in syntax tree", select_all_siblings, "Select all siblings of the current node", select_all_children, "Select all children of the current node", + expand_selection_around, "Expand selection to parent syntax node, but exclude the selection you started with", jump_forward, "Jump forward on jumplist", jump_backward, "Jump backward on jumplist", save_selection, "Save current selection to jumplist", @@ -5127,6 +5128,8 @@ fn reverse_selection_contents(cx: &mut Context) { // tree sitter node selection const EXPAND_KEY: &str = "expand"; +const EXPAND_AROUND_BASE_KEY: &str = "expand_around_base"; +const PARENTS_KEY: &str = "parents"; fn expand_selection(cx: &mut Context) { let motion = |editor: &mut Editor| { @@ -5170,6 +5173,33 @@ fn shrink_selection(cx: &mut Context) { if let Some(prev_selection) = prev_expansions.pop() { // allow shrinking the selection only if current selection contains the previous object selection doc.set_selection_clear(view.id, prev_selection, false); + + // Do a corresponding pop of the parents from `expand_selection_around` + doc.view_data_mut(view.id) + .object_selections + .entry(PARENTS_KEY) + .and_modify(|parents| { + parents.pop(); + }); + + // need to do this again because borrowing + let prev_expansions = doc + .view_data_mut(view.id) + .object_selections + .entry(EXPAND_KEY) + .or_default(); + + // if we've emptied out the previous expansions, then clear out the + // base history as well so it doesn't get used again erroneously + if prev_expansions.is_empty() { + doc.view_data_mut(view.id) + .object_selections + .entry(EXPAND_AROUND_BASE_KEY) + .and_modify(|base| { + base.clear(); + }); + } + return; } @@ -5184,6 +5214,81 @@ fn shrink_selection(cx: &mut Context) { cx.editor.apply_motion(motion); } +fn expand_selection_around(cx: &mut Context) { + let motion = |editor: &mut Editor| { + let (view, doc) = current!(editor); + + if doc.syntax().is_some() { + // [NOTE] we do this pop and push dance because if we don't take + // ownership of the objects, then we require multiple + // mutable references to the view's object selections + let mut parents_selection = doc + .view_data_mut(view.id) + .object_selections + .entry(PARENTS_KEY) + .or_default() + .pop(); + + let mut base_selection = doc + .view_data_mut(view.id) + .object_selections + .entry(EXPAND_AROUND_BASE_KEY) + .or_default() + .pop(); + + let current_selection = doc.selection(view.id).clone(); + + if parents_selection.is_none() || base_selection.is_none() { + parents_selection = Some(current_selection.clone()); + base_selection = Some(current_selection.clone()); + } + + let text = doc.text().slice(..); + let syntax = doc.syntax().unwrap(); + + let outside_selection = + object::expand_selection(syntax, text, parents_selection.clone().unwrap()); + + let target_selection = match outside_selection + .clone() + .without(&base_selection.clone().unwrap()) + { + Some(sel) => sel, + None => outside_selection.clone(), + }; + + // check if selection is different from the last one + if target_selection != current_selection { + // save current selection so it can be restored using shrink_selection + doc.view_data_mut(view.id) + .object_selections + .entry(EXPAND_KEY) + .or_default() + .push(current_selection); + + doc.set_selection_clear(view.id, target_selection, false); + } + + let parents = doc + .view_data_mut(view.id) + .object_selections + .entry(PARENTS_KEY) + .or_default(); + + parents.push(parents_selection.unwrap()); + parents.push(outside_selection); + + doc.view_data_mut(view.id) + .object_selections + .entry(EXPAND_AROUND_BASE_KEY) + .or_default() + .push(base_selection.unwrap()); + } + }; + + cx.editor.apply_motion(motion); +} + fn select_sibling_impl(cx: &mut Context, sibling_fn: F) where F: Fn(&helix_core::Syntax, RopeSlice, Selection) -> Selection + 'static, @@ -5198,6 +5303,7 @@ where doc.set_selection(view.id, selection); } }; + cx.editor.apply_motion(motion); } diff --git a/helix-term/src/keymap/default.rs b/helix-term/src/keymap/default.rs index c6cefd927574..96d8853a13bf 100644 --- a/helix-term/src/keymap/default.rs +++ b/helix-term/src/keymap/default.rs @@ -86,6 +86,7 @@ pub fn default() -> HashMap { ";" => collapse_selection, "A-;" => flip_selections, "A-o" | "A-up" => expand_selection, + "A-O" => expand_selection_around, "A-i" | "A-down" => shrink_selection, "A-I" | "A-S-down" => select_all_children, "A-p" | "A-left" => select_prev_sibling, diff --git a/helix-term/tests/test/commands/movement.rs b/helix-term/tests/test/commands/movement.rs index dd5a3beef576..684f1dcaed73 100644 --- a/helix-term/tests/test/commands/movement.rs +++ b/helix-term/tests/test/commands/movement.rs @@ -1067,6 +1067,72 @@ async fn expand_shrink_selection() -> anyhow::Result<()> { #[|Some(thing)]#, Some(other_thing), ) + + "##}, + ), + ]; + + for test in tests { + test_with_config(AppBuilder::new().with_file("foo.rs", None), test).await?; + } + + Ok(()) +} + +#[tokio::test(flavor = "multi_thread")] +async fn expand_selection_around() -> anyhow::Result<()> { + let tests = vec![ + // single cursor stays single cursor, first goes to end of current + // node, then parent + ( + indoc! {r##" + Some(#[thing|]#) + "##}, + "", + indoc! {r##" + #[Some(|]#thing#()|)# + "##}, + ), + // shrinking restores previous selection + ( + indoc! {r##" + Some(#[thing|]#) + "##}, + "", + indoc! {r##" + Some(#[thing|]#) + "##}, + ), + // multi range collision merges expand as normal, except with the + // original selection removed from the result + ( + indoc! {r##" + ( + Some(#[thing|]#), + Some(#(other_thing|)#), + ) + "##}, + "", + indoc! {r##" + #[( + Some(|]#thing#(), + Some(|)#other_thing#(), + )|)# + "##}, + ), + ( + indoc! {r##" + ( + Some(#[thing|]#), + Some(#(other_thing|)#), + ) + "##}, + "", + indoc! {r##" + ( + Some(#[thing|]#), + Some(#(other_thing|)#), + ) "##}, ), ];