From 7eb8fa3682e4deae482d46905ff8b2bacaf64014 Mon Sep 17 00:00:00 2001 From: Nikita Revenco <154856872+NikitaRevenco@users.noreply.github.com> Date: Sat, 11 Jan 2025 15:12:40 +0000 Subject: [PATCH 1/4] feat: jump to matching single pair if on "ambiguous" pair --- helix-core/src/surround.rs | 31 +++++++++++++--- helix-core/src/textobject.rs | 2 +- helix-term/src/commands.rs | 69 ++++-------------------------------- 3 files changed, 33 insertions(+), 69 deletions(-) diff --git a/helix-core/src/surround.rs b/helix-core/src/surround.rs index e45346c92..d8fda0c05 100644 --- a/helix-core/src/surround.rs +++ b/helix-core/src/surround.rs @@ -3,7 +3,7 @@ use std::fmt::Display; use crate::{ graphemes::next_grapheme_boundary, match_brackets::{ - find_matching_bracket, find_matching_bracket_fuzzy, get_pair, is_close_bracket, + self, find_matching_bracket, find_matching_bracket_fuzzy, get_pair, is_close_bracket, is_open_bracket, }, movement::Direction, @@ -162,6 +162,7 @@ pub fn find_nth_pairs_pos( ch: char, range: Range, n: usize, + syntax: Option<&Syntax>, ) -> Result<(usize, usize)> { if text.len_chars() < 2 { return Err(Error::PairNotFound); @@ -175,6 +176,26 @@ pub fn find_nth_pairs_pos( let (open, close) = if open == close { if Some(open) == text.get_char(pos) { + // In this case, the Cursor is directly on the match char. There is still hope to find a matching char. + + if let Some(final_attempt) = syntax + .map_or_else( + || match_brackets::find_matching_bracket_plaintext(text.slice(..), pos), + |syntax| { + match_brackets::find_matching_bracket_fuzzy(syntax, text.slice(..), pos) + }, + ) + .map(|matching_pair_pos| { + if matching_pair_pos > pos { + (pos, matching_pair_pos) + } else { + (matching_pair_pos, pos) + } + }) + { + return Ok(final_attempt); + }; + // Cursor is directly on match char. We return no match // because there's no way to know which side of the char // we should be searching on. @@ -298,7 +319,7 @@ pub fn get_surround_pos( for &range in selection { let (open_pos, close_pos) = { let range_raw = match ch { - Some(ch) => find_nth_pairs_pos(text, ch, range, skip)?, + Some(ch) => find_nth_pairs_pos(text, ch, range, skip, syntax)?, None => find_nth_closest_pairs_pos(syntax, text, range, skip)?, }; let range = Range::new(range_raw.0, range_raw.1); @@ -392,7 +413,7 @@ mod test { assert_eq!(2, expectations.len()); assert_eq!( - find_nth_pairs_pos(doc.slice(..), '\'', selection.primary(), 1) + find_nth_pairs_pos(doc.slice(..), '\'', selection.primary(), 1, None) .expect("find should succeed"), (expectations[0], expectations[1]) ) @@ -409,7 +430,7 @@ mod test { assert_eq!(2, expectations.len()); assert_eq!( - find_nth_pairs_pos(doc.slice(..), '\'', selection.primary(), 2) + find_nth_pairs_pos(doc.slice(..), '\'', selection.primary(), 2, None) .expect("find should succeed"), (expectations[0], expectations[1]) ) @@ -425,7 +446,7 @@ mod test { ); assert_eq!( - find_nth_pairs_pos(doc.slice(..), '\'', selection.primary(), 1), + find_nth_pairs_pos(doc.slice(..), '\'', selection.primary(), 1, None), Err(Error::CursorOnAmbiguousPair) ) } diff --git a/helix-core/src/textobject.rs b/helix-core/src/textobject.rs index 7576b3a78..752975e85 100644 --- a/helix-core/src/textobject.rs +++ b/helix-core/src/textobject.rs @@ -228,7 +228,7 @@ fn textobject_pair_surround_impl( count: usize, ) -> Range { let pair_pos = match ch { - Some(ch) => surround::find_nth_pairs_pos(slice, ch, range, count), + Some(ch) => surround::find_nth_pairs_pos(slice, ch, range, count, syntax), None => surround::find_nth_closest_pairs_pos(syntax, slice, range, count), }; pair_pos diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs index 06d892adc..e9fbebd69 100644 --- a/helix-term/src/commands.rs +++ b/helix-term/src/commands.rs @@ -5627,73 +5627,16 @@ fn select_textobject(cx: &mut Context, objtype: textobject::TextObject) { let (view, doc) = current!(editor); let text = doc.text().slice(..); - let textobject_treesitter = |obj_name: &str, range: Range| -> Range { - let (lang_config, syntax) = match doc.language_config().zip(doc.syntax()) { - Some(t) => t, - None => return range, - }; - textobject::textobject_treesitter( + let selection = doc.selection(view.id).clone().transform(|range| match ch { + ch if !ch.is_ascii_alphanumeric() => textobject::textobject_pair_surround( + doc.syntax(), text, range, objtype, - obj_name, - syntax.tree().root_node(), - lang_config, + ch, count, - ) - }; - - if ch == 'g' && doc.diff_handle().is_none() { - editor.set_status("Diff is not available in current buffer"); - return; - } - - let textobject_change = |range: Range| -> Range { - let diff_handle = doc.diff_handle().unwrap(); - let diff = diff_handle.load(); - let line = range.cursor_line(text); - let hunk_idx = if let Some(hunk_idx) = diff.hunk_at(line as u32, false) { - hunk_idx - } else { - return range; - }; - let hunk = diff.nth_hunk(hunk_idx).after; - - let start = text.line_to_char(hunk.start as usize); - let end = text.line_to_char(hunk.end as usize); - Range::new(start, end).with_direction(range.direction()) - }; - - let selection = doc.selection(view.id).clone().transform(|range| { - match ch { - 'w' => textobject::textobject_word(text, range, objtype, count, false), - 'W' => textobject::textobject_word(text, range, objtype, count, true), - 't' => textobject_treesitter("class", range), - 'f' => textobject_treesitter("function", range), - 'a' => textobject_treesitter("parameter", range), - 'c' => textobject_treesitter("comment", range), - 'T' => textobject_treesitter("test", range), - 'e' => textobject_treesitter("entry", range), - 'p' => textobject::textobject_paragraph(text, range, objtype, count), - 'm' => textobject::textobject_pair_surround_closest( - doc.syntax(), - text, - range, - objtype, - count, - ), - 'g' => textobject_change(range), - // TODO: cancel new ranges if inconsistent surround matches across lines - ch if !ch.is_ascii_alphanumeric() => textobject::textobject_pair_surround( - doc.syntax(), - text, - range, - objtype, - ch, - count, - ), - _ => range, - } + ), + _ => range, }); doc.set_selection(view.id, selection); }; From b83d120295641cf3f6f21aae934938ea34320bc1 Mon Sep 17 00:00:00 2001 From: Nikita Revenco <154856872+NikitaRevenco@users.noreply.github.com> Date: Sat, 11 Jan 2025 15:26:59 +0000 Subject: [PATCH 2/4] refactor: use single expression --- helix-core/src/surround.rs | 29 ++++++++++++----------------- 1 file changed, 12 insertions(+), 17 deletions(-) diff --git a/helix-core/src/surround.rs b/helix-core/src/surround.rs index d8fda0c05..7c94140c5 100644 --- a/helix-core/src/surround.rs +++ b/helix-core/src/surround.rs @@ -176,9 +176,10 @@ pub fn find_nth_pairs_pos( let (open, close) = if open == close { if Some(open) == text.get_char(pos) { - // In this case, the Cursor is directly on the match char. There is still hope to find a matching char. - - if let Some(final_attempt) = syntax + // Cursor is directly on match character for which the opening and closing pairs are the same. For instance: ", ', ` + // + // This is potentially ambiguous + syntax .map_or_else( || match_brackets::find_matching_bracket_plaintext(text.slice(..), pos), |syntax| { @@ -187,24 +188,18 @@ pub fn find_nth_pairs_pos( ) .map(|matching_pair_pos| { if matching_pair_pos > pos { - (pos, matching_pair_pos) + (Some(pos), Some(matching_pair_pos)) } else { - (matching_pair_pos, pos) + (Some(matching_pair_pos), Some(pos)) } }) - { - return Ok(final_attempt); - }; - - // Cursor is directly on match char. We return no match - // because there's no way to know which side of the char - // we should be searching on. - return Err(Error::CursorOnAmbiguousPair); + .ok_or(Error::CursorOnAmbiguousPair)? + } else { + ( + search::find_nth_prev(text, open, pos, n), + search::find_nth_next(text, close, pos, n), + ) } - ( - search::find_nth_prev(text, open, pos, n), - search::find_nth_next(text, close, pos, n), - ) } else { ( find_nth_open_pair(text, open, close, pos, n), From 0e5c160a475ccb7ec2bf3b257c31a84fa255274c Mon Sep 17 00:00:00 2001 From: Nikita Revenco <154856872+NikitaRevenco@users.noreply.github.com> Date: Sat, 11 Jan 2025 15:33:45 +0000 Subject: [PATCH 3/4] fix: add back unnecesserily removed content --- helix-term/src/commands.rs | 69 ++++++++++++++++++++++++++++++++++---- 1 file changed, 63 insertions(+), 6 deletions(-) diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs index e9fbebd69..06d892adc 100644 --- a/helix-term/src/commands.rs +++ b/helix-term/src/commands.rs @@ -5627,16 +5627,73 @@ fn select_textobject(cx: &mut Context, objtype: textobject::TextObject) { let (view, doc) = current!(editor); let text = doc.text().slice(..); - let selection = doc.selection(view.id).clone().transform(|range| match ch { - ch if !ch.is_ascii_alphanumeric() => textobject::textobject_pair_surround( - doc.syntax(), + let textobject_treesitter = |obj_name: &str, range: Range| -> Range { + let (lang_config, syntax) = match doc.language_config().zip(doc.syntax()) { + Some(t) => t, + None => return range, + }; + textobject::textobject_treesitter( text, range, objtype, - ch, + obj_name, + syntax.tree().root_node(), + lang_config, count, - ), - _ => range, + ) + }; + + if ch == 'g' && doc.diff_handle().is_none() { + editor.set_status("Diff is not available in current buffer"); + return; + } + + let textobject_change = |range: Range| -> Range { + let diff_handle = doc.diff_handle().unwrap(); + let diff = diff_handle.load(); + let line = range.cursor_line(text); + let hunk_idx = if let Some(hunk_idx) = diff.hunk_at(line as u32, false) { + hunk_idx + } else { + return range; + }; + let hunk = diff.nth_hunk(hunk_idx).after; + + let start = text.line_to_char(hunk.start as usize); + let end = text.line_to_char(hunk.end as usize); + Range::new(start, end).with_direction(range.direction()) + }; + + let selection = doc.selection(view.id).clone().transform(|range| { + match ch { + 'w' => textobject::textobject_word(text, range, objtype, count, false), + 'W' => textobject::textobject_word(text, range, objtype, count, true), + 't' => textobject_treesitter("class", range), + 'f' => textobject_treesitter("function", range), + 'a' => textobject_treesitter("parameter", range), + 'c' => textobject_treesitter("comment", range), + 'T' => textobject_treesitter("test", range), + 'e' => textobject_treesitter("entry", range), + 'p' => textobject::textobject_paragraph(text, range, objtype, count), + 'm' => textobject::textobject_pair_surround_closest( + doc.syntax(), + text, + range, + objtype, + count, + ), + 'g' => textobject_change(range), + // TODO: cancel new ranges if inconsistent surround matches across lines + ch if !ch.is_ascii_alphanumeric() => textobject::textobject_pair_surround( + doc.syntax(), + text, + range, + objtype, + ch, + count, + ), + _ => range, + } }); doc.set_selection(view.id, selection); }; From b8e1621538422a896d6a8efe67534b4b86fcd38f Mon Sep 17 00:00:00 2001 From: Nikita Revenco <154856872+NikitaRevenco@users.noreply.github.com> Date: Sat, 11 Jan 2025 16:53:49 +0000 Subject: [PATCH 4/4] chore: add comment --- helix-core/src/surround.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/helix-core/src/surround.rs b/helix-core/src/surround.rs index 7c94140c5..e3eb168da 100644 --- a/helix-core/src/surround.rs +++ b/helix-core/src/surround.rs @@ -178,7 +178,7 @@ pub fn find_nth_pairs_pos( if Some(open) == text.get_char(pos) { // Cursor is directly on match character for which the opening and closing pairs are the same. For instance: ", ', ` // - // This is potentially ambiguous + // This is potentially ambiguous, because there's no way to know which side of the char we should be searching on. syntax .map_or_else( || match_brackets::find_matching_bracket_plaintext(text.slice(..), pos),