highlight both brackets, only when empty selection, and add test

Keith Simmons created

Change summary

crates/editor/src/editor.rs                     |  12 +
crates/editor/src/highlight_matching_bracket.rs | 150 +++++++++++++++---
crates/editor/src/multi_buffer.rs               |   2 
crates/editor/src/test.rs                       |  64 +++++--
crates/util/src/test/assertions.rs              |   4 
crates/util/src/test/marked_text.rs             |  17 ++
6 files changed, 200 insertions(+), 49 deletions(-)

Detailed changes

crates/editor/src/editor.rs 🔗

@@ -5372,7 +5372,7 @@ impl Editor {
             .map(|h| &h.1);
         let write_highlights = self
             .background_highlights
-            .get(&TypeId::of::<DocumentHighlightRead>())
+            .get(&TypeId::of::<DocumentHighlightWrite>())
             .map(|h| &h.1);
         let left_position = position.bias_left(buffer);
         let right_position = position.bias_right(buffer);
@@ -10281,3 +10281,13 @@ impl<T: Ord + Clone> RangeExt<T> for Range<T> {
         self.start.clone()..=self.end.clone()
     }
 }
+
+trait RangeToAnchorExt {
+    fn to_anchors(self, snapshot: &MultiBufferSnapshot) -> Range<Anchor>;
+}
+
+impl<T: ToOffset> RangeToAnchorExt for Range<T> {
+    fn to_anchors(self, snapshot: &MultiBufferSnapshot) -> Range<Anchor> {
+        snapshot.anchor_after(self.start)..snapshot.anchor_before(self.end)
+    }
+}

crates/editor/src/highlight_matching_bracket.rs 🔗

@@ -1,6 +1,6 @@
 use gpui::ViewContext;
 
-use crate::Editor;
+use crate::{Editor, RangeToAnchorExt};
 
 enum MatchingBracketHighlight {}
 
@@ -8,33 +8,135 @@ pub fn refresh_matching_bracket_highlights(editor: &mut Editor, cx: &mut ViewCon
     editor.clear_background_highlights::<MatchingBracketHighlight>(cx);
 
     let newest_selection = editor.selections.newest::<usize>(cx);
+    // Don't highlight brackets if the selection isn't empty
+    if !newest_selection.is_empty() {
+        return;
+    }
+
+    let head = newest_selection.head();
     let snapshot = editor.snapshot(cx);
     if let Some((opening_range, closing_range)) = snapshot
         .buffer_snapshot
-        .enclosing_bracket_ranges(newest_selection.range())
+        .enclosing_bracket_ranges(head..head)
     {
-        let head = newest_selection.head();
-        let range_to_highlight = if opening_range.contains(&head) {
-            Some(closing_range)
-        } else if closing_range.contains(&head) {
-            Some(opening_range)
-        } else {
-            None
-        };
-
-        if let Some(range_to_highlight) = range_to_highlight {
-            let anchor_range = snapshot
-                .buffer_snapshot
-                .anchor_before(range_to_highlight.start)
-                ..snapshot
-                    .buffer_snapshot
-                    .anchor_after(range_to_highlight.end);
-
-            editor.highlight_background::<MatchingBracketHighlight>(
-                vec![anchor_range],
-                |theme| theme.editor.document_highlight_read_background,
-                cx,
+        editor.highlight_background::<MatchingBracketHighlight>(
+            vec![
+                opening_range.to_anchors(&snapshot.buffer_snapshot),
+                closing_range.to_anchors(&snapshot.buffer_snapshot),
+            ],
+            |theme| theme.editor.document_highlight_read_background,
+            cx,
+        )
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use indoc::indoc;
+
+    use language::{BracketPair, Language, LanguageConfig};
+
+    use crate::test::EditorLspTestContext;
+
+    use super::*;
+
+    #[gpui::test]
+    async fn test_matching_bracket_highlights(cx: &mut gpui::TestAppContext) {
+        let mut cx = EditorLspTestContext::new(
+            Language::new(
+                LanguageConfig {
+                    name: "Rust".into(),
+                    path_suffixes: vec!["rs".to_string()],
+                    brackets: vec![
+                        BracketPair {
+                            start: "{".to_string(),
+                            end: "}".to_string(),
+                            close: false,
+                            newline: true,
+                        },
+                        BracketPair {
+                            start: "(".to_string(),
+                            end: ")".to_string(),
+                            close: false,
+                            newline: true,
+                        },
+                    ],
+                    ..Default::default()
+                },
+                Some(tree_sitter_rust::language()),
             )
-        }
+            .with_brackets_query(indoc! {r#"
+                ("{" @open "}" @close)
+                ("(" @open ")" @close)
+                "#})
+            .unwrap(),
+            Default::default(),
+            cx,
+        )
+        .await;
+
+        // positioning cursor inside bracket highlights both
+        cx.set_state_by(
+            vec!['|'.into()],
+            indoc! {r#"
+                pub fn test("Test |argument") {
+                    another_test(1, 2, 3);
+                }"#},
+        );
+        cx.assert_editor_background_highlights::<MatchingBracketHighlight>(indoc! {r#"
+                pub fn test[(]"Test argument"[)] {
+                    another_test(1, 2, 3);
+                }"#});
+
+        cx.set_state_by(
+            vec!['|'.into()],
+            indoc! {r#"
+                pub fn test("Test argument") {
+                    another_test(1, |2, 3);
+                }"#},
+        );
+        cx.assert_editor_background_highlights::<MatchingBracketHighlight>(indoc! {r#"
+            pub fn test("Test argument") {
+                another_test[(]1, 2, 3[)];
+            }"#});
+
+        cx.set_state_by(
+            vec!['|'.into()],
+            indoc! {r#"
+                pub fn test("Test argument") {
+                    another|_test(1, 2, 3);
+                }"#},
+        );
+        cx.assert_editor_background_highlights::<MatchingBracketHighlight>(indoc! {r#"
+            pub fn test("Test argument") [{]
+                another_test(1, 2, 3);
+            [}]"#});
+
+        // positioning outside of brackets removes highlight
+        cx.set_state_by(
+            vec!['|'.into()],
+            indoc! {r#"
+                pub f|n test("Test argument") {
+                    another_test(1, 2, 3);
+                }"#},
+        );
+        cx.assert_editor_background_highlights::<MatchingBracketHighlight>(indoc! {r#"
+            pub fn test("Test argument") {
+                another_test(1, 2, 3);
+            }"#});
+
+        // non empty selection dismisses highlight
+        // positioning outside of brackets removes highlight
+        cx.set_state_by(
+            vec![('<', '>').into()],
+            indoc! {r#"
+                pub fn test("Te<st arg>ument") {
+                    another_test(1, 2, 3);
+                }"#},
+        );
+        cx.assert_editor_background_highlights::<MatchingBracketHighlight>(indoc! {r#"
+            pub fn test("Test argument") {
+                another_test(1, 2, 3);
+            }"#});
     }
 }

crates/editor/src/multi_buffer.rs 🔗

@@ -2321,7 +2321,7 @@ impl MultiBufferSnapshot {
                     .enclosing_bracket_ranges(start_in_buffer..end_in_buffer)?;
 
                 if start_bracket_range.start >= excerpt_buffer_start
-                    && end_bracket_range.end < excerpt_buffer_end
+                    && end_bracket_range.end <= excerpt_buffer_end
                 {
                     start_bracket_range.start =
                         cursor.start() + (start_bracket_range.start - excerpt_buffer_start);

crates/editor/src/test.rs 🔗

@@ -1,4 +1,5 @@
 use std::{
+    any::TypeId,
     ops::{Deref, DerefMut, Range},
     sync::Arc,
 };
@@ -13,7 +14,7 @@ use project::Project;
 use settings::Settings;
 use util::{
     assert_set_eq, set_eq,
-    test::{marked_text, marked_text_ranges, marked_text_ranges_by, SetEqError},
+    test::{marked_text, marked_text_ranges, marked_text_ranges_by, SetEqError, TextRangeMarker},
 };
 use workspace::{pane, AppState, Workspace, WorkspaceHandle};
 
@@ -159,29 +160,30 @@ impl<'a> EditorTestContext<'a> {
     // `[` to `}` represents a non empty selection with the head at `}`
     // `{` to `]` represents a non empty selection with the head at `{`
     pub fn set_state(&mut self, text: &str) {
+        self.set_state_by(
+            vec![
+                '|'.into(),
+                ('[', '}').into(),
+                TextRangeMarker::ReverseRange('{', ']'),
+            ],
+            text,
+        );
+    }
+
+    pub fn set_state_by(&mut self, range_markers: Vec<TextRangeMarker>, text: &str) {
         self.editor.update(self.cx, |editor, cx| {
-            let (unmarked_text, mut selection_ranges) = marked_text_ranges_by(
-                &text,
-                vec!['|'.into(), ('[', '}').into(), ('{', ']').into()],
-            );
+            let (unmarked_text, selection_ranges) = marked_text_ranges_by(&text, range_markers);
             editor.set_text(unmarked_text, cx);
 
-            let mut selections: Vec<Range<usize>> =
-                selection_ranges.remove(&'|'.into()).unwrap_or_default();
-            selections.extend(
-                selection_ranges
-                    .remove(&('{', ']').into())
-                    .unwrap_or_default()
-                    .into_iter()
-                    .map(|range| range.end..range.start),
-            );
-            selections.extend(
-                selection_ranges
-                    .remove(&('[', '}').into())
-                    .unwrap_or_default(),
-            );
-
-            editor.change_selections(Some(Autoscroll::Fit), cx, |s| s.select_ranges(selections));
+            let selection_ranges: Vec<Range<usize>> = selection_ranges
+                .values()
+                .into_iter()
+                .flatten()
+                .cloned()
+                .collect();
+            editor.change_selections(Some(Autoscroll::Fit), cx, |s| {
+                s.select_ranges(selection_ranges)
+            })
         })
     }
 
@@ -216,6 +218,26 @@ impl<'a> EditorTestContext<'a> {
         )
     }
 
+    pub fn assert_editor_background_highlights<Tag: 'static>(&mut self, marked_text: &str) {
+        let (unmarked, mut ranges) = marked_text_ranges_by(marked_text, vec![('[', ']').into()]);
+        assert_eq!(unmarked, self.buffer_text());
+
+        let asserted_ranges = ranges.remove(&('[', ']').into()).unwrap();
+        let actual_ranges: Vec<Range<usize>> = self.update_editor(|editor, cx| {
+            let snapshot = editor.snapshot(cx);
+            editor
+                .background_highlights
+                .get(&TypeId::of::<Tag>())
+                .map(|h| h.1.clone())
+                .unwrap_or_default()
+                .into_iter()
+                .map(|range| range.to_offset(&snapshot.buffer_snapshot))
+                .collect()
+        });
+
+        assert_set_eq!(asserted_ranges, actual_ranges);
+    }
+
     pub fn assert_editor_text_highlights<Tag: ?Sized + 'static>(&mut self, marked_text: &str) {
         let (unmarked, mut ranges) = marked_text_ranges_by(marked_text, vec![('[', ']').into()]);
         assert_eq!(unmarked, self.buffer_text());

crates/util/src/test/assertions.rs 🔗

@@ -51,10 +51,10 @@ macro_rules! assert_set_eq {
 
         match set_eq!(&left, &right) {
             Err(SetEqError::LeftMissing(missing)) => {
-                panic!("assertion failed: `(left == right)`\n left: {:?}\nright: {:?}\nright does not contain {:?}", &left, &right, &missing);
+                panic!("assertion failed: `(left == right)`\n left: {:?}\nright: {:?}\nleft does not contain {:?}", &left, &right, &missing);
             },
             Err(SetEqError::RightMissing(missing)) => {
-                panic!("assertion failed: `(left == right)`\n left: {:?}\nright: {:?}\nleft does not contain {:?}", &left, &right, &missing);
+                panic!("assertion failed: `(left == right)`\n left: {:?}\nright: {:?}\nright does not contain {:?}", &left, &right, &missing);
             },
             _ => {}
         }

crates/util/src/test/marked_text.rs 🔗

@@ -28,6 +28,7 @@ pub fn marked_text(marked_text: &str) -> (String, Vec<usize>) {
 pub enum TextRangeMarker {
     Empty(char),
     Range(char, char),
+    ReverseRange(char, char),
 }
 
 impl TextRangeMarker {
@@ -35,6 +36,7 @@ impl TextRangeMarker {
         match self {
             Self::Empty(m) => vec![*m],
             Self::Range(l, r) => vec![*l, *r],
+            Self::ReverseRange(l, r) => vec![*l, *r],
         }
     }
 }
@@ -85,6 +87,21 @@ pub fn marked_text_ranges_by(
                     .collect::<Vec<Range<usize>>>();
                 (marker, ranges)
             }
+            TextRangeMarker::ReverseRange(start_marker, end_marker) => {
+                let starts = marker_offsets.remove(&start_marker).unwrap_or_default();
+                let ends = marker_offsets.remove(&end_marker).unwrap_or_default();
+                assert_eq!(starts.len(), ends.len(), "marked ranges are unbalanced");
+
+                let ranges = starts
+                    .into_iter()
+                    .zip(ends)
+                    .map(|(start, end)| {
+                        assert!(start >= end, "marked ranges must be disjoint");
+                        end..start
+                    })
+                    .collect::<Vec<Range<usize>>>();
+                (marker, ranges)
+            }
         })
         .collect();