mirror of
https://github.com/zed-industries/zed.git
synced 2026-10-19 02:54:11 +00:00
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
This commit is contained in:
Kirill Bulatov
GitHub
parent
5f08939a5d
commit
2c99f547cc
1 changed files with 92 additions and 26 deletions
|
|
@ -1502,7 +1502,10 @@ pub struct ParsedMarkdown {
|
|||
pub(crate) code_block_highlights: Arc<CodeBlockHighlights>,
|
||||
}
|
||||
|
||||
pub(crate) type CodeBlockHighlights = HashMap<usize, ResolvedHighlights>;
|
||||
#[derive(Clone, Copy, Debug, Eq, Hash, PartialEq)]
|
||||
pub(crate) struct MarkdownEventIndex(usize);
|
||||
|
||||
pub(crate) type CodeBlockHighlights = HashMap<MarkdownEventIndex, ResolvedHighlights>;
|
||||
|
||||
impl ParsedMarkdown {
|
||||
pub fn source(&self) -> &SharedString {
|
||||
|
|
@ -1626,13 +1629,14 @@ impl ParsedMarkdown {
|
|||
|
||||
struct PendingCodeBlock<'a> {
|
||||
language: Arc<Language>,
|
||||
texts: Vec<(Range<usize>, &'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<PendingCodeBlock> = 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<TextStyleRefinement>,
|
||||
code_block_stack: Vec<Option<Arc<Language>>>,
|
||||
code_block_highlights: Arc<CodeBlockHighlights>,
|
||||
link_depth: usize,
|
||||
list_stack: Vec<ListStackEntry>,
|
||||
table: TableState,
|
||||
|
|
@ -3866,7 +3877,6 @@ impl MarkdownElementBuilder {
|
|||
base_text_style: TextStyle,
|
||||
syntax_theme: Arc<SyntaxTheme>,
|
||||
highlights: MarkdownHighlights,
|
||||
code_block_highlights: Arc<CodeBlockHighlights>,
|
||||
) -> 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<usize>) {
|
||||
self.push_highlighted_text(text, source_range, None);
|
||||
}
|
||||
|
||||
fn push_highlighted_text(
|
||||
&mut self,
|
||||
text: &str,
|
||||
source_range: Range<usize>,
|
||||
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<_>>(),
|
||||
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()
|
||||
})
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue