From 2c99f547ccf6843adca27fc2b17576991d70925e Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Fri, 9 Oct 2026 15:56:46 +0000 Subject: [PATCH] markdown: Fix crashes on tab-indented code blocks (#65398) Fixes https://github.com/zed-industries/zed/issues/65320 Key cached highlights by typed event indices instead of source offsets, which can collide during tab expansion. Add a layout regression test covering the crash and related edge cases. Release Notes: - Fixed crashes on tab-indented code blocks --- crates/markdown/src/markdown.rs | 118 +++++++++++++++++++++++++------- 1 file changed, 92 insertions(+), 26 deletions(-) diff --git a/crates/markdown/src/markdown.rs b/crates/markdown/src/markdown.rs index 1862eab1c15..5a28ca7988a 100644 --- a/crates/markdown/src/markdown.rs +++ b/crates/markdown/src/markdown.rs @@ -1502,7 +1502,10 @@ pub struct ParsedMarkdown { pub(crate) code_block_highlights: Arc, } -pub(crate) type CodeBlockHighlights = HashMap; +#[derive(Clone, Copy, Debug, Eq, Hash, PartialEq)] +pub(crate) struct MarkdownEventIndex(usize); + +pub(crate) type CodeBlockHighlights = HashMap; impl ParsedMarkdown { pub fn source(&self) -> &SharedString { @@ -1626,13 +1629,14 @@ impl ParsedMarkdown { struct PendingCodeBlock<'a> { language: Arc, - texts: Vec<(Range, &'a str)>, + texts: Vec<(MarkdownEventIndex, &'a str)>, } fn compute_code_block_highlights(parsed: &ParsedMarkdown) -> CodeBlockHighlights { let mut code_block_highlights = CodeBlockHighlights::default(); let mut pending_block: Option = None; - for (range, event) in parsed.events.iter() { + for (event_index, (range, event)) in parsed.events.iter().enumerate() { + let event_index = MarkdownEventIndex(event_index); match event { MarkdownEvent::Start(MarkdownTag::CodeBlock { kind, .. }) => { if parsed.mermaid_diagrams.contains_key(&range.start) { @@ -1655,12 +1659,12 @@ fn compute_code_block_highlights(parsed: &ParsedMarkdown) -> CodeBlockHighlights if let Some(block) = &mut pending_block { block .texts - .push((range.clone(), &parsed.source[range.clone()])); + .push((event_index, &parsed.source[range.clone()])); } } MarkdownEvent::SubstitutedText(text) => { if let Some(block) = &mut pending_block { - block.texts.push((range.clone(), text.as_str())); + block.texts.push((event_index, text.as_str())); } } _ => {} @@ -1682,12 +1686,12 @@ fn highlight_code_block(block: PendingCodeBlock, code_block_highlights: &mut Cod if resolved.runs.is_empty() { return; } - if let [(source_range, _)] = block.texts.as_slice() { - code_block_highlights.insert(source_range.start, resolved); + if let [(event_index, _)] = block.texts.as_slice() { + code_block_highlights.insert(*event_index, resolved); return; } let mut runs = resolved.runs.iter().peekable(); - for ((source_range, text), text_offset) in block.texts.iter().zip(text_offsets) { + for ((event_index, text), text_offset) in block.texts.iter().zip(text_offsets) { let text_end = text_offset + text.len(); let mut text_runs = Vec::new(); while let Some((run_range, highlight_id)) = runs.peek() { @@ -1706,7 +1710,7 @@ fn highlight_code_block(block: PendingCodeBlock, code_block_highlights: &mut Cod } if !text_runs.is_empty() { code_block_highlights.insert( - source_range.start, + *event_index, ResolvedHighlights { sources: resolved.sources.clone(), runs: text_runs.into(), @@ -2626,7 +2630,6 @@ impl Element for MarkdownElement { self.style.base_text_style.clone(), self.style.syntax.clone(), highlights, - parsed_markdown.code_block_highlights.clone(), ); let markdown_end = if let Some(last) = parsed_markdown.events.last() { last.0.end @@ -2641,6 +2644,7 @@ impl Element for MarkdownElement { let mut rendered_mermaid_block = false; let mut rendered_metadata_block = false; for (index, (range, event)) in parsed_markdown.events.iter().enumerate() { + let event_index = MarkdownEventIndex(index); // Skip alt text for images that rendered if let Some(current_img_block_range) = ¤t_img_block_range && current_img_block_range.end > range.end @@ -3262,13 +3266,21 @@ impl Element for MarkdownElement { if let Some(current_code_block_text) = &mut current_code_block_text { current_code_block_text.push_str(text); } - builder.push_text(text, range.clone()); + builder.push_highlighted_text( + text, + range.clone(), + parsed_markdown.code_block_highlights.get(&event_index), + ); } MarkdownEvent::SubstitutedText(text) => { if let Some(current_code_block_text) = &mut current_code_block_text { current_code_block_text.push_str(text); } - builder.push_text(text, range.clone()); + builder.push_highlighted_text( + text, + range.clone(), + parsed_markdown.code_block_highlights.get(&event_index), + ); } MarkdownEvent::Code => { self.push_markdown_code_span( @@ -3764,7 +3776,6 @@ struct MarkdownElementBuilder { base_text_style: TextStyle, text_style_stack: Vec, code_block_stack: Vec>>, - code_block_highlights: Arc, link_depth: usize, list_stack: Vec, table: TableState, @@ -3866,7 +3877,6 @@ impl MarkdownElementBuilder { base_text_style: TextStyle, syntax_theme: Arc, highlights: MarkdownHighlights, - code_block_highlights: Arc, ) -> Self { Self { div_stack: vec![{ @@ -3885,7 +3895,6 @@ impl MarkdownElementBuilder { base_text_style, text_style_stack: Vec::new(), code_block_stack: Vec::new(), - code_block_highlights, link_depth: 0, list_stack: Vec::new(), table: TableState::default(), @@ -4135,6 +4144,15 @@ impl MarkdownElementBuilder { } fn push_text(&mut self, text: &str, source_range: Range) { + self.push_highlighted_text(text, source_range, None); + } + + fn push_highlighted_text( + &mut self, + text: &str, + source_range: Range, + highlights: Option<&ResolvedHighlights>, + ) { self.pending_line.source_mappings.push(SourceMapping { rendered_index: self.pending_line.text.len(), source_index: source_range.start, @@ -4146,7 +4164,7 @@ impl MarkdownElementBuilder { let text_style = self.text_style(); if let Some(language) = self.code_block_stack.last().and_then(Option::as_ref) - && let Some(resolved) = self.code_block_highlights.get(&source_range.start) + && let Some(resolved) = highlights { let runs = if resolved.is_current() { resolved.runs.clone() @@ -5643,6 +5661,45 @@ mod tests { ); } + #[gpui::test] + fn test_code_block_highlights_with_partial_tabs(cx: &mut TestAppContext) { + ensure_theme_initialized(cx); + + for (source, expected) in [ + (" ```rust\n\tfn main() {}\n ```\n", " fn main() {}"), + (" ```rust\n\tfn café() {}\n ```\n", " fn café() {}"), + (" ```rust\r\n\tfn 界() {}\r\n ```\r\n", " fn 界() {}"), + ("- ```rust\n\tfn main() {}\n ```\n", " fn main() {}"), + (" ```rust\n\tfn main() {}", " fn main() {}"), + ] { + let (language, markdown) = markdown_with_rust_language(source, cx); + for stale in [false, true] { + if stale { + language.set_theme(&rust_test_theme()); + } + let rendered = render_markdown_entity_in_view( + markdown.clone(), + MarkdownStyle { + syntax: Arc::new(rust_test_theme()), + ..MarkdownStyle::default() + }, + None, + None, + cx, + ); + assert_eq!( + rendered + .lines + .iter() + .map(|line| line.layout.text()) + .collect::>(), + vec![expected.to_owned()], + "source: {source:?}, stale: {stale}" + ); + } + } + } + #[gpui::test] fn test_code_block_language_uses_first_word_of_info_string(cx: &mut TestAppContext) { let source = "```rust import.meta.vitest\nfn main() {}\n```"; @@ -5794,14 +5851,11 @@ mod tests { .as_ref() .expect("the fallback language must be resolved for untagged code blocks"); assert_eq!(fallback.name(), language.name()); - - let code_start = source.find("fn main").unwrap(); - let cached = parsed - .code_block_highlights - .get(&code_start) - .expect("untagged code blocks must be highlighted with the fallback language"); - assert!(!cached.runs.is_empty()); }); + + let code_start = source.find("fn main").unwrap(); + let cached = cached_code_block_highlights(&markdown, code_start, cx); + assert!(!cached.runs.is_empty()); } #[gpui::test] @@ -9055,10 +9109,22 @@ mod tests { cx: &mut TestAppContext, ) -> ResolvedHighlights { markdown.read_with(cx, |markdown, _| { - markdown - .parsed_markdown() + let parsed = markdown.parsed_markdown(); + let event_index = parsed + .events + .iter() + .position(|(range, event)| { + range.start == code_start + && matches!( + event, + MarkdownEvent::Text | MarkdownEvent::SubstitutedText(_) + ) + }) + .map(MarkdownEventIndex) + .expect("code block text event must exist"); + parsed .code_block_highlights - .get(&code_start) + .get(&event_index) .expect("code block highlights must be computed during parse") .clone() })