From 43163a0154e94518f67a6a935c0760348274ab6a Mon Sep 17 00:00:00 2001 From: Bennet Bo Fenner <53836821+bennetbo@users.noreply.github.com> Date: Mon, 26 Feb 2024 20:04:48 +0100 Subject: [PATCH] Support rendering strikethrough text in markdown (#8287) Just noticed strikethrough text handling was not implemented for the following: Chat ![image](https://github.com/zed-industries/zed/assets/53836821/ddd98272-d4d4-4a94-bd79-77e967f3ca15) Markdown Preview ![image](https://github.com/zed-industries/zed/assets/53836821/9087635c-5b89-40e6-8e4d-2785a43ef318) Code Documentation ![image](https://github.com/zed-industries/zed/assets/53836821/5ed55c60-3e5e-4fc2-86c2-a81fac7de038) It looks like there are three different markdown parsing/rendering implementations, might be worth to investigate if any of these can be combined into a single crate (looks like a lot of work though). Release Notes: - Added support for rendering strikethrough text in markdown elements --- Cargo.lock | 6 +- Cargo.toml | 2 +- crates/language/src/markdown.rs | 47 ++++-- .../markdown_preview/src/markdown_elements.rs | 13 +- .../markdown_preview/src/markdown_parser.rs | 149 ++++++++++++------ crates/rich_text/src/rich_text.rs | 39 +++-- 6 files changed, 187 insertions(+), 69 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 629086d364..f15a53d2d1 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -7042,11 +7042,11 @@ dependencies = [ [[package]] name = "pulldown-cmark" -version = "0.9.3" +version = "0.10.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "77a1a2f1f0a7ecff9c31abbe177637be0e97a0aef46cf8738ece09327985d998" +checksum = "dce76ce678ffc8e5675b22aa1405de0b7037e2fdf8913fea40d1926c6fe1e6e7" dependencies = [ - "bitflags 1.3.2", + "bitflags 2.4.1", "memchr", "unicase", ] diff --git a/Cargo.toml b/Cargo.toml index 9a62339018..78d48688ee 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -218,7 +218,7 @@ profiling = "1" postage = { version = "0.5", features = ["futures-traits"] } pretty_assertions = "1.3.0" prost = "0.8" -pulldown-cmark = { version = "0.9.2", default-features = false } +pulldown-cmark = { version = "0.10.0", default-features = false } rand = "0.8.5" refineable = { path = "./crates/refineable" } regex = "1.5" diff --git a/crates/language/src/markdown.rs b/crates/language/src/markdown.rs index 0ff78e49a9..cc0c0becce 100644 --- a/crates/language/src/markdown.rs +++ b/crates/language/src/markdown.rs @@ -4,8 +4,8 @@ use std::sync::Arc; use std::{ops::Range, path::PathBuf}; use crate::{HighlightId, Language, LanguageRegistry}; -use gpui::{px, FontStyle, FontWeight, HighlightStyle, UnderlineStyle}; -use pulldown_cmark::{CodeBlockKind, Event, Options, Parser, Tag}; +use gpui::{px, FontStyle, FontWeight, HighlightStyle, StrikethroughStyle, UnderlineStyle}; +use pulldown_cmark::{CodeBlockKind, Event, Options, Parser, Tag, TagEnd}; /// Parsed Markdown content. #[derive(Debug, Clone)] @@ -47,6 +47,13 @@ impl MarkdownHighlight { }); } + if style.strikethrough { + highlight.strikethrough = Some(StrikethroughStyle { + thickness: px(1.), + ..Default::default() + }); + } + if style.weight != FontWeight::default() { highlight.font_weight = Some(style.weight); } @@ -66,6 +73,8 @@ pub struct MarkdownHighlightStyle { pub italic: bool, /// Whether the text should be underlined. pub underline: bool, + /// Whether the text should be struck through. + pub strikethrough: bool, /// The weight of the text. pub weight: FontWeight, } @@ -151,6 +160,7 @@ pub async fn parse_markdown_block( ) { let mut bold_depth = 0; let mut italic_depth = 0; + let mut strikethrough_depth = 0; let mut link_url = None; let mut current_language = None; let mut list_stack = Vec::new(); @@ -174,6 +184,10 @@ pub async fn parse_markdown_block( style.italic = true; } + if strikethrough_depth > 0 { + style.strikethrough = true; + } + if let Some(link) = link_url.clone().and_then(|u| Link::identify(u)) { region_ranges.push(prev_len..text.len()); regions.push(ParsedRegion { @@ -221,7 +235,12 @@ pub async fn parse_markdown_block( Event::Start(tag) => match tag { Tag::Paragraph => new_paragraph(text, &mut list_stack), - Tag::Heading(_, _, _) => { + Tag::Heading { + level: _, + id: _, + classes: _, + attrs: _, + } => { new_paragraph(text, &mut list_stack); bold_depth += 1; } @@ -242,7 +261,14 @@ pub async fn parse_markdown_block( Tag::Strong => bold_depth += 1, - Tag::Link(_, url, _) => link_url = Some(url.to_string()), + Tag::Strikethrough => strikethrough_depth += 1, + + Tag::Link { + link_type: _, + dest_url, + title: _, + id: _, + } => link_url = Some(dest_url.to_string()), Tag::List(number) => { list_stack.push((number, false)); @@ -272,12 +298,13 @@ pub async fn parse_markdown_block( }, Event::End(tag) => match tag { - Tag::Heading(_, _, _) => bold_depth -= 1, - Tag::CodeBlock(_) => current_language = None, - Tag::Emphasis => italic_depth -= 1, - Tag::Strong => bold_depth -= 1, - Tag::Link(_, _, _) => link_url = None, - Tag::List(_) => drop(list_stack.pop()), + TagEnd::Heading(_) => bold_depth -= 1, + TagEnd::CodeBlock => current_language = None, + TagEnd::Emphasis => italic_depth -= 1, + TagEnd::Strong => bold_depth -= 1, + TagEnd::Strikethrough => strikethrough_depth -= 1, + TagEnd::Link => link_url = None, + TagEnd::List(_) => drop(list_stack.pop()), _ => {} }, diff --git a/crates/markdown_preview/src/markdown_elements.rs b/crates/markdown_preview/src/markdown_elements.rs index 54cd01f8cd..61f4459d62 100644 --- a/crates/markdown_preview/src/markdown_elements.rs +++ b/crates/markdown_preview/src/markdown_elements.rs @@ -1,4 +1,6 @@ -use gpui::{px, FontStyle, FontWeight, HighlightStyle, SharedString, UnderlineStyle}; +use gpui::{ + px, FontStyle, FontWeight, HighlightStyle, SharedString, StrikethroughStyle, UnderlineStyle, +}; use language::HighlightId; use std::{ops::Range, path::PathBuf}; @@ -170,6 +172,13 @@ impl MarkdownHighlight { }); } + if style.strikethrough { + highlight.strikethrough = Some(StrikethroughStyle { + thickness: px(1.), + ..Default::default() + }); + } + if style.weight != FontWeight::default() { highlight.font_weight = Some(style.weight); } @@ -189,6 +198,8 @@ pub struct MarkdownHighlightStyle { pub italic: bool, /// Whether the text should be underlined. pub underline: bool, + /// Whether the text should be struck through. + pub strikethrough: bool, /// The weight of the text. pub weight: FontWeight, } diff --git a/crates/markdown_preview/src/markdown_parser.rs b/crates/markdown_preview/src/markdown_parser.rs index f860e211e2..b4e9d9829c 100644 --- a/crates/markdown_preview/src/markdown_parser.rs +++ b/crates/markdown_preview/src/markdown_parser.rs @@ -1,6 +1,6 @@ use crate::markdown_elements::*; use gpui::FontWeight; -use pulldown_cmark::{Alignment, Event, Options, Parser, Tag}; +use pulldown_cmark::{Alignment, Event, Options, Parser, Tag, TagEnd}; use std::{ops::Range, path::PathBuf}; pub fn parse_markdown( @@ -70,11 +70,11 @@ impl<'a> MarkdownParser<'a> { | Event::Code(_) | Event::Html(_) | Event::FootnoteReference(_) - | Event::Start(Tag::Link(_, _, _)) + | Event::Start(Tag::Link { link_type: _, dest_url: _, title: _, id: _ }) | Event::Start(Tag::Emphasis) | Event::Start(Tag::Strong) | Event::Start(Tag::Strikethrough) - | Event::Start(Tag::Image(_, _, _)) => { + | Event::Start(Tag::Image { link_type: _, dest_url: _, title: _, id: _ }) => { return true; } _ => return false, @@ -99,15 +99,21 @@ impl<'a> MarkdownParser<'a> { let text = self.parse_text(false); Some(ParsedMarkdownElement::Paragraph(text)) } - Tag::Heading(level, _, _) => { + Tag::Heading { + level, + id: _, + classes: _, + attrs: _, + } => { let level = level.clone(); self.cursor += 1; let heading = self.parse_heading(level); Some(ParsedMarkdownElement::Heading(heading)) } - Tag::Table(_) => { + Tag::Table(alignment) => { + let alignment = alignment.clone(); self.cursor += 1; - let table = self.parse_table(); + let table = self.parse_table(alignment); Some(ParsedMarkdownElement::Table(table)) } Tag::List(order) => { @@ -162,6 +168,7 @@ impl<'a> MarkdownParser<'a> { let mut text = String::new(); let mut bold_depth = 0; let mut italic_depth = 0; + let mut strikethrough_depth = 0; let mut link: Option = None; let mut region_ranges: Vec> = vec![]; let mut regions: Vec = vec![]; @@ -201,6 +208,10 @@ impl<'a> MarkdownParser<'a> { style.italic = true; } + if strikethrough_depth > 0 { + style.strikethrough = true; + } + if let Some(link) = link.clone() { region_ranges.push(prev_len..text.len()); regions.push(ParsedRegion { @@ -248,39 +259,40 @@ impl<'a> MarkdownParser<'a> { }); } - Event::Start(tag) => { - match tag { - Tag::Emphasis => italic_depth += 1, - Tag::Strong => bold_depth += 1, - Tag::Link(_type, url, _title) => { - link = Link::identify( - self.file_location_directory.clone(), - url.to_string(), - ); - } - Tag::Strikethrough => { - // TODO: Confirm that gpui currently doesn't support strikethroughs - } - _ => { - break; - } + Event::Start(tag) => match tag { + Tag::Emphasis => italic_depth += 1, + Tag::Strong => bold_depth += 1, + Tag::Strikethrough => strikethrough_depth += 1, + Tag::Link { + link_type: _, + dest_url, + title: _, + id: _, + } => { + link = Link::identify( + self.file_location_directory.clone(), + dest_url.to_string(), + ); } - } + _ => { + break; + } + }, Event::End(tag) => match tag { - Tag::Emphasis => { + TagEnd::Emphasis => { italic_depth -= 1; } - Tag::Strong => { + TagEnd::Strong => { bold_depth -= 1; } - Tag::Link(_, _, _) => { + TagEnd::Strikethrough => { + strikethrough_depth -= 1; + } + TagEnd::Link => { link = None; } - Tag::Strikethrough => { - // TODO: Confirm that gpui currently doesn't support strikethroughs - } - Tag::Paragraph => { + TagEnd::Paragraph => { self.cursor += 1; break; } @@ -328,14 +340,17 @@ impl<'a> MarkdownParser<'a> { } } - fn parse_table(&mut self) -> ParsedMarkdownTable { + fn parse_table(&mut self, alignment: Vec) -> ParsedMarkdownTable { let (_event, source_range) = self.previous().unwrap(); let source_range = source_range.clone(); let mut header = ParsedMarkdownTableRow::new(); let mut body = vec![]; let mut current_row = vec![]; let mut in_header = true; - let mut alignment: Vec = vec![]; + let column_alignments = alignment + .iter() + .map(|a| Self::convert_alignment(a)) + .collect(); loop { if self.eof() { @@ -346,7 +361,7 @@ impl<'a> MarkdownParser<'a> { match current { Event::Start(Tag::TableHead) | Event::Start(Tag::TableRow) - | Event::End(Tag::TableCell) => { + | Event::End(TagEnd::TableCell) => { self.cursor += 1; } Event::Start(Tag::TableCell) => { @@ -354,7 +369,7 @@ impl<'a> MarkdownParser<'a> { let cell_contents = self.parse_text(false); current_row.push(cell_contents); } - Event::End(Tag::TableHead) | Event::End(Tag::TableRow) => { + Event::End(TagEnd::TableHead) | Event::End(TagEnd::TableRow) => { self.cursor += 1; let new_row = std::mem::replace(&mut current_row, vec![]); if in_header { @@ -365,11 +380,7 @@ impl<'a> MarkdownParser<'a> { body.push(row); } } - Event::End(Tag::Table(table_alignment)) => { - alignment = table_alignment - .iter() - .map(|a| Self::convert_alignment(a)) - .collect(); + Event::End(TagEnd::Table) => { self.cursor += 1; break; } @@ -383,7 +394,7 @@ impl<'a> MarkdownParser<'a> { source_range, header, body, - column_alignments: alignment, + column_alignments, } } @@ -417,7 +428,7 @@ impl<'a> MarkdownParser<'a> { let block = ParsedMarkdownElement::List(inner_list); current_list_items.push(Box::new(block)); } - Event::End(Tag::List(_)) => { + Event::End(TagEnd::List(_)) => { self.cursor += 1; break; } @@ -451,7 +462,7 @@ impl<'a> MarkdownParser<'a> { } } } - Event::End(Tag::Item) => { + Event::End(TagEnd::Item) => { self.cursor += 1; let item_type = if let Some(checked) = task_item { @@ -525,7 +536,7 @@ impl<'a> MarkdownParser<'a> { Event::Start(Tag::BlockQuote) => { nested_depth += 1; } - Event::End(Tag::BlockQuote) => { + Event::End(TagEnd::BlockQuote) => { nested_depth -= 1; if nested_depth == 0 { self.cursor += 1; @@ -554,7 +565,7 @@ impl<'a> MarkdownParser<'a> { code.push_str(&text); self.cursor += 1; } - Event::End(Tag::CodeBlock(_)) => { + Event::End(TagEnd::CodeBlock) => { self.cursor += 1; break; } @@ -642,6 +653,56 @@ mod tests { ); } + #[test] + fn test_nested_bold_strikethrough_text() { + let parsed = parse("Some **bo~~strikethrough~~ld** text"); + + assert_eq!(parsed.children.len(), 1); + assert_eq!( + parsed.children[0], + ParsedMarkdownElement::Paragraph(ParsedMarkdownText { + source_range: 0..35, + contents: "Some bostrikethroughld text".to_string(), + highlights: Vec::new(), + region_ranges: Vec::new(), + regions: Vec::new(), + }) + ); + + let paragraph = if let ParsedMarkdownElement::Paragraph(text) = &parsed.children[0] { + text + } else { + panic!("Expected a paragraph"); + }; + assert_eq!( + paragraph.highlights, + vec![ + ( + 5..7, + MarkdownHighlight::Style(MarkdownHighlightStyle { + weight: FontWeight::BOLD, + ..Default::default() + }), + ), + ( + 7..20, + MarkdownHighlight::Style(MarkdownHighlightStyle { + weight: FontWeight::BOLD, + strikethrough: true, + ..Default::default() + }), + ), + ( + 20..22, + MarkdownHighlight::Style(MarkdownHighlightStyle { + weight: FontWeight::BOLD, + ..Default::default() + }), + ), + ] + ); + } + #[test] fn test_header_only_table() { let markdown = "\ diff --git a/crates/rich_text/src/rich_text.rs b/crates/rich_text/src/rich_text.rs index 4f61cd5b9a..effa671590 100644 --- a/crates/rich_text/src/rich_text.rs +++ b/crates/rich_text/src/rich_text.rs @@ -1,7 +1,7 @@ use futures::FutureExt; use gpui::{ AnyElement, ElementId, FontStyle, FontWeight, HighlightStyle, InteractiveText, IntoElement, - SharedString, StyledText, UnderlineStyle, WindowContext, + SharedString, StrikethroughStyle, StyledText, UnderlineStyle, WindowContext, }; use language::{HighlightId, Language, LanguageRegistry}; use std::{ops::Range, sync::Arc}; @@ -134,10 +134,11 @@ pub fn render_markdown_mut( link_ranges: &mut Vec>, link_urls: &mut Vec, ) { - use pulldown_cmark::{CodeBlockKind, Event, Options, Parser, Tag}; + use pulldown_cmark::{CodeBlockKind, Event, Options, Parser, Tag, TagEnd}; let mut bold_depth = 0; let mut italic_depth = 0; + let mut strikethrough_depth = 0; let mut link_url = None; let mut current_language = None; let mut list_stack = Vec::new(); @@ -175,6 +176,12 @@ pub fn render_markdown_mut( if italic_depth > 0 { style.font_style = Some(FontStyle::Italic); } + if strikethrough_depth > 0 { + style.strikethrough = Some(StrikethroughStyle { + thickness: 1.0.into(), + ..Default::default() + }); + } let last_run_len = if let Some(link_url) = link_url.clone() { link_ranges.push(prev_len..text.len()); link_urls.push(link_url); @@ -249,7 +256,12 @@ pub fn render_markdown_mut( } Event::Start(tag) => match tag { Tag::Paragraph => new_paragraph(text, &mut list_stack), - Tag::Heading(_, _, _) => { + Tag::Heading { + level: _, + id: _, + classes: _, + attrs: _, + } => { new_paragraph(text, &mut list_stack); bold_depth += 1; } @@ -266,7 +278,13 @@ pub fn render_markdown_mut( } Tag::Emphasis => italic_depth += 1, Tag::Strong => bold_depth += 1, - Tag::Link(_, url, _) => link_url = Some(url.to_string()), + Tag::Strikethrough => strikethrough_depth += 1, + Tag::Link { + link_type: _, + dest_url, + title: _, + id: _, + } => link_url = Some(dest_url.to_string()), Tag::List(number) => { list_stack.push((number, false)); } @@ -292,12 +310,13 @@ pub fn render_markdown_mut( _ => {} }, Event::End(tag) => match tag { - Tag::Heading(_, _, _) => bold_depth -= 1, - Tag::CodeBlock(_) => current_language = None, - Tag::Emphasis => italic_depth -= 1, - Tag::Strong => bold_depth -= 1, - Tag::Link(_, _, _) => link_url = None, - Tag::List(_) => drop(list_stack.pop()), + TagEnd::Heading(_) => bold_depth -= 1, + TagEnd::CodeBlock => current_language = None, + TagEnd::Emphasis => italic_depth -= 1, + TagEnd::Strong => bold_depth -= 1, + TagEnd::Strikethrough => strikethrough_depth -= 1, + TagEnd::Link => link_url = None, + TagEnd::List(_) => drop(list_stack.pop()), _ => {} }, Event::HardBreak => text.push('\n'),