From a3ac59573799d10ec55f88426438a45af08a56fa Mon Sep 17 00:00:00 2001 From: Serophots <47299955+Serophots@users.noreply.github.com> Date: Mon, 15 Dec 2025 13:30:13 +0000 Subject: [PATCH] gpui: Make refining a `Style` properly refine the `TextStyle` (#42852) ## 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 --- crates/acp_tools/src/acp_tools.rs | 4 +- crates/agent_ui/src/acp/thread_view.rs | 4 +- .../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 +-- .../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(-) diff --git a/crates/acp_tools/src/acp_tools.rs b/crates/acp_tools/src/acp_tools.rs index 0905effce38d1bfd4fa18e1d00169d6c7ef6c2d7..b0d30367da0634dc82f8db96fc099e268aa4790e 100644 --- a/crates/acp_tools/src/acp_tools.rs +++ b/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() diff --git a/crates/agent_ui/src/acp/thread_view.rs b/crates/agent_ui/src/acp/thread_view.rs index 63ea9eb279d26ff610c12f9785ef882be61f5e26..6cd2ec2fa3442bbf4961dffb0c4538ac9615d982 100644 --- a/crates/agent_ui/src/acp/thread_view.rs +++ b/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 { diff --git a/crates/edit_prediction_ui/src/rate_prediction_modal.rs b/crates/edit_prediction_ui/src/rate_prediction_modal.rs index 54933fbf904f8fc7146dcce9a6bd3340884cc8bf..22e82bc445b394cc122e1cb1aa3604b45c25d1d1 100644 --- a/crates/edit_prediction_ui/src/rate_prediction_modal.rs +++ b/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.)), diff --git a/crates/gpui/src/style.rs b/crates/gpui/src/style.rs index 42f8f25e47620fe673720055037b7f91f44165a2..446c3ad2a325681a39689577a261ed1ffdde6d5b 100644 --- a/crates/gpui/src/style.rs +++ b/crates/gpui/src/style.rs @@ -252,6 +252,7 @@ pub struct Style { pub box_shadow: Vec, /// 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 + ); + } } diff --git a/crates/gpui/src/styled.rs b/crates/gpui/src/styled.rs index 752038c1ed63a1d0d5960bf0a74a1c2fdbc43392..e01649be481e27f89643db2ffb3a9ccd294b9b73 100644 --- a/crates/gpui/src/styled.rs +++ b/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 { + 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) -> 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) -> 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) -> 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) -> 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) -> 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) -> 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 } diff --git a/crates/markdown/examples/markdown_as_child.rs b/crates/markdown/examples/markdown_as_child.rs index 6affa243ae5cc5f4cac1dc7fea0af9b9cc183aa6..775e2a141a849636512264dda2628e28254c8e2b 100644 --- a/crates/markdown/examples/markdown_as_child.rs +++ b/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())), diff --git a/crates/markdown/src/markdown.rs b/crates/markdown/src/markdown.rs index 2e9103787bf2705732e1dad2276ebbdb21c5c2bc..d6ba3babecf3b6b43155780e569bdc4515762d40 100644 --- a/crates/markdown/src/markdown.rs +++ b/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(); } } diff --git a/crates/refineable/derive_refineable/src/derive_refineable.rs b/crates/refineable/derive_refineable/src/derive_refineable.rs index ddf3855a4dc5ae6917309ced57391bd244f1b465..c7c8a91ad9b05d054a94c8ca7f55a54c75150d81 100644 --- a/crates/refineable/derive_refineable/src/derive_refineable.rs +++ b/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 { diff --git a/crates/refineable/src/refineable.rs b/crates/refineable/src/refineable.rs index d2a7c3d3f2148ae174be10121dc23b4dbfc5a650..b2305d4b5a7c1e5e45287394a49a258d98767c66 100644 --- a/crates/refineable/src/refineable.rs +++ b/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`): Remain as `Option` /// - **Regular fields**: Become `Option` /// diff --git a/crates/ui/src/components/keybinding_hint.rs b/crates/ui/src/components/keybinding_hint.rs index c998e29f0ed6f5bccab976b11080320d4d65a7dd..7c19953ca43c907070829f7140f97a4fde495b57 100644 --- a/crates/ui/src/components/keybinding_hint.rs +++ b/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) diff --git a/crates/ui/src/components/label/label_like.rs b/crates/ui/src/components/label/label_like.rs index e51d65c3b6c8ecb38ba26a1926c3bfdbb988a1f8..31fb7bfd88f1343ac6145c86f228bdcbd6a22e10 100644 --- a/crates/ui/src/components/label/label_like.rs +++ b/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,