Account for markdown styling in mentions offset calculation.

Piotr Osiewicz and Max created

This also means that we can support smart punctuation.

Co-authored-by: Max <max@zed.dev>

Change summary

Cargo.lock                                        |   1 
crates/collab_ui/Cargo.toml                       |   1 
crates/collab_ui/src/chat_panel.rs                | 119 +++++++++++++---
crates/collab_ui/src/chat_panel/message_editor.rs |   2 
crates/rich_text/src/rich_text.rs                 | 105 +++++++++++----
crates/theme/src/theme.rs                         |  30 ++--
styles/src/style_tree/chat_panel.ts               |  45 ++----
7 files changed, 207 insertions(+), 96 deletions(-)

Detailed changes

Cargo.lock 🔗

@@ -1566,6 +1566,7 @@ dependencies = [
  "notifications",
  "picker",
  "postage",
+ "pretty_assertions",
  "project",
  "recent_projects",
  "rich_text",

crates/collab_ui/Cargo.toml 🔗

@@ -75,4 +75,5 @@ settings = { path = "../settings", features = ["test-support"] }
 util = { path = "../util", features = ["test-support"] }
 workspace = { path = "../workspace", features = ["test-support"] }
 
+pretty_assertions.workspace = true
 tree-sitter-markdown.workspace = true

crates/collab_ui/src/chat_panel.rs 🔗

@@ -382,20 +382,7 @@ impl ChatPanel {
         let is_pending = message.is_pending();
         let theme = theme::current(cx);
         let text = self.markdown_data.entry(message.id).or_insert_with(|| {
-            let mut markdown =
-                rich_text::render_markdown(message.body.clone(), &self.languages, None);
-            let self_client_id = self.client.id();
-            for (mention_range, user_id) in message.mentions {
-                let is_current_user = self_client_id == user_id;
-                markdown
-                    .add_mention(
-                        mention_range,
-                        is_current_user,
-                        theme.chat_panel.mention_highlight.clone(),
-                    )
-                    .log_err();
-            }
-            markdown
+            Self::render_markdown_with_mentions(&self.languages, self.client.id(), &message)
         });
 
         let now = OffsetDateTime::now_utc();
@@ -419,15 +406,13 @@ impl ChatPanel {
 
         enum MessageBackgroundHighlight {}
         MouseEventHandler::new::<MessageBackgroundHighlight, _>(ix, cx, |state, cx| {
-            let container = style.container.style_for(state);
+            let container = style.style_for(state);
             if is_continuation {
                 Flex::row()
                     .with_child(
                         text.element(
                             theme.editor.syntax.clone(),
-                            style.body.clone(),
-                            theme.editor.document_highlight_read_background,
-                            theme.chat_panel.self_mention_background,
+                            theme.chat_panel.rich_text.clone(),
                             cx,
                         )
                         .flex(1., true),
@@ -455,10 +440,10 @@ impl ChatPanel {
                                     .with_child(
                                         Label::new(
                                             message.sender.github_login.clone(),
-                                            style.sender.text.clone(),
+                                            theme.chat_panel.message_sender.text.clone(),
                                         )
                                         .contained()
-                                        .with_style(style.sender.container),
+                                        .with_style(theme.chat_panel.message_sender.container),
                                     )
                                     .with_child(
                                         Label::new(
@@ -467,10 +452,10 @@ impl ChatPanel {
                                                 now,
                                                 self.local_timezone,
                                             ),
-                                            style.timestamp.text.clone(),
+                                            theme.chat_panel.message_timestamp.text.clone(),
                                         )
                                         .contained()
-                                        .with_style(style.timestamp.container),
+                                        .with_style(theme.chat_panel.message_timestamp.container),
                                     )
                                     .align_children_center()
                                     .flex(1., true),
@@ -483,9 +468,7 @@ impl ChatPanel {
                             .with_child(
                                 text.element(
                                     theme.editor.syntax.clone(),
-                                    style.body.clone(),
-                                    theme.editor.document_highlight_read_background,
-                                    theme.chat_panel.self_mention_background,
+                                    theme.chat_panel.rich_text.clone(),
                                     cx,
                                 )
                                 .flex(1., true),
@@ -506,6 +489,23 @@ impl ChatPanel {
         .into_any()
     }
 
+    fn render_markdown_with_mentions(
+        language_registry: &Arc<LanguageRegistry>,
+        current_user_id: u64,
+        message: &channel::ChannelMessage,
+    ) -> RichText {
+        let mentions = message
+            .mentions
+            .iter()
+            .map(|(range, user_id)| rich_text::Mention {
+                range: range.clone(),
+                is_self_mention: *user_id == current_user_id,
+            })
+            .collect::<Vec<_>>();
+
+        rich_text::render_markdown(message.body.clone(), &mentions, language_registry, None)
+    }
+
     fn render_input_box(&self, theme: &Arc<Theme>, cx: &AppContext) -> AnyElement<Self> {
         ChildView::new(&self.input_editor, cx)
             .contained()
@@ -879,3 +879,72 @@ fn render_icon_button<V: View>(style: &IconButton, svg_path: &'static str) -> im
         .contained()
         .with_style(style.container)
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use gpui::fonts::HighlightStyle;
+    use pretty_assertions::assert_eq;
+    use rich_text::{BackgroundKind, Highlight, RenderedRegion};
+    use util::test::marked_text_ranges;
+
+    #[gpui::test]
+    fn test_render_markdown_with_mentions() {
+        let language_registry = Arc::new(LanguageRegistry::test());
+        let (body, ranges) = marked_text_ranges("*hi*, «@abc», let's **call** «@fgh»", false);
+        let message = channel::ChannelMessage {
+            id: ChannelMessageId::Saved(0),
+            body,
+            timestamp: OffsetDateTime::now_utc(),
+            sender: Arc::new(client::User {
+                github_login: "fgh".into(),
+                avatar: None,
+                id: 103,
+            }),
+            nonce: 5,
+            mentions: vec![(ranges[0].clone(), 101), (ranges[1].clone(), 102)],
+        };
+
+        let message = ChatPanel::render_markdown_with_mentions(&language_registry, 102, &message);
+
+        // Note that the "'" was replaced with ’ due to smart punctuation.
+        let (body, ranges) = marked_text_ranges("«hi», «@abc», let’s «call» «@fgh»", false);
+        assert_eq!(message.text, body);
+        assert_eq!(
+            message.highlights,
+            vec![
+                (
+                    ranges[0].clone(),
+                    HighlightStyle {
+                        italic: Some(true),
+                        ..Default::default()
+                    }
+                    .into()
+                ),
+                (ranges[1].clone(), Highlight::Mention),
+                (
+                    ranges[2].clone(),
+                    HighlightStyle {
+                        weight: Some(gpui::fonts::Weight::BOLD),
+                        ..Default::default()
+                    }
+                    .into()
+                ),
+                (ranges[3].clone(), Highlight::SelfMention)
+            ]
+        );
+        assert_eq!(
+            message.regions,
+            vec![
+                RenderedRegion {
+                    background_kind: Some(BackgroundKind::Mention),
+                    link_url: None
+                },
+                RenderedRegion {
+                    background_kind: Some(BackgroundKind::SelfMention),
+                    link_url: None
+                },
+            ]
+        );
+    }
+}

crates/collab_ui/src/chat_panel/message_editor.rs 🔗

@@ -179,7 +179,7 @@ impl MessageEditor {
                 editor.clear_highlights::<Self>(cx);
                 editor.highlight_text::<Self>(
                     anchor_ranges,
-                    theme::current(cx).chat_panel.mention_highlight,
+                    theme::current(cx).chat_panel.rich_text.mention_highlight,
                     cx,
                 )
             });

crates/rich_text/src/rich_text.rs 🔗

@@ -3,19 +3,33 @@ use std::{ops::Range, sync::Arc};
 use anyhow::bail;
 use futures::FutureExt;
 use gpui::{
-    color::Color,
     elements::Text,
-    fonts::{HighlightStyle, TextStyle, Underline, Weight},
+    fonts::{HighlightStyle, Underline, Weight},
     platform::{CursorStyle, MouseButton},
     AnyElement, CursorRegion, Element, MouseRegion, ViewContext,
 };
 use language::{HighlightId, Language, LanguageRegistry};
-use theme::SyntaxTheme;
+use theme::{RichTextStyle, SyntaxTheme};
+use util::RangeExt;
 
 #[derive(Debug, Clone, PartialEq, Eq)]
 pub enum Highlight {
     Id(HighlightId),
     Highlight(HighlightStyle),
+    Mention,
+    SelfMention,
+}
+
+impl From<HighlightStyle> for Highlight {
+    fn from(style: HighlightStyle) -> Self {
+        Self::Highlight(style)
+    }
+}
+
+impl From<HighlightId> for Highlight {
+    fn from(style: HighlightId) -> Self {
+        Self::Id(style)
+    }
 }
 
 #[derive(Debug, Clone)]
@@ -26,25 +40,32 @@ pub struct RichText {
     pub regions: Vec<RenderedRegion>,
 }
 
-#[derive(Clone, Copy, Debug, PartialEq)]
-enum BackgroundKind {
+#[derive(Clone, Copy, Debug, PartialEq, Eq)]
+pub enum BackgroundKind {
     Code,
+    /// A mention background for non-self user.
     Mention,
+    SelfMention,
 }
 
-#[derive(Debug, Clone)]
+#[derive(Debug, Clone, PartialEq, Eq)]
 pub struct RenderedRegion {
-    background_kind: Option<BackgroundKind>,
-    link_url: Option<String>,
+    pub background_kind: Option<BackgroundKind>,
+    pub link_url: Option<String>,
+}
+
+/// Allows one to specify extra links to the rendered markdown, which can be used
+/// for e.g. mentions.
+pub struct Mention {
+    pub range: Range<usize>,
+    pub is_self_mention: bool,
 }
 
 impl RichText {
     pub fn element<V: 'static>(
         &self,
         syntax: Arc<SyntaxTheme>,
-        style: TextStyle,
-        code_span_background_color: Color,
-        self_mention_span_background_color: Color,
+        style: RichTextStyle,
         cx: &mut ViewContext<V>,
     ) -> AnyElement<V> {
         let mut region_id = 0;
@@ -53,7 +74,7 @@ impl RichText {
         let regions = self.regions.clone();
 
         enum Markdown {}
-        Text::new(self.text.clone(), style.clone())
+        Text::new(self.text.clone(), style.text.clone())
             .with_highlights(
                 self.highlights
                     .iter()
@@ -61,6 +82,8 @@ impl RichText {
                         let style = match highlight {
                             Highlight::Id(id) => id.style(&syntax)?,
                             Highlight::Highlight(style) => style.clone(),
+                            Highlight::Mention => style.mention_highlight,
+                            Highlight::SelfMention => style.self_mention_highlight,
                         };
                         Some((range.clone(), style))
                     })
@@ -83,21 +106,24 @@ impl RichText {
                 }
                 if let Some(region_kind) = &region.background_kind {
                     let background = match region_kind {
-                        BackgroundKind::Code => code_span_background_color,
-                        BackgroundKind::Mention => self_mention_span_background_color,
+                        BackgroundKind::Code => style.code_background,
+                        BackgroundKind::Mention => style.mention_background,
+                        BackgroundKind::SelfMention => style.self_mention_background,
+                    };
+                    if background.is_some() {
+                        cx.scene().push_quad(gpui::Quad {
+                            bounds,
+                            background,
+                            border: Default::default(),
+                            corner_radii: (2.0).into(),
+                        });
                     }
-                    .into();
-                    cx.scene().push_quad(gpui::Quad {
-                        bounds,
-                        background,
-                        border: Default::default(),
-                        corner_radii: (2.0).into(),
-                    });
                 }
             })
             .with_soft_wrap(true)
             .into_any()
     }
+
     pub fn add_mention(
         &mut self,
         range: Range<usize>,
@@ -126,6 +152,7 @@ impl RichText {
 
 pub fn render_markdown_mut(
     block: &str,
+    mut mentions: &[Mention],
     language_registry: &Arc<LanguageRegistry>,
     language: Option<&Arc<Language>>,
     data: &mut RichText,
@@ -138,19 +165,40 @@ pub fn render_markdown_mut(
     let mut current_language = None;
     let mut list_stack = Vec::new();
 
-    // Smart Punctuation is disabled as that messes with offsets within the message.
-    let mut options = Options::all();
-    options.remove(Options::ENABLE_SMART_PUNCTUATION);
-
-    for event in Parser::new_ext(&block, options) {
+    let options = Options::all();
+    for (event, source_range) in Parser::new_ext(&block, options).into_offset_iter() {
         let prev_len = data.text.len();
         match event {
             Event::Text(t) => {
                 if let Some(language) = &current_language {
                     render_code(&mut data.text, &mut data.highlights, t.as_ref(), language);
                 } else {
-                    data.text.push_str(t.as_ref());
+                    if let Some(mention) = mentions.first() {
+                        if source_range.contains_inclusive(&mention.range) {
+                            mentions = &mentions[1..];
+                            let range = (prev_len + mention.range.start - source_range.start)
+                                ..(prev_len + mention.range.end - source_range.start);
+                            data.highlights.push((
+                                range.clone(),
+                                if mention.is_self_mention {
+                                    Highlight::SelfMention
+                                } else {
+                                    Highlight::Mention
+                                },
+                            ));
+                            data.region_ranges.push(range);
+                            data.regions.push(RenderedRegion {
+                                background_kind: Some(if mention.is_self_mention {
+                                    BackgroundKind::SelfMention
+                                } else {
+                                    BackgroundKind::Mention
+                                }),
+                                link_url: None,
+                            });
+                        }
+                    }
 
+                    data.text.push_str(t.as_ref());
                     let mut style = HighlightStyle::default();
                     if bold_depth > 0 {
                         style.weight = Some(Weight::BOLD);
@@ -269,6 +317,7 @@ pub fn render_markdown_mut(
 
 pub fn render_markdown(
     block: String,
+    mentions: &[Mention],
     language_registry: &Arc<LanguageRegistry>,
     language: Option<&Arc<Language>>,
 ) -> RichText {
@@ -279,7 +328,7 @@ pub fn render_markdown(
         regions: Default::default(),
     };
 
-    render_markdown_mut(&block, language_registry, language, &mut data);
+    render_markdown_mut(&block, mentions, language_registry, language, &mut data);
 
     data.text = data.text.trim().to_string();
 

crates/theme/src/theme.rs 🔗

@@ -639,16 +639,27 @@ pub struct ChatPanel {
     pub input_editor: FieldEditor,
     pub avatar: AvatarStyle,
     pub avatar_container: ContainerStyle,
-    pub message: ChatMessage,
-    pub mention_highlight: HighlightStyle,
-    pub self_mention_background: Color,
-    pub continuation_message: ChatMessage,
+    pub rich_text: RichTextStyle,
+    pub message_sender: ContainedText,
+    pub message_timestamp: ContainedText,
+    pub message: Interactive<ContainerStyle>,
+    pub continuation_message: Interactive<ContainerStyle>,
+    pub pending_message: Interactive<ContainerStyle>,
     pub last_message_bottom_spacing: f32,
-    pub pending_message: ChatMessage,
     pub sign_in_prompt: Interactive<TextStyle>,
     pub icon_button: Interactive<IconButton>,
 }
 
+#[derive(Clone, Deserialize, Default, JsonSchema)]
+pub struct RichTextStyle {
+    pub text: TextStyle,
+    pub mention_highlight: HighlightStyle,
+    pub mention_background: Option<Color>,
+    pub self_mention_highlight: HighlightStyle,
+    pub self_mention_background: Option<Color>,
+    pub code_background: Option<Color>,
+}
+
 #[derive(Deserialize, Default, JsonSchema)]
 pub struct NotificationPanel {
     #[serde(flatten)]
@@ -667,15 +678,6 @@ pub struct NotificationPanel {
     pub button: Interactive<ContainedText>,
 }
 
-#[derive(Deserialize, Default, JsonSchema)]
-pub struct ChatMessage {
-    #[serde(flatten)]
-    pub container: Interactive<ContainerStyle>,
-    pub body: TextStyle,
-    pub sender: ContainedText,
-    pub timestamp: ContainedText,
-}
-
 #[derive(Deserialize, Default, JsonSchema)]
 pub struct ChannelSelect {
     #[serde(flatten)]

styles/src/style_tree/chat_panel.ts 🔗

@@ -1,6 +1,6 @@
-import { background, border, text } from "./components"
+import { background, border, foreground, text } from "./components"
 import { icon_button } from "../component/icon_button"
-import { useTheme } from "../theme"
+import { useTheme, with_opacity } from "../theme"
 import { interactive } from "../element"
 import { Color } from "ayu/dist/color"
 
@@ -86,8 +86,21 @@ export default function chat_panel(): any {
                 top: 4,
             },
         },
-        mention_highlight: { weight: "bold" },
-        self_mention_background: background(layer, "active"),
+
+        rich_text: {
+            text: text(layer, "sans", "base"),
+            code_background: with_opacity(foreground(layer, "accent"), 0.1),
+            mention_highlight: { weight: "bold" },
+            self_mention_highlight: { weight: "bold" },
+            self_mention_background: background(layer, "active"),
+        },
+        message_sender: {
+            margin: {
+                right: 8,
+            },
+            ...text(layer, "sans", "base", { weight: "bold" }),
+        },
+        message_timestamp: text(layer, "sans", "base", "disabled"),
         message: {
             ...interactive({
                 base: {
@@ -105,25 +118,9 @@ export default function chat_panel(): any {
                     },
                 },
             }),
-            body: text(layer, "sans", "base"),
-            sender: {
-                margin: {
-                    right: 8,
-                },
-                ...text(layer, "sans", "base", { weight: "bold" }),
-            },
-            timestamp: text(layer, "sans", "base", "disabled"),
         },
         last_message_bottom_spacing: SPACING,
         continuation_message: {
-            body: text(layer, "sans", "base"),
-            sender: {
-                margin: {
-                    right: 8,
-                },
-                ...text(layer, "sans", "base", { weight: "bold" }),
-            },
-            timestamp: text(layer, "sans", "base", "disabled"),
             ...interactive({
                 base: {
                     padding: {
@@ -141,14 +138,6 @@ export default function chat_panel(): any {
             }),
         },
         pending_message: {
-            body: text(layer, "sans", "base"),
-            sender: {
-                margin: {
-                    right: 8,
-                },
-                ...text(layer, "sans", "base", "disabled"),
-            },
-            timestamp: text(layer, "sans", "base"),
             ...interactive({
                 base: {
                     padding: {