theme: Properly merge SyntaxTheme styles to allow for partial overrides (#11911)

This PR improves the merging behavior for the `SyntaxTheme` such that
user-provided values get merged into the base theme.

This makes it possible to override individual styles without clobbering
the unspecified styles in the base theme.

Release Notes:

- Improved merging of `syntax` styles in the theme.

crates/theme/src/registry.rs

@@ -118,10 +118,30 @@ impl ThemeRegistry {
             };
             player_colors.merge(&user_theme.style.players);
 
-            let mut syntax_colors = match user_theme.appearance {
+            let syntax_theme = match user_theme.appearance {
                 AppearanceContent::Light => SyntaxTheme::light(),
                 AppearanceContent::Dark => SyntaxTheme::dark(),
             };
+            let syntax_highlights = user_theme
+                .style
+                .syntax
+                .iter()
+                .map(|(syntax_token, highlight)| {
+                    (
+                        syntax_token.clone(),
+                        HighlightStyle {
+                            color: highlight
+                                .color
+                                .as_ref()
+                                .and_then(|color| try_parse_color(color).ok()),
+                            font_style: highlight.font_style.map(Into::into),
+                            font_weight: highlight.font_weight.map(Into::into),
+                            ..Default::default()
+                        },
+                    )
+                })
+                .collect::<Vec<_>>();
+            let syntax_theme = SyntaxTheme::merge(Arc::new(syntax_theme), syntax_highlights);
 
             let window_background_appearance = user_theme
                 .style
@@ -129,28 +149,6 @@ impl ThemeRegistry {
                 .map(Into::into)
                 .unwrap_or_default();
 
-            if !user_theme.style.syntax.is_empty() {
-                syntax_colors.highlights = user_theme
-                    .style
-                    .syntax
-                    .iter()
-                    .map(|(syntax_token, highlight)| {
-                        (
-                            syntax_token.clone(),
-                            HighlightStyle {
-                                color: highlight
-                                    .color
-                                    .as_ref()
-                                    .and_then(|color| try_parse_color(color).ok()),
-                                font_style: highlight.font_style.map(Into::into),
-                                font_weight: highlight.font_weight.map(Into::into),
-                                ..Default::default()
-                            },
-                        )
-                    })
-                    .collect::<Vec<_>>();
-            }
-
             Theme {
                 id: uuid::Uuid::new_v4().to_string(),
                 name: user_theme.name.into(),
@@ -164,7 +162,7 @@ impl ThemeRegistry {
                     colors: theme_colors,
                     status: status_colors,
                     player: player_colors,
-                    syntax: Arc::new(syntax_colors),
+                    syntax: syntax_theme,
                     accents: Vec::new(),
                 },
             }

crates/theme/src/settings.rs

@@ -325,15 +325,8 @@ impl ThemeSettings {
                 .status
                 .refine(&theme_overrides.status_colors_refinement());
             base_theme.styles.player.merge(&theme_overrides.players);
-            base_theme.styles.syntax = Arc::new(SyntaxTheme {
+            base_theme.styles.syntax =
-                highlights: {
+                SyntaxTheme::merge(base_theme.styles.syntax, theme_overrides.syntax_overrides());
-                    let mut highlights = base_theme.styles.syntax.highlights.clone();
-                    // Overrides come second in the highlight list so that they take precedence
-                    // over the ones in the base theme.
-                    highlights.extend(theme_overrides.syntax_overrides());
-                    highlights
-                },
-            });
 
             self.active_theme = Arc::new(base_theme);
         }

crates/theme/src/styles/syntax.rs

@@ -1,3 +1,5 @@
+use std::sync::Arc;
+
 use gpui::{HighlightStyle, Hsla};
 
 use crate::{
@@ -5,7 +7,7 @@ use crate::{
     tomato, yellow,
 };
 
-#[derive(Clone, Default)]
+#[derive(Debug, PartialEq, Eq, Clone, Default)]
 pub struct SyntaxTheme {
     pub highlights: Vec<(String, HighlightStyle)>,
 }
@@ -129,18 +131,25 @@ impl SyntaxTheme {
 
     #[cfg(any(test, feature = "test-support"))]
     pub fn new_test(colors: impl IntoIterator<Item = (&'static str, Hsla)>) -> Self {
-        SyntaxTheme {
+        Self::new_test_styles(colors.into_iter().map(|(key, color)| {
+            (
+                key,
+                HighlightStyle {
+                    color: Some(color),
+                    ..Default::default()
+                },
+            )
+        }))
+    }
+
+    #[cfg(any(test, feature = "test-support"))]
+    pub fn new_test_styles(
+        colors: impl IntoIterator<Item = (&'static str, HighlightStyle)>,
+    ) -> Self {
+        Self {
             highlights: colors
                 .into_iter()
-                .map(|(key, color)| {
+                .map(|(key, style)| (key.to_owned(), style))
-                    (
-                        key.to_owned(),
-                        HighlightStyle {
-                            color: Some(color),
-                            ..Default::default()
-                        },
-                    )
-                })
                 .collect(),
         }
     }
@@ -155,4 +164,148 @@ impl SyntaxTheme {
     pub fn color(&self, name: &str) -> Hsla {
         self.get(name).color.unwrap_or_default()
     }
+
+    /// Returns a new [`Arc<SyntaxTheme>`] with the given syntax styles merged in.
+    pub fn merge(base: Arc<Self>, user_syntax_styles: Vec<(String, HighlightStyle)>) -> Arc<Self> {
+        if user_syntax_styles.is_empty() {
+            return base;
+        }
+
+        let mut merged_highlights = base.highlights.clone();
+
+        for (name, highlight) in user_syntax_styles {
+            if let Some((_, existing_highlight)) = merged_highlights
+                .iter_mut()
+                .find(|(existing_name, _)| existing_name == &name)
+            {
+                existing_highlight.color = highlight.color.or(existing_highlight.color);
+                existing_highlight.font_weight =
+                    highlight.font_weight.or(existing_highlight.font_weight);
+                existing_highlight.font_style =
+                    highlight.font_style.or(existing_highlight.font_style);
+                existing_highlight.background_color = highlight
+                    .background_color
+                    .or(existing_highlight.background_color);
+                existing_highlight.underline = highlight.underline.or(existing_highlight.underline);
+                existing_highlight.strikethrough =
+                    highlight.strikethrough.or(existing_highlight.strikethrough);
+                existing_highlight.fade_out = highlight.fade_out.or(existing_highlight.fade_out);
+            } else {
+                merged_highlights.push((name, highlight));
+            }
+        }
+
+        Arc::new(Self {
+            highlights: merged_highlights,
+        })
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use gpui::FontStyle;
+
+    use super::*;
+
+    #[test]
+    fn test_syntax_theme_merge() {
+        // Merging into an empty `SyntaxTheme` keeps all the user-defined styles.
+        let syntax_theme = SyntaxTheme::merge(
+            Arc::new(SyntaxTheme::new_test([])),
+            vec![
+                (
+                    "foo".to_string(),
+                    HighlightStyle {
+                        color: Some(gpui::red()),
+                        ..Default::default()
+                    },
+                ),
+                (
+                    "foo.bar".to_string(),
+                    HighlightStyle {
+                        color: Some(gpui::green()),
+                        ..Default::default()
+                    },
+                ),
+            ],
+        );
+        assert_eq!(
+            syntax_theme,
+            Arc::new(SyntaxTheme::new_test([
+                ("foo", gpui::red()),
+                ("foo.bar", gpui::green())
+            ]))
+        );
+
+        // Merging empty user-defined styles keeps all the base styles.
+        let syntax_theme = SyntaxTheme::merge(
+            Arc::new(SyntaxTheme::new_test([
+                ("foo", gpui::blue()),
+                ("foo.bar", gpui::red()),
+            ])),
+            Vec::new(),
+        );
+        assert_eq!(
+            syntax_theme,
+            Arc::new(SyntaxTheme::new_test([
+                ("foo", gpui::blue()),
+                ("foo.bar", gpui::red())
+            ]))
+        );
+
+        let syntax_theme = SyntaxTheme::merge(
+            Arc::new(SyntaxTheme::new_test([
+                ("foo", gpui::red()),
+                ("foo.bar", gpui::green()),
+            ])),
+            vec![(
+                "foo.bar".to_string(),
+                HighlightStyle {
+                    color: Some(gpui::yellow()),
+                    ..Default::default()
+                },
+            )],
+        );
+        assert_eq!(
+            syntax_theme,
+            Arc::new(SyntaxTheme::new_test([
+                ("foo", gpui::red()),
+                ("foo.bar", gpui::yellow())
+            ]))
+        );
+
+        let syntax_theme = SyntaxTheme::merge(
+            Arc::new(SyntaxTheme::new_test([
+                ("foo", gpui::red()),
+                ("foo.bar", gpui::green()),
+            ])),
+            vec![(
+                "foo.bar".to_string(),
+                HighlightStyle {
+                    font_style: Some(FontStyle::Italic),
+                    ..Default::default()
+                },
+            )],
+        );
+        assert_eq!(
+            syntax_theme,
+            Arc::new(SyntaxTheme::new_test_styles([
+                (
+                    "foo",
+                    HighlightStyle {
+                        color: Some(gpui::red()),
+                        ..Default::default()
+                    }
+                ),
+                (
+                    "foo.bar",
+                    HighlightStyle {
+                        color: Some(gpui::green()),
+                        font_style: Some(FontStyle::Italic),
+                        ..Default::default()
+                    }
+                )
+            ]))
+        );
+    }
 }