From 57a7ff9a6fce4797ebe13533d0ba911ff832fe31 Mon Sep 17 00:00:00 2001 From: Kay Simmons Date: Fri, 17 Feb 2023 14:52:36 -0800 Subject: [PATCH] fix vim percent motion to better match the docs and observed behavior --- crates/editor/src/multi_buffer.rs | 84 ++++++++++++------- .../src/test/editor_lsp_test_context.rs | 7 ++ crates/language/src/buffer.rs | 19 ++--- crates/language/src/buffer_tests.rs | 4 +- crates/vim/src/motion.rs | 58 ++++++++++--- crates/vim/src/normal.rs | 53 ++++++++---- crates/vim/test_data/test_o.json | 2 +- crates/vim/test_data/test_percent.json | 1 + 8 files changed, 150 insertions(+), 78 deletions(-) create mode 100644 crates/vim/test_data/test_percent.json diff --git a/crates/editor/src/multi_buffer.rs b/crates/editor/src/multi_buffer.rs index 7cc0031c4f..ad661b84ee 100644 --- a/crates/editor/src/multi_buffer.rs +++ b/crates/editor/src/multi_buffer.rs @@ -2630,6 +2630,9 @@ impl MultiBufferSnapshot { self.parse_count } + /// Returns the smallest enclosing bracket ranges containing the given range or + /// None if no brackets contain range or the range is not contained in a single + /// excerpt pub fn innermost_enclosing_bracket_ranges( &self, range: Range, @@ -2657,46 +2660,59 @@ impl MultiBufferSnapshot { result } - /// Returns enclosinng bracket ranges containing the given range or returns None if the range is + /// Returns enclosing 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(excerpt_offset); - let end_in_buffer = excerpt_buffer_start + range.end.saturating_sub(excerpt_offset); + self.bracket_ranges(range.clone()).map(|range_pairs| { + range_pairs + .filter(move |(open, close)| open.start <= range.start && close.end >= range.end) + }) + } - 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; - } + /// Returns bracket range pairs overlapping the given `range` or returns None if the `range` is + /// not contained in a single excerpt + pub fn bracket_ranges<'a, T: ToOffset>( + &'a self, + range: Range, + ) -> Option, Range)> + 'a> { + let range = range.start.to_offset(self)..range.end.to_offset(self); + let excerpt = self.excerpt_containing(range.clone()); + excerpt.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 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 start_in_buffer = excerpt_buffer_start + range.start.saturating_sub(excerpt_offset); + let end_in_buffer = excerpt_buffer_start + range.end.saturating_sub(excerpt_offset); - 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)) - }) - }) + excerpt + .buffer + .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)) + }) + }) } pub fn diagnostics_update_count(&self) -> usize { @@ -2945,10 +2961,14 @@ impl MultiBufferSnapshot { let range = range.start.to_offset(self)..range.end.to_offset(self); let mut cursor = self.excerpts.cursor::(); - cursor.seek(&range.start, Bias::Right, &()); + cursor.seek(&dbg!(range.start), Bias::Right, &()); let start_excerpt = cursor.item(); - cursor.seek(&range.end, Bias::Right, &()); + if range.start == range.end { + return start_excerpt.map(|excerpt| (excerpt, *cursor.start())); + } + + cursor.seek(&dbg!(range.end), Bias::Right, &()); let end_excerpt = cursor.item(); start_excerpt diff --git a/crates/editor/src/test/editor_lsp_test_context.rs b/crates/editor/src/test/editor_lsp_test_context.rs index 938de169a7..345709abf3 100644 --- a/crates/editor/src/test/editor_lsp_test_context.rs +++ b/crates/editor/src/test/editor_lsp_test_context.rs @@ -139,6 +139,13 @@ impl<'a> EditorLspTestContext<'a> { (_ "<" ">" @end) @indent (_ "{" "}" @end) @indent (_ "(" ")" @end) @indent"#})), + brackets: Some(Cow::from(indoc! {r#" + ("(" @open ")" @close) + ("[" @open "]" @close) + ("{" @open "}" @close) + ("<" @open ">" @close) + ("\"" @open "\"" @close) + (closure_parameters "|" @open "|" @close)"#})), ..Default::default() }) .expect("Could not parse queries"); diff --git a/crates/language/src/buffer.rs b/crates/language/src/buffer.rs index 2119f86bbd..5f7dd97049 100644 --- a/crates/language/src/buffer.rs +++ b/crates/language/src/buffer.rs @@ -2346,18 +2346,18 @@ impl BufferSnapshot { Some(items) } - pub fn enclosing_bracket_ranges<'a, T: ToOffset>( + /// Returns bracket range pairs overlapping `range` + pub fn bracket_ranges<'a, T: ToOffset>( &'a self, 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 range = range.start.to_offset(self).saturating_sub(1) + ..self.len().min(range.end.to_offset(self) + 1); - let mut matches = self.syntax.matches( - range.start.saturating_sub(1)..self.len().min(range.end + 1), - &self.text, - |grammar| grammar.brackets_config.as_ref().map(|c| &c.query), - ); + let mut matches = self.syntax.matches(range, &self.text, |grammar| { + grammar.brackets_config.as_ref().map(|c| &c.query) + }); let configs = matches .grammars() .iter() @@ -2380,11 +2380,6 @@ impl BufferSnapshot { matches.advance(); let Some((open, close)) = open.zip(close) else { continue }; - - if open.start > range.start || close.end < range.end { - continue; - } - return Some((open, close)); } None diff --git a/crates/language/src/buffer_tests.rs b/crates/language/src/buffer_tests.rs index 4d2c9670c6..a24c7e227f 100644 --- a/crates/language/src/buffer_tests.rs +++ b/crates/language/src/buffer_tests.rs @@ -2072,9 +2072,7 @@ fn assert_enclosing_bracket_pairs( .collect::>(); assert_set_eq!( - buffer - .enclosing_bracket_ranges(selection_range) - .collect::>(), + buffer.bracket_ranges(selection_range).collect::>(), bracket_pairs ); } diff --git a/crates/vim/src/motion.rs b/crates/vim/src/motion.rs index 7c821d6fec..25188a466c 100644 --- a/crates/vim/src/motion.rs +++ b/crates/vim/src/motion.rs @@ -3,7 +3,7 @@ use std::sync::Arc; use editor::{ char_kind, display_map::{DisplaySnapshot, ToDisplayPoint}, - movement, Bias, CharKind, DisplayPoint, + movement, Bias, CharKind, DisplayPoint, ToOffset, }; use gpui::{actions, impl_actions, MutableAppContext}; use language::{Point, Selection, SelectionGoal}; @@ -450,19 +450,53 @@ fn end_of_document(map: &DisplaySnapshot, point: DisplayPoint, line: usize) -> D map.clip_point(new_point, Bias::Left) } -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 - .innermost_enclosing_bracket_ranges(offset..offset) - { - if open_range.contains(&offset) { - close_range.start.to_display_point(map) - } else { - open_range.start.to_display_point(map) +fn matching(map: &DisplaySnapshot, display_point: DisplayPoint) -> DisplayPoint { + // https://github.com/vim/vim/blob/1d87e11a1ef201b26ed87585fba70182ad0c468a/runtime/doc/motion.txt#L1200 + let point = display_point.to_point(map); + let offset = point.to_offset(&map.buffer_snapshot); + + // Ensure the range is contained by the current line. + let mut line_end = map.next_line_boundary(point).0; + if line_end == point { + line_end = map.max_point().to_point(map); + } + line_end.column = line_end.column.saturating_sub(1); + + let line_range = map.prev_line_boundary(point).0..line_end; + let ranges = map.buffer_snapshot.bracket_ranges(line_range.clone()); + if let Some(ranges) = ranges { + let line_range = line_range.start.to_offset(&map.buffer_snapshot) + ..line_range.end.to_offset(&map.buffer_snapshot); + let mut closest_pair_destination = None; + let mut closest_distance = usize::MAX; + + for (open_range, close_range) in ranges { + if open_range.start >= offset && line_range.contains(&open_range.start) { + let distance = open_range.start - offset; + if distance < closest_distance { + closest_pair_destination = Some(close_range.start); + closest_distance = distance; + continue; + } + } + + if close_range.start >= offset && line_range.contains(&close_range.start) { + let distance = close_range.start - offset; + if distance < closest_distance { + closest_pair_destination = Some(open_range.start); + closest_distance = distance; + continue; + } + } + + continue; } + + closest_pair_destination + .map(|destination| destination.to_display_point(map)) + .unwrap_or(display_point) } else { - point + display_point } } diff --git a/crates/vim/src/normal.rs b/crates/vim/src/normal.rs index 2dd2d489a5..0cac45fd18 100644 --- a/crates/vim/src/normal.rs +++ b/crates/vim/src/normal.rs @@ -824,17 +824,34 @@ mod test { ˇ brown fox"}) .await; - cx.assert(indoc! {" + + cx.assert_manual( + indoc! {" fn test() { println!(ˇ); - } - "}) - .await; - cx.assert(indoc! {" + }"}, + Mode::Normal, + indoc! {" + fn test() { + println!(); + ˇ + }"}, + Mode::Insert, + ); + + cx.assert_manual( + indoc! {" fn test(ˇ) { println!(); - }"}) - .await; + }"}, + Mode::Normal, + indoc! {" + fn test() { + ˇ + println!(); + }"}, + Mode::Insert, + ); } #[gpui::test] @@ -996,14 +1013,14 @@ mod test { #[gpui::test] async fn test_capital_f_and_capital_t(cx: &mut gpui::TestAppContext) { let mut cx = NeovimBackedTestContext::new(cx).await; - for count in 1..=3 { - let test_case = indoc! {" - ˇaaaˇbˇ ˇbˇ ˇbˇbˇ aˇaaˇbaaa - ˇ ˇbˇaaˇa ˇbˇbˇb - ˇ - ˇb + let test_case = indoc! {" + ˇaaaˇbˇ ˇbˇ ˇbˇbˇ aˇaaˇbaaa + ˇ ˇbˇaaˇa ˇbˇbˇb + ˇ + ˇb "}; + for count in 1..=3 { cx.assert_binding_matches_all([&count.to_string(), "shift-f", "b"], test_case) .await; @@ -1014,10 +1031,10 @@ mod test { #[gpui::test] async fn test_percent(cx: &mut gpui::TestAppContext) { - let mut cx = NeovimBackedTestContext::new(cx).await; - for count in 1..=2 { - // let test_case = indoc! {" - // "} - } + let mut cx = NeovimBackedTestContext::new(cx).await.binding(["%"]); + cx.assert_all("ˇconsole.logˇ(ˇvaˇrˇ)ˇ;").await; + cx.assert_all("ˇconsole.logˇ(ˇ'var', ˇ[ˇ1, ˇ2, 3ˇ]ˇ)ˇ;") + .await; + cx.assert_all("let result = curried_funˇ(ˇ)ˇ(ˇ)ˇ;").await; } } diff --git a/crates/vim/test_data/test_o.json b/crates/vim/test_data/test_o.json index 08bea7cae8..fa1a400bc0 100644 --- a/crates/vim/test_data/test_o.json +++ b/crates/vim/test_data/test_o.json @@ -1 +1 @@ -[{"Text":"\n"},{"Mode":"Insert"},{"Selection":{"start":[1,0],"end":[1,0]}},{"Mode":"Insert"},{"Text":"The quick\n"},{"Mode":"Insert"},{"Selection":{"start":[1,0],"end":[1,0]}},{"Mode":"Insert"},{"Text":"The quick\n\nbrown fox\njumps over"},{"Mode":"Insert"},{"Selection":{"start":[1,0],"end":[1,0]}},{"Mode":"Insert"},{"Text":"The quick\nbrown fox\n\njumps over"},{"Mode":"Insert"},{"Selection":{"start":[2,0],"end":[2,0]}},{"Mode":"Insert"},{"Text":"The quick\nbrown fox\njumps over\n"},{"Mode":"Insert"},{"Selection":{"start":[3,0],"end":[3,0]}},{"Mode":"Insert"},{"Text":"The quick\n\n\nbrown fox"},{"Mode":"Insert"},{"Selection":{"start":[2,0],"end":[2,0]}},{"Mode":"Insert"},{"Text":"fn test() {\n println!();\n \n}\n"},{"Mode":"Insert"},{"Selection":{"start":[2,4],"end":[2,4]}},{"Mode":"Insert"},{"Text":"fn test() {\n\n println!();\n}"},{"Mode":"Insert"},{"Selection":{"start":[1,0],"end":[1,0]}},{"Mode":"Insert"}] \ No newline at end of file +[{"Text":"\n"},{"Mode":"Insert"},{"Selection":{"start":[1,0],"end":[1,0]}},{"Mode":"Insert"},{"Text":"The quick\n"},{"Mode":"Insert"},{"Selection":{"start":[1,0],"end":[1,0]}},{"Mode":"Insert"},{"Text":"The quick\n\nbrown fox\njumps over"},{"Mode":"Insert"},{"Selection":{"start":[1,0],"end":[1,0]}},{"Mode":"Insert"},{"Text":"The quick\nbrown fox\n\njumps over"},{"Mode":"Insert"},{"Selection":{"start":[2,0],"end":[2,0]}},{"Mode":"Insert"},{"Text":"The quick\nbrown fox\njumps over\n"},{"Mode":"Insert"},{"Selection":{"start":[3,0],"end":[3,0]}},{"Mode":"Insert"},{"Text":"The quick\n\n\nbrown fox"},{"Mode":"Insert"},{"Selection":{"start":[2,0],"end":[2,0]}},{"Mode":"Insert"}] \ No newline at end of file diff --git a/crates/vim/test_data/test_percent.json b/crates/vim/test_data/test_percent.json new file mode 100644 index 0000000000..9dc0fc655b --- /dev/null +++ b/crates/vim/test_data/test_percent.json @@ -0,0 +1 @@ +[{"Text":"console.log(var);"},{"Mode":"Normal"},{"Selection":{"start":[0,15],"end":[0,15]}},{"Mode":"Normal"},{"Text":"console.log(var);"},{"Mode":"Normal"},{"Selection":{"start":[0,15],"end":[0,15]}},{"Mode":"Normal"},{"Text":"console.log(var);"},{"Mode":"Normal"},{"Selection":{"start":[0,11],"end":[0,11]}},{"Mode":"Normal"},{"Text":"console.log(var);"},{"Mode":"Normal"},{"Selection":{"start":[0,11],"end":[0,11]}},{"Mode":"Normal"},{"Text":"console.log(var);"},{"Mode":"Normal"},{"Selection":{"start":[0,11],"end":[0,11]}},{"Mode":"Normal"},{"Text":"console.log(var);"},{"Mode":"Normal"},{"Selection":{"start":[0,16],"end":[0,16]}},{"Mode":"Normal"},{"Text":"console.log('var', [1, 2, 3]);"},{"Mode":"Normal"},{"Selection":{"start":[0,28],"end":[0,28]}},{"Mode":"Normal"},{"Text":"console.log('var', [1, 2, 3]);"},{"Mode":"Normal"},{"Selection":{"start":[0,28],"end":[0,28]}},{"Mode":"Normal"},{"Text":"console.log('var', [1, 2, 3]);"},{"Mode":"Normal"},{"Selection":{"start":[0,27],"end":[0,27]}},{"Mode":"Normal"},{"Text":"console.log('var', [1, 2, 3]);"},{"Mode":"Normal"},{"Selection":{"start":[0,27],"end":[0,27]}},{"Mode":"Normal"},{"Text":"console.log('var', [1, 2, 3]);"},{"Mode":"Normal"},{"Selection":{"start":[0,19],"end":[0,19]}},{"Mode":"Normal"},{"Text":"console.log('var', [1, 2, 3]);"},{"Mode":"Normal"},{"Selection":{"start":[0,19],"end":[0,19]}},{"Mode":"Normal"},{"Text":"console.log('var', [1, 2, 3]);"},{"Mode":"Normal"},{"Selection":{"start":[0,19],"end":[0,19]}},{"Mode":"Normal"},{"Text":"console.log('var', [1, 2, 3]);"},{"Mode":"Normal"},{"Selection":{"start":[0,11],"end":[0,11]}},{"Mode":"Normal"},{"Text":"console.log('var', [1, 2, 3]);"},{"Mode":"Normal"},{"Selection":{"start":[0,29],"end":[0,29]}},{"Mode":"Normal"},{"Text":"let result = curried_fun()();"},{"Mode":"Normal"},{"Selection":{"start":[0,25],"end":[0,25]}},{"Mode":"Normal"},{"Text":"let result = curried_fun()();"},{"Mode":"Normal"},{"Selection":{"start":[0,24],"end":[0,24]}},{"Mode":"Normal"},{"Text":"let result = curried_fun()();"},{"Mode":"Normal"},{"Selection":{"start":[0,27],"end":[0,27]}},{"Mode":"Normal"},{"Text":"let result = curried_fun()();"},{"Mode":"Normal"},{"Selection":{"start":[0,26],"end":[0,26]}},{"Mode":"Normal"},{"Text":"let result = curried_fun()();"},{"Mode":"Normal"},{"Selection":{"start":[0,28],"end":[0,28]}},{"Mode":"Normal"}] \ No newline at end of file