From 2bb25624f5f6f46cc7ea366d1865f7dccb190013 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexandre=20Vinyals=20Valdepe=C3=B1as?= Date: Sat, 15 Oct 2022 00:07:32 +0200 Subject: [PATCH 01/15] bracket_matching: add plaintext matching fallback to tree-sitter matching This patch introduces bracket matching independent of tree-sitter grammar. For the initial iteration of this feature, only match on the current line. This matching is introduced as a fallback in cases where the tree-sitter matcher does not match any bracket. This fallback should provide a better experience to users that are editing documents without tree-sitter grammar, but also provides a better experience in cases like the ones reported in #3614 If we find that this feature works well, we could consider extending it for multi-line matching, but I wanted to keep it small for the first iteration and gather thoughts beforehand. --- helix-core/src/match_brackets.rs | 96 ++++++++++++++++++++++++++++++++ helix-term/src/commands.rs | 9 ++- 2 files changed, 104 insertions(+), 1 deletion(-) diff --git a/helix-core/src/match_brackets.rs b/helix-core/src/match_brackets.rs index 0189deddbb21..30b0d01d3bf1 100644 --- a/helix-core/src/match_brackets.rs +++ b/helix-core/src/match_brackets.rs @@ -70,10 +70,63 @@ fn find_pair(syntax: &Syntax, doc: &Rope, pos: usize, traverse_parents: bool) -> } } +// Returns the position of the bracket that is closing, searching only in +// the current line, and works on plain text, ignoring the tree-sitter grammar. +// +// If the cursor is on an opening or closing bracket, the function +// behaves equivalent to [`find_matching_bracket`]. +// +// If no matchig bracket is found on the current line, returns None. +#[must_use] +pub fn find_matching_bracket_current_line_plaintext(doc: &Rope, pos: usize) -> Option { + // Don't do anything when the cursor is not on top of a bracket. + let mut c = doc.char(pos); + if !is_valid_bracket(c) { + return None; + } + + let bracket = c; + let bracket_pos = pos; + + // Determine the direction of the matching + let is_fwd = is_forward_bracket(c); + let line = doc.byte_to_line(pos); + let end = doc.line_to_byte(if is_fwd { line + 1 } else { line }); + let range = if is_fwd { + Range::Forward((pos + 1)..end) + } else { + Range::Backward((end..pos).rev()) + }; + + let mut open_cnt = 1; + + for pos in range { + c = doc.char(pos); + + if !is_valid_bracket(c) { + continue; + } else if bracket == c { + open_cnt += 1; + } else if is_valid_pair(doc, bracket_pos, pos) || is_valid_pair(doc, pos, bracket_pos) { + open_cnt -= 1; + } + + if open_cnt == 0 { + return Some(pos); + } + } + + None +} + fn is_valid_bracket(c: char) -> bool { PAIRS.iter().any(|(l, r)| *l == c || *r == c) } +fn is_forward_bracket(c: char) -> bool { + PAIRS.iter().any(|(l, _)| *l == c) +} + fn is_valid_pair(doc: &Rope, start_char: usize, end_char: usize) -> bool { PAIRS.contains(&(doc.char(start_char), doc.char(end_char))) } @@ -90,3 +143,46 @@ fn surrounding_bytes(doc: &Rope, node: &Node) -> Option<(usize, usize)> { Some((start_byte, end_byte)) } + +enum Range { + Forward(std::ops::Range), + Backward(std::iter::Rev>), +} + +impl Iterator for Range { + type Item = usize; + fn next(&mut self) -> Option { + match self { + Range::Forward(range) => range.next(), + Range::Backward(range) => range.next(), + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_find_matching_bracket_current_line_plaintext() { + let assert = |input: &str, pos, expected| { + let input = &Rope::from(input); + let actual = find_matching_bracket_current_line_plaintext(input, pos); + assert_eq!(expected, actual.unwrap()); + + let reverse = find_matching_bracket_current_line_plaintext(input, actual.unwrap()); + assert_eq!(pos, reverse.unwrap(), "expected symmetrical behaviour"); + }; + + assert("(hello)", 0, 6); + assert("((hello))", 0, 8); + assert("((hello))", 1, 7); + assert("(((hello)))", 2, 8); + + assert("key: ${value}", 6, 12); + + assert("(paren (paren {bracket}))", 0, 24); + assert("(paren (paren {bracket}))", 7, 23); + assert("(paren (paren {bracket}))", 14, 22); + } +} diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs index 911c9c1f7e40..e77a29e8e2c4 100644 --- a/helix-term/src/commands.rs +++ b/helix-term/src/commands.rs @@ -4537,7 +4537,14 @@ fn match_brackets(cx: &mut Context) { { range.put_cursor(text, pos, cx.editor.mode == Mode::Select) } else { - range + if let Some(pos) = match_brackets::find_matching_bracket_current_line_plaintext( + doc.text(), + range.cursor(text), + ) { + range.put_cursor(text, pos, cx.editor.mode == Mode::Select) + } else { + range + } } }); doc.set_selection(view.id, selection); From 138fe384bfb16c2ec08ae7ebea89eebf0349ac80 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexandre=20Vinyals=20Valdepe=C3=B1as?= Date: Sat, 15 Oct 2022 13:31:10 +0200 Subject: [PATCH 02/15] feedback: address clippy complaints, and extract variables. --- helix-term/src/commands.rs | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs index e77a29e8e2c4..b568c8d130db 100644 --- a/helix-term/src/commands.rs +++ b/helix-term/src/commands.rs @@ -4530,21 +4530,19 @@ fn match_brackets(cx: &mut Context) { let (view, doc) = current!(cx.editor); if let Some(syntax) = doc.syntax() { - let text = doc.text().slice(..); + let text = doc.text(); + let text_slice = text.slice(..); + let is_select = cx.editor.mode == Mode::Select; let selection = doc.selection(view.id).clone().transform(|range| { - if let Some(pos) = - match_brackets::find_matching_bracket_fuzzy(syntax, doc.text(), range.cursor(text)) + let pos = range.cursor(text_slice); + if let Some(pos) = match_brackets::find_matching_bracket_fuzzy(syntax, text, pos) { + range.put_cursor(text_slice, pos, is_select) + } else if let Some(pos) = + match_brackets::find_matching_bracket_current_line_plaintext(text, pos) { - range.put_cursor(text, pos, cx.editor.mode == Mode::Select) + range.put_cursor(text_slice, pos, is_select) } else { - if let Some(pos) = match_brackets::find_matching_bracket_current_line_plaintext( - doc.text(), - range.cursor(text), - ) { - range.put_cursor(text, pos, cx.editor.mode == Mode::Select) - } else { - range - } + range } }); doc.set_selection(view.id, selection); From add3cd146205c339ba78be2b7e4919543d721a22 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexandre=20Vinyals=20Valdepe=C3=B1as?= Date: Sat, 15 Oct 2022 13:51:59 +0200 Subject: [PATCH 03/15] self-review: make the implementation more concise - Introduce new variables only when needed - Avoid calling twice `is_valid_pair` on worst-case scenarios - Remove checking if we've closed all brackets on each iteration, this check only makes sense whenever we encouter a closing bracket. --- helix-core/src/match_brackets.rs | 37 ++++++++++++++++---------------- 1 file changed, 18 insertions(+), 19 deletions(-) diff --git a/helix-core/src/match_brackets.rs b/helix-core/src/match_brackets.rs index 30b0d01d3bf1..1c011a2b1ab5 100644 --- a/helix-core/src/match_brackets.rs +++ b/helix-core/src/match_brackets.rs @@ -70,26 +70,22 @@ fn find_pair(syntax: &Syntax, doc: &Rope, pos: usize, traverse_parents: bool) -> } } -// Returns the position of the bracket that is closing, searching only in -// the current line, and works on plain text, ignoring the tree-sitter grammar. -// -// If the cursor is on an opening or closing bracket, the function -// behaves equivalent to [`find_matching_bracket`]. -// -// If no matchig bracket is found on the current line, returns None. +// Returns the position of the bracket that is closing. +// This function only searches on the current line, either forwards or backwards. +// This function matches plain text, it ignores tree-sitter grammar. +// Returns None when no matching bracket is found. #[must_use] pub fn find_matching_bracket_current_line_plaintext(doc: &Rope, pos: usize) -> Option { // Don't do anything when the cursor is not on top of a bracket. - let mut c = doc.char(pos); - if !is_valid_bracket(c) { + let bracket = doc.char(pos); + if !is_valid_bracket(bracket) { return None; } - let bracket = c; let bracket_pos = pos; // Determine the direction of the matching - let is_fwd = is_forward_bracket(c); + let is_fwd = is_forward_bracket(bracket); let line = doc.byte_to_line(pos); let end = doc.line_to_byte(if is_fwd { line + 1 } else { line }); let range = if is_fwd { @@ -101,18 +97,21 @@ pub fn find_matching_bracket_current_line_plaintext(doc: &Rope, pos: usize) -> O let mut open_cnt = 1; for pos in range { - c = doc.char(pos); + let c = doc.char(pos); - if !is_valid_bracket(c) { - continue; - } else if bracket == c { + if c == bracket { open_cnt += 1; - } else if is_valid_pair(doc, bracket_pos, pos) || is_valid_pair(doc, pos, bracket_pos) { + } else if is_valid_pair( + doc, + if is_fwd { bracket_pos } else { pos }, + if is_fwd { pos } else { bracket_pos }, + ) { open_cnt -= 1; - } - if open_cnt == 0 { - return Some(pos); + // Return when all pending brackets have been closed. + if open_cnt == 0 { + return Some(pos); + } } } From 9f34f13d776687b301d59dcbd027c125703f451b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexandre=20Vinyals=20Valdepe=C3=B1as?= Date: Sat, 15 Oct 2022 17:40:38 +0200 Subject: [PATCH 04/15] self-review: better variable names, improved test coverage Less overlapping variable names, now it should be even more clear to read. Also added some extra testing, specially to verify that this feature only works on the current line. --- helix-core/src/match_brackets.rs | 41 +++++++++++++++++++++----------- 1 file changed, 27 insertions(+), 14 deletions(-) diff --git a/helix-core/src/match_brackets.rs b/helix-core/src/match_brackets.rs index 1c011a2b1ab5..6c74924833e8 100644 --- a/helix-core/src/match_brackets.rs +++ b/helix-core/src/match_brackets.rs @@ -75,36 +75,35 @@ fn find_pair(syntax: &Syntax, doc: &Rope, pos: usize, traverse_parents: bool) -> // This function matches plain text, it ignores tree-sitter grammar. // Returns None when no matching bracket is found. #[must_use] -pub fn find_matching_bracket_current_line_plaintext(doc: &Rope, pos: usize) -> Option { +pub fn find_matching_bracket_current_line_plaintext( + doc: &Rope, + cursor_pos: usize, +) -> Option { // Don't do anything when the cursor is not on top of a bracket. - let bracket = doc.char(pos); + let bracket = doc.char(cursor_pos); if !is_valid_bracket(bracket) { return None; } - let bracket_pos = pos; - // Determine the direction of the matching let is_fwd = is_forward_bracket(bracket); - let line = doc.byte_to_line(pos); + let line = doc.byte_to_line(cursor_pos); let end = doc.line_to_byte(if is_fwd { line + 1 } else { line }); let range = if is_fwd { - Range::Forward((pos + 1)..end) + Range::Forward((cursor_pos + 1)..end) } else { - Range::Backward((end..pos).rev()) + Range::Backward((end..cursor_pos).rev()) }; let mut open_cnt = 1; for pos in range { - let c = doc.char(pos); - - if c == bracket { + if doc.char(pos) == bracket { open_cnt += 1; } else if is_valid_pair( doc, - if is_fwd { bracket_pos } else { pos }, - if is_fwd { pos } else { bracket_pos }, + if is_fwd { cursor_pos } else { pos }, + if is_fwd { pos } else { cursor_pos }, ) { open_cnt -= 1; @@ -169,8 +168,14 @@ mod tests { let actual = find_matching_bracket_current_line_plaintext(input, pos); assert_eq!(expected, actual.unwrap()); - let reverse = find_matching_bracket_current_line_plaintext(input, actual.unwrap()); - assert_eq!(pos, reverse.unwrap(), "expected symmetrical behaviour"); + let actual = find_matching_bracket_current_line_plaintext(input, expected); + assert_eq!(pos, actual.unwrap(), "expected symmetrical behaviour"); + }; + + let assert_no_match = |input: &str, pos| { + let input = &Rope::from(input); + let actual = find_matching_bracket_current_line_plaintext(input, pos); + assert!(actual.is_none(), "expected no match"); }; assert("(hello)", 0, 6); @@ -179,9 +184,17 @@ mod tests { assert("(((hello)))", 2, 8); assert("key: ${value}", 6, 12); + assert("key: ${value} # (some comment)", 16, 29); assert("(paren (paren {bracket}))", 0, 24); assert("(paren (paren {bracket}))", 7, 23); assert("(paren (paren {bracket}))", 14, 22); + + // Verify that this feature only works on the current line. + assert_no_match("(prev line\n ) (middle) ( \n next line)", 0); + assert_no_match("(prev line\n ) (middle) ( \n next line)", 12); + assert("(prev line\n ) (middle) ( \n next line)", 14, 21); + assert_no_match("(prev line\n ) (middle) ( \n next line)", 23); + assert_no_match("(prev line\n ) (middle) ( \n next line)", 36); } } From 9de70296ccfcc6f3250a95da2874b3e07553279f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexandre=20Vinyals=20Valdepe=C3=B1as?= Date: Sat, 15 Oct 2022 18:00:59 +0200 Subject: [PATCH 05/15] docs: make the documentation of the new function consistent with the rest Also fix some typos. --- helix-core/src/match_brackets.rs | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/helix-core/src/match_brackets.rs b/helix-core/src/match_brackets.rs index 6c74924833e8..64a1b6cedfec 100644 --- a/helix-core/src/match_brackets.rs +++ b/helix-core/src/match_brackets.rs @@ -15,11 +15,13 @@ const PAIRS: &[(char, char)] = &[ // Returns the position of the matching bracket under cursor. // -// If the cursor is one the opening bracket, the position of -// the closing bracket is returned. If the cursor in the closing +// If the cursor is on the opening bracket, the position of +// the closing bracket is returned. If the cursor on the closing // bracket, the position of the opening bracket is returned. // // If the cursor is not on a bracket, `None` is returned. +// +// If no matching bracket is found, `None` is returned. #[must_use] pub fn find_matching_bracket(syntax: &Syntax, doc: &Rope, pos: usize) -> Option { if pos >= doc.len_chars() || !is_valid_bracket(doc.char(pos)) { @@ -70,10 +72,17 @@ fn find_pair(syntax: &Syntax, doc: &Rope, pos: usize, traverse_parents: bool) -> } } -// Returns the position of the bracket that is closing. -// This function only searches on the current line, either forwards or backwards. -// This function matches plain text, it ignores tree-sitter grammar. -// Returns None when no matching bracket is found. +// Returns the position of the matching bracket under cursor, the search +// is limited to the current line. This function works on plain text, it +// ignores tree-sitter grammar. +// +// If the cursor is on the opening bracket, the position of +// the closing bracket is returned. If the cursor on the closing +// bracket, the position of the opening bracket is returned. +// +// If the cursor is not on a bracket, `None` is returned. +// +// If no matching bracket is found, `None` is returned. #[must_use] pub fn find_matching_bracket_current_line_plaintext( doc: &Rope, From dd44778976d3eb20c1b891a8be89f205bce54ee3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexandre=20Vinyals=20Valdepe=C3=B1as?= Date: Sat, 15 Oct 2022 20:10:44 +0200 Subject: [PATCH 06/15] optimize: avoid wasteful decrement when closing last bracket Move the decrement operation *after* checking if this is the last bracket to close. This avoids a useless operation. When `is_valid_pair` is entered, we already know we are closing a bracket, when there's only 1 bracket pending to be closed, we already know we're going to return immediately anyway, no need to decrement. --- helix-core/src/match_brackets.rs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/helix-core/src/match_brackets.rs b/helix-core/src/match_brackets.rs index 64a1b6cedfec..a233025bd3d8 100644 --- a/helix-core/src/match_brackets.rs +++ b/helix-core/src/match_brackets.rs @@ -94,7 +94,7 @@ pub fn find_matching_bracket_current_line_plaintext( return None; } - // Determine the direction of the matching + // Determine the direction of the matching. let is_fwd = is_forward_bracket(bracket); let line = doc.byte_to_line(cursor_pos); let end = doc.line_to_byte(if is_fwd { line + 1 } else { line }); @@ -114,12 +114,11 @@ pub fn find_matching_bracket_current_line_plaintext( if is_fwd { cursor_pos } else { pos }, if is_fwd { pos } else { cursor_pos }, ) { - open_cnt -= 1; - // Return when all pending brackets have been closed. - if open_cnt == 0 { + if open_cnt == 1 { return Some(pos); } + open_cnt -= 1; } } From ac239af736ecb7aa01bdd77f381439e3ba977b09 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexandre=20Vinyals=20Valdepe=C3=B1as?= Date: Sat, 15 Oct 2022 20:36:42 +0200 Subject: [PATCH 07/15] self-review: use `or_else` to make the fallback more concise --- helix-term/src/commands.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs index b568c8d130db..c9a54d5195cd 100644 --- a/helix-term/src/commands.rs +++ b/helix-term/src/commands.rs @@ -4535,10 +4535,8 @@ fn match_brackets(cx: &mut Context) { let is_select = cx.editor.mode == Mode::Select; let selection = doc.selection(view.id).clone().transform(|range| { let pos = range.cursor(text_slice); - if let Some(pos) = match_brackets::find_matching_bracket_fuzzy(syntax, text, pos) { - range.put_cursor(text_slice, pos, is_select) - } else if let Some(pos) = - match_brackets::find_matching_bracket_current_line_plaintext(text, pos) + if let Some(pos) = match_brackets::find_matching_bracket_fuzzy(syntax, text, pos) + .or_else(|| match_brackets::find_matching_bracket_current_line_plaintext(text, pos)) { range.put_cursor(text_slice, pos, is_select) } else { From 1e932a9960061dea2b5e35a843f3476ab79fcefa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexandre=20Vinyals=20Valdepe=C3=B1as?= Date: Mon, 27 Feb 2023 23:09:35 +0100 Subject: [PATCH 08/15] Apply feedback - Remove constraint to work only on current line - Improve iteration by properly using ropes --- helix-core/src/match_brackets.rs | 44 ++++++++++---------------------- 1 file changed, 14 insertions(+), 30 deletions(-) diff --git a/helix-core/src/match_brackets.rs b/helix-core/src/match_brackets.rs index a233025bd3d8..57bb801c3ac8 100644 --- a/helix-core/src/match_brackets.rs +++ b/helix-core/src/match_brackets.rs @@ -96,27 +96,29 @@ pub fn find_matching_bracket_current_line_plaintext( // Determine the direction of the matching. let is_fwd = is_forward_bracket(bracket); - let line = doc.byte_to_line(cursor_pos); - let end = doc.line_to_byte(if is_fwd { line + 1 } else { line }); - let range = if is_fwd { - Range::Forward((cursor_pos + 1)..end) + let chars_iter = if is_fwd { + doc.chars_at(cursor_pos + 1) } else { - Range::Backward((end..cursor_pos).rev()) + doc.chars_at(cursor_pos).reversed() }; let mut open_cnt = 1; - for pos in range { - if doc.char(pos) == bracket { + for (i, candidate) in chars_iter.enumerate() { + if candidate == bracket { open_cnt += 1; } else if is_valid_pair( doc, - if is_fwd { cursor_pos } else { pos }, - if is_fwd { pos } else { cursor_pos }, + if is_fwd { cursor_pos } else { cursor_pos - i - 1 }, + if is_fwd { cursor_pos + i + 1 } else { cursor_pos }, ) { // Return when all pending brackets have been closed. if open_cnt == 1 { - return Some(pos); + return Some(if is_fwd { + cursor_pos + i + 1 + } else { + cursor_pos - i - 1 + }); } open_cnt -= 1; } @@ -150,21 +152,6 @@ fn surrounding_bytes(doc: &Rope, node: &Node) -> Option<(usize, usize)> { Some((start_byte, end_byte)) } -enum Range { - Forward(std::ops::Range), - Backward(std::iter::Rev>), -} - -impl Iterator for Range { - type Item = usize; - fn next(&mut self) -> Option { - match self { - Range::Forward(range) => range.next(), - Range::Backward(range) => range.next(), - } - } -} - #[cfg(test)] mod tests { use super::*; @@ -198,11 +185,8 @@ mod tests { assert("(paren (paren {bracket}))", 7, 23); assert("(paren (paren {bracket}))", 14, 22); - // Verify that this feature only works on the current line. - assert_no_match("(prev line\n ) (middle) ( \n next line)", 0); - assert_no_match("(prev line\n ) (middle) ( \n next line)", 12); + assert("(prev line\n ) (middle) ( \n next line)", 0, 12); assert("(prev line\n ) (middle) ( \n next line)", 14, 21); - assert_no_match("(prev line\n ) (middle) ( \n next line)", 23); - assert_no_match("(prev line\n ) (middle) ( \n next line)", 36); + assert("(prev line\n ) (middle) ( \n next line)", 23, 36); } } From 3370b051e88fc8eea860699ceb26ce251fefe28d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexandre=20Vinyals=20Valdepe=C3=B1as?= Date: Thu, 2 Mar 2023 09:20:48 +0100 Subject: [PATCH 09/15] Ensure cargo fmt --- helix-core/src/match_brackets.rs | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/helix-core/src/match_brackets.rs b/helix-core/src/match_brackets.rs index 57bb801c3ac8..6db66549e9f4 100644 --- a/helix-core/src/match_brackets.rs +++ b/helix-core/src/match_brackets.rs @@ -109,8 +109,16 @@ pub fn find_matching_bracket_current_line_plaintext( open_cnt += 1; } else if is_valid_pair( doc, - if is_fwd { cursor_pos } else { cursor_pos - i - 1 }, - if is_fwd { cursor_pos + i + 1 } else { cursor_pos }, + if is_fwd { + cursor_pos + } else { + cursor_pos - i - 1 + }, + if is_fwd { + cursor_pos + i + 1 + } else { + cursor_pos + }, ) { // Return when all pending brackets have been closed. if open_cnt == 1 { From 9ee32af12ec3fd82bbab8c9d7d9c6ad982f762ed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexandre=20Vinyals=20Valdepe=C3=B1as?= Date: Thu, 2 Mar 2023 18:22:47 +0100 Subject: [PATCH 10/15] Remove unused assertion function --- helix-core/src/match_brackets.rs | 6 ------ 1 file changed, 6 deletions(-) diff --git a/helix-core/src/match_brackets.rs b/helix-core/src/match_brackets.rs index 6db66549e9f4..f76450c6a1f7 100644 --- a/helix-core/src/match_brackets.rs +++ b/helix-core/src/match_brackets.rs @@ -175,12 +175,6 @@ mod tests { assert_eq!(pos, actual.unwrap(), "expected symmetrical behaviour"); }; - let assert_no_match = |input: &str, pos| { - let input = &Rope::from(input); - let actual = find_matching_bracket_current_line_plaintext(input, pos); - assert!(actual.is_none(), "expected no match"); - }; - assert("(hello)", 0, 6); assert("((hello))", 0, 8); assert("((hello))", 1, 7); From f5cb382af064d6db6df2e5da89913ae24e363ee5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexandre=20Vinyals=20Valdepe=C3=B1as?= Date: Thu, 1 Jun 2023 15:40:41 +0200 Subject: [PATCH 11/15] feedback: convert some of the comments to doc-comments --- helix-core/src/match_brackets.rs | 40 ++++++++++++++++---------------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/helix-core/src/match_brackets.rs b/helix-core/src/match_brackets.rs index f76450c6a1f7..fc21f32b1e11 100644 --- a/helix-core/src/match_brackets.rs +++ b/helix-core/src/match_brackets.rs @@ -13,15 +13,15 @@ const PAIRS: &[(char, char)] = &[ // limit matching pairs to only ( ) { } [ ] < > ' ' " " -// Returns the position of the matching bracket under cursor. -// -// If the cursor is on the opening bracket, the position of -// the closing bracket is returned. If the cursor on the closing -// bracket, the position of the opening bracket is returned. -// -// If the cursor is not on a bracket, `None` is returned. -// -// If no matching bracket is found, `None` is returned. +/// Returns the position of the matching bracket under cursor. +/// +/// If the cursor is on the opening bracket, the position of +/// the closing bracket is returned. If the cursor on the closing +/// bracket, the position of the opening bracket is returned. +/// +/// If the cursor is not on a bracket, `None` is returned. +/// +/// If no matching bracket is found, `None` is returned. #[must_use] pub fn find_matching_bracket(syntax: &Syntax, doc: &Rope, pos: usize) -> Option { if pos >= doc.len_chars() || !is_valid_bracket(doc.char(pos)) { @@ -72,17 +72,17 @@ fn find_pair(syntax: &Syntax, doc: &Rope, pos: usize, traverse_parents: bool) -> } } -// Returns the position of the matching bracket under cursor, the search -// is limited to the current line. This function works on plain text, it -// ignores tree-sitter grammar. -// -// If the cursor is on the opening bracket, the position of -// the closing bracket is returned. If the cursor on the closing -// bracket, the position of the opening bracket is returned. -// -// If the cursor is not on a bracket, `None` is returned. -// -// If no matching bracket is found, `None` is returned. +/// Returns the position of the matching bracket under cursor, the search +/// is limited to the current line. This function works on plain text, it +/// ignores tree-sitter grammar. +/// +/// If the cursor is on the opening bracket, the position of +/// the closing bracket is returned. If the cursor on the closing +/// bracket, the position of the opening bracket is returned. +/// +/// If the cursor is not on a bracket, `None` is returned. +/// +/// If no matching bracket is found, `None` is returned. #[must_use] pub fn find_matching_bracket_current_line_plaintext( doc: &Rope, From 715b928dae00d8048f32ce8c6527a9d45f7d4f70 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexandre=20Vinyals=20Valdepe=C3=B1as?= Date: Thu, 1 Jun 2023 15:48:09 +0200 Subject: [PATCH 12/15] feedback: limit plain-text scanning to 10k characters at most, to avoid freezes --- helix-core/src/match_brackets.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/helix-core/src/match_brackets.rs b/helix-core/src/match_brackets.rs index fc21f32b1e11..0ec040e54053 100644 --- a/helix-core/src/match_brackets.rs +++ b/helix-core/src/match_brackets.rs @@ -2,6 +2,9 @@ use tree_sitter::Node; use crate::{Rope, Syntax}; +const MAX_PLAINTEXT_SCAN: usize = 10000; + +// Limit matching pairs to only ( ) { } [ ] < > ' ' " " const PAIRS: &[(char, char)] = &[ ('(', ')'), ('{', '}'), @@ -11,8 +14,6 @@ const PAIRS: &[(char, char)] = &[ ('\"', '\"'), ]; -// limit matching pairs to only ( ) { } [ ] < > ' ' " " - /// Returns the position of the matching bracket under cursor. /// /// If the cursor is on the opening bracket, the position of @@ -104,7 +105,7 @@ pub fn find_matching_bracket_current_line_plaintext( let mut open_cnt = 1; - for (i, candidate) in chars_iter.enumerate() { + for (i, candidate) in chars_iter.take(MAX_PLAINTEXT_SCAN).enumerate() { if candidate == bracket { open_cnt += 1; } else if is_valid_pair( From adf0fbd3c8a663ceeaba3050f955c4bdec242381 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexandre=20Vinyals=20Valdepe=C3=B1as?= Date: Thu, 1 Jun 2023 16:51:26 +0200 Subject: [PATCH 13/15] feedback: add plain-text fallback only when there is no syntax (tree-sitter) --- helix-term/src/commands.rs | 33 +++++++++++++++++---------------- 1 file changed, 17 insertions(+), 16 deletions(-) diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs index c9a54d5195cd..540ca9b36618 100644 --- a/helix-term/src/commands.rs +++ b/helix-term/src/commands.rs @@ -4528,23 +4528,24 @@ fn select_prev_sibling(cx: &mut Context) { fn match_brackets(cx: &mut Context) { let (view, doc) = current!(cx.editor); + let is_select = cx.editor.mode == Mode::Select; + let text = doc.text(); + let text_slice = text.slice(..); - if let Some(syntax) = doc.syntax() { - let text = doc.text(); - let text_slice = text.slice(..); - let is_select = cx.editor.mode == Mode::Select; - let selection = doc.selection(view.id).clone().transform(|range| { - let pos = range.cursor(text_slice); - if let Some(pos) = match_brackets::find_matching_bracket_fuzzy(syntax, text, pos) - .or_else(|| match_brackets::find_matching_bracket_current_line_plaintext(text, pos)) - { - range.put_cursor(text_slice, pos, is_select) - } else { - range - } - }); - doc.set_selection(view.id, selection); - } + let selection = doc.selection(view.id).clone().transform(|range| { + let pos = range.cursor(text_slice); + if let Some(matched_pos) = doc + .syntax() + .and_then(|syntax| match_brackets::find_matching_bracket_fuzzy(syntax, text, pos)) + .or_else(|| match_brackets::find_matching_bracket_current_line_plaintext(text, pos)) + { + range.put_cursor(text_slice, matched_pos, is_select) + } else { + range + } + }); + + doc.set_selection(view.id, selection); } // From 67099d1a77f6868e5774b84a3e220d719747d02c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexandre=20Vinyals=20Valdepe=C3=B1as?= Date: Mon, 5 Jun 2023 11:30:45 +0200 Subject: [PATCH 14/15] feedback: fix bug, use to ensure it only runs when there's no syntax --- helix-term/src/commands.rs | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs index 540ca9b36618..1e89fe1cb69b 100644 --- a/helix-term/src/commands.rs +++ b/helix-term/src/commands.rs @@ -4534,11 +4534,10 @@ fn match_brackets(cx: &mut Context) { let selection = doc.selection(view.id).clone().transform(|range| { let pos = range.cursor(text_slice); - if let Some(matched_pos) = doc - .syntax() - .and_then(|syntax| match_brackets::find_matching_bracket_fuzzy(syntax, text, pos)) - .or_else(|| match_brackets::find_matching_bracket_current_line_plaintext(text, pos)) - { + if let Some(matched_pos) = doc.syntax().map_or_else( + || match_brackets::find_matching_bracket_current_line_plaintext(text, pos), + |syntax| match_brackets::find_matching_bracket_fuzzy(syntax, text, pos), + ) { range.put_cursor(text_slice, matched_pos, is_select) } else { range From b745fb0feea0cd38a079ff474272b7c78e28742a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexandre=20Vinyals=20Valdepe=C3=B1as?= Date: Mon, 5 Jun 2023 15:33:14 +0200 Subject: [PATCH 15/15] feedback: fix documentation --- helix-core/src/match_brackets.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/helix-core/src/match_brackets.rs b/helix-core/src/match_brackets.rs index 0ec040e54053..fd43a110e796 100644 --- a/helix-core/src/match_brackets.rs +++ b/helix-core/src/match_brackets.rs @@ -73,9 +73,9 @@ fn find_pair(syntax: &Syntax, doc: &Rope, pos: usize, traverse_parents: bool) -> } } -/// Returns the position of the matching bracket under cursor, the search -/// is limited to the current line. This function works on plain text, it -/// ignores tree-sitter grammar. +/// Returns the position of the matching bracket under cursor. +/// This function works on plain text and ignores tree-sitter grammar. +/// The search is limited to `MAX_PLAINTEXT_SCAN` characters /// /// If the cursor is on the opening bracket, the position of /// the closing bracket is returned. If the cursor on the closing