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

@@ -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<F>(
-    syntax: &Syntax,
-    text: RopeSlice,
-    selection: Selection,
-    motion: F,
-    direction: Option<Direction>,
-) -> 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()))
-    })
-}

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<Node<'n>> {
+        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 {
+    pub fn children(&'n mut self) -> ChildIter {
         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 {
+    pub fn named_children(&'n mut self) -> ChildIter {
         let parent = self.node();
 
         ChildIter {

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),
+                )|]#
+            "##},
+            "<A-i>",
             indoc! {r##"
                 (
                     #[Some(thing)|]#,
                     Some(other_thing),
                 )
             "##},
-            "<A-i>",
+        ),
+        // any movement cancels selection history and falls back to first child
+        (
+            indoc! {r##"
+                (
+                    Some(#[thing|]#),
+                    Some(#(other_thing|)#),
+                )
+
+            "##},
+            "<A-o><A-o><A-o>jkvkkk<A-i>",
             indoc! {r##"
                 (
-                    #[Some|]#(thing),
+                    #[|Some(thing)]#,
                     Some(other_thing),
                 )
             "##},