From e606e2e2398b1664ae86a4385a2850ba6739bb89 Mon Sep 17 00:00:00 2001 From: Remco Smits Date: Mon, 25 Aug 2025 08:53:41 +0200 Subject: [PATCH] Fix review comments --- .../markdown_preview/src/markdown_elements.rs | 4 +-- .../markdown_preview/src/markdown_parser.rs | 29 +++++-------------- .../markdown_preview/src/markdown_renderer.rs | 11 +++---- 3 files changed, 13 insertions(+), 31 deletions(-) diff --git a/crates/markdown_preview/src/markdown_elements.rs b/crates/markdown_preview/src/markdown_elements.rs index f87543b046..560e468439 100644 --- a/crates/markdown_preview/src/markdown_elements.rs +++ b/crates/markdown_preview/src/markdown_elements.rs @@ -1,6 +1,6 @@ -use gpui::DefiniteLength; use gpui::{ - FontStyle, FontWeight, HighlightStyle, SharedString, StrikethroughStyle, UnderlineStyle, px, + DefiniteLength, FontStyle, FontWeight, HighlightStyle, SharedString, StrikethroughStyle, + UnderlineStyle, px, }; use language::HighlightId; use std::{fmt::Display, ops::Range, path::PathBuf}; diff --git a/crates/markdown_preview/src/markdown_parser.rs b/crates/markdown_preview/src/markdown_parser.rs index a8154b1752..1b116c50d9 100644 --- a/crates/markdown_preview/src/markdown_parser.rs +++ b/crates/markdown_preview/src/markdown_parser.rs @@ -938,7 +938,6 @@ mod tests { HighlightId, Language, LanguageConfig, LanguageMatcher, LanguageRegistry, tree_sitter_rust, }; use pretty_assertions::assert_eq; - use ui::Pixels; async fn parse(input: &str) -> ParsedMarkdown { parse_markdown(input, None, None).await @@ -1226,35 +1225,25 @@ mod tests { // Test pixel values assert_eq!( MarkdownParser::parse_length("100px"), - Some(DefiniteLength::Absolute(AbsoluteLength::Pixels(Pixels( - 100.0 - )))) + Some(DefiniteLength::Absolute(AbsoluteLength::Pixels(px(100.0)))) ); assert_eq!( MarkdownParser::parse_length("50px"), - Some(DefiniteLength::Absolute(AbsoluteLength::Pixels(Pixels( - 50.0 - )))) + Some(DefiniteLength::Absolute(AbsoluteLength::Pixels(px(50.0)))) ); assert_eq!( MarkdownParser::parse_length("0px"), - Some(DefiniteLength::Absolute(AbsoluteLength::Pixels(Pixels( - 0.0 - )))) + Some(DefiniteLength::Absolute(AbsoluteLength::Pixels(px(0.0)))) ); // Test values without units (should be treated as pixels) assert_eq!( MarkdownParser::parse_length("100"), - Some(DefiniteLength::Absolute(AbsoluteLength::Pixels(Pixels( - 100.0 - )))) + Some(DefiniteLength::Absolute(AbsoluteLength::Pixels(px(100.0)))) ); assert_eq!( MarkdownParser::parse_length("42"), - Some(DefiniteLength::Absolute(AbsoluteLength::Pixels(Pixels( - 42.0 - )))) + Some(DefiniteLength::Absolute(AbsoluteLength::Pixels(px(42.0)))) ); // Test invalid values @@ -1272,15 +1261,11 @@ mod tests { ); assert_eq!( MarkdownParser::parse_length("100.25px"), - Some(DefiniteLength::Absolute(AbsoluteLength::Pixels(Pixels( - 100.25 - )))) + Some(DefiniteLength::Absolute(AbsoluteLength::Pixels(px(100.25)))) ); assert_eq!( MarkdownParser::parse_length("42.0"), - Some(DefiniteLength::Absolute(AbsoluteLength::Pixels(Pixels( - 42.0 - )))) + Some(DefiniteLength::Absolute(AbsoluteLength::Pixels(px(42.0)))) ); } diff --git a/crates/markdown_preview/src/markdown_renderer.rs b/crates/markdown_preview/src/markdown_renderer.rs index 80e99f304f..b07b4686a4 100644 --- a/crates/markdown_preview/src/markdown_renderer.rs +++ b/crates/markdown_preview/src/markdown_renderer.rs @@ -26,9 +26,6 @@ use ui::{ }; use workspace::{OpenOptions, OpenVisible, Workspace}; -const OPEN_IMAGE_TOOLTIP: SharedString = SharedString::new_static("open image"); -const TOGGLE_CHECKBOX_TOOLTIP: SharedString = SharedString::new_static("toggle checkbox"); - pub struct CheckboxClickedEvent { pub checked: bool, pub source_range: Range, @@ -263,7 +260,7 @@ fn render_markdown_list_item( ) .hover(|s| s.cursor_pointer()) .tooltip(|_, cx| { - InteractiveMarkdownElementTooltip::new(None, TOGGLE_CHECKBOX_TOOLTIP, cx).into() + InteractiveMarkdownElementTooltip::new(None, "toggle checkbox", cx).into() }) .into_any_element(), }; @@ -767,7 +764,7 @@ fn render_markdown_image(image: &Image, cx: &mut RenderContext) -> AnyElement { move |_, cx| { InteractiveMarkdownElementTooltip::new( Some(alt_text.clone().unwrap_or(link.to_string().into())), - OPEN_IMAGE_TOOLTIP, + "open image", cx, ) .into() @@ -811,14 +808,14 @@ struct InteractiveMarkdownElementTooltip { impl InteractiveMarkdownElementTooltip { pub fn new( tooltip_text: Option, - action_text: SharedString, + action_text: impl Into, cx: &mut App, ) -> Entity { let tooltip_text = tooltip_text.map(|t| util::truncate_and_trailoff(&t, 50).into()); cx.new(|_cx| Self { tooltip_text, - action_text, + action_text: action_text.into(), }) } }