Fix incorrect highlighting when an empty range is highlighted via the DisplayMap

Co-Authored-By: Keith Simmons <keith@zed.dev>

crates/editor/src/display_map.rs

@@ -489,13 +489,17 @@ impl ToDisplayPoint for Anchor {
 #[cfg(test)]
 mod tests {
     use super::*;
-    use crate::movement;
+    use crate::{
+        movement,
+        test::{marked_text, marked_text_ranges},
+    };
     use gpui::{color::Color, elements::*, test::observe, MutableAppContext};
     use language::{Buffer, Language, LanguageConfig, RandomCharIter, SelectionGoal};
     use rand::{prelude::*, Rng};
     use smol::stream::StreamExt;
     use std::{env, sync::Arc};
     use theme::SyntaxTheme;
+    use unindent::Unindent;
     use util::test::sample_text;
     use Bias::*;
 
@@ -929,7 +933,7 @@ mod tests {
         let map = cx
             .add_model(|cx| DisplayMap::new(buffer, tab_size, font_id, font_size, None, 1, 1, cx));
         assert_eq!(
-            cx.update(|cx| chunks(0..5, &map, &theme, cx)),
+            cx.update(|cx| syntax_chunks(0..5, &map, &theme, cx)),
             vec![
                 ("fn ".to_string(), None),
                 ("outer".to_string(), Some(Color::blue())),
@@ -940,7 +944,7 @@ mod tests {
             ]
         );
         assert_eq!(
-            cx.update(|cx| chunks(3..5, &map, &theme, cx)),
+            cx.update(|cx| syntax_chunks(3..5, &map, &theme, cx)),
             vec![
                 ("    fn ".to_string(), Some(Color::red())),
                 ("inner".to_string(), Some(Color::blue())),
@@ -952,7 +956,7 @@ mod tests {
             map.fold(vec![Point::new(0, 6)..Point::new(3, 2)], cx)
         });
         assert_eq!(
-            cx.update(|cx| chunks(0..2, &map, &theme, cx)),
+            cx.update(|cx| syntax_chunks(0..2, &map, &theme, cx)),
             vec![
                 ("fn ".to_string(), None),
                 ("out".to_string(), Some(Color::blue())),
@@ -1018,7 +1022,7 @@ mod tests {
             DisplayMap::new(buffer, tab_size, font_id, font_size, Some(40.0), 1, 1, cx)
         });
         assert_eq!(
-            cx.update(|cx| chunks(0..5, &map, &theme, cx)),
+            cx.update(|cx| syntax_chunks(0..5, &map, &theme, cx)),
             [
                 ("fn \n".to_string(), None),
                 ("oute\nr".to_string(), Some(Color::blue())),
@@ -1026,7 +1030,7 @@ mod tests {
             ]
         );
         assert_eq!(
-            cx.update(|cx| chunks(3..5, &map, &theme, cx)),
+            cx.update(|cx| syntax_chunks(3..5, &map, &theme, cx)),
             [("{}\n\n".to_string(), None)]
         );
 
@@ -1034,7 +1038,7 @@ mod tests {
             map.fold(vec![Point::new(0, 6)..Point::new(3, 2)], cx)
         });
         assert_eq!(
-            cx.update(|cx| chunks(1..4, &map, &theme, cx)),
+            cx.update(|cx| syntax_chunks(1..4, &map, &theme, cx)),
             [
                 ("out".to_string(), Some(Color::blue())),
                 ("…\n".to_string(), None),
@@ -1044,6 +1048,89 @@ mod tests {
         );
     }
 
+    #[gpui::test]
+    async fn test_chunks_with_text_highlights(cx: &mut gpui::TestAppContext) {
+        cx.foreground().set_block_on_ticks(usize::MAX..=usize::MAX);
+
+        let theme = SyntaxTheme::new(vec![
+            ("operator".to_string(), Color::red().into()),
+            ("string".to_string(), Color::green().into()),
+        ]);
+        let language = Arc::new(
+            Language::new(
+                LanguageConfig {
+                    name: "Test".into(),
+                    path_suffixes: vec![".test".to_string()],
+                    ..Default::default()
+                },
+                Some(tree_sitter_rust::language()),
+            )
+            .with_highlights_query(
+                r#"
+                ":" @operator
+                (string_literal) @string
+                "#,
+            )
+            .unwrap(),
+        );
+        language.set_theme(&theme);
+
+        let (text, highlighted_ranges) = marked_text_ranges(
+            r#"const{} <a>: B = "c [d]""#,
+            vec![('{', '}'), ('<', '>'), ('[', ']')],
+        );
+
+        let buffer = cx.add_model(|cx| Buffer::new(0, text, cx).with_language(language, cx));
+        buffer.condition(&cx, |buf, _| !buf.is_parsing()).await;
+
+        let buffer = cx.add_model(|cx| MultiBuffer::singleton(buffer, cx));
+        let buffer_snapshot = buffer.read_with(cx, |buffer, cx| buffer.snapshot(cx));
+
+        let font_cache = cx.font_cache();
+        let tab_size = 4;
+        let family_id = font_cache.load_family(&["Courier"]).unwrap();
+        let font_id = font_cache
+            .select_font(family_id, &Default::default())
+            .unwrap();
+        let font_size = 16.0;
+        let map = cx
+            .add_model(|cx| DisplayMap::new(buffer, tab_size, font_id, font_size, None, 1, 1, cx));
+
+        enum MyType {}
+
+        let style = HighlightStyle {
+            color: Some(Color::blue()),
+            ..Default::default()
+        };
+
+        map.update(cx, |map, _cx| {
+            map.highlight_text(
+                TypeId::of::<MyType>(),
+                highlighted_ranges
+                    .into_iter()
+                    .map(|range| {
+                        buffer_snapshot.anchor_before(range.start)
+                            ..buffer_snapshot.anchor_before(range.end)
+                    })
+                    .collect(),
+                style,
+            );
+        });
+
+        assert_eq!(
+            cx.update(|cx| chunks(0..10, &map, &theme, cx)),
+            [
+                ("const ".to_string(), None, None),
+                ("a".to_string(), None, Some(Color::blue())),
+                (":".to_string(), Some(Color::red()), None),
+                (" B = ".to_string(), None, None),
+                ("\"c ".to_string(), Some(Color::green()), None),
+                ("d".to_string(), Some(Color::green()), Some(Color::blue())),
+                ("\"".to_string(), Some(Color::green()), None),
+            ]
+        );
+    }
+
     #[gpui::test]
     fn test_clip_point(cx: &mut gpui::MutableAppContext) {
         use Bias::{Left, Right};
@@ -1170,27 +1257,38 @@ mod tests {
         )
     }
 
-    fn chunks<'a>(
+    fn syntax_chunks<'a>(
         rows: Range<u32>,
         map: &ModelHandle<DisplayMap>,
         theme: &'a SyntaxTheme,
         cx: &mut MutableAppContext,
     ) -> Vec<(String, Option<Color>)> {
+        chunks(rows, map, theme, cx)
+            .into_iter()
+            .map(|(text, color, _)| (text, color))
+            .collect()
+    }
+
+    fn chunks<'a>(
+        rows: Range<u32>,
+        map: &ModelHandle<DisplayMap>,
+        theme: &'a SyntaxTheme,
+        cx: &mut MutableAppContext,
+    ) -> Vec<(String, Option<Color>, Option<Color>)> {
         let snapshot = map.update(cx, |map, cx| map.snapshot(cx));
-        let mut chunks: Vec<(String, Option<Color>)> = Vec::new();
+        let mut chunks: Vec<(String, Option<Color>, Option<Color>)> = Vec::new();
         for chunk in snapshot.chunks(rows, true) {
-            let color = chunk
+            let syntax_color = chunk
                 .syntax_highlight_id
                 .and_then(|id| id.style(theme)?.color);
-            if let Some((last_chunk, last_color)) = chunks.last_mut() {
+            let highlight_color = chunk.highlight_style.and_then(|style| style.color);
-                if color == *last_color {
+            if let Some((last_chunk, last_syntax_color, last_highlight_color)) = chunks.last_mut() {
+                if syntax_color == *last_syntax_color && highlight_color == *last_highlight_color {
                     last_chunk.push_str(chunk.text);
-                } else {
+                    continue;
-                    chunks.push((chunk.text.to_string(), color));
                 }
-            } else {
-                chunks.push((chunk.text.to_string(), color));
             }
+            chunks.push((chunk.text.to_string(), syntax_color, highlight_color));
         }
         chunks
     }

crates/editor/src/display_map/fold_map.rs

@@ -1156,7 +1156,7 @@ impl Ord for HighlightEndpoint {
     fn cmp(&self, other: &Self) -> Ordering {
         self.offset
             .cmp(&other.offset)
-            .then_with(|| self.is_start.cmp(&other.is_start))
+            .then_with(|| other.is_start.cmp(&self.is_start))
     }
 }
 

crates/editor/src/editor.rs

@@ -4,6 +4,8 @@ pub mod items;
 pub mod movement;
 mod multi_buffer;
 
+pub mod repro;
+
 #[cfg(test)]
 mod test;
 
@@ -4404,7 +4406,15 @@ impl Editor {
                                 .into_iter()
                                 .flat_map(|(_, ranges)| ranges),
                         )
-                        .collect();
+                        .collect::<Vec<_>>();
+
+                    let snapshot = this.buffer.read(cx).snapshot(cx);
+                    let point_ranges = ranges
+                        .iter()
+                        .map(|range| range.to_point(&snapshot))
+                        .collect::<Vec<_>>();
+                    dbg!(point_ranges);
+
                     this.highlight_text::<Rename>(
                         ranges,
                         HighlightStyle {

crates/editor/src/test.rs

@@ -1,3 +1,5 @@
+use std::ops::Range;
+
 use collections::HashMap;
 
 #[cfg(test)]
@@ -31,3 +33,24 @@ pub fn marked_text(marked_text: &str) -> (String, Vec<usize>) {
     let (unmarked_text, mut markers) = marked_text_by(marked_text, vec!['|']);
     (unmarked_text, markers.remove(&'|').unwrap_or_else(Vec::new))
 }
+
+pub fn marked_text_ranges(
+    marked_text: &str,
+    range_markers: Vec<(char, char)>,
+) -> (String, Vec<Range<usize>>) {
+    let mut marker_chars = Vec::new();
+    for (start, end) in range_markers.iter() {
+        marker_chars.push(*start);
+        marker_chars.push(*end);
+    }
+    let (unmarked_text, markers) = marked_text_by(marked_text, marker_chars);
+    let ranges = range_markers
+        .iter()
+        .map(|(start_marker, end_marker)| {
+            let start = markers.get(start_marker).unwrap()[0];
+            let end = markers.get(end_marker).unwrap()[0];
+            start..end
+        })
+        .collect();
+    (unmarked_text, ranges)
+}