gpui: Make refining a `Style` properly refine the `TextStyle` (#42852)

Serophots and Antonio Scandurra created 20 hours ago

## Motivating problem
The gpui API currently has this counter intuitive behaviour

```rust
 div()
            .id("hallo")
            .cursor_pointer()
            .text_color(white())
            .font_weight(FontWeight::SEMIBOLD)
            .text_size(px(20.0))
            .child("hallo")
            .active(|this| this.text_color(red()))
```
By changing the text_color when the div is active, the current behaviour
is to overwrite all of the text styling rather than do a proper
refinement of the existing text styling leading to this odd result:
The button being active inadvertently changes the font size.


https://github.com/user-attachments/assets/1ff51169-0d76-4ee5-bbb0-004eb9ffdf2c



## Solution
Previously refining a Style would not recursively refine the TextStyle
inside of it, leading to this behaviour:
```rust
let mut style = Style::default();
style.refine(&StyleRefinement::default().text_size(px(20.0)));
style.refine(&StyleRefinement::default().font_weight(FontWeight::SEMIBOLD));

assert!(style.text_style().unwrap().font_size.is_none());
//assertion passes
```

(As best as I can tell) Style deliberately has `pub text:
TextStyleRefinement` storing the `TextStyleRefinement` rather than the
absolute `TextStyle` so that these refinements can be elsewhere used in
cascading text styles down to element's children. But a consequence of
that is that the refine macro was not properly recursively refining the
`text` field as it ought to.

I've modified the refine macro so that the `#[refineable]` attribute
works with `TextStyleRefinement` as well as the usual `TextStyle`.
(Perhaps a little bit haphazardly by simply checking whether the name
ends in Refinement - there may be a better solution there).

This PR resolves the motivating problem and triggers the assertion in
the above code as you'd expect. I've compiled zed under these changes
and all seems to be in order there.

Release Notes:

- N/A

---------

Co-authored-by: Antonio Scandurra <[e-mail]>

Change summary

crates/acp_tools/src/acp_tools.rs                            |   4 
crates/agent_ui/src/acp/thread_view.rs                       |   4 
crates/edit_prediction_ui/src/rate_prediction_modal.rs       |   4 
crates/gpui/src/style.rs                                     |  18 
crates/gpui/src/styled.rs                                    | 112 +----
crates/markdown/examples/markdown_as_child.rs                |   4 
crates/markdown/src/markdown.rs                              |  14 
crates/refineable/derive_refineable/src/derive_refineable.rs |   7 
crates/refineable/src/refineable.rs                          |   2 
crates/ui/src/components/keybinding_hint.rs                  |   4 
crates/ui/src/components/label/label_like.rs                 |   4 
11 files changed, 74 insertions(+), 103 deletions(-)

Detailed changes

crates/acp_tools/src/acp_tools.rs

@@ -371,13 +371,13 @@ impl AcpTools {
                                     syntax: cx.theme().syntax().clone(),
                                     code_block_overflow_x_scroll: true,
                                     code_block: StyleRefinement {
-                                        text: Some(TextStyleRefinement {
+                                        text: TextStyleRefinement {
                                             font_family: Some(
                                                 theme_settings.buffer_font.family.clone(),
                                             ),
                                             font_size: Some((base_size * 0.8).into()),
                                             ..Default::default()
-                                        }),
+                                        },
                                         ..Default::default()
                                     },
                                     ..Default::default()

crates/agent_ui/src/acp/thread_view.rs

@@ -6053,13 +6053,13 @@ fn default_markdown_style(
             },
             border_color: Some(colors.border_variant),
             background: Some(colors.editor_background.into()),
-            text: Some(TextStyleRefinement {
+            text: TextStyleRefinement {
                 font_family: Some(theme_settings.buffer_font.family.clone()),
                 font_fallbacks: theme_settings.buffer_font.fallbacks.clone(),
                 font_features: Some(theme_settings.buffer_font.features.clone()),
                 font_size: Some(buffer_font_size.into()),
                 ..Default::default()
-            }),
+            },
             ..Default::default()
         },
         inline_code: TextStyleRefinement {

crates/edit_prediction_ui/src/rate_prediction_modal.rs

@@ -510,13 +510,13 @@ impl RatePredictionsModal {
                                     base_text_style: window.text_style(),
                                     syntax: cx.theme().syntax().clone(),
                                     code_block: StyleRefinement {
-                                        text: Some(TextStyleRefinement {
+                                        text: TextStyleRefinement {
                                             font_family: Some(
                                                 theme_settings.buffer_font.family.clone(),
                                             ),
                                             font_size: Some(buffer_font_size.into()),
                                             ..Default::default()
-                                        }),
+                                        },
                                         padding: EdgesRefinement {
                                             top: Some(DefiniteLength::Absolute(
                                                 AbsoluteLength::Pixels(px(8.)),

crates/gpui/src/style.rs

@@ -252,6 +252,7 @@ pub struct Style {
     pub box_shadow: Vec<BoxShadow>,
 
     /// The text style of this element
+    #[refineable]
     pub text: TextStyleRefinement,
 
     /// The mouse cursor style shown when the mouse pointer is over an element.
@@ -1469,4 +1470,21 @@ mod tests {
             ]
         );
     }
+
+    #[perf]
+    fn test_text_style_refinement() {
+        let mut style = Style::default();
+        style.refine(&StyleRefinement::default().text_size(px(20.0)));
+        style.refine(&StyleRefinement::default().font_weight(FontWeight::SEMIBOLD));
+
+        assert_eq!(
+            Some(AbsoluteLength::from(px(20.0))),
+            style.text_style().unwrap().font_size
+        );
+
+        assert_eq!(
+            Some(FontWeight::SEMIBOLD),
+            style.text_style().unwrap().font_weight
+        );
+    }
 }

crates/gpui/src/styled.rs

@@ -64,43 +64,33 @@ pub trait Styled: Sized {
     /// Sets the whitespace of the element to `normal`.
     /// [Docs](https://tailwindcss.com/docs/whitespace#normal)
     fn whitespace_normal(mut self) -> Self {
-        self.text_style()
-            .get_or_insert_with(Default::default)
-            .white_space = Some(WhiteSpace::Normal);
+        self.text_style().white_space = Some(WhiteSpace::Normal);
         self
     }
 
     /// Sets the whitespace of the element to `nowrap`.
     /// [Docs](https://tailwindcss.com/docs/whitespace#nowrap)
     fn whitespace_nowrap(mut self) -> Self {
-        self.text_style()
-            .get_or_insert_with(Default::default)
-            .white_space = Some(WhiteSpace::Nowrap);
+        self.text_style().white_space = Some(WhiteSpace::Nowrap);
         self
     }
 
     /// Sets the truncate overflowing text with an ellipsis (…) if needed.
     /// [Docs](https://tailwindcss.com/docs/text-overflow#ellipsis)
     fn text_ellipsis(mut self) -> Self {
-        self.text_style()
-            .get_or_insert_with(Default::default)
-            .text_overflow = Some(TextOverflow::Truncate(ELLIPSIS));
+        self.text_style().text_overflow = Some(TextOverflow::Truncate(ELLIPSIS));
         self
     }
 
     /// Sets the text overflow behavior of the element.
     fn text_overflow(mut self, overflow: TextOverflow) -> Self {
-        self.text_style()
-            .get_or_insert_with(Default::default)
-            .text_overflow = Some(overflow);
+        self.text_style().text_overflow = Some(overflow);
         self
     }
 
     /// Set the text alignment of the element.
     fn text_align(mut self, align: TextAlign) -> Self {
-        self.text_style()
-            .get_or_insert_with(Default::default)
-            .text_align = Some(align);
+        self.text_style().text_align = Some(align);
         self
     }
 
@@ -128,7 +118,7 @@ pub trait Styled: Sized {
     /// Sets number of lines to show before truncating the text.
     /// [Docs](https://tailwindcss.com/docs/line-clamp)
     fn line_clamp(mut self, lines: usize) -> Self {
-        let mut text_style = self.text_style().get_or_insert_with(Default::default);
+        let mut text_style = self.text_style();
         text_style.line_clamp = Some(lines);
         self.overflow_hidden()
     }
@@ -396,7 +386,7 @@ pub trait Styled: Sized {
     }
 
     /// Returns a mutable reference to the text style that has been configured on this element.
-    fn text_style(&mut self) -> &mut Option<TextStyleRefinement> {
+    fn text_style(&mut self) -> &mut TextStyleRefinement {
         let style: &mut StyleRefinement = self.style();
         &mut style.text
     }
@@ -405,7 +395,7 @@ pub trait Styled: Sized {
     ///
     /// This value cascades to its child elements.
     fn text_color(mut self, color: impl Into<Hsla>) -> Self {
-        self.text_style().get_or_insert_with(Default::default).color = Some(color.into());
+        self.text_style().color = Some(color.into());
         self
     }
 
@@ -413,9 +403,7 @@ pub trait Styled: Sized {
     ///
     /// This value cascades to its child elements.
     fn font_weight(mut self, weight: FontWeight) -> Self {
-        self.text_style()
-            .get_or_insert_with(Default::default)
-            .font_weight = Some(weight);
+        self.text_style().font_weight = Some(weight);
         self
     }
 
@@ -423,9 +411,7 @@ pub trait Styled: Sized {
     ///
     /// This value cascades to its child elements.
     fn text_bg(mut self, bg: impl Into<Hsla>) -> Self {
-        self.text_style()
-            .get_or_insert_with(Default::default)
-            .background_color = Some(bg.into());
+        self.text_style().background_color = Some(bg.into());
         self
     }
 
@@ -433,97 +419,77 @@ pub trait Styled: Sized {
     ///
     /// This value cascades to its child elements.
     fn text_size(mut self, size: impl Into<AbsoluteLength>) -> Self {
-        self.text_style()
-            .get_or_insert_with(Default::default)
-            .font_size = Some(size.into());
+        self.text_style().font_size = Some(size.into());
         self
     }
 
     /// Sets the text size to 'extra small'.
     /// [Docs](https://tailwindcss.com/docs/font-size#setting-the-font-size)
     fn text_xs(mut self) -> Self {
-        self.text_style()
-            .get_or_insert_with(Default::default)
-            .font_size = Some(rems(0.75).into());
+        self.text_style().font_size = Some(rems(0.75).into());
         self
     }
 
     /// Sets the text size to 'small'.
     /// [Docs](https://tailwindcss.com/docs/font-size#setting-the-font-size)
     fn text_sm(mut self) -> Self {
-        self.text_style()
-            .get_or_insert_with(Default::default)
-            .font_size = Some(rems(0.875).into());
+        self.text_style().font_size = Some(rems(0.875).into());
         self
     }
 
     /// Sets the text size to 'base'.
     /// [Docs](https://tailwindcss.com/docs/font-size#setting-the-font-size)
     fn text_base(mut self) -> Self {
-        self.text_style()
-            .get_or_insert_with(Default::default)
-            .font_size = Some(rems(1.0).into());
+        self.text_style().font_size = Some(rems(1.0).into());
         self
     }
 
     /// Sets the text size to 'large'.
     /// [Docs](https://tailwindcss.com/docs/font-size#setting-the-font-size)
     fn text_lg(mut self) -> Self {
-        self.text_style()
-            .get_or_insert_with(Default::default)
-            .font_size = Some(rems(1.125).into());
+        self.text_style().font_size = Some(rems(1.125).into());
         self
     }
 
     /// Sets the text size to 'extra large'.
     /// [Docs](https://tailwindcss.com/docs/font-size#setting-the-font-size)
     fn text_xl(mut self) -> Self {
-        self.text_style()
-            .get_or_insert_with(Default::default)
-            .font_size = Some(rems(1.25).into());
+        self.text_style().font_size = Some(rems(1.25).into());
         self
     }
 
     /// Sets the text size to 'extra extra large'.
     /// [Docs](https://tailwindcss.com/docs/font-size#setting-the-font-size)
     fn text_2xl(mut self) -> Self {
-        self.text_style()
-            .get_or_insert_with(Default::default)
-            .font_size = Some(rems(1.5).into());
+        self.text_style().font_size = Some(rems(1.5).into());
         self
     }
 
     /// Sets the text size to 'extra extra extra large'.
     /// [Docs](https://tailwindcss.com/docs/font-size#setting-the-font-size)
     fn text_3xl(mut self) -> Self {
-        self.text_style()
-            .get_or_insert_with(Default::default)
-            .font_size = Some(rems(1.875).into());
+        self.text_style().font_size = Some(rems(1.875).into());
         self
     }
 
     /// Sets the font style of the element to italic.
     /// [Docs](https://tailwindcss.com/docs/font-style#italicizing-text)
     fn italic(mut self) -> Self {
-        self.text_style()
-            .get_or_insert_with(Default::default)
-            .font_style = Some(FontStyle::Italic);
+        self.text_style().font_style = Some(FontStyle::Italic);
         self
     }
 
     /// Sets the font style of the element to normal (not italic).
     /// [Docs](https://tailwindcss.com/docs/font-style#displaying-text-normally)
     fn not_italic(mut self) -> Self {
-        self.text_style()
-            .get_or_insert_with(Default::default)
-            .font_style = Some(FontStyle::Normal);
+        self.text_style().font_style = Some(FontStyle::Normal);
         self
     }
 
     /// Sets the text decoration to underline.
     /// [Docs](https://tailwindcss.com/docs/text-decoration-line#underling-text)
     fn underline(mut self) -> Self {
-        let style = self.text_style().get_or_insert_with(Default::default);
+        let style = self.text_style();
         style.underline = Some(UnderlineStyle {
             thickness: px(1.),
             ..Default::default()
@@ -534,7 +500,7 @@ pub trait Styled: Sized {
     /// Sets the decoration of the text to have a line through it.
     /// [Docs](https://tailwindcss.com/docs/text-decoration-line#adding-a-line-through-text)
     fn line_through(mut self) -> Self {
-        let style = self.text_style().get_or_insert_with(Default::default);
+        let style = self.text_style();
         style.strikethrough = Some(StrikethroughStyle {
             thickness: px(1.),
             ..Default::default()
@@ -546,15 +512,13 @@ pub trait Styled: Sized {
     ///
     /// This value cascades to its child elements.
     fn text_decoration_none(mut self) -> Self {
-        self.text_style()
-            .get_or_insert_with(Default::default)
-            .underline = None;
+        self.text_style().underline = None;
         self
     }
 
     /// Sets the color for the underline on this element
     fn text_decoration_color(mut self, color: impl Into<Hsla>) -> Self {
-        let style = self.text_style().get_or_insert_with(Default::default);
+        let style = self.text_style();
         let underline = style.underline.get_or_insert_with(Default::default);
         underline.color = Some(color.into());
         self
@@ -563,7 +527,7 @@ pub trait Styled: Sized {
     /// Sets the text decoration style to a solid line.
     /// [Docs](https://tailwindcss.com/docs/text-decoration-style)
     fn text_decoration_solid(mut self) -> Self {
-        let style = self.text_style().get_or_insert_with(Default::default);
+        let style = self.text_style();
         let underline = style.underline.get_or_insert_with(Default::default);
         underline.wavy = false;
         self
@@ -572,7 +536,7 @@ pub trait Styled: Sized {
     /// Sets the text decoration style to a wavy line.
     /// [Docs](https://tailwindcss.com/docs/text-decoration-style)
     fn text_decoration_wavy(mut self) -> Self {
-        let style = self.text_style().get_or_insert_with(Default::default);
+        let style = self.text_style();
         let underline = style.underline.get_or_insert_with(Default::default);
         underline.wavy = true;
         self
@@ -581,7 +545,7 @@ pub trait Styled: Sized {
     /// Sets the text decoration to be 0px thick.
     /// [Docs](https://tailwindcss.com/docs/text-decoration-thickness)
     fn text_decoration_0(mut self) -> Self {
-        let style = self.text_style().get_or_insert_with(Default::default);
+        let style = self.text_style();
         let underline = style.underline.get_or_insert_with(Default::default);
         underline.thickness = px(0.);
         self
@@ -590,7 +554,7 @@ pub trait Styled: Sized {
     /// Sets the text decoration to be 1px thick.
     /// [Docs](https://tailwindcss.com/docs/text-decoration-thickness)
     fn text_decoration_1(mut self) -> Self {
-        let style = self.text_style().get_or_insert_with(Default::default);
+        let style = self.text_style();
         let underline = style.underline.get_or_insert_with(Default::default);
         underline.thickness = px(1.);
         self
@@ -599,7 +563,7 @@ pub trait Styled: Sized {
     /// Sets the text decoration to be 2px thick.
     /// [Docs](https://tailwindcss.com/docs/text-decoration-thickness)
     fn text_decoration_2(mut self) -> Self {
-        let style = self.text_style().get_or_insert_with(Default::default);
+        let style = self.text_style();
         let underline = style.underline.get_or_insert_with(Default::default);
         underline.thickness = px(2.);
         self
@@ -608,7 +572,7 @@ pub trait Styled: Sized {
     /// Sets the text decoration to be 4px thick.
     /// [Docs](https://tailwindcss.com/docs/text-decoration-thickness)
     fn text_decoration_4(mut self) -> Self {
-        let style = self.text_style().get_or_insert_with(Default::default);
+        let style = self.text_style();
         let underline = style.underline.get_or_insert_with(Default::default);
         underline.thickness = px(4.);
         self
@@ -617,7 +581,7 @@ pub trait Styled: Sized {
     /// Sets the text decoration to be 8px thick.
     /// [Docs](https://tailwindcss.com/docs/text-decoration-thickness)
     fn text_decoration_8(mut self) -> Self {
-        let style = self.text_style().get_or_insert_with(Default::default);
+        let style = self.text_style();
         let underline = style.underline.get_or_insert_with(Default::default);
         underline.thickness = px(8.);
         self
@@ -625,17 +589,13 @@ pub trait Styled: Sized {
 
     /// Sets the font family of this element and its children.
     fn font_family(mut self, family_name: impl Into<SharedString>) -> Self {
-        self.text_style()
-            .get_or_insert_with(Default::default)
-            .font_family = Some(family_name.into());
+        self.text_style().font_family = Some(family_name.into());
         self
     }
 
     /// Sets the font features of this element and its children.
     fn font_features(mut self, features: FontFeatures) -> Self {
-        self.text_style()
-            .get_or_insert_with(Default::default)
-            .font_features = Some(features);
+        self.text_style().font_features = Some(features);
         self
     }
 
@@ -649,7 +609,7 @@ pub trait Styled: Sized {
             style,
         } = font;
 
-        let text_style = self.text_style().get_or_insert_with(Default::default);
+        let text_style = self.text_style();
         text_style.font_family = Some(family);
         text_style.font_features = Some(features);
         text_style.font_weight = Some(weight);
@@ -661,9 +621,7 @@ pub trait Styled: Sized {
 
     /// Sets the line height of this element and its children.
     fn line_height(mut self, line_height: impl Into<DefiniteLength>) -> Self {
-        self.text_style()
-            .get_or_insert_with(Default::default)
-            .line_height = Some(line_height.into());
+        self.text_style().line_height = Some(line_height.into());
         self
     }
 

crates/markdown/examples/markdown_as_child.rs

@@ -54,11 +54,11 @@ impl Render for HelloWorld {
                 ..Default::default()
             },
             code_block: StyleRefinement {
-                text: Some(gpui::TextStyleRefinement {
+                text: gpui::TextStyleRefinement {
                     font_family: Some("Zed Mono".into()),
                     background_color: Some(cx.theme().colors().editor_background),
                     ..Default::default()
-                }),
+                },
                 margin: gpui::EdgesRefinement {
                     top: Some(Length::Definite(rems(4.).into())),
                     left: Some(Length::Definite(rems(4.).into())),

crates/markdown/src/markdown.rs

@@ -838,8 +838,7 @@ impl Element for MarkdownElement {
 
                             heading.style().refine(&self.style.heading);
 
-                            let text_style =
-                                self.style.heading.text_style().clone().unwrap_or_default();
+                            let text_style = self.style.heading.text_style().clone();
 
                             builder.push_text_style(text_style);
                             builder.push_div(heading, range, markdown_end);
@@ -933,10 +932,7 @@ impl Element for MarkdownElement {
                                             }
                                         });
 
-                                    if let Some(code_block_text_style) = &self.style.code_block.text
-                                    {
-                                        builder.push_text_style(code_block_text_style.to_owned());
-                                    }
+                                    builder.push_text_style(self.style.code_block.text.to_owned());
                                     builder.push_code_block(language);
                                     builder.push_div(code_block, range, markdown_end);
                                 }
@@ -1091,9 +1087,7 @@ impl Element for MarkdownElement {
 
                         builder.pop_div();
                         builder.pop_code_block();
-                        if self.style.code_block.text.is_some() {
-                            builder.pop_text_style();
-                        }
+                        builder.pop_text_style();
 
                         if let CodeBlockRenderer::Default {
                             copy_button: true, ..
@@ -1346,7 +1340,7 @@ fn apply_heading_style(
         };
 
         if let Some(style) = style_opt {
-            heading.style().text = Some(style.clone());
+            heading.style().text = style.clone();
         }
     }
 

crates/refineable/derive_refineable/src/derive_refineable.rs

@@ -528,7 +528,12 @@ fn get_wrapper_type(field: &Field, ty: &Type) -> syn::Type {
         } else {
             panic!("Expected struct type for a refineable field");
         };
-        let refinement_struct_name = format_ident!("{}Refinement", struct_name);
+
+        let refinement_struct_name = if struct_name.to_string().ends_with("Refinement") {
+            format_ident!("{}", struct_name)
+        } else {
+            format_ident!("{}Refinement", struct_name)
+        };
         let generics = if let Type::Path(tp) = ty {
             &tp.path.segments.last().unwrap().arguments
         } else {

crates/refineable/src/refineable.rs

@@ -13,7 +13,7 @@ pub use derive_refineable::Refineable;
 /// wrapped appropriately:
 ///
 /// - **Refineable fields** (marked with `#[refineable]`): Become the corresponding refinement type
-///   (e.g., `Bar` becomes `BarRefinement`)
+///   (e.g., `Bar` becomes `BarRefinement`, or `BarRefinement` remains `BarRefinement`)
 /// - **Optional fields** (`Option<T>`): Remain as `Option<T>`
 /// - **Regular fields**: Become `Option<T>`
 ///

crates/ui/src/components/keybinding_hint.rs

@@ -234,9 +234,7 @@ impl RenderOnce for KeybindingHint {
 
         let mut base = h_flex();
 
-        base.text_style()
-            .get_or_insert_with(Default::default)
-            .font_style = Some(FontStyle::Italic);
+        base.text_style().font_style = Some(FontStyle::Italic);
 
         base.gap_1()
             .font_buffer(cx)

crates/ui/src/components/label/label_like.rs

@@ -223,9 +223,7 @@ impl RenderOnce for LabelLike {
             })
             .when(self.italic, |this| this.italic())
             .when(self.underline, |mut this| {
-                this.text_style()
-                    .get_or_insert_with(Default::default)
-                    .underline = Some(UnderlineStyle {
+                this.text_style().underline = Some(UnderlineStyle {
                     thickness: px(1.),
                     color: Some(cx.theme().colors().text_muted.opacity(0.4)),
                     wavy: false,