Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add select_next_sibling and select_prev_sibling commands #1495

Merged
merged 9 commits into from
Jan 20, 2022
Merged

add select_next_sibling and select_prev_sibling commands #1495

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
855a0de
add select_next_sibling and select_prev_sibling commands
the-mikedavis Jan 12, 2022
7f56104
refactor objects to use higher order functions
the-mikedavis Jan 13, 2022
807b64f
address clippy feedback
the-mikedavis Jan 13, 2022
0c74d4e
move selection cloning into commands
the-mikedavis Jan 14, 2022
d1df47f
add default keybindings under left/right brackets
the-mikedavis Jan 14, 2022
5549acd
use [+t,]+t for selecting sibling syntax nodes
the-mikedavis Jan 14, 2022
198600c
setup Alt-{j,k,h,l} default keymaps for syntax selection commands
the-mikedavis Jan 16, 2022
b337a5b
reduce boilerplate of select_next/prev_sibling in commands
the-mikedavis Jan 16, 2022
1ce524b
import tree-sitter Node type in commands
the-mikedavis Jan 16, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 65 additions & 0 deletions helix-core/src/object.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use crate::{Range, RopeSlice, Selection, Syntax};
use tree_sitter::Node;

pub fn expand_selection(syntax: &Syntax, text: RopeSlice, selection: &Selection) -> Selection {
let tree = syntax.tree();
Expand Down Expand Up @@ -59,3 +60,67 @@ pub fn shrink_selection(syntax: &Syntax, text: RopeSlice, selection: &Selection)
}
})
}

pub fn select_next_sibling(syntax: &Syntax, text: RopeSlice, selection: &Selection) -> Selection {
let tree = syntax.tree();

selection.clone().transform(|range| {
let from = text.char_to_byte(range.from());
let to = text.char_to_byte(range.to());

let sibling = match tree
.root_node()
.descendant_for_byte_range(from, to)
.and_then(find_next_sibling)
{
Some(sibling) => sibling,
None => return range,
};

let from = text.byte_to_char(sibling.start_byte());
let to = text.byte_to_char(sibling.end_byte());

if range.head < range.anchor {
Range::new(to, from)
} else {
Range::new(from, to)
}
})
}

fn find_next_sibling(node: Node) -> Option<Node> {
node.next_sibling()
.or_else(|| node.parent().and_then(find_next_sibling))
}

pub fn select_prev_sibling(syntax: &Syntax, text: RopeSlice, selection: &Selection) -> Selection {
let tree = syntax.tree();

selection.clone().transform(|range| {
let from = text.char_to_byte(range.from());
let to = text.char_to_byte(range.to());

let sibling = match tree
.root_node()
.descendant_for_byte_range(from, to)
.and_then(find_prev_sibling)
{
Some(sibling) => sibling,
None => return range,
};

let from = text.byte_to_char(sibling.start_byte());
let to = text.byte_to_char(sibling.end_byte());

if range.head < range.anchor {
Range::new(to, from)
} else {
Range::new(from, to)
}
})
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This whole thing seemed like it is duplicated, probably can reuse.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep it's all duplicated. I tried out refactoring it to take the find_prev_sibling/find_next_sibling function as an argument but I couldn't figure out the borrowing rules. I'll keep looking into it 🤔

(ofc if you'd like to push a commit to cut down on that boilerplate, I'd be grateful 🙂)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I figured out how to do higher order functions but now I think I may have overdone it 😄

}

fn find_prev_sibling(node: Node) -> Option<Node> {
node.prev_sibling()
.or_else(|| node.parent().and_then(find_prev_sibling))
}
36 changes: 36 additions & 0 deletions helix-term/src/commands.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -363,6 +363,8 @@ impl MappableCommand {
rotate_selection_contents_backward, "Rotate selections contents backward",
expand_selection, "Expand selection to parent syntax node",
shrink_selection, "Shrink selection to previously expanded syntax node",
select_next_sibling, "Select the next sibling in the syntax tree",
select_prev_sibling, "Select the previous sibling in the syntax tree",
jump_forward, "Jump forward on jumplist",
jump_backward, "Jump backward on jumplist",
save_selection, "Save the current selection to the jumplist",
Expand Down Expand Up @@ -5524,6 +5526,40 @@ fn shrink_selection(cx: &mut Context) {
cx.editor.last_motion = Some(Motion(Box::new(motion)));
}

fn select_next_sibling(cx: &mut Context) {
let motion = |editor: &mut Editor| {
let (view, doc) = current!(editor);

if let Some(syntax) = doc.syntax() {
let text = doc.text().slice(..);

let current_selection = doc.selection(view.id);

let selection = object::select_next_sibling(syntax, text, current_selection);
doc.set_selection(view.id, selection);
}
};
motion(cx.editor);
cx.editor.last_motion = Some(Motion(Box::new(motion)));
}

fn select_prev_sibling(cx: &mut Context) {
let motion = |editor: &mut Editor| {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried to factor these too so it's not duplicated everywhere except the sibling call, but I couldn't figure out how to make the borrow checker happy. I tried making a fn(&Syntax, RopeSlice, &Selection) argument, but it didn't like that the function argument gets captured by the motion closure.

Copy link
Member Author

@the-mikedavis the-mikedavis Jan 14, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a little afraid of the borrow checker 😅, I'll give this a shot but I'm not optimistic

edit: it didn't go well 🙃

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could do something like this:

helix/helix-term/src/commands.rs

Lines 488 to 535 in 5549acd

fn move_impl<F>(cx: &mut Context, move_fn: F, dir: Direction, behaviour: Movement)
where
F: Fn(RopeSlice, Range, Direction, usize, Movement) -> Range,
{
let count = cx.count();
let (view, doc) = current!(cx.editor);
let text = doc.text().slice(..);
let selection = doc
.selection(view.id)
.clone()
.transform(|range| move_fn(text, range, dir, count, behaviour));
doc.set_selection(view.id, selection);
}
use helix_core::movement::{move_horizontally, move_vertically};
fn move_char_left(cx: &mut Context) {
move_impl(cx, move_horizontally, Direction::Backward, Movement::Move)
}
fn move_char_right(cx: &mut Context) {
move_impl(cx, move_horizontally, Direction::Forward, Movement::Move)
}
fn move_line_up(cx: &mut Context) {
move_impl(cx, move_vertically, Direction::Backward, Movement::Move)
}
fn move_line_down(cx: &mut Context) {
move_impl(cx, move_vertically, Direction::Forward, Movement::Move)
}
fn extend_char_left(cx: &mut Context) {
move_impl(cx, move_horizontally, Direction::Backward, Movement::Extend)
}
fn extend_char_right(cx: &mut Context) {
move_impl(cx, move_horizontally, Direction::Forward, Movement::Extend)
}
fn extend_line_up(cx: &mut Context) {
move_impl(cx, move_vertically, Direction::Backward, Movement::Extend)
}
fn extend_line_down(cx: &mut Context) {
move_impl(cx, move_vertically, Direction::Forward, Movement::Extend)
}

let (view, doc) = current!(editor);

if let Some(syntax) = doc.syntax() {
let text = doc.text().slice(..);

let current_selection = doc.selection(view.id);

let selection = object::select_prev_sibling(syntax, text, current_selection);
doc.set_selection(view.id, selection);
}
};
motion(cx.editor);
cx.editor.last_motion = Some(Motion(Box::new(motion)));
}

fn match_brackets(cx: &mut Context) {
let (view, doc) = current!(cx.editor);

Expand Down