Skip to content

Commit

Permalink
improve match bracket matching (#10613)
Browse files Browse the repository at this point in the history
  • Loading branch information
@pascalkuthe
pascalkuthe authored May 7, 2024
1 parent 0432d9c commit 5b8b2f4
Show file tree
Hide file tree
Showing 2 changed files with 187 additions and 62 deletions.
123 changes: 61 additions & 62 deletions helix-core/src/match_brackets.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -75,60 +75,64 @@ fn find_pair(
) -> Option<usize> {
let pos = doc.char_to_byte(pos_);

let root = syntax.tree_for_byte_range(pos, pos + 1).root_node();
let mut node = root.descendant_for_byte_range(pos, pos + 1)?;
let root = syntax.tree_for_byte_range(pos, pos).root_node();
let mut node = root.descendant_for_byte_range(pos, pos)?;

loop {
if node.is_named() {
let (start_byte, end_byte) = surrounding_bytes(doc, &node)?;
let (start_char, end_char) = (doc.byte_to_char(start_byte), doc.byte_to_char(end_byte));

if is_valid_pair_on_pos(doc, start_char, end_char) {
if end_byte == pos {
return Some(start_char);
}

// We return the end char if the cursor is either on the start char
// or at some arbitrary position between start and end char.
if traverse_parents || start_byte == pos {
return Some(end_char);
if node.is_named() && node.child_count() >= 2 {
let open = node.child(0).unwrap();
let close = node.child(node.child_count() - 1).unwrap();

if let (Some((start_pos, open)), Some((end_pos, close))) =
(as_char(doc, &open), as_char(doc, &close))
{
if PAIRS.contains(&(open, close)) {
if start_pos == pos_ {
return Some(start_pos);
}

// We return the end char if the cursor is either on the start char
// or at some arbitrary position between start and end char.
if traverse_parents || end_pos == pos_ {
return Some(end_pos);
}
}
}
}
// this node itselt wasn't a pair but maybe its siblings are

// check if we are *on* the pair (special cased so we don't look
// at the current node twice and to jump to the start on that case)
if let Some(open) = as_close_pair(doc, &node) {
if let Some(pair_start) = find_pair_end(doc, node.prev_sibling(), open, Backward) {
if let Some((start_char, end_char)) = as_close_pair(doc, &node) {
if let Some(pair_start) =
find_pair_end(doc, node.prev_sibling(), start_char, end_char, Backward)
{
return Some(pair_start);
}
}

if !traverse_parents {
// check if we are *on* the opening pair (special cased here as
// an opptimization since we only care about bracket on the cursor
// here)
if let Some(close) = as_open_pair(doc, &node) {
if let Some(pair_end) = find_pair_end(doc, node.next_sibling(), close, Forward) {
return Some(pair_end);
}
}
if node.is_named() {
break;
if let Some((start_char, end_char)) = as_open_pair(doc, &node) {
if let Some(pair_end) =
find_pair_end(doc, node.next_sibling(), start_char, end_char, Forward)
{
return Some(pair_end);
}
}

for close in
iter::successors(node.next_sibling(), |node| node.next_sibling()).take(MATCH_LIMIT)
{
let Some(open) = as_close_pair(doc, &close) else {
continue;
};
if find_pair_end(doc, Some(node), open, Backward).is_some() {
return doc.try_byte_to_char(close.start_byte()).ok();
if traverse_parents {
for sibling in
iter::successors(node.next_sibling(), |node| node.next_sibling()).take(MATCH_LIMIT)
{
let Some((start_char, end_char)) = as_close_pair(doc, &sibling) else {
continue;
};
if find_pair_end(doc, sibling.prev_sibling(), start_char, end_char, Backward)
.is_some()
{
return doc.try_byte_to_char(sibling.start_byte()).ok();
}
}
} else if node.is_named() {
break;
}

let Some(parent) = node.parent() else {
break;
};
Expand Down Expand Up @@ -241,29 +245,13 @@ pub fn is_valid_pair(ch: char) -> bool {
PAIRS.iter().any(|(l, r)| *l == ch || *r == ch)
}

fn is_valid_pair_on_pos(doc: RopeSlice, start_char: usize, end_char: usize) -> bool {
PAIRS.contains(&(doc.char(start_char), doc.char(end_char)))
}

fn surrounding_bytes(doc: RopeSlice, node: &Node) -> Option<(usize, usize)> {
let len = doc.len_bytes();

let start_byte = node.start_byte();
let end_byte = node.end_byte().saturating_sub(1);

if start_byte >= len || end_byte >= len {
return None;
}

Some((start_byte, end_byte))
}

/// Tests if this node is a pair close char and returns the expected open char
fn as_close_pair(doc: RopeSlice, node: &Node) -> Option<char> {
/// and close char contained in this node
fn as_close_pair(doc: RopeSlice, node: &Node) -> Option<(char, char)> {
let close = as_char(doc, node)?.1;
PAIRS
.iter()
.find_map(|&(open, close_)| (close_ == close).then_some(open))
.find_map(|&(open, close_)| (close_ == close).then_some((close, open)))
}

/// Checks if `node` or its siblings (at most MATCH_LIMIT nodes) is the specified closing char
Expand All @@ -274,27 +262,38 @@ fn as_close_pair(doc: RopeSlice, node: &Node) -> Option<char> {
fn find_pair_end(
doc: RopeSlice,
node: Option<Node>,
start_char: char,
end_char: char,
direction: Direction,
) -> Option<usize> {
let advance = match direction {
Forward => Node::next_sibling,
Backward => Node::prev_sibling,
};
let mut depth = 0;
iter::successors(node, advance)
.take(MATCH_LIMIT)
.find_map(|node| {
let (pos, c) = as_char(doc, &node)?;
(end_char == c).then_some(pos)
if c == end_char {
if depth == 0 {
return Some(pos);
}
depth -= 1;
} else if c == start_char {
depth += 1;
}
None
})
}

/// Tests if this node is a pair close char and returns the expected open char
fn as_open_pair(doc: RopeSlice, node: &Node) -> Option<char> {
/// Tests if this node is a pair open char and returns the expected close char
/// and open char contained in this node
fn as_open_pair(doc: RopeSlice, node: &Node) -> Option<(char, char)> {
let open = as_char(doc, node)?.1;
PAIRS
.iter()
.find_map(|&(open_, close)| (open_ == open).then_some(close))
.find_map(|&(open_, close)| (open_ == open).then_some((open, close)))
}

/// If node is a single char return it (and its char position)
Expand Down
126 changes: 126 additions & 0 deletions helix-term/tests/test/commands/movement.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -806,3 +806,129 @@ async fn test_select_prev_sibling() -> anyhow::Result<()> {

Ok(())
}

#[tokio::test(flavor = "multi_thread")]
async fn match_bracket() -> anyhow::Result<()> {
let rust_tests = vec![
// fwd
(
indoc! {r##"
fn foo(x: usize) -> usize { #[x|]# + 1 }
"##},
"mm",
indoc! {r##"
fn foo(x: usize) -> usize { x + 1 #[}|]#
"##},
),
// backward
(
indoc! {r##"
fn foo(x: usize) -> usize { #[x|]# + 1 }
"##},
"mmmm",
indoc! {r##"
fn foo(x: usize) -> usize #[{|]# x + 1 }
"##},
),
// avoid false positive inside string literal
(
indoc! {r##"
fn foo() -> &'static str { "(hello#[ |]#world)" }
"##},
"mm",
indoc! {r##"
fn foo() -> &'static str { "(hello world)#["|]# }
"##},
),
// make sure matching on quotes works
(
indoc! {r##"
fn foo() -> &'static str { "(hello#[ |]#world)" }
"##},
"mm",
indoc! {r##"
fn foo() -> &'static str { "(hello world)#["|]# }
"##},
),
// .. on both ends
(
indoc! {r##"
fn foo() -> &'static str { "(hello#[ |]#world)" }
"##},
"mmmm",
indoc! {r##"
fn foo() -> &'static str { #["|]#(hello world)" }
"##},
),
// match on siblings nodes
(
indoc! {r##"
fn foo(bar: Option<usize>) -> usize {
match bar {
Some(b#[a|]#r) => bar,
None => 42,
}
}
"##},
"mmmm",
indoc! {r##"
fn foo(bar: Option<usize>) -> usize {
match bar {
Some#[(|]#bar) => bar,
None => 42,
}
}
"##},
),
// gracefully handle multiple sibling brackets (usally for errors/incomplete syntax trees)
// in the past we selected the first > instead of the second > here
(
indoc! {r##"
fn foo() {
foo::<b#[a|]#r<>>
}
"##},
"mm",
indoc! {r##"
fn foo() {
foo::<bar<>#[>|]#
}
"##},
),
];

let python_tests = vec![
// python quotes have a slightly more complex syntax tree
// that triggerd a bug in an old implementation so we test
// them here
(
indoc! {r##"
foo_python = "mm does not#[ |]#work on this string"
"##},
"mm",
indoc! {r##"
foo_python = "mm does not work on this string#["|]#
"##},
),
(
indoc! {r##"
foo_python = "mm does not#[ |]#work on this string"
"##},
"mmmm",
indoc! {r##"
foo_python = #["|]#mm does not work on this string"
"##},
),
];

for test in rust_tests {
println!("{test:?}");
test_with_config(AppBuilder::new().with_file("foo.rs", None), test).await?;
}
for test in python_tests {
println!("{test:?}");
test_with_config(AppBuilder::new().with_file("foo.py", None), test).await?;
}

Ok(())
}

0 comments on commit 5b8b2f4

Please sign in to comment.