From 0ba051a7545d1e7486659a315e41b818a4e75acc Mon Sep 17 00:00:00 2001 From: Kay Simmons Date: Wed, 15 Feb 2023 14:04:16 -0800 Subject: [PATCH 1/3] use more predictable rules for selecting which bracket to jump to and where --- Cargo.lock | 2 + crates/editor/Cargo.toml | 2 + crates/editor/src/editor.rs | 65 +++-- crates/editor/src/editor_tests.rs | 48 ++++ .../editor/src/highlight_matching_bracket.rs | 2 +- crates/editor/src/multi_buffer.rs | 151 +++++----- crates/editor/src/selections_collection.rs | 25 ++ .../src/test/editor_lsp_test_context.rs | 30 +- crates/editor/src/test/editor_test_context.rs | 5 +- crates/gpui/src/app/ref_counts.rs | 11 +- crates/gpui/src/app/test_app_context.rs | 5 + crates/language/Cargo.toml | 1 + crates/language/src/buffer.rs | 51 ++-- crates/language/src/buffer_tests.rs | 271 ++++++++++++------ crates/vim/src/motion.rs | 5 +- 15 files changed, 457 insertions(+), 217 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 82e9197631..c748774237 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1956,6 +1956,7 @@ dependencies = [ "tree-sitter-html", "tree-sitter-javascript", "tree-sitter-rust", + "tree-sitter-typescript", "unindent", "util", "workspace", @@ -3249,6 +3250,7 @@ dependencies = [ "fuzzy", "git", "gpui", + "indoc", "lazy_static", "log", "lsp", diff --git a/crates/editor/Cargo.toml b/crates/editor/Cargo.toml index 26dd371041..7f1bec1d85 100644 --- a/crates/editor/Cargo.toml +++ b/crates/editor/Cargo.toml @@ -58,6 +58,7 @@ smol = "1.2" tree-sitter-rust = { version = "*", optional = true } tree-sitter-html = { version = "*", optional = true } tree-sitter-javascript = { version = "*", optional = true } +tree-sitter-typescript = { version = "*", optional = true } [dev-dependencies] text = { path = "../text", features = ["test-support"] } @@ -75,4 +76,5 @@ unindent = "0.1.7" tree-sitter = "0.20" tree-sitter-rust = "0.20" tree-sitter-html = "0.19" +tree-sitter-typescript = "0.20.1" tree-sitter-javascript = "0.20" diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index c649fed7ce..eafcfd694a 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -4752,27 +4752,52 @@ impl Editor { _: &MoveToEnclosingBracket, cx: &mut ViewContext, ) { - let buffer = self.buffer.read(cx).snapshot(cx); - let mut selections = self.selections.all::(cx); - for selection in &mut selections { - if let Some((open_range, close_range)) = - buffer.enclosing_bracket_ranges(selection.start..selection.end) - { - let close_range = close_range.to_inclusive(); - let destination = if close_range.contains(&selection.start) - && close_range.contains(&selection.end) - { - open_range.end - } else { - *close_range.start() - }; - selection.start = destination; - selection.end = destination; - } - } - self.change_selections(Some(Autoscroll::fit()), cx, |s| { - s.select(selections); + s.move_offsets_with(|snapshot, selection| { + let Some(enclosing_bracket_ranges) = snapshot.enclosing_bracket_ranges(selection.start..selection.end) else { return; }; + + let mut best_length = usize::MAX; + let mut best_inside = false; + let mut best_in_bracket_range = false; + let mut best_destination = None; + for (open, close) in enclosing_bracket_ranges { + let close = close.to_inclusive(); + let length = close.end() - open.start; + let inside = selection.start >= open.end && selection.end <= *close.start(); + let in_bracket_range = open.to_inclusive().contains(&selection.head()) || close.contains(&selection.head()); + + // If best is next to a bracket and current isn't, skip + if !in_bracket_range && best_in_bracket_range { + continue; + } + + // Prefer smaller lengths unless best is inside and current isn't + if length > best_length && (best_inside || !inside) { + continue; + } + + best_length = length; + best_inside = inside; + best_in_bracket_range = in_bracket_range; + best_destination = Some(if close.contains(&selection.start) && close.contains(&selection.end) { + if inside { + open.end + } else { + open.start + } + } else { + if inside { + *close.start() + } else { + *close.end() + } + }); + } + + if let Some(destination) = best_destination { + selection.collapse_to(destination, SelectionGoal::None); + } + }) }); } diff --git a/crates/editor/src/editor_tests.rs b/crates/editor/src/editor_tests.rs index ff59c5dc14..5b023bfe82 100644 --- a/crates/editor/src/editor_tests.rs +++ b/crates/editor/src/editor_tests.rs @@ -5459,6 +5459,54 @@ fn test_split_words() { assert_eq!(split("helloworld"), &["helloworld"]); } +#[gpui::test] +async fn test_move_to_enclosing_bracket(cx: &mut gpui::TestAppContext) { + let mut cx = EditorLspTestContext::new_typescript(Default::default(), cx).await; + let mut assert = |before, after| { + let _state_context = cx.set_state(before); + cx.update_editor(|editor, cx| { + editor.move_to_enclosing_bracket(&MoveToEnclosingBracket, cx) + }); + cx.assert_editor_state(after); + }; + + // Outside bracket jumps to outside of matching bracket + assert("console.logˇ(var);", "console.log(var)ˇ;"); + assert("console.log(var)ˇ;", "console.logˇ(var);"); + + // Inside bracket jumps to inside of matching bracket + assert("console.log(ˇvar);", "console.log(varˇ);"); + assert("console.log(varˇ);", "console.log(ˇvar);"); + + // When outside a bracket and inside, favor jumping to the inside bracket + assert( + "console.log('foo', [1, 2, 3]ˇ);", + "console.log(ˇ'foo', [1, 2, 3]);", + ); + assert( + "console.log(ˇ'foo', [1, 2, 3]);", + "console.log('foo', [1, 2, 3]ˇ);", + ); + + // Bias forward if two options are equally likely + assert( + "let result = curried_fun()ˇ();", + "let result = curried_fun()()ˇ;", + ); + + // If directly adjacent to a smaller pair but inside a larger (not adjacent), pick the smaller + assert( + indoc! {" + function test() { + console.log('test')ˇ + }"}, + indoc! {" + function test() { + console.logˇ('test') + }"}, + ); +} + fn empty_range(row: usize, column: usize) -> Range { let point = DisplayPoint::new(row as u32, column as u32); point..point diff --git a/crates/editor/src/highlight_matching_bracket.rs b/crates/editor/src/highlight_matching_bracket.rs index 043b21db21..0d868d460c 100644 --- a/crates/editor/src/highlight_matching_bracket.rs +++ b/crates/editor/src/highlight_matching_bracket.rs @@ -17,7 +17,7 @@ pub fn refresh_matching_bracket_highlights(editor: &mut Editor, cx: &mut ViewCon let snapshot = editor.snapshot(cx); if let Some((opening_range, closing_range)) = snapshot .buffer_snapshot - .enclosing_bracket_ranges(head..head) + .innermost_enclosing_bracket_ranges(head..head) { editor.highlight_background::( vec![ diff --git a/crates/editor/src/multi_buffer.rs b/crates/editor/src/multi_buffer.rs index 7079d197f9..7b85799b31 100644 --- a/crates/editor/src/multi_buffer.rs +++ b/crates/editor/src/multi_buffer.rs @@ -2621,56 +2621,71 @@ impl MultiBufferSnapshot { self.parse_count } - pub fn enclosing_bracket_ranges( + pub fn innermost_enclosing_bracket_ranges( &self, range: Range, ) -> Option<(Range, Range)> { let range = range.start.to_offset(self)..range.end.to_offset(self); - let mut cursor = self.excerpts.cursor::(); - cursor.seek(&range.start, Bias::Right, &()); - let start_excerpt = cursor.item(); + // Get the ranges of the innermost pair of brackets. + let mut result: Option<(Range, Range)> = None; - cursor.seek(&range.end, Bias::Right, &()); - let end_excerpt = cursor.item(); + let Some(enclosing_bracket_ranges) = self.enclosing_bracket_ranges(range.clone()) else { return None; }; - start_excerpt - .zip(end_excerpt) - .and_then(|(start_excerpt, end_excerpt)| { - if start_excerpt.id != end_excerpt.id { - return None; + for (open, close) in enclosing_bracket_ranges { + let len = close.end - open.start; + + if let Some((existing_open, existing_close)) = &result { + let existing_len = existing_close.end - existing_open.start; + if len > existing_len { + continue; } + } - let excerpt_buffer_start = start_excerpt - .range - .context - .start - .to_offset(&start_excerpt.buffer); - let excerpt_buffer_end = excerpt_buffer_start + start_excerpt.text_summary.len; + result = Some((open, close)); + } + + result + } + + /// Returns enclosingn bracket ranges containing the given range or returns None if the range is not contained in a single excerpt + pub fn enclosing_bracket_ranges<'a, T: ToOffset>( + &'a self, + range: Range, + ) -> Option, Range)> + 'a> { + let range = range.start.to_offset(self)..range.end.to_offset(self); + self.excerpt_containing(range.clone()) + .map(|(excerpt, excerpt_offset)| { + let excerpt_buffer_start = excerpt.range.context.start.to_offset(&excerpt.buffer); + let excerpt_buffer_end = excerpt_buffer_start + excerpt.text_summary.len; let start_in_buffer = - excerpt_buffer_start + range.start.saturating_sub(*cursor.start()); - let end_in_buffer = - excerpt_buffer_start + range.end.saturating_sub(*cursor.start()); - let (mut start_bracket_range, mut end_bracket_range) = start_excerpt - .buffer - .enclosing_bracket_ranges(start_in_buffer..end_in_buffer)?; + excerpt_buffer_start + range.start.saturating_sub(excerpt_offset); + let end_in_buffer = excerpt_buffer_start + range.end.saturating_sub(excerpt_offset); - if start_bracket_range.start >= excerpt_buffer_start - && end_bracket_range.end <= excerpt_buffer_end - { - start_bracket_range.start = - cursor.start() + (start_bracket_range.start - excerpt_buffer_start); - start_bracket_range.end = - cursor.start() + (start_bracket_range.end - excerpt_buffer_start); - end_bracket_range.start = - cursor.start() + (end_bracket_range.start - excerpt_buffer_start); - end_bracket_range.end = - cursor.start() + (end_bracket_range.end - excerpt_buffer_start); - Some((start_bracket_range, end_bracket_range)) - } else { - None - } + excerpt + .buffer + .enclosing_bracket_ranges(start_in_buffer..end_in_buffer) + .filter_map(move |(start_bracket_range, end_bracket_range)| { + if start_bracket_range.start < excerpt_buffer_start + || end_bracket_range.end > excerpt_buffer_end + { + return None; + } + + let mut start_bracket_range = start_bracket_range.clone(); + start_bracket_range.start = + excerpt_offset + (start_bracket_range.start - excerpt_buffer_start); + start_bracket_range.end = + excerpt_offset + (start_bracket_range.end - excerpt_buffer_start); + + let mut end_bracket_range = end_bracket_range.clone(); + end_bracket_range.start = + excerpt_offset + (end_bracket_range.start - excerpt_buffer_start); + end_bracket_range.end = + excerpt_offset + (end_bracket_range.end - excerpt_buffer_start); + Some((start_bracket_range, end_bracket_range)) + }) }) } @@ -2812,40 +2827,23 @@ impl MultiBufferSnapshot { pub fn range_for_syntax_ancestor(&self, range: Range) -> Option> { let range = range.start.to_offset(self)..range.end.to_offset(self); - let mut cursor = self.excerpts.cursor::(); - cursor.seek(&range.start, Bias::Right, &()); - let start_excerpt = cursor.item(); - - cursor.seek(&range.end, Bias::Right, &()); - let end_excerpt = cursor.item(); - - start_excerpt - .zip(end_excerpt) - .and_then(|(start_excerpt, end_excerpt)| { - if start_excerpt.id != end_excerpt.id { - return None; - } - - let excerpt_buffer_start = start_excerpt - .range - .context - .start - .to_offset(&start_excerpt.buffer); - let excerpt_buffer_end = excerpt_buffer_start + start_excerpt.text_summary.len; + self.excerpt_containing(range.clone()) + .and_then(|(excerpt, excerpt_offset)| { + let excerpt_buffer_start = excerpt.range.context.start.to_offset(&excerpt.buffer); + let excerpt_buffer_end = excerpt_buffer_start + excerpt.text_summary.len; let start_in_buffer = - excerpt_buffer_start + range.start.saturating_sub(*cursor.start()); - let end_in_buffer = - excerpt_buffer_start + range.end.saturating_sub(*cursor.start()); - let mut ancestor_buffer_range = start_excerpt + excerpt_buffer_start + range.start.saturating_sub(excerpt_offset); + let end_in_buffer = excerpt_buffer_start + range.end.saturating_sub(excerpt_offset); + let mut ancestor_buffer_range = excerpt .buffer .range_for_syntax_ancestor(start_in_buffer..end_in_buffer)?; ancestor_buffer_range.start = cmp::max(ancestor_buffer_range.start, excerpt_buffer_start); ancestor_buffer_range.end = cmp::min(ancestor_buffer_range.end, excerpt_buffer_end); - let start = cursor.start() + (ancestor_buffer_range.start - excerpt_buffer_start); - let end = cursor.start() + (ancestor_buffer_range.end - excerpt_buffer_start); + let start = excerpt_offset + (ancestor_buffer_range.start - excerpt_buffer_start); + let end = excerpt_offset + (ancestor_buffer_range.end - excerpt_buffer_start); Some(start..end) }) } @@ -2929,6 +2927,31 @@ impl MultiBufferSnapshot { None } + /// Returns the excerpt containing range and its offset start within the multibuffer or none if `range` spans multiple excerpts + fn excerpt_containing<'a, T: ToOffset>( + &'a self, + range: Range, + ) -> Option<(&'a Excerpt, usize)> { + let range = range.start.to_offset(self)..range.end.to_offset(self); + + let mut cursor = self.excerpts.cursor::(); + cursor.seek(&range.start, Bias::Right, &()); + let start_excerpt = cursor.item(); + + cursor.seek(&range.end, Bias::Right, &()); + let end_excerpt = cursor.item(); + + start_excerpt + .zip(end_excerpt) + .and_then(|(start_excerpt, end_excerpt)| { + if start_excerpt.id != end_excerpt.id { + return None; + } + + Some((start_excerpt, *cursor.start())) + }) + } + pub fn remote_selections_in_range<'a>( &'a self, range: &'a Range, diff --git a/crates/editor/src/selections_collection.rs b/crates/editor/src/selections_collection.rs index f1c19bca8a..f3ce89adc5 100644 --- a/crates/editor/src/selections_collection.rs +++ b/crates/editor/src/selections_collection.rs @@ -659,6 +659,31 @@ impl<'a> MutableSelectionsCollection<'a> { } } + pub fn move_offsets_with( + &mut self, + mut move_selection: impl FnMut(&MultiBufferSnapshot, &mut Selection), + ) { + let mut changed = false; + let snapshot = self.buffer().clone(); + let selections = self + .all::(self.cx) + .into_iter() + .map(|selection| { + let mut moved_selection = selection.clone(); + move_selection(&snapshot, &mut moved_selection); + if selection != moved_selection { + changed = true; + } + moved_selection + }) + .collect(); + drop(snapshot); + + if changed { + self.select(selections) + } + } + pub fn move_heads_with( &mut self, mut update_head: impl FnMut( diff --git a/crates/editor/src/test/editor_lsp_test_context.rs b/crates/editor/src/test/editor_lsp_test_context.rs index b65b09cf17..7f92190489 100644 --- a/crates/editor/src/test/editor_lsp_test_context.rs +++ b/crates/editor/src/test/editor_lsp_test_context.rs @@ -1,4 +1,5 @@ use std::{ + borrow::Cow, ops::{Deref, DerefMut, Range}, sync::Arc, }; @@ -7,7 +8,8 @@ use anyhow::Result; use futures::Future; use gpui::{json, ViewContext, ViewHandle}; -use language::{point_to_lsp, FakeLspAdapter, Language, LanguageConfig}; +use indoc::indoc; +use language::{point_to_lsp, FakeLspAdapter, Language, LanguageConfig, LanguageQueries}; use lsp::{notification, request}; use project::Project; use smol::stream::StreamExt; @@ -125,6 +127,32 @@ impl<'a> EditorLspTestContext<'a> { Self::new(language, capabilities, cx).await } + pub async fn new_typescript( + capabilities: lsp::ServerCapabilities, + cx: &'a mut gpui::TestAppContext, + ) -> EditorLspTestContext<'a> { + let language = Language::new( + LanguageConfig { + name: "Typescript".into(), + path_suffixes: vec!["ts".to_string()], + ..Default::default() + }, + Some(tree_sitter_typescript::language_typescript()), + ) + .with_queries(LanguageQueries { + brackets: Some(Cow::from(indoc! {r#" + ("(" @open ")" @close) + ("[" @open "]" @close) + ("{" @open "}" @close) + ("<" @open ">" @close) + ("\"" @open "\"" @close)"#})), + ..Default::default() + }) + .expect("Could not parse brackets"); + + Self::new(language, capabilities, cx).await + } + // Constructs lsp range using a marked string with '[', ']' range delimiters pub fn lsp_range(&mut self, marked_text: &str) -> lsp::Range { let ranges = self.ranges(marked_text); diff --git a/crates/editor/src/test/editor_test_context.rs b/crates/editor/src/test/editor_test_context.rs index d8dbfee171..8f8647e88d 100644 --- a/crates/editor/src/test/editor_test_context.rs +++ b/crates/editor/src/test/editor_test_context.rs @@ -162,10 +162,13 @@ impl<'a> EditorTestContext<'a> { /// embedded range markers that represent the ranges and directions of /// each selection. /// + /// Returns a context handle so that assertion failures can print what + /// editor state was needed to cause the failure. + /// /// See the `util::test::marked_text_ranges` function for more information. pub fn set_state(&mut self, marked_text: &str) -> ContextHandle { let _state_context = self.add_assertion_context(format!( - "Editor State: \"{}\"", + "Initial Editor State: \"{}\"", marked_text.escape_debug().to_string() )); let (unmarked_text, selection_ranges) = marked_text_ranges(marked_text, true); diff --git a/crates/gpui/src/app/ref_counts.rs b/crates/gpui/src/app/ref_counts.rs index a9ae6e7a6c..f0c1699f16 100644 --- a/crates/gpui/src/app/ref_counts.rs +++ b/crates/gpui/src/app/ref_counts.rs @@ -1,11 +1,15 @@ +#[cfg(any(test, feature = "test-support"))] use std::sync::Arc; use lazy_static::lazy_static; +#[cfg(any(test, feature = "test-support"))] use parking_lot::Mutex; use collections::{hash_map::Entry, HashMap, HashSet}; -use crate::{util::post_inc, ElementStateId}; +#[cfg(any(test, feature = "test-support"))] +use crate::util::post_inc; +use crate::ElementStateId; lazy_static! { static ref LEAK_BACKTRACE: bool = @@ -30,9 +34,8 @@ pub struct RefCounts { } impl RefCounts { - pub fn new( - #[cfg(any(test, feature = "test-support"))] leak_detector: Arc>, - ) -> Self { + #[cfg(any(test, feature = "test-support"))] + pub fn new(leak_detector: Arc>) -> Self { Self { #[cfg(any(test, feature = "test-support"))] leak_detector, diff --git a/crates/gpui/src/app/test_app_context.rs b/crates/gpui/src/app/test_app_context.rs index 0805cdd865..1366e7e6ed 100644 --- a/crates/gpui/src/app/test_app_context.rs +++ b/crates/gpui/src/app/test_app_context.rs @@ -621,6 +621,8 @@ impl ViewHandle { } } +/// Tracks string context to be printed when assertions fail. +/// Often this is done by storing a context string in the manager and returning the handle. #[derive(Clone)] pub struct AssertionContextManager { id: Arc, @@ -651,6 +653,9 @@ impl AssertionContextManager { } } +/// Used to track the lifetime of a piece of context so that it can be provided when an assertion fails. +/// For example, in the EditorTestContext, `set_state` returns a context handle so that if an assertion fails, +/// the state that was set initially for the failure can be printed in the error message pub struct ContextHandle { id: usize, manager: AssertionContextManager, diff --git a/crates/language/Cargo.toml b/crates/language/Cargo.toml index 8f1d3d39ed..3e6561e471 100644 --- a/crates/language/Cargo.toml +++ b/crates/language/Cargo.toml @@ -66,6 +66,7 @@ settings = { path = "../settings", features = ["test-support"] } util = { path = "../util", features = ["test-support"] } ctor = "0.1" env_logger = "0.9" +indoc = "1.0.4" rand = "0.8.3" tree-sitter-embedded-template = "*" tree-sitter-html = "*" diff --git a/crates/language/src/buffer.rs b/crates/language/src/buffer.rs index 13b3a86822..953e059518 100644 --- a/crates/language/src/buffer.rs +++ b/crates/language/src/buffer.rs @@ -2346,12 +2346,13 @@ impl BufferSnapshot { Some(items) } - pub fn enclosing_bracket_ranges( - &self, + pub fn enclosing_bracket_ranges<'a, T: ToOffset>( + &'a self, range: Range, - ) -> Option<(Range, Range)> { + ) -> impl Iterator, Range)> + 'a { // Find bracket pairs that *inclusively* contain the given range. let range = range.start.to_offset(self)..range.end.to_offset(self); + let mut matches = self.syntax.matches( range.start.saturating_sub(1)..self.len().min(range.end + 1), &self.text, @@ -2363,39 +2364,31 @@ impl BufferSnapshot { .map(|grammar| grammar.brackets_config.as_ref().unwrap()) .collect::>(); - // Get the ranges of the innermost pair of brackets. - let mut result: Option<(Range, Range)> = None; - while let Some(mat) = matches.peek() { - let mut open = None; - let mut close = None; - let config = &configs[mat.grammar_index]; - for capture in mat.captures { - if capture.index == config.open_capture_ix { - open = Some(capture.node.byte_range()); - } else if capture.index == config.close_capture_ix { - close = Some(capture.node.byte_range()); + iter::from_fn(move || { + while let Some(mat) = matches.peek() { + let mut open = None; + let mut close = None; + let config = &configs[mat.grammar_index]; + for capture in mat.captures { + if capture.index == config.open_capture_ix { + open = Some(capture.node.byte_range()); + } else if capture.index == config.close_capture_ix { + close = Some(capture.node.byte_range()); + } } - } - matches.advance(); + matches.advance(); - let Some((open, close)) = open.zip(close) else { continue }; - if open.start > range.start || close.end < range.end { - continue; - } - let len = close.end - open.start; + let Some((open, close)) = open.zip(close) else { continue }; - if let Some((existing_open, existing_close)) = &result { - let existing_len = existing_close.end - existing_open.start; - if len > existing_len { + if open.start > range.start || close.end < range.end { continue; } + + return Some((open, close)); } - - result = Some((open, close)); - } - - result + None + }) } #[allow(clippy::type_complexity)] diff --git a/crates/language/src/buffer_tests.rs b/crates/language/src/buffer_tests.rs index 59fd6a534b..4d2c9670c6 100644 --- a/crates/language/src/buffer_tests.rs +++ b/crates/language/src/buffer_tests.rs @@ -3,6 +3,7 @@ use clock::ReplicaId; use collections::BTreeMap; use fs::LineEnding; use gpui::{ModelHandle, MutableAppContext}; +use indoc::indoc; use proto::deserialize_operation; use rand::prelude::*; use settings::Settings; @@ -15,7 +16,7 @@ use std::{ }; use text::network::Network; use unindent::Unindent as _; -use util::{post_inc, test::marked_text_ranges, RandomCharIter}; +use util::{assert_set_eq, post_inc, test::marked_text_ranges, RandomCharIter}; #[cfg(test)] #[ctor::ctor] @@ -576,53 +577,117 @@ async fn test_symbols_containing(cx: &mut gpui::TestAppContext) { #[gpui::test] fn test_enclosing_bracket_ranges(cx: &mut MutableAppContext) { - cx.set_global(Settings::test(cx)); - let buffer = cx.add_model(|cx| { - let text = " - mod x { - mod y { + let mut assert = |selection_text, range_markers| { + assert_enclosing_bracket_pairs(selection_text, range_markers, rust_lang(), cx) + }; + assert( + indoc! {" + mod x { + moˇd y { + } } - " - .unindent(); - Buffer::new(0, text, cx).with_language(Arc::new(rust_lang()), cx) - }); - let buffer = buffer.read(cx); - assert_eq!( - buffer.enclosing_bracket_point_ranges(Point::new(1, 6)..Point::new(1, 6)), - Some(( - Point::new(0, 6)..Point::new(0, 7), - Point::new(4, 0)..Point::new(4, 1) - )) - ); - assert_eq!( - buffer.enclosing_bracket_point_ranges(Point::new(1, 10)..Point::new(1, 10)), - Some(( - Point::new(1, 10)..Point::new(1, 11), - Point::new(3, 4)..Point::new(3, 5) - )) - ); - assert_eq!( - buffer.enclosing_bracket_point_ranges(Point::new(3, 5)..Point::new(3, 5)), - Some(( - Point::new(1, 10)..Point::new(1, 11), - Point::new(3, 4)..Point::new(3, 5) - )) + let foo = 1;"}, + vec![indoc! {" + mod x «{» + mod y { + + } + «}» + let foo = 1;"}], ); - assert_eq!( - buffer.enclosing_bracket_point_ranges(Point::new(4, 1)..Point::new(4, 1)), - Some(( - Point::new(0, 6)..Point::new(0, 7), - Point::new(4, 0)..Point::new(4, 1) - )) + assert( + indoc! {" + mod x { + mod y ˇ{ + + } + } + let foo = 1;"}, + vec![ + indoc! {" + mod x «{» + mod y { + + } + «}» + let foo = 1;"}, + indoc! {" + mod x { + mod y «{» + + «}» + } + let foo = 1;"}, + ], + ); + + assert( + indoc! {" + mod x { + mod y { + + }ˇ + } + let foo = 1;"}, + vec![ + indoc! {" + mod x «{» + mod y { + + } + «}» + let foo = 1;"}, + indoc! {" + mod x { + mod y «{» + + «}» + } + let foo = 1;"}, + ], + ); + + assert( + indoc! {" + mod x { + mod y { + + } + ˇ} + let foo = 1;"}, + vec![indoc! {" + mod x «{» + mod y { + + } + «}» + let foo = 1;"}], + ); + + assert( + indoc! {" + mod x { + mod y { + + } + } + let fˇoo = 1;"}, + vec![], ); // Regression test: avoid crash when querying at the end of the buffer. - assert_eq!( - buffer.enclosing_bracket_point_ranges(Point::new(4, 1)..Point::new(5, 0)), - None + assert( + indoc! {" + mod x { + mod y { + + } + } + let foo = 1;ˇ"}, + vec![], ); } @@ -630,52 +695,33 @@ fn test_enclosing_bracket_ranges(cx: &mut MutableAppContext) { fn test_enclosing_bracket_ranges_where_brackets_are_not_outermost_children( cx: &mut MutableAppContext, ) { - let javascript_language = Arc::new( - Language::new( - LanguageConfig { - name: "JavaScript".into(), - ..Default::default() - }, - Some(tree_sitter_javascript::language()), - ) - .with_brackets_query( - r#" - ("{" @open "}" @close) - ("(" @open ")" @close) - "#, - ) - .unwrap(), - ); + let mut assert = |selection_text, bracket_pair_texts| { + assert_enclosing_bracket_pairs(selection_text, bracket_pair_texts, javascript_lang(), cx) + }; - cx.set_global(Settings::test(cx)); - let buffer = cx.add_model(|cx| { - let text = " - for (const a in b) { - // a comment that's longer than the for-loop header - } - " - .unindent(); - Buffer::new(0, text, cx).with_language(javascript_language, cx) - }); - - let buffer = buffer.read(cx); - assert_eq!( - buffer.enclosing_bracket_point_ranges(Point::new(0, 18)..Point::new(0, 18)), - Some(( - Point::new(0, 4)..Point::new(0, 5), - Point::new(0, 17)..Point::new(0, 18) - )) + assert( + indoc! {" + for (const a in b)ˇ { + // a comment that's longer than the for-loop header + }"}, + vec![indoc! {" + for «(»const a in b«)» { + // a comment that's longer than the for-loop header + }"}], ); // Regression test: even though the parent node of the parentheses (the for loop) does // intersect the given range, the parentheses themselves do not contain the range, so // they should not be returned. Only the curly braces contain the range. - assert_eq!( - buffer.enclosing_bracket_point_ranges(Point::new(0, 20)..Point::new(0, 20)), - Some(( - Point::new(0, 19)..Point::new(0, 20), - Point::new(2, 0)..Point::new(2, 1) - )) + assert( + indoc! {" + for (const a in b) {ˇ + // a comment that's longer than the for-loop header + }"}, + vec![indoc! {" + for (const a in b) «{» + // a comment that's longer than the for-loop header + «}»"}], ); } @@ -1892,21 +1938,6 @@ fn test_contiguous_ranges() { ); } -impl Buffer { - pub fn enclosing_bracket_point_ranges( - &self, - range: Range, - ) -> Option<(Range, Range)> { - self.snapshot() - .enclosing_bracket_ranges(range) - .map(|(start, end)| { - let point_start = start.start.to_point(self)..start.end.to_point(self); - let point_end = end.start.to_point(self)..end.end.to_point(self); - (point_start, point_end) - }) - } -} - fn ruby_lang() -> Language { Language::new( LanguageConfig { @@ -1990,6 +2021,23 @@ fn json_lang() -> Language { ) } +fn javascript_lang() -> Language { + Language::new( + LanguageConfig { + name: "JavaScript".into(), + ..Default::default() + }, + Some(tree_sitter_javascript::language()), + ) + .with_brackets_query( + r#" + ("{" @open "}" @close) + ("(" @open ")" @close) + "#, + ) + .unwrap() +} + fn get_tree_sexp(buffer: &ModelHandle, cx: &gpui::TestAppContext) -> String { buffer.read_with(cx, |buffer, _| { let snapshot = buffer.snapshot(); @@ -1997,3 +2045,36 @@ fn get_tree_sexp(buffer: &ModelHandle, cx: &gpui::TestAppContext) -> Str layers[0].node.to_sexp() }) } + +// Assert that the enclosing bracket ranges around the selection match the pairs indicated by the marked text in `range_markers` +fn assert_enclosing_bracket_pairs( + selection_text: &'static str, + bracket_pair_texts: Vec<&'static str>, + language: Language, + cx: &mut MutableAppContext, +) { + cx.set_global(Settings::test(cx)); + let (expected_text, selection_ranges) = marked_text_ranges(selection_text, false); + let buffer = cx.add_model(|cx| { + Buffer::new(0, expected_text.clone(), cx).with_language(Arc::new(language), cx) + }); + let buffer = buffer.update(cx, |buffer, _cx| buffer.snapshot()); + + let selection_range = selection_ranges[0].clone(); + + let bracket_pairs = bracket_pair_texts + .into_iter() + .map(|pair_text| { + let (bracket_text, ranges) = marked_text_ranges(pair_text, false); + assert_eq!(bracket_text, expected_text); + (ranges[0].clone(), ranges[1].clone()) + }) + .collect::>(); + + assert_set_eq!( + buffer + .enclosing_bracket_ranges(selection_range) + .collect::>(), + bracket_pairs + ); +} diff --git a/crates/vim/src/motion.rs b/crates/vim/src/motion.rs index 8bc7c756e0..7c821d6fec 100644 --- a/crates/vim/src/motion.rs +++ b/crates/vim/src/motion.rs @@ -452,8 +452,9 @@ fn end_of_document(map: &DisplaySnapshot, point: DisplayPoint, line: usize) -> D fn matching(map: &DisplaySnapshot, point: DisplayPoint) -> DisplayPoint { let offset = point.to_offset(map, Bias::Left); - if let Some((open_range, close_range)) = - map.buffer_snapshot.enclosing_bracket_ranges(offset..offset) + if let Some((open_range, close_range)) = map + .buffer_snapshot + .innermost_enclosing_bracket_ranges(offset..offset) { if open_range.contains(&offset) { close_range.start.to_display_point(map) From 30caeeaeb597470fb714f5caccdffbe11eae3a14 Mon Sep 17 00:00:00 2001 From: Kay Simmons Date: Wed, 15 Feb 2023 14:11:00 -0800 Subject: [PATCH 2/3] fix comment typo --- crates/editor/src/multi_buffer.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/crates/editor/src/multi_buffer.rs b/crates/editor/src/multi_buffer.rs index 7b85799b31..908e5c827d 100644 --- a/crates/editor/src/multi_buffer.rs +++ b/crates/editor/src/multi_buffer.rs @@ -2648,7 +2648,8 @@ impl MultiBufferSnapshot { result } - /// Returns enclosingn bracket ranges containing the given range or returns None if the range is not contained in a single excerpt + /// Returns enclosinng bracket ranges containing the given range or returns None if the range is + /// not contained in a single excerpt pub fn enclosing_bracket_ranges<'a, T: ToOffset>( &'a self, range: Range, From 33306846a6d94bad5e6f143473c1f363b9c026ac Mon Sep 17 00:00:00 2001 From: Kay Simmons Date: Wed, 15 Feb 2023 14:28:50 -0800 Subject: [PATCH 3/3] add tree-sitter-typescript to editor crate test support features --- crates/editor/Cargo.toml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/crates/editor/Cargo.toml b/crates/editor/Cargo.toml index 7f1bec1d85..fd704becd2 100644 --- a/crates/editor/Cargo.toml +++ b/crates/editor/Cargo.toml @@ -17,7 +17,8 @@ test-support = [ "project/test-support", "util/test-support", "workspace/test-support", - "tree-sitter-rust" + "tree-sitter-rust", + "tree-sitter-typescript" ] [dependencies]