From 89a9f2be788017615f6d2406d4e12f50949a5492 Mon Sep 17 00:00:00 2001
From: Krishan <54745129+krish-r@users.noreply.github.com>
Date: Tue, 23 Apr 2024 19:11:03 +0530
Subject: [PATCH] specify direction for select_prev_sibling and
 select_next_sibling (#10542)

* specify direction for select_prev_sibling and select_next_sibling

* fix failing integration-test
---
 helix-core/src/object.rs                   | 53 +++++++++-----
 helix-term/tests/test/commands/movement.rs | 80 ++++++++++++++++++++++
 helix-term/tests/test/movement.rs          |  2 +-
 3 files changed, 117 insertions(+), 18 deletions(-)

diff --git a/helix-core/src/object.rs b/helix-core/src/object.rs
index 28629235f..17a393caf 100644
--- a/helix-core/src/object.rs
+++ b/helix-core/src/object.rs
@@ -1,4 +1,4 @@
-use crate::{syntax::TreeCursor, Range, RopeSlice, Selection, Syntax};
+use crate::{movement::Direction, syntax::TreeCursor, Range, RopeSlice, Selection, Syntax};
 
 pub fn expand_selection(syntax: &Syntax, text: RopeSlice, selection: Selection) -> Selection {
     let cursor = &mut syntax.walk();
@@ -25,19 +25,31 @@ 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();
-    })
+    select_node_impl(
+        syntax,
+        text,
+        selection,
+        |cursor| {
+            cursor.goto_first_child();
+        },
+        None,
+    )
 }
 
 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;
+    select_node_impl(
+        syntax,
+        text,
+        selection,
+        |cursor| {
+            while !cursor.goto_next_sibling() {
+                if !cursor.goto_parent() {
+                    break;
+                }
             }
-        }
-    })
+        },
+        Some(Direction::Forward),
+    )
 }
 
 pub fn select_all_siblings(syntax: &Syntax, text: RopeSlice, selection: Selection) -> Selection {
@@ -81,13 +93,19 @@ fn select_children<'n>(
 }
 
 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;
+    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>(
@@ -95,6 +113,7 @@ fn select_node_impl<F>(
     text: RopeSlice,
     selection: Selection,
     motion: F,
+    direction: Option<Direction>,
 ) -> Selection
 where
     F: Fn(&mut TreeCursor),
@@ -113,6 +132,6 @@ where
         let from = text.byte_to_char(node.start_byte());
         let to = text.byte_to_char(node.end_byte());
 
-        Range::new(from, to).with_direction(range.direction())
+        Range::new(from, to).with_direction(direction.unwrap_or_else(|| range.direction()))
     })
 }
diff --git a/helix-term/tests/test/commands/movement.rs b/helix-term/tests/test/commands/movement.rs
index 1f33b3944..b2471f635 100644
--- a/helix-term/tests/test/commands/movement.rs
+++ b/helix-term/tests/test/commands/movement.rs
@@ -726,3 +726,83 @@ async fn select_all_children() -> anyhow::Result<()> {
 
     Ok(())
 }
+
+#[tokio::test(flavor = "multi_thread")]
+async fn test_select_next_sibling() -> anyhow::Result<()> {
+    let tests = vec![
+        // basic test
+        (
+            indoc! {r##"
+                fn inc(x: usize) -> usize { x + 1 #[}|]#
+                fn dec(x: usize) -> usize { x - 1 }
+                fn ident(x: usize) -> usize { x }
+            "##},
+            "<A-n>",
+            indoc! {r##"
+                fn inc(x: usize) -> usize { x + 1 }
+                #[fn dec(x: usize) -> usize { x - 1 }|]#
+                fn ident(x: usize) -> usize { x }
+            "##},
+        ),
+        // direction is not preserved and is always forward.
+        (
+            indoc! {r##"
+                fn inc(x: usize) -> usize { x + 1 #[}|]#
+                fn dec(x: usize) -> usize { x - 1 }
+                fn ident(x: usize) -> usize { x }
+            "##},
+            "<A-n><A-;><A-n>",
+            indoc! {r##"
+                fn inc(x: usize) -> usize { x + 1 }
+                fn dec(x: usize) -> usize { x - 1 }
+                #[fn ident(x: usize) -> usize { x }|]#
+            "##},
+        ),
+    ];
+
+    for test in tests {
+        test_with_config(AppBuilder::new().with_file("foo.rs", None), test).await?;
+    }
+
+    Ok(())
+}
+
+#[tokio::test(flavor = "multi_thread")]
+async fn test_select_prev_sibling() -> anyhow::Result<()> {
+    let tests = vec![
+        // basic test
+        (
+            indoc! {r##"
+                fn inc(x: usize) -> usize { x + 1 }
+                fn dec(x: usize) -> usize { x - 1 }
+                #[|f]#n ident(x: usize) -> usize { x }
+            "##},
+            "<A-p>",
+            indoc! {r##"
+                fn inc(x: usize) -> usize { x + 1 }
+                #[|fn dec(x: usize) -> usize { x - 1 }]#
+                fn ident(x: usize) -> usize { x }
+            "##},
+        ),
+        // direction is not preserved and is always backward.
+        (
+            indoc! {r##"
+                fn inc(x: usize) -> usize { x + 1 }
+                fn dec(x: usize) -> usize { x - 1 }
+                #[|f]#n ident(x: usize) -> usize { x }
+            "##},
+            "<A-p><A-;><A-p>",
+            indoc! {r##"
+                #[|fn inc(x: usize) -> usize { x + 1 }]#
+                fn dec(x: usize) -> usize { x - 1 }
+                fn ident(x: usize) -> usize { x }
+            "##},
+        ),
+    ];
+
+    for test in tests {
+        test_with_config(AppBuilder::new().with_file("foo.rs", None), test).await?;
+    }
+
+    Ok(())
+}
diff --git a/helix-term/tests/test/movement.rs b/helix-term/tests/test/movement.rs
index 9ecaf6bb0..3fa850929 100644
--- a/helix-term/tests/test/movement.rs
+++ b/helix-term/tests/test/movement.rs
@@ -666,7 +666,7 @@ async fn tree_sitter_motions_work_across_injections() -> anyhow::Result<()> {
         (
             "<script>let #[|x]# = 1;</script>",
             "<A-n>",
-            "<script>let x #[|=]# 1;</script>",
+            "<script>let x #[=|]# 1;</script>",
         ),
     )
     .await?;