From 6fb5e07cb27ef16584a7ca3e093e7721b15500b3 Mon Sep 17 00:00:00 2001 From: Remco Smits Date: Thu, 21 Aug 2025 18:04:39 +0200 Subject: [PATCH 01/14] Require scraper for parsing the raw html --- Cargo.lock | 103 ++++++++++++++++++++++++++++- Cargo.toml | 1 + crates/markdown_preview/Cargo.toml | 5 +- 3 files changed, 106 insertions(+), 3 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 76f8672d4d..94dd63c66c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5117,6 +5117,12 @@ dependencies = [ "zlog", ] +[[package]] +name = "ego-tree" +version = "0.10.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b2972feb8dffe7bc8c5463b1dacda1b0dfbed3710e50f977d965429692d74cd8" + [[package]] name = "either" version = "1.15.0" @@ -6329,6 +6335,15 @@ dependencies = [ "thread_local", ] +[[package]] +name = "fxhash" +version = "0.2.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c31b6d751ae2c7f11320402d34e41349dd1016f8d5d45e48c4312bc8625af50c" +dependencies = [ + "byteorder", +] + [[package]] name = "generator" version = "0.8.5" @@ -6363,6 +6378,15 @@ dependencies = [ "windows-targets 0.48.5", ] +[[package]] +name = "getopts" +version = "0.2.23" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "cba6ae63eb948698e300f645f87c70f76630d505f23b8907cf1e193ee85048c1" +dependencies = [ + "unicode-width 0.2.0", +] + [[package]] name = "getrandom" version = "0.2.15" @@ -7889,7 +7913,18 @@ dependencies = [ "log", "mac", "markup5ever 0.16.1", - "match_token", + "match_token 0.1.0", +] + +[[package]] +name = "html5ever" +version = "0.35.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "55d958c2f74b664487a2035fe1dadb032c48718a03b63f3ab0b8537db8549ed4" +dependencies = [ + "log", + "markup5ever 0.35.0", + "match_token 0.35.0", ] [[package]] @@ -9935,6 +9970,7 @@ dependencies = [ "log", "pretty_assertions", "pulldown-cmark 0.12.2", + "scraper", "settings", "theme", "ui", @@ -9968,6 +10004,17 @@ dependencies = [ "web_atoms", ] +[[package]] +name = "markup5ever" +version = "0.35.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "311fe69c934650f8f19652b3946075f0fc41ad8757dbb68f1ca14e7900ecc1c3" +dependencies = [ + "log", + "tendril", + "web_atoms", +] + [[package]] name = "markup5ever_rcdom" version = "0.3.0" @@ -9991,6 +10038,17 @@ dependencies = [ "syn 2.0.101", ] +[[package]] +name = "match_token" +version = "0.35.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ac84fd3f360fcc43dc5f5d186f02a94192761a080e8bc58621ad4d12296a58cf" +dependencies = [ + "proc-macro2", + "quote", + "syn 2.0.101", +] + [[package]] name = "matchers" version = "0.1.0" @@ -14402,6 +14460,21 @@ version = "1.2.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "94143f37725109f92c262ed2cf5e59bce7498c01bcc1502d7b9afe439a4e9f49" +[[package]] +name = "scraper" +version = "0.24.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e5f3a24d916e78954af99281a455168d4a9515d65eca99a18da1b813689c4ad9" +dependencies = [ + "cssparser", + "ego-tree", + "getopts", + "html5ever 0.35.0", + "precomputed-hash", + "selectors", + "tendril", +] + [[package]] name = "scratch" version = "1.0.8" @@ -14646,6 +14719,25 @@ dependencies = [ "libc", ] +[[package]] +name = "selectors" +version = "0.31.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5685b6ae43bfcf7d2e7dfcfb5d8e8f61b46442c902531e41a32a9a8bf0ee0fb6" +dependencies = [ + "bitflags 2.9.0", + "cssparser", + "derive_more 2.0.1", + "fxhash", + "log", + "new_debug_unreachable", + "phf", + "phf_codegen", + "precomputed-hash", + "servo_arc", + "smallvec", +] + [[package]] name = "self_cell" version = "1.2.0" @@ -14832,6 +14924,15 @@ dependencies = [ "winapi", ] +[[package]] +name = "servo_arc" +version = "0.4.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "204ea332803bd95a0b60388590d59cf6468ec9becf626e2451f1d26a1d972de4" +dependencies = [ + "stable_deref_trait", +] + [[package]] name = "session" version = "0.1.0" diff --git a/Cargo.toml b/Cargo.toml index b13795e1e1..2d5a57ca3e 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -574,6 +574,7 @@ rustls = { version = "0.23.26" } rustls-platform-verifier = "0.5.0" scap = { git = "https://github.com/zed-industries/scap", rev = "808aa5c45b41e8f44729d02e38fd00a2fe2722e7", default-features = false } schemars = { version = "1.0", features = ["indexmap2"] } +scraper = "0.24.0" semver = "1.0" serde = { version = "1.0", features = ["derive", "rc"] } serde_derive = { version = "1.0", features = ["deserialize_in_place"] } diff --git a/crates/markdown_preview/Cargo.toml b/crates/markdown_preview/Cargo.toml index ebdd8a9eb6..0a9d228de2 100644 --- a/crates/markdown_preview/Cargo.toml +++ b/crates/markdown_preview/Cargo.toml @@ -19,19 +19,20 @@ anyhow.workspace = true async-recursion.workspace = true collections.workspace = true editor.workspace = true +fs.workspace = true gpui.workspace = true language.workspace = true linkify.workspace = true log.workspace = true pretty_assertions.workspace = true pulldown-cmark.workspace = true +scraper.workspace = true settings.workspace = true theme.workspace = true ui.workspace = true util.workspace = true -workspace.workspace = true workspace-hack.workspace = true -fs.workspace = true +workspace.workspace = true [dev-dependencies] editor = { workspace = true, features = ["test-support"] } From cff0f2871fe3025eb4b867a7c870c932d7db1166 Mon Sep 17 00:00:00 2001 From: Remco Smits Date: Thu, 21 Aug 2025 18:08:26 +0200 Subject: [PATCH 02/14] Add html image tag support Supported examples: ```markdown Description of image Description of image ![alt text](https://picsum.photos/200/300) ``` --- .../markdown_preview/src/markdown_elements.rs | 14 ++ .../markdown_preview/src/markdown_parser.rs | 89 ++++++++++- .../markdown_preview/src/markdown_renderer.rs | 145 ++++++++++-------- 3 files changed, 182 insertions(+), 66 deletions(-) diff --git a/crates/markdown_preview/src/markdown_elements.rs b/crates/markdown_preview/src/markdown_elements.rs index a570e79f53..cb5c819e8a 100644 --- a/crates/markdown_preview/src/markdown_elements.rs +++ b/crates/markdown_preview/src/markdown_elements.rs @@ -15,6 +15,7 @@ pub enum ParsedMarkdownElement { /// A paragraph of text and other inline elements. Paragraph(MarkdownParagraph), HorizontalRule(Range), + Image(Image), } impl ParsedMarkdownElement { @@ -30,6 +31,7 @@ impl ParsedMarkdownElement { MarkdownParagraphChunk::Image(image) => image.source_range.clone(), }, Self::HorizontalRule(range) => range.clone(), + Self::Image(image) => image.source_range.clone(), }) } @@ -290,6 +292,8 @@ pub struct Image { pub link: Link, pub source_range: Range, pub alt_text: Option, + pub width: Option, + pub height: Option, } impl Image { @@ -303,10 +307,20 @@ impl Image { source_range, link, alt_text: None, + width: None, + height: None, }) } pub fn set_alt_text(&mut self, alt_text: SharedString) { self.alt_text = Some(alt_text); } + + pub fn set_width(&mut self, width: u32) { + self.width = Some(width); + } + + pub fn set_height(&mut self, height: u32) { + self.height = Some(height); + } } diff --git a/crates/markdown_preview/src/markdown_parser.rs b/crates/markdown_preview/src/markdown_parser.rs index b51b98a2ed..d525787562 100644 --- a/crates/markdown_preview/src/markdown_parser.rs +++ b/crates/markdown_preview/src/markdown_parser.rs @@ -175,6 +175,11 @@ impl<'a> MarkdownParser<'a> { let code_block = self.parse_code_block(language).await; Some(vec![ParsedMarkdownElement::CodeBlock(code_block)]) } + Tag::HtmlBlock => { + self.cursor += 1; + + Some(self.parse_html_block().await) + } _ => None, }, Event::Rule => { @@ -378,7 +383,7 @@ impl<'a> MarkdownParser<'a> { TagEnd::Image => { if let Some(mut image) = image.take() { if !text.is_empty() { - image.alt_text = Some(std::mem::take(&mut text).into()); + image.set_alt_text(std::mem::take(&mut text).into()); } markdown_text_like.push(MarkdownParagraphChunk::Image(image)); } @@ -741,6 +746,78 @@ impl<'a> MarkdownParser<'a> { highlights, } } + + async fn parse_html_block(&mut self) -> Vec { + let (_event, _source_range) = self.previous().unwrap(); + let mut elements = Vec::new(); + + while !self.eof() { + let (current, source_range) = self.current().unwrap(); + let source_range = source_range.clone(); + match current { + Event::Html(html) => { + let fragment = scraper::Html::parse_fragment(html); + self.cursor += 1; + + elements.extend(self.parse_html_image(fragment, source_range)); + } + Event::End(TagEnd::CodeBlock) => { + self.cursor += 1; + break; + } + _ => { + break; + } + } + } + + elements + } + + fn parse_html_image( + &self, + html: scraper::Html, + source_range: Range, + ) -> Vec { + let mut images = Vec::new(); + let selector = scraper::Selector::parse("img").unwrap(); + + for element in html.select(&selector).into_iter() { + let Some(src) = element.attr("src") else { + continue; + }; + + let Some(mut image) = Image::identify( + src.to_string(), + source_range.clone(), + self.file_location_directory.clone(), + ) else { + continue; + }; + + if let Some(alt) = element.attr("alt") { + image.set_alt_text(alt.to_string().into()); + } + + if let Some(width) = element + .attr("width") + .and_then(|width| width.parse::().ok()) + { + image.set_width(width); + } + + if let Some(height) = element + .attr("height") + .and_then(|height| height.parse::().ok()) + { + image.set_height(height); + } + + images.push(ParsedMarkdownElement::Image(image)); + } + + images + } } #[cfg(test)] @@ -925,6 +1002,8 @@ mod tests { url: "https://blog.logrocket.com/wp-content/uploads/2024/04/exploring-zed-open-source-code-editor-rust-2.png".to_string(), }, alt_text: Some("test".into()), + height: None, + width: None, },) ); } @@ -946,6 +1025,8 @@ mod tests { url: "http://example.com/foo.png".to_string(), }, alt_text: None, + height: None, + width: None, },) ); } @@ -965,6 +1046,8 @@ mod tests { url: "http://example.com/foo.png".to_string(), }, alt_text: Some("foo bar baz".into()), + height: None, + width: None, }),], ); } @@ -990,6 +1073,8 @@ mod tests { url: "http://example.com/foo.png".to_string(), }, alt_text: Some("foo".into()), + height: None, + width: None, }), MarkdownParagraphChunk::Text(ParsedMarkdownText { source_range: 0..81, @@ -1004,6 +1089,8 @@ mod tests { url: "http://example.com/bar.png".to_string(), }, alt_text: Some("bar".into()), + height: None, + width: None, }) ] ); diff --git a/crates/markdown_preview/src/markdown_renderer.rs b/crates/markdown_preview/src/markdown_renderer.rs index b0b10e927c..df134014b0 100644 --- a/crates/markdown_preview/src/markdown_renderer.rs +++ b/crates/markdown_preview/src/markdown_renderer.rs @@ -1,5 +1,5 @@ use crate::markdown_elements::{ - HeadingLevel, Link, MarkdownParagraph, MarkdownParagraphChunk, ParsedMarkdown, + HeadingLevel, Image, Link, MarkdownParagraph, MarkdownParagraphChunk, ParsedMarkdown, ParsedMarkdownBlockQuote, ParsedMarkdownCodeBlock, ParsedMarkdownElement, ParsedMarkdownHeading, ParsedMarkdownListItem, ParsedMarkdownListItemType, ParsedMarkdownTable, ParsedMarkdownTableAlignment, ParsedMarkdownTableRow, @@ -22,10 +22,13 @@ use ui::{ ButtonCommon, Clickable, Color, FluentBuilder, IconButton, IconName, IconSize, InteractiveElement, Label, LabelCommon, LabelSize, LinkPreview, Pixels, Rems, StatefulInteractiveElement, StyledExt, StyledImage, ToggleState, Tooltip, VisibleOnHover, - h_flex, relative, tooltip_container, v_flex, + h_flex, px, relative, tooltip_container, v_flex, }; 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, @@ -164,6 +167,7 @@ pub fn render_markdown_block(block: &ParsedMarkdownElement, cx: &mut RenderConte BlockQuote(block_quote) => render_markdown_block_quote(block_quote, cx), CodeBlock(code_block) => render_markdown_code_block(code_block, cx), HorizontalRule(_) => render_markdown_rule(cx), + Image(image) => render_markdown_image(image, cx), } } @@ -259,7 +263,7 @@ fn render_markdown_list_item( ) .hover(|s| s.cursor_pointer()) .tooltip(|_, cx| { - InteractiveMarkdownElementTooltip::new(None, "toggle checkbox", cx).into() + InteractiveMarkdownElementTooltip::new(None, TOGGLE_CHECKBOX_TOOLTIP, cx).into() }) .into_any_element(), }; @@ -722,65 +726,7 @@ fn render_markdown_text(parsed_new: &MarkdownParagraph, cx: &mut RenderContext) } MarkdownParagraphChunk::Image(image) => { - let image_resource = match image.link.clone() { - Link::Web { url } => Resource::Uri(url.into()), - Link::Path { path, .. } => Resource::Path(Arc::from(path)), - }; - - let element_id = cx.next_id(&image.source_range); - - let image_element = div() - .id(element_id) - .cursor_pointer() - .child( - img(ImageSource::Resource(image_resource)) - .max_w_full() - .with_fallback({ - let alt_text = image.alt_text.clone(); - move || div().children(alt_text.clone()).into_any_element() - }), - ) - .tooltip({ - let link = image.link.clone(); - move |_, cx| { - InteractiveMarkdownElementTooltip::new( - Some(link.to_string()), - "open image", - cx, - ) - .into() - } - }) - .on_click({ - let workspace = workspace_clone.clone(); - let link = image.link.clone(); - move |_, window, cx| { - if window.modifiers().secondary() { - match &link { - Link::Web { url } => cx.open_url(url), - Link::Path { path, .. } => { - if let Some(workspace) = &workspace { - _ = workspace.update(cx, |workspace, cx| { - workspace - .open_abs_path( - path.clone(), - OpenOptions { - visible: Some(OpenVisible::None), - ..Default::default() - }, - window, - cx, - ) - .detach(); - }); - } - } - } - } - } - }) - .into_any(); - any_element.push(image_element); + any_element.push(render_markdown_image(image, cx)); } } } @@ -793,18 +739,87 @@ fn render_markdown_rule(cx: &mut RenderContext) -> AnyElement { div().py(cx.scaled_rems(0.5)).child(rule).into_any() } +fn render_markdown_image(image: &Image, cx: &mut RenderContext) -> AnyElement { + let image_resource = match image.link.clone() { + Link::Web { url } => Resource::Uri(url.into()), + Link::Path { path, .. } => Resource::Path(Arc::from(path)), + }; + + let element_id = cx.next_id(&image.source_range); + let workspace_clone = cx.workspace.clone(); + + div() + .id(element_id) + .cursor_pointer() + .child( + img(ImageSource::Resource(image_resource)) + .max_w_full() + .with_fallback({ + let alt_text = image.alt_text.clone(); + move || div().children(alt_text.clone()).into_any_element() + }) + .when_some(image.height, |this, height| this.h(px(height as f32))) + .when_some(image.width, |this, width| this.w(px(width as f32))), + ) + .tooltip({ + let link = image.link.clone(); + let alt_text = image.alt_text.clone(); + move |_, cx| { + InteractiveMarkdownElementTooltip::new( + Some(alt_text.clone().unwrap_or(link.to_string().into())), + OPEN_IMAGE_TOOLTIP, + cx, + ) + .into() + } + }) + .on_click({ + let workspace = workspace_clone.clone(); + let link = image.link.clone(); + move |_, window, cx| { + if window.modifiers().secondary() { + match &link { + Link::Web { url } => cx.open_url(url), + Link::Path { path, .. } => { + if let Some(workspace) = &workspace { + _ = workspace.update(cx, |workspace, cx| { + workspace + .open_abs_path( + path.clone(), + OpenOptions { + visible: Some(OpenVisible::None), + ..Default::default() + }, + window, + cx, + ) + .detach(); + }); + } + } + } + } + } + }) + .into_any() +} + struct InteractiveMarkdownElementTooltip { tooltip_text: Option, - action_text: String, + action_text: SharedString, } impl InteractiveMarkdownElementTooltip { - pub fn new(tooltip_text: Option, action_text: &str, cx: &mut App) -> Entity { + pub fn new( + tooltip_text: Option, + action_text: SharedString, + 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.to_string(), + action_text, }) } } From 6fa5ae021a198ee4d566cdbe493a9dcf16de18ad Mon Sep 17 00:00:00 2001 From: Remco Smits Date: Thu, 21 Aug 2025 18:52:03 +0200 Subject: [PATCH 03/14] Add tests --- .../markdown_preview/src/markdown_parser.rs | 64 +++++++++++++++++++ 1 file changed, 64 insertions(+) diff --git a/crates/markdown_preview/src/markdown_parser.rs b/crates/markdown_preview/src/markdown_parser.rs index d525787562..29c0ec5175 100644 --- a/crates/markdown_preview/src/markdown_parser.rs +++ b/crates/markdown_preview/src/markdown_parser.rs @@ -1096,6 +1096,70 @@ mod tests { ); } + #[gpui::test] + async fn test_html_image_tag() { + let parsed = parse("").await; + + let ParsedMarkdownElement::Image(image) = &parsed.children[0] else { + panic!("Expected a image element"); + }; + assert_eq!( + image.clone(), + Image { + source_range: 0..40, + link: Link::Web { + url: "http://example.com/foo.png".to_string(), + }, + alt_text: None, + height: None, + width: None, + }, + ); + } + + #[gpui::test] + async fn test_html_image_tag_with_alt_text() { + let parsed = parse("\"Foo\"").await; + + let ParsedMarkdownElement::Image(image) = &parsed.children[0] else { + panic!("Expected a image element"); + }; + assert_eq!( + image.clone(), + Image { + source_range: 0..50, + link: Link::Web { + url: "http://example.com/foo.png".to_string(), + }, + alt_text: Some("Foo".into()), + height: None, + width: None, + }, + ); + } + + #[gpui::test] + async fn test_html_image_tag_with_height_and_width() { + let parsed = + parse("").await; + + let ParsedMarkdownElement::Image(image) = &parsed.children[0] else { + panic!("Expected a image element"); + }; + assert_eq!( + image.clone(), + Image { + source_range: 0..65, + link: Link::Web { + url: "http://example.com/foo.png".to_string(), + }, + alt_text: None, + height: Some(100), + width: Some(200), + }, + ); + } + #[gpui::test] async fn test_header_only_table() { let markdown = "\ From 25610fb3d3ca6c106c5df50edff45e481e29a116 Mon Sep 17 00:00:00 2001 From: Remco Smits Date: Thu, 21 Aug 2025 19:02:37 +0200 Subject: [PATCH 04/14] Clippppyyy --- crates/markdown_preview/src/markdown_parser.rs | 2 +- crates/markdown_preview/src/markdown_renderer.rs | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/crates/markdown_preview/src/markdown_parser.rs b/crates/markdown_preview/src/markdown_parser.rs index 29c0ec5175..1d3b3c37c6 100644 --- a/crates/markdown_preview/src/markdown_parser.rs +++ b/crates/markdown_preview/src/markdown_parser.rs @@ -782,7 +782,7 @@ impl<'a> MarkdownParser<'a> { let mut images = Vec::new(); let selector = scraper::Selector::parse("img").unwrap(); - for element in html.select(&selector).into_iter() { + for element in html.select(&selector) { let Some(src) = element.attr("src") else { continue; }; diff --git a/crates/markdown_preview/src/markdown_renderer.rs b/crates/markdown_preview/src/markdown_renderer.rs index df134014b0..674efdabdd 100644 --- a/crates/markdown_preview/src/markdown_renderer.rs +++ b/crates/markdown_preview/src/markdown_renderer.rs @@ -746,7 +746,7 @@ fn render_markdown_image(image: &Image, cx: &mut RenderContext) -> AnyElement { }; let element_id = cx.next_id(&image.source_range); - let workspace_clone = cx.workspace.clone(); + let workspace = cx.workspace.clone(); div() .id(element_id) @@ -774,7 +774,6 @@ fn render_markdown_image(image: &Image, cx: &mut RenderContext) -> AnyElement { } }) .on_click({ - let workspace = workspace_clone.clone(); let link = image.link.clone(); move |_, window, cx| { if window.modifiers().secondary() { From 03487fff5b057fd4c9a47a7a5cd47c3b0d172a80 Mon Sep 17 00:00:00 2001 From: Remco Smits Date: Fri, 22 Aug 2025 19:02:36 +0200 Subject: [PATCH 05/14] Parse more variants of the height/width allowed values --- .../markdown_preview/src/markdown_elements.rs | 9 ++--- .../markdown_preview/src/markdown_parser.rs | 33 ++++++++++++++++--- .../markdown_preview/src/markdown_renderer.rs | 6 ++-- 3 files changed, 37 insertions(+), 11 deletions(-) diff --git a/crates/markdown_preview/src/markdown_elements.rs b/crates/markdown_preview/src/markdown_elements.rs index cb5c819e8a..9da94f30db 100644 --- a/crates/markdown_preview/src/markdown_elements.rs +++ b/crates/markdown_preview/src/markdown_elements.rs @@ -3,6 +3,7 @@ use gpui::{ }; use language::HighlightId; use std::{fmt::Display, ops::Range, path::PathBuf}; +use ui::DefiniteLength; #[derive(Debug)] #[cfg_attr(test, derive(PartialEq))] @@ -292,8 +293,8 @@ pub struct Image { pub link: Link, pub source_range: Range, pub alt_text: Option, - pub width: Option, - pub height: Option, + pub width: Option, + pub height: Option, } impl Image { @@ -316,11 +317,11 @@ impl Image { self.alt_text = Some(alt_text); } - pub fn set_width(&mut self, width: u32) { + pub fn set_width(&mut self, width: DefiniteLength) { self.width = Some(width); } - pub fn set_height(&mut self, height: u32) { + pub fn set_height(&mut self, height: DefiniteLength) { self.height = Some(height); } } diff --git a/crates/markdown_preview/src/markdown_parser.rs b/crates/markdown_preview/src/markdown_parser.rs index 1d3b3c37c6..0562ae4932 100644 --- a/crates/markdown_preview/src/markdown_parser.rs +++ b/crates/markdown_preview/src/markdown_parser.rs @@ -5,6 +5,7 @@ use gpui::FontWeight; use language::LanguageRegistry; use pulldown_cmark::{Alignment, Event, Options, Parser, Tag, TagEnd}; use std::{ops::Range, path::PathBuf, sync::Arc, vec}; +use ui::{px, relative}; pub async fn parse_markdown( markdown_input: &str, @@ -774,6 +775,29 @@ impl<'a> MarkdownParser<'a> { elements } + /// Parses the width/height attribute value of an html element (e.g. img element) + fn parse_length(value: &str) -> Option { + if value.ends_with("px") { + value + .trim_end_matches("px") + .parse() + .ok() + .map(|value| px(value).into()) + } else if value.ends_with("%") { + value + .trim_end_matches("%") + .parse::() + .ok() + .map(|value| relative(value / 100.)) + } else { + value + .trim_end_matches("px") + .parse() + .ok() + .map(|value| px(value).into()) + } + } + fn parse_html_image( &self, html: scraper::Html, @@ -801,14 +825,14 @@ impl<'a> MarkdownParser<'a> { if let Some(width) = element .attr("width") - .and_then(|width| width.parse::().ok()) + .and_then(|width| Self::parse_length(width)) { image.set_width(width); } if let Some(height) = element .attr("height") - .and_then(|height| height.parse::().ok()) + .and_then(|height| Self::parse_length(height)) { image.set_height(height); } @@ -832,6 +856,7 @@ mod tests { HighlightId, Language, LanguageConfig, LanguageMatcher, LanguageRegistry, tree_sitter_rust, }; use pretty_assertions::assert_eq; + use ui::{AbsoluteLength, DefiniteLength}; async fn parse(input: &str) -> ParsedMarkdown { parse_markdown(input, None, None).await @@ -1154,8 +1179,8 @@ mod tests { url: "http://example.com/foo.png".to_string(), }, alt_text: None, - height: Some(100), - width: Some(200), + height: Some(DefiniteLength::Absolute(AbsoluteLength::Pixels(px(100.)))), + width: Some(DefiniteLength::Absolute(AbsoluteLength::Pixels(px(200.)))), }, ); } diff --git a/crates/markdown_preview/src/markdown_renderer.rs b/crates/markdown_preview/src/markdown_renderer.rs index 674efdabdd..80e99f304f 100644 --- a/crates/markdown_preview/src/markdown_renderer.rs +++ b/crates/markdown_preview/src/markdown_renderer.rs @@ -22,7 +22,7 @@ use ui::{ ButtonCommon, Clickable, Color, FluentBuilder, IconButton, IconName, IconSize, InteractiveElement, Label, LabelCommon, LabelSize, LinkPreview, Pixels, Rems, StatefulInteractiveElement, StyledExt, StyledImage, ToggleState, Tooltip, VisibleOnHover, - h_flex, px, relative, tooltip_container, v_flex, + h_flex, relative, tooltip_container, v_flex, }; use workspace::{OpenOptions, OpenVisible, Workspace}; @@ -758,8 +758,8 @@ fn render_markdown_image(image: &Image, cx: &mut RenderContext) -> AnyElement { let alt_text = image.alt_text.clone(); move || div().children(alt_text.clone()).into_any_element() }) - .when_some(image.height, |this, height| this.h(px(height as f32))) - .when_some(image.width, |this, width| this.w(px(width as f32))), + .when_some(image.height, |this, height| this.h(height)) + .when_some(image.width, |this, width| this.w(width)), ) .tooltip({ let link = image.link.clone(); From 85321152cf53e47bd55cadf61c1eb85273e7c240 Mon Sep 17 00:00:00 2001 From: Remco Smits Date: Fri, 22 Aug 2025 20:09:45 +0200 Subject: [PATCH 06/14] Refactoring so we can support other html types aswell --- Cargo.lock | 105 +------------ Cargo.toml | 1 - crates/markdown_preview/Cargo.toml | 3 +- .../markdown_preview/src/markdown_parser.rs | 148 ++++++++++++------ 4 files changed, 105 insertions(+), 152 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 94dd63c66c..e210baa814 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5117,12 +5117,6 @@ dependencies = [ "zlog", ] -[[package]] -name = "ego-tree" -version = "0.10.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b2972feb8dffe7bc8c5463b1dacda1b0dfbed3710e50f977d965429692d74cd8" - [[package]] name = "either" version = "1.15.0" @@ -6335,15 +6329,6 @@ dependencies = [ "thread_local", ] -[[package]] -name = "fxhash" -version = "0.2.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c31b6d751ae2c7f11320402d34e41349dd1016f8d5d45e48c4312bc8625af50c" -dependencies = [ - "byteorder", -] - [[package]] name = "generator" version = "0.8.5" @@ -6378,15 +6363,6 @@ dependencies = [ "windows-targets 0.48.5", ] -[[package]] -name = "getopts" -version = "0.2.23" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "cba6ae63eb948698e300f645f87c70f76630d505f23b8907cf1e193ee85048c1" -dependencies = [ - "unicode-width 0.2.0", -] - [[package]] name = "getrandom" version = "0.2.15" @@ -7913,18 +7889,7 @@ dependencies = [ "log", "mac", "markup5ever 0.16.1", - "match_token 0.1.0", -] - -[[package]] -name = "html5ever" -version = "0.35.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "55d958c2f74b664487a2035fe1dadb032c48718a03b63f3ab0b8537db8549ed4" -dependencies = [ - "log", - "markup5ever 0.35.0", - "match_token 0.35.0", + "match_token", ] [[package]] @@ -9965,12 +9930,13 @@ dependencies = [ "editor", "fs", "gpui", + "html5ever 0.27.0", "language", "linkify", "log", + "markup5ever_rcdom", "pretty_assertions", "pulldown-cmark 0.12.2", - "scraper", "settings", "theme", "ui", @@ -10004,17 +9970,6 @@ dependencies = [ "web_atoms", ] -[[package]] -name = "markup5ever" -version = "0.35.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "311fe69c934650f8f19652b3946075f0fc41ad8757dbb68f1ca14e7900ecc1c3" -dependencies = [ - "log", - "tendril", - "web_atoms", -] - [[package]] name = "markup5ever_rcdom" version = "0.3.0" @@ -10038,17 +9993,6 @@ dependencies = [ "syn 2.0.101", ] -[[package]] -name = "match_token" -version = "0.35.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ac84fd3f360fcc43dc5f5d186f02a94192761a080e8bc58621ad4d12296a58cf" -dependencies = [ - "proc-macro2", - "quote", - "syn 2.0.101", -] - [[package]] name = "matchers" version = "0.1.0" @@ -14460,21 +14404,6 @@ version = "1.2.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "94143f37725109f92c262ed2cf5e59bce7498c01bcc1502d7b9afe439a4e9f49" -[[package]] -name = "scraper" -version = "0.24.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e5f3a24d916e78954af99281a455168d4a9515d65eca99a18da1b813689c4ad9" -dependencies = [ - "cssparser", - "ego-tree", - "getopts", - "html5ever 0.35.0", - "precomputed-hash", - "selectors", - "tendril", -] - [[package]] name = "scratch" version = "1.0.8" @@ -14719,25 +14648,6 @@ dependencies = [ "libc", ] -[[package]] -name = "selectors" -version = "0.31.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5685b6ae43bfcf7d2e7dfcfb5d8e8f61b46442c902531e41a32a9a8bf0ee0fb6" -dependencies = [ - "bitflags 2.9.0", - "cssparser", - "derive_more 2.0.1", - "fxhash", - "log", - "new_debug_unreachable", - "phf", - "phf_codegen", - "precomputed-hash", - "servo_arc", - "smallvec", -] - [[package]] name = "self_cell" version = "1.2.0" @@ -14924,15 +14834,6 @@ dependencies = [ "winapi", ] -[[package]] -name = "servo_arc" -version = "0.4.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "204ea332803bd95a0b60388590d59cf6468ec9becf626e2451f1d26a1d972de4" -dependencies = [ - "stable_deref_trait", -] - [[package]] name = "session" version = "0.1.0" diff --git a/Cargo.toml b/Cargo.toml index 2d5a57ca3e..b13795e1e1 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -574,7 +574,6 @@ rustls = { version = "0.23.26" } rustls-platform-verifier = "0.5.0" scap = { git = "https://github.com/zed-industries/scap", rev = "808aa5c45b41e8f44729d02e38fd00a2fe2722e7", default-features = false } schemars = { version = "1.0", features = ["indexmap2"] } -scraper = "0.24.0" semver = "1.0" serde = { version = "1.0", features = ["derive", "rc"] } serde_derive = { version = "1.0", features = ["deserialize_in_place"] } diff --git a/crates/markdown_preview/Cargo.toml b/crates/markdown_preview/Cargo.toml index 0a9d228de2..55646cdcf4 100644 --- a/crates/markdown_preview/Cargo.toml +++ b/crates/markdown_preview/Cargo.toml @@ -21,12 +21,13 @@ collections.workspace = true editor.workspace = true fs.workspace = true gpui.workspace = true +html5ever.workspace = true language.workspace = true linkify.workspace = true log.workspace = true +markup5ever_rcdom.workspace = true pretty_assertions.workspace = true pulldown-cmark.workspace = true -scraper.workspace = true settings.workspace = true theme.workspace = true ui.workspace = true diff --git a/crates/markdown_preview/src/markdown_parser.rs b/crates/markdown_preview/src/markdown_parser.rs index 0562ae4932..9e8e179560 100644 --- a/crates/markdown_preview/src/markdown_parser.rs +++ b/crates/markdown_preview/src/markdown_parser.rs @@ -2,9 +2,11 @@ use crate::markdown_elements::*; use async_recursion::async_recursion; use collections::FxHashMap; use gpui::FontWeight; +use html5ever::{ParseOpts, local_name, parse_document, tendril::TendrilSink}; use language::LanguageRegistry; +use markup5ever_rcdom::RcDom; use pulldown_cmark::{Alignment, Event, Options, Parser, Tag, TagEnd}; -use std::{ops::Range, path::PathBuf, sync::Arc, vec}; +use std::{cell::RefCell, ops::Range, path::PathBuf, rc::Rc, sync::Arc, vec}; use ui::{px, relative}; pub async fn parse_markdown( @@ -757,10 +759,19 @@ impl<'a> MarkdownParser<'a> { let source_range = source_range.clone(); match current { Event::Html(html) => { - let fragment = scraper::Html::parse_fragment(html); + let mut cursor = std::io::Cursor::new(html.as_bytes()); + let Some(dom) = parse_document(RcDom::default(), ParseOpts::default()) + .from_utf8() + .read_from(&mut cursor) + .ok() + else { + self.cursor += 1; + continue; + }; + self.cursor += 1; - elements.extend(self.parse_html_image(fragment, source_range)); + self.parse_html_node(source_range, &dom.document, &mut elements); } Event::End(TagEnd::CodeBlock) => { self.cursor += 1; @@ -775,6 +786,92 @@ impl<'a> MarkdownParser<'a> { elements } + fn attr_value( + attrs: &RefCell>, + name: html5ever::LocalName, + ) -> Option { + attrs.borrow().iter().find_map(|attr| { + if attr.name.local == name { + Some(attr.value.to_string()) + } else { + None + } + }) + } + + fn parse_html_node( + &self, + source_range: Range, + node: &Rc, + elements: &mut Vec, + ) { + match &node.data { + markup5ever_rcdom::NodeData::Document => { + self.consume_children(source_range, node, elements); + } + markup5ever_rcdom::NodeData::Doctype { .. } => {} + markup5ever_rcdom::NodeData::Text { contents } => { + elements.push(ParsedMarkdownElement::Paragraph(vec![ + MarkdownParagraphChunk::Text(ParsedMarkdownText { + source_range, + contents: contents.borrow().to_string(), + highlights: Vec::default(), + region_ranges: Vec::default(), + regions: Vec::default(), + }), + ])); + } + markup5ever_rcdom::NodeData::Comment { .. } => {} + markup5ever_rcdom::NodeData::Element { name, attrs, .. } => { + if local_name!("img") == name.local { + let Some(src) = Self::attr_value(attrs, local_name!("src")) else { + return; + }; + + let Some(mut image) = Image::identify( + src.to_string(), + source_range, + self.file_location_directory.clone(), + ) else { + return; + }; + + if let Some(alt) = Self::attr_value(attrs, local_name!("alt")) { + image.set_alt_text(alt.to_string().into()); + } + + if let Some(width) = Self::attr_value(attrs, local_name!("width")) + .and_then(|width| Self::parse_length(&width)) + { + image.set_width(width); + } + + if let Some(height) = Self::attr_value(attrs, local_name!("height")) + .and_then(|height| Self::parse_length(&height)) + { + image.set_height(height); + } + + elements.push(ParsedMarkdownElement::Image(image)); + } else { + self.consume_children(source_range, node, elements); + } + } + markup5ever_rcdom::NodeData::ProcessingInstruction { .. } => {} + } + } + + fn consume_children( + &self, + source_range: Range, + node: &Rc, + elements: &mut Vec, + ) { + for node in node.children.borrow().iter() { + self.parse_html_node(source_range.clone(), node, elements); + } + } + /// Parses the width/height attribute value of an html element (e.g. img element) fn parse_length(value: &str) -> Option { if value.ends_with("px") { @@ -797,51 +894,6 @@ impl<'a> MarkdownParser<'a> { .map(|value| px(value).into()) } } - - fn parse_html_image( - &self, - html: scraper::Html, - source_range: Range, - ) -> Vec { - let mut images = Vec::new(); - let selector = scraper::Selector::parse("img").unwrap(); - - for element in html.select(&selector) { - let Some(src) = element.attr("src") else { - continue; - }; - - let Some(mut image) = Image::identify( - src.to_string(), - source_range.clone(), - self.file_location_directory.clone(), - ) else { - continue; - }; - - if let Some(alt) = element.attr("alt") { - image.set_alt_text(alt.to_string().into()); - } - - if let Some(width) = element - .attr("width") - .and_then(|width| Self::parse_length(width)) - { - image.set_width(width); - } - - if let Some(height) = element - .attr("height") - .and_then(|height| Self::parse_length(height)) - { - image.set_height(height); - } - - images.push(ParsedMarkdownElement::Image(image)); - } - - images - } } #[cfg(test)] From d00c24453387a1370e238ace3a6721acb9509da8 Mon Sep 17 00:00:00 2001 From: Remco Smits Date: Fri, 22 Aug 2025 20:13:24 +0200 Subject: [PATCH 07/14] Clippy --- crates/markdown_preview/src/markdown_parser.rs | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/crates/markdown_preview/src/markdown_parser.rs b/crates/markdown_preview/src/markdown_parser.rs index 9e8e179560..30bb97e89d 100644 --- a/crates/markdown_preview/src/markdown_parser.rs +++ b/crates/markdown_preview/src/markdown_parser.rs @@ -828,16 +828,14 @@ impl<'a> MarkdownParser<'a> { return; }; - let Some(mut image) = Image::identify( - src.to_string(), - source_range, - self.file_location_directory.clone(), - ) else { + let Some(mut image) = + Image::identify(src, source_range, self.file_location_directory.clone()) + else { return; }; if let Some(alt) = Self::attr_value(attrs, local_name!("alt")) { - image.set_alt_text(alt.to_string().into()); + image.set_alt_text(alt.into()); } if let Some(width) = Self::attr_value(attrs, local_name!("width")) From ac8fdfb48f9110b4f8caf38e90926f36b64a0bd5 Mon Sep 17 00:00:00 2001 From: Remco Smits Date: Sat, 23 Aug 2025 16:36:42 +0200 Subject: [PATCH 08/14] Extract width and height from style attribute --- .../markdown_preview/src/markdown_parser.rs | 108 +++++++++++------- 1 file changed, 68 insertions(+), 40 deletions(-) diff --git a/crates/markdown_preview/src/markdown_parser.rs b/crates/markdown_preview/src/markdown_parser.rs index 30bb97e89d..409a838658 100644 --- a/crates/markdown_preview/src/markdown_parser.rs +++ b/crates/markdown_preview/src/markdown_parser.rs @@ -6,7 +6,7 @@ use html5ever::{ParseOpts, local_name, parse_document, tendril::TendrilSink}; use language::LanguageRegistry; use markup5ever_rcdom::RcDom; use pulldown_cmark::{Alignment, Event, Options, Parser, Tag, TagEnd}; -use std::{cell::RefCell, ops::Range, path::PathBuf, rc::Rc, sync::Arc, vec}; +use std::{cell::RefCell, collections::HashMap, ops::Range, path::PathBuf, rc::Rc, sync::Arc, vec}; use ui::{px, relative}; pub async fn parse_markdown( @@ -786,19 +786,6 @@ impl<'a> MarkdownParser<'a> { elements } - fn attr_value( - attrs: &RefCell>, - name: html5ever::LocalName, - ) -> Option { - attrs.borrow().iter().find_map(|attr| { - if attr.name.local == name { - Some(attr.value.to_string()) - } else { - None - } - }) - } - fn parse_html_node( &self, source_range: Range, @@ -824,33 +811,9 @@ impl<'a> MarkdownParser<'a> { markup5ever_rcdom::NodeData::Comment { .. } => {} markup5ever_rcdom::NodeData::Element { name, attrs, .. } => { if local_name!("img") == name.local { - let Some(src) = Self::attr_value(attrs, local_name!("src")) else { - return; - }; - - let Some(mut image) = - Image::identify(src, source_range, self.file_location_directory.clone()) - else { - return; - }; - - if let Some(alt) = Self::attr_value(attrs, local_name!("alt")) { - image.set_alt_text(alt.into()); + if let Some(image) = self.extract_image(source_range, attrs) { + elements.push(ParsedMarkdownElement::Image(image)); } - - if let Some(width) = Self::attr_value(attrs, local_name!("width")) - .and_then(|width| Self::parse_length(&width)) - { - image.set_width(width); - } - - if let Some(height) = Self::attr_value(attrs, local_name!("height")) - .and_then(|height| Self::parse_length(&height)) - { - image.set_height(height); - } - - elements.push(ParsedMarkdownElement::Image(image)); } else { self.consume_children(source_range, node, elements); } @@ -870,6 +833,71 @@ impl<'a> MarkdownParser<'a> { } } + fn attr_value( + attrs: &RefCell>, + name: html5ever::LocalName, + ) -> Option { + attrs.borrow().iter().find_map(|attr| { + if attr.name.local == name { + Some(attr.value.to_string()) + } else { + None + } + }) + } + + fn extract_styles_from_attributes( + attrs: &RefCell>, + ) -> HashMap { + let mut styles = HashMap::new(); + + if let Some(style) = Self::attr_value(attrs, local_name!("style")) { + for decl in style.split(';') { + let mut parts = decl.splitn(2, ':'); + if let Some((key, value)) = parts.next().zip(parts.next()) { + styles.insert( + key.trim().to_lowercase().to_string(), + value.trim().to_string(), + ); + } + } + } + + styles + } + + fn extract_image( + &self, + source_range: Range, + attrs: &RefCell>, + ) -> Option { + let src = Self::attr_value(attrs, local_name!("src"))?; + + let mut image = Image::identify(src, source_range, self.file_location_directory.clone())?; + + if let Some(alt) = Self::attr_value(attrs, local_name!("alt")) { + image.set_alt_text(alt.into()); + } + + let styles = Self::extract_styles_from_attributes(attrs); + + if let Some(width) = Self::attr_value(attrs, local_name!("width")) + .or_else(|| styles.get("width").cloned()) + .and_then(|width| Self::parse_length(&width)) + { + image.set_width(width); + } + + if let Some(height) = Self::attr_value(attrs, local_name!("height")) + .or_else(|| styles.get("height").cloned()) + .and_then(|height| Self::parse_length(&height)) + { + image.set_height(height); + } + + Some(image) + } + /// Parses the width/height attribute value of an html element (e.g. img element) fn parse_length(value: &str) -> Option { if value.ends_with("px") { From 392f4b461ad8fce4dae41dfda30489083318e6fb Mon Sep 17 00:00:00 2001 From: Remco Smits Date: Mon, 25 Aug 2025 07:31:00 +0200 Subject: [PATCH 09/14] Fix review issues --- .../markdown_preview/src/markdown_parser.rs | 34 ++++++++++--------- 1 file changed, 18 insertions(+), 16 deletions(-) diff --git a/crates/markdown_preview/src/markdown_parser.rs b/crates/markdown_preview/src/markdown_parser.rs index 409a838658..e583c2ac99 100644 --- a/crates/markdown_preview/src/markdown_parser.rs +++ b/crates/markdown_preview/src/markdown_parser.rs @@ -1,13 +1,12 @@ use crate::markdown_elements::*; use async_recursion::async_recursion; use collections::FxHashMap; -use gpui::FontWeight; +use gpui::{DefiniteLength, FontWeight, px, relative}; use html5ever::{ParseOpts, local_name, parse_document, tendril::TendrilSink}; use language::LanguageRegistry; use markup5ever_rcdom::RcDom; use pulldown_cmark::{Alignment, Event, Options, Parser, Tag, TagEnd}; use std::{cell::RefCell, collections::HashMap, ops::Range, path::PathBuf, rc::Rc, sync::Arc, vec}; -use ui::{px, relative}; pub async fn parse_markdown( markdown_input: &str, @@ -175,7 +174,7 @@ impl<'a> MarkdownParser<'a> { self.cursor += 1; - let code_block = self.parse_code_block(language).await; + let code_block = self.parse_code_block(language).await?; Some(vec![ParsedMarkdownElement::CodeBlock(code_block)]) } Tag::HtmlBlock => { @@ -703,13 +702,22 @@ impl<'a> MarkdownParser<'a> { } } - async fn parse_code_block(&mut self, language: Option) -> ParsedMarkdownCodeBlock { - let (_event, source_range) = self.previous().unwrap(); + async fn parse_code_block( + &mut self, + language: Option, + ) -> Option { + let Some((_event, source_range)) = self.previous() else { + return None; + }; + let source_range = source_range.clone(); let mut code = String::new(); while !self.eof() { - let (current, _source_range) = self.current().unwrap(); + let Some((current, _source_range)) = self.current() else { + break; + }; + match current { Event::Text(text) => { code.push_str(text); @@ -742,12 +750,12 @@ impl<'a> MarkdownParser<'a> { None }; - ParsedMarkdownCodeBlock { + Some(ParsedMarkdownCodeBlock { source_range, contents: code.into(), language, highlights, - } + }) } async fn parse_html_block(&mut self) -> Vec { @@ -899,14 +907,8 @@ impl<'a> MarkdownParser<'a> { } /// Parses the width/height attribute value of an html element (e.g. img element) - fn parse_length(value: &str) -> Option { - if value.ends_with("px") { - value - .trim_end_matches("px") - .parse() - .ok() - .map(|value| px(value).into()) - } else if value.ends_with("%") { + fn parse_length(value: &str) -> Option { + if value.ends_with("%") { value .trim_end_matches("%") .parse::() From 6426342cfa8127e944e8527d6ff7d048786dadd9 Mon Sep 17 00:00:00 2001 From: Remco Smits Date: Mon, 25 Aug 2025 07:31:28 +0200 Subject: [PATCH 10/14] Add more tests --- .../markdown_preview/src/markdown_parser.rs | 83 +++++++++++++++++-- 1 file changed, 78 insertions(+), 5 deletions(-) diff --git a/crates/markdown_preview/src/markdown_parser.rs b/crates/markdown_preview/src/markdown_parser.rs index e583c2ac99..475cd1127b 100644 --- a/crates/markdown_preview/src/markdown_parser.rs +++ b/crates/markdown_preview/src/markdown_parser.rs @@ -926,17 +926,14 @@ impl<'a> MarkdownParser<'a> { #[cfg(test)] mod tests { - use core::panic; - use super::*; - use ParsedMarkdownListItemType::*; - use gpui::BackgroundExecutor; + use core::panic; + use gpui::{AbsoluteLength, BackgroundExecutor, DefiniteLength}; use language::{ HighlightId, Language, LanguageConfig, LanguageMatcher, LanguageRegistry, tree_sitter_rust, }; use pretty_assertions::assert_eq; - use ui::{AbsoluteLength, DefiniteLength}; async fn parse(input: &str) -> ParsedMarkdown { parse_markdown(input, None, None).await @@ -1201,6 +1198,58 @@ mod tests { ); } + #[test] + fn test_parse_length() { + // Test percentage values + assert_eq!( + MarkdownParser::parse_length("50%"), + Some(relative(0.5).into()) + ); + assert_eq!( + MarkdownParser::parse_length("100%"), + Some(relative(1.0).into()) + ); + assert_eq!( + MarkdownParser::parse_length("25%"), + Some(relative(0.25).into()) + ); + assert_eq!( + MarkdownParser::parse_length("0%"), + Some(relative(0.0).into()) + ); + + // Test pixel values + assert_eq!( + MarkdownParser::parse_length("100px"), + Some(px(100.0).into()) + ); + assert_eq!(MarkdownParser::parse_length("50px"), Some(px(50.0).into())); + assert_eq!(MarkdownParser::parse_length("0px"), Some(px(0.0).into())); + + // Test values without units (should be treated as pixels) + assert_eq!(MarkdownParser::parse_length("100"), Some(px(100.0).into())); + assert_eq!(MarkdownParser::parse_length("42"), Some(px(42.0).into())); + + // Test invalid values + assert_eq!(MarkdownParser::parse_length("invalid"), None); + assert_eq!(MarkdownParser::parse_length("px"), None); + assert_eq!(MarkdownParser::parse_length("%"), None); + assert_eq!(MarkdownParser::parse_length(""), None); + assert_eq!(MarkdownParser::parse_length("abc%"), None); + assert_eq!(MarkdownParser::parse_length("abcpx"), None); + + // Test decimal values + assert_eq!( + MarkdownParser::parse_length("50.5%"), + Some(relative(0.505).into()) + ); + assert_eq!( + MarkdownParser::parse_length("100.25px"), + Some(px(100.25).into()) + ); + assert_eq!(MarkdownParser::parse_length("42.0"), Some(px(42.0).into())); + } + #[gpui::test] async fn test_html_image_tag() { let parsed = parse("").await; @@ -1265,6 +1314,30 @@ mod tests { ); } + #[gpui::test] + async fn test_html_image_style_tag_with_height_and_width() { + let parsed = parse( + "", + ) + .await; + + let ParsedMarkdownElement::Image(image) = &parsed.children[0] else { + panic!("Expected a image element"); + }; + assert_eq!( + image.clone(), + Image { + source_range: 0..65, + link: Link::Web { + url: "http://example.com/foo.png".to_string(), + }, + alt_text: None, + height: Some(DefiniteLength::Absolute(AbsoluteLength::Pixels(px(100.)))), + width: Some(DefiniteLength::Absolute(AbsoluteLength::Pixels(px(200.)))), + }, + ); + } + #[gpui::test] async fn test_header_only_table() { let markdown = "\ From 1db824fbc1e1c9795442166444d2ab67c5130021 Mon Sep 17 00:00:00 2001 From: Remco Smits Date: Mon, 25 Aug 2025 07:33:13 +0200 Subject: [PATCH 11/14] Remove one more unwrap --- crates/markdown_preview/src/markdown_parser.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/crates/markdown_preview/src/markdown_parser.rs b/crates/markdown_preview/src/markdown_parser.rs index 475cd1127b..bf968007a7 100644 --- a/crates/markdown_preview/src/markdown_parser.rs +++ b/crates/markdown_preview/src/markdown_parser.rs @@ -759,11 +759,15 @@ impl<'a> MarkdownParser<'a> { } async fn parse_html_block(&mut self) -> Vec { - let (_event, _source_range) = self.previous().unwrap(); let mut elements = Vec::new(); + let Some((_event, _source_range)) = self.previous() else { + return elements; + }; while !self.eof() { - let (current, source_range) = self.current().unwrap(); + let Some((current, source_range)) = self.current() else { + break; + }; let source_range = source_range.clone(); match current { Event::Html(html) => { From 5e71f4cb3fa41699da66fca62bb897f3ed1ff86b Mon Sep 17 00:00:00 2001 From: Remco Smits Date: Mon, 25 Aug 2025 07:43:45 +0200 Subject: [PATCH 12/14] Fix clippy --- .../markdown_preview/src/markdown_parser.rs | 56 ++++++++++++++----- 1 file changed, 43 insertions(+), 13 deletions(-) diff --git a/crates/markdown_preview/src/markdown_parser.rs b/crates/markdown_preview/src/markdown_parser.rs index bf968007a7..a8154b1752 100644 --- a/crates/markdown_preview/src/markdown_parser.rs +++ b/crates/markdown_preview/src/markdown_parser.rs @@ -938,6 +938,7 @@ 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 @@ -1207,32 +1208,54 @@ mod tests { // Test percentage values assert_eq!( MarkdownParser::parse_length("50%"), - Some(relative(0.5).into()) + Some(DefiniteLength::Fraction(0.5)) ); assert_eq!( MarkdownParser::parse_length("100%"), - Some(relative(1.0).into()) + Some(DefiniteLength::Fraction(1.0)) ); assert_eq!( MarkdownParser::parse_length("25%"), - Some(relative(0.25).into()) + Some(DefiniteLength::Fraction(0.25)) ); assert_eq!( MarkdownParser::parse_length("0%"), - Some(relative(0.0).into()) + Some(DefiniteLength::Fraction(0.0)) ); // Test pixel values assert_eq!( MarkdownParser::parse_length("100px"), - Some(px(100.0).into()) + Some(DefiniteLength::Absolute(AbsoluteLength::Pixels(Pixels( + 100.0 + )))) + ); + assert_eq!( + MarkdownParser::parse_length("50px"), + Some(DefiniteLength::Absolute(AbsoluteLength::Pixels(Pixels( + 50.0 + )))) + ); + assert_eq!( + MarkdownParser::parse_length("0px"), + Some(DefiniteLength::Absolute(AbsoluteLength::Pixels(Pixels( + 0.0 + )))) ); - assert_eq!(MarkdownParser::parse_length("50px"), Some(px(50.0).into())); - assert_eq!(MarkdownParser::parse_length("0px"), Some(px(0.0).into())); // Test values without units (should be treated as pixels) - assert_eq!(MarkdownParser::parse_length("100"), Some(px(100.0).into())); - assert_eq!(MarkdownParser::parse_length("42"), Some(px(42.0).into())); + assert_eq!( + MarkdownParser::parse_length("100"), + Some(DefiniteLength::Absolute(AbsoluteLength::Pixels(Pixels( + 100.0 + )))) + ); + assert_eq!( + MarkdownParser::parse_length("42"), + Some(DefiniteLength::Absolute(AbsoluteLength::Pixels(Pixels( + 42.0 + )))) + ); // Test invalid values assert_eq!(MarkdownParser::parse_length("invalid"), None); @@ -1245,13 +1268,20 @@ mod tests { // Test decimal values assert_eq!( MarkdownParser::parse_length("50.5%"), - Some(relative(0.505).into()) + Some(DefiniteLength::Fraction(0.505)) ); assert_eq!( MarkdownParser::parse_length("100.25px"), - Some(px(100.25).into()) + Some(DefiniteLength::Absolute(AbsoluteLength::Pixels(Pixels( + 100.25 + )))) + ); + assert_eq!( + MarkdownParser::parse_length("42.0"), + Some(DefiniteLength::Absolute(AbsoluteLength::Pixels(Pixels( + 42.0 + )))) ); - assert_eq!(MarkdownParser::parse_length("42.0"), Some(px(42.0).into())); } #[gpui::test] @@ -1331,7 +1361,7 @@ mod tests { assert_eq!( image.clone(), Image { - source_range: 0..65, + source_range: 0..75, link: Link::Web { url: "http://example.com/foo.png".to_string(), }, From 9db3f54ac326d35b62c1e236db3921d301737f81 Mon Sep 17 00:00:00 2001 From: Remco Smits Date: Mon, 25 Aug 2025 08:23:19 +0200 Subject: [PATCH 13/14] fix review --- crates/markdown_preview/src/markdown_elements.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/markdown_preview/src/markdown_elements.rs b/crates/markdown_preview/src/markdown_elements.rs index 9da94f30db..f87543b046 100644 --- a/crates/markdown_preview/src/markdown_elements.rs +++ b/crates/markdown_preview/src/markdown_elements.rs @@ -1,9 +1,9 @@ +use gpui::DefiniteLength; use gpui::{ FontStyle, FontWeight, HighlightStyle, SharedString, StrikethroughStyle, UnderlineStyle, px, }; use language::HighlightId; use std::{fmt::Display, ops::Range, path::PathBuf}; -use ui::DefiniteLength; #[derive(Debug)] #[cfg_attr(test, derive(PartialEq))] From e606e2e2398b1664ae86a4385a2850ba6739bb89 Mon Sep 17 00:00:00 2001 From: Remco Smits Date: Mon, 25 Aug 2025 08:53:41 +0200 Subject: [PATCH 14/14] 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(), }) } }