From 90cd009f238fbe6e88ad2013b1dc2253df21cce0 Mon Sep 17 00:00:00 2001 From: Cole Helbling Date: Wed, 26 Apr 2023 15:28:22 -0700 Subject: [PATCH 1/2] helix-core/surround: `Syntax` aware Prior to this change, attempting to replace the double quotes in `" asdf "` would fail with "Cursor on ambiguous surround pair". Now, after running e.g. `:set-language rust` in a file with the contents `" asdf "`, it is now possible to replace the double quotes with `mr` as you could with any other pair. This change essentially makes any pair that you can use `mm` on to go to the matching pair also able to be replaced with `mr` or deleted with `md` (whether they be built-in, or provided by a tree-sitter grammar). --- helix-core/src/surround.rs | 49 +++++++++++++++++++++++------------- helix-core/src/textobject.rs | 14 +++++++---- helix-term/src/commands.rs | 47 +++++++++++++++++++++------------- 3 files changed, 70 insertions(+), 40 deletions(-) diff --git a/helix-core/src/surround.rs b/helix-core/src/surround.rs index f430aee8a152..bf85b3825d09 100644 --- a/helix-core/src/surround.rs +++ b/helix-core/src/surround.rs @@ -1,6 +1,8 @@ use std::fmt::Display; -use crate::{movement::Direction, search, Range, Selection}; +use crate::{ + match_brackets::find_matching_bracket, movement::Direction, search, Range, Selection, Syntax, +}; use ropey::RopeSlice; pub const PAIRS: &[(char, char)] = &[ @@ -124,6 +126,7 @@ pub fn find_nth_closest_pairs_pos( /// or opening pair. `n` will skip n - 1 pairs (eg. n=2 will discard (only) /// the first pair found and keep looking) pub fn find_nth_pairs_pos( + syntax: Option<&Syntax>, text: RopeSlice, ch: char, range: Range, @@ -141,15 +144,26 @@ pub fn find_nth_pairs_pos( let (open, close) = if open == close { if Some(open) == text.get_char(pos) { - // 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); + // Cursor is directly on match char and we don't have any tree-sitter syntax available + // to help identify the matching pair. We return no match because there's no way to know + // which side of the char we should be searching on. + let syntax = syntax.ok_or(Error::CursorOnAmbiguousPair)?; + + let match_pos = find_matching_bracket(syntax, &text.into(), pos) + .ok_or(Error::CursorOnAmbiguousPair)?; + let close_char = text.char(match_pos); + let search_pos = pos.max(match_pos); + + ( + search::find_nth_prev(text, open, search_pos, n), + search::find_nth_next(text, close_char, search_pos, n), + ) + } 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), @@ -245,6 +259,7 @@ fn find_nth_close_pair( /// are automatically detected around each cursor (note that this may result /// in them selecting different surround characters for each selection). pub fn get_surround_pos( + syntax: Option<&Syntax>, text: RopeSlice, selection: &Selection, ch: Option, @@ -254,7 +269,7 @@ pub fn get_surround_pos( for &range in selection { let (open_pos, close_pos) = match ch { - Some(ch) => find_nth_pairs_pos(text, ch, range, skip)?, + Some(ch) => find_nth_pairs_pos(syntax, text, ch, range, skip)?, None => find_nth_closest_pairs_pos(text, range, skip)?, }; if change_pos.contains(&open_pos) || change_pos.contains(&close_pos) { @@ -283,7 +298,7 @@ mod test { ); assert_eq!( - get_surround_pos(doc.slice(..), &selection, Some('('), 1).unwrap(), + get_surround_pos(None, doc.slice(..), &selection, Some('('), 1).unwrap(), expectations ); } @@ -298,7 +313,7 @@ mod test { ); assert_eq!( - get_surround_pos(doc.slice(..), &selection, Some('('), 1), + get_surround_pos(None, doc.slice(..), &selection, Some('('), 1), Err(Error::PairNotFound) ); } @@ -313,7 +328,7 @@ mod test { ); assert_eq!( - get_surround_pos(doc.slice(..), &selection, Some('('), 1), + get_surround_pos(None, doc.slice(..), &selection, Some('('), 1), Err(Error::PairNotFound) // overlapping surround chars ); } @@ -328,7 +343,7 @@ mod test { ); assert_eq!( - get_surround_pos(doc.slice(..), &selection, Some('['), 1), + get_surround_pos(None, doc.slice(..), &selection, Some('['), 1), Err(Error::CursorOverlap) ); } @@ -344,7 +359,7 @@ mod test { assert_eq!(2, expectations.len()); assert_eq!( - find_nth_pairs_pos(doc.slice(..), '\'', selection.primary(), 1) + find_nth_pairs_pos(None, doc.slice(..), '\'', selection.primary(), 1) .expect("find should succeed"), (expectations[0], expectations[1]) ) @@ -361,7 +376,7 @@ mod test { assert_eq!(2, expectations.len()); assert_eq!( - find_nth_pairs_pos(doc.slice(..), '\'', selection.primary(), 2) + find_nth_pairs_pos(None, doc.slice(..), '\'', selection.primary(), 2) .expect("find should succeed"), (expectations[0], expectations[1]) ) @@ -377,7 +392,7 @@ mod test { ); assert_eq!( - find_nth_pairs_pos(doc.slice(..), '\'', selection.primary(), 1), + find_nth_pairs_pos(None, doc.slice(..), '\'', selection.primary(), 1), Err(Error::CursorOnAmbiguousPair) ) } diff --git a/helix-core/src/textobject.rs b/helix-core/src/textobject.rs index bf00a4580491..a726512306c3 100644 --- a/helix-core/src/textobject.rs +++ b/helix-core/src/textobject.rs @@ -7,9 +7,9 @@ use crate::chars::{categorize_char, char_is_whitespace, CharCategory}; use crate::graphemes::{next_grapheme_boundary, prev_grapheme_boundary}; use crate::line_ending::rope_is_line_ending; use crate::movement::Direction; -use crate::surround; use crate::syntax::LanguageConfiguration; use crate::Range; +use crate::{surround, Syntax}; fn find_word_boundary(slice: RopeSlice, mut pos: usize, direction: Direction, long: bool) -> usize { use CharCategory::{Eol, Whitespace}; @@ -199,25 +199,28 @@ pub fn textobject_paragraph( } pub fn textobject_pair_surround( + syntax: Option<&Syntax>, slice: RopeSlice, range: Range, textobject: TextObject, ch: char, count: usize, ) -> Range { - textobject_pair_surround_impl(slice, range, textobject, Some(ch), count) + textobject_pair_surround_impl(syntax, slice, range, textobject, Some(ch), count) } pub fn textobject_pair_surround_closest( + syntax: Option<&Syntax>, slice: RopeSlice, range: Range, textobject: TextObject, count: usize, ) -> Range { - textobject_pair_surround_impl(slice, range, textobject, None, count) + textobject_pair_surround_impl(syntax, slice, range, textobject, None, count) } fn textobject_pair_surround_impl( + syntax: Option<&Syntax>, slice: RopeSlice, range: Range, textobject: TextObject, @@ -225,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(syntax, slice, ch, range, count), // Automatically find the closest surround pairs None => surround::find_nth_closest_pairs_pos(slice, range, count), }; @@ -574,7 +577,8 @@ mod test { let slice = doc.slice(..); for &case in scenario { let (pos, objtype, expected_range, ch, count) = case; - let result = textobject_pair_surround(slice, Range::point(pos), objtype, ch, count); + let result = + textobject_pair_surround(None, slice, Range::point(pos), objtype, ch, count); assert_eq!( result, expected_range.into(), diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs index 5a75553cd6da..582b89b9c297 100644 --- a/helix-term/src/commands.rs +++ b/helix-term/src/commands.rs @@ -4840,13 +4840,22 @@ fn select_textobject(cx: &mut Context, objtype: textobject::TextObject) { 'T' => textobject_treesitter("test", range), 'p' => textobject::textobject_paragraph(text, range, objtype, count), 'm' => textobject::textobject_pair_surround_closest( - text, range, objtype, count, + 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(text, range, objtype, ch, count) - } + ch if !ch.is_ascii_alphanumeric() => textobject::textobject_pair_surround( + doc.syntax(), + text, + range, + objtype, + ch, + count, + ), _ => range, } }); @@ -4935,13 +4944,14 @@ fn surround_replace(cx: &mut Context) { let text = doc.text().slice(..); let selection = doc.selection(view.id); - let change_pos = match surround::get_surround_pos(text, selection, surround_ch, count) { - Ok(c) => c, - Err(err) => { - cx.editor.set_error(err.to_string()); - return; - } - }; + let change_pos = + match surround::get_surround_pos(doc.syntax(), text, selection, surround_ch, count) { + Ok(c) => c, + Err(err) => { + cx.editor.set_error(err.to_string()); + return; + } + }; cx.on_next_key(move |cx, event| { let (view, doc) = current!(cx.editor); @@ -4976,13 +4986,14 @@ fn surround_delete(cx: &mut Context) { let text = doc.text().slice(..); let selection = doc.selection(view.id); - let change_pos = match surround::get_surround_pos(text, selection, surround_ch, count) { - Ok(c) => c, - Err(err) => { - cx.editor.set_error(err.to_string()); - return; - } - }; + let change_pos = + match surround::get_surround_pos(doc.syntax(), text, selection, surround_ch, count) { + Ok(c) => c, + Err(err) => { + cx.editor.set_error(err.to_string()); + return; + } + }; let transaction = Transaction::change(doc.text(), change_pos.into_iter().map(|p| (p, p + 1, None))); From da73d79d9b1b0d800e163d583a6d90a88fefdc53 Mon Sep 17 00:00:00 2001 From: Cole Helbling Date: Thu, 27 Apr 2023 17:04:50 -0700 Subject: [PATCH 2/2] helix-term/tests: test new `Syntax` aware surround functionality --- helix-term/tests/integration.rs | 1 + helix-term/tests/test/surround.rs | 46 +++++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+) create mode 100644 helix-term/tests/test/surround.rs diff --git a/helix-term/tests/integration.rs b/helix-term/tests/integration.rs index d77eefed07d8..ac66708ecf6b 100644 --- a/helix-term/tests/integration.rs +++ b/helix-term/tests/integration.rs @@ -21,4 +21,5 @@ mod test { mod movement; mod prompt; mod splits; + mod surround; } diff --git a/helix-term/tests/test/surround.rs b/helix-term/tests/test/surround.rs new file mode 100644 index 000000000000..4cc49fcc4bfc --- /dev/null +++ b/helix-term/tests/test/surround.rs @@ -0,0 +1,46 @@ +use super::*; + +#[tokio::test(flavor = "multi_thread")] +async fn replace_matching_double_quotes_with_treesitter() -> anyhow::Result<()> { + test_with_config( + AppBuilder::new().with_file("foo.rs", None), + ( + helpers::platform_line(r#"#["|]#hello world""#), + r#"mr"{"#, + helpers::platform_line(r#"#[{|]#hello world}"#), + ), + ) + .await?; + + Ok(()) +} + +#[tokio::test(flavor = "multi_thread")] +async fn delete_matching_double_quotes_with_treesitter() -> anyhow::Result<()> { + test_with_config( + AppBuilder::new().with_file("foo.rs", None), + ( + helpers::platform_line(r#"#["|]#hello world""#), + r#"md""#, + helpers::platform_line(r#"#[h|]#ello world"#), + ), + ) + .await?; + + Ok(()) +} + +#[tokio::test(flavor = "multi_thread")] +async fn replace_matching_single_quotes_with_treesitter() -> anyhow::Result<()> { + test_with_config( + AppBuilder::new().with_file("foo.rs", None), + ( + helpers::platform_line(r#"#['|]#h'"#), + r#"mr'""#, + helpers::platform_line(r#"#["|]#h""#), + ), + ) + .await?; + + Ok(()) +}