-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
refactor(markdown-parser): promote fenced code block skipped trivia to explicit CST nodes #9321
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 5 commits
041e82b
8d084b9
582f51a
870df65
27f50ef
4dbe781
a8280e0
d7b10d8
d44b297
cf5d8f5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -33,6 +33,7 @@ use biome_parser::{ | |||||||||
| }; | ||||||||||
|
|
||||||||||
| use crate::syntax::parse_error::unterminated_fenced_code; | ||||||||||
| use crate::syntax::quote::try_bump_quote_marker; | ||||||||||
| use crate::syntax::{MAX_BLOCK_PREFIX_INDENT, TAB_STOP_SPACES}; | ||||||||||
|
|
||||||||||
| /// Minimum number of fence characters required per CommonMark §4.5. | ||||||||||
|
|
@@ -276,70 +277,131 @@ fn parse_code_content( | |||||||||
|
|
||||||||||
| // Consume all tokens until we see the matching closing fence or EOF | ||||||||||
| while !p.at(T![EOF]) { | ||||||||||
| if at_line_start && quote_depth > 0 { | ||||||||||
| let prev_virtual = p.state().virtual_line_start; | ||||||||||
| p.state_mut().virtual_line_start = Some(p.cur_range().start()); | ||||||||||
| p.skip_line_indent(MAX_BLOCK_PREFIX_INDENT); | ||||||||||
| p.state_mut().virtual_line_start = prev_virtual; | ||||||||||
|
|
||||||||||
| let mut ok = true; | ||||||||||
| for _ in 0..quote_depth { | ||||||||||
| if p.at(MD_TEXTUAL_LITERAL) && p.cur_text().starts_with('>') { | ||||||||||
| p.force_relex_regular(); | ||||||||||
| } | ||||||||||
| match prepare_next_code_content_token( | ||||||||||
| p, | ||||||||||
| is_tilde_fence, | ||||||||||
| fence_len, | ||||||||||
| fence_indent, | ||||||||||
| quote_depth, | ||||||||||
| &mut at_line_start, | ||||||||||
| ) { | ||||||||||
| CodeContentLoopAction::Break => break, | ||||||||||
| CodeContentLoopAction::Continue => continue, | ||||||||||
| CodeContentLoopAction::ConsumeText => {} | ||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These arms have different semantics — Open to restructuring if you have a different approach in mind — what would you prefer here?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe something like this that inverts the enum? enum CodeContentTokenAction {
Break,
Skip, // renamed from Continue
Consume, // renamed from ConsumeText
}
fn parse_code_content(...) {
// ...
while !p.at(T![EOF]) {
match prepare_next_code_content_token(...) {
CodeContentTokenAction::Break => break,
CodeContentTokenAction::Skip => continue,
CodeContentTokenAction::Consume => {
bump_code_textual(p);
at_line_start = false;
}
}
}
}
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Much better yes!
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Resolved: I've refactored the control flow to make all three code paths explicit within the match statement (d44b297) |
||||||||||
| } | ||||||||||
|
|
||||||||||
| if p.at(T![>]) { | ||||||||||
| p.parse_as_skipped_trivia_tokens(|p| p.bump(T![>])); | ||||||||||
| } else if p.at(MD_TEXTUAL_LITERAL) && p.cur_text() == ">" { | ||||||||||
| p.parse_as_skipped_trivia_tokens(|p| p.bump_remap(T![>])); | ||||||||||
| } else { | ||||||||||
| ok = false; | ||||||||||
| break; | ||||||||||
| } | ||||||||||
| consume_code_textual(p); | ||||||||||
| at_line_start = false; | ||||||||||
| } | ||||||||||
|
|
||||||||||
| if p.at(MD_TEXTUAL_LITERAL) { | ||||||||||
| let text = p.cur_text(); | ||||||||||
| if text == " " || text == "\t" { | ||||||||||
| p.parse_as_skipped_trivia_tokens(|p| p.bump(MD_TEXTUAL_LITERAL)); | ||||||||||
| } | ||||||||||
| } | ||||||||||
| } | ||||||||||
| m.complete(p, MD_INLINE_ITEM_LIST); | ||||||||||
| } | ||||||||||
|
|
||||||||||
| if !ok { | ||||||||||
| break; | ||||||||||
| } | ||||||||||
| at_line_start = false; | ||||||||||
| } | ||||||||||
| enum CodeContentLoopAction { | ||||||||||
| Break, | ||||||||||
| Continue, | ||||||||||
| ConsumeText, | ||||||||||
| } | ||||||||||
|
|
||||||||||
| if p.at(NEWLINE) { | ||||||||||
| // Preserve newlines as code content and reset virtual line start. | ||||||||||
| let text_m = p.start(); | ||||||||||
| p.bump_remap(MD_TEXTUAL_LITERAL); | ||||||||||
| text_m.complete(p, MD_TEXTUAL); | ||||||||||
| p.set_virtual_line_start(); | ||||||||||
| at_line_start = true; | ||||||||||
| continue; | ||||||||||
| } | ||||||||||
| fn prepare_next_code_content_token( | ||||||||||
|
ematipico marked this conversation as resolved.
|
||||||||||
| p: &mut MarkdownParser, | ||||||||||
| is_tilde_fence: bool, | ||||||||||
| fence_len: usize, | ||||||||||
| fence_indent: usize, | ||||||||||
| quote_depth: usize, | ||||||||||
| at_line_start: &mut bool, | ||||||||||
| ) -> CodeContentLoopAction { | ||||||||||
| if *at_line_start && quote_depth > 0 && !consume_quote_prefixes_in_code_content(p, quote_depth) | ||||||||||
| { | ||||||||||
| return CodeContentLoopAction::Break; | ||||||||||
| } | ||||||||||
|
|
||||||||||
| if consume_code_newline(p) { | ||||||||||
| *at_line_start = true; | ||||||||||
| return CodeContentLoopAction::Continue; | ||||||||||
| } | ||||||||||
|
|
||||||||||
| if at_closing_fence(p, is_tilde_fence, fence_len) { | ||||||||||
| return CodeContentLoopAction::Break; | ||||||||||
| } | ||||||||||
|
|
||||||||||
| if *at_line_start && fence_indent > 0 { | ||||||||||
| skip_fenced_content_indent(p, fence_indent); | ||||||||||
| if at_closing_fence(p, is_tilde_fence, fence_len) { | ||||||||||
| break; | ||||||||||
| return CodeContentLoopAction::Break; | ||||||||||
| } | ||||||||||
| } | ||||||||||
|
|
||||||||||
| if at_line_start && fence_indent > 0 { | ||||||||||
| skip_fenced_content_indent(p, fence_indent); | ||||||||||
| if at_closing_fence(p, is_tilde_fence, fence_len) { | ||||||||||
| break; | ||||||||||
| } | ||||||||||
| CodeContentLoopAction::ConsumeText | ||||||||||
| } | ||||||||||
|
coderabbitai[bot] marked this conversation as resolved.
Outdated
|
||||||||||
|
|
||||||||||
| fn consume_quote_prefixes_in_code_content(p: &mut MarkdownParser, quote_depth: usize) -> bool { | ||||||||||
|
ematipico marked this conversation as resolved.
|
||||||||||
| let prev_virtual = p.state().virtual_line_start; | ||||||||||
| p.state_mut().virtual_line_start = Some(p.cur_range().start()); | ||||||||||
| p.skip_line_indent(MAX_BLOCK_PREFIX_INDENT); | ||||||||||
| p.state_mut().virtual_line_start = prev_virtual; | ||||||||||
|
|
||||||||||
| for _ in 0..quote_depth { | ||||||||||
| if !consume_quote_prefix_in_code_content(p) { | ||||||||||
| return false; | ||||||||||
| } | ||||||||||
| } | ||||||||||
|
coderabbitai[bot] marked this conversation as resolved.
|
||||||||||
|
|
||||||||||
| // Consume the token as code content (including NEWLINE tokens) | ||||||||||
| let text_m = p.start(); | ||||||||||
| p.bump_remap(MD_TEXTUAL_LITERAL); | ||||||||||
| text_m.complete(p, MD_TEXTUAL); | ||||||||||
| at_line_start = false; | ||||||||||
| p.set_virtual_line_start(); | ||||||||||
| true | ||||||||||
| } | ||||||||||
|
|
||||||||||
| fn consume_quote_prefix_in_code_content(p: &mut MarkdownParser) -> bool { | ||||||||||
|
ematipico marked this conversation as resolved.
|
||||||||||
| if p.at(MD_TEXTUAL_LITERAL) && p.cur_text().starts_with('>') { | ||||||||||
| p.force_relex_regular(); | ||||||||||
| } | ||||||||||
|
|
||||||||||
| m.complete(p, MD_INLINE_ITEM_LIST); | ||||||||||
| if !(p.at(T![>]) || (p.at(MD_TEXTUAL_LITERAL) && p.cur_text() == ">")) { | ||||||||||
| return false; | ||||||||||
| } | ||||||||||
|
|
||||||||||
| let prefix_m = p.start(); | ||||||||||
|
|
||||||||||
| // Empty pre-marker indent list (initial indent handled by skip_line_indent). | ||||||||||
| let indent_list_m = p.start(); | ||||||||||
| indent_list_m.complete(p, MD_QUOTE_INDENT_LIST); | ||||||||||
|
|
||||||||||
| let marker_bumped = try_bump_quote_marker(p); | ||||||||||
| debug_assert!(marker_bumped, "guard above guarantees marker present"); | ||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We usually try to use messages to understand what went wrong and/or how to fix it. For example, if a developer lands here, the message should tell what caused the problem, and where to look at for possible fixes (if applicable)
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed — replaced |
||||||||||
| if !marker_bumped { | ||||||||||
| unreachable!("guard above guarantees marker present"); | ||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No code that panics in production. Let's find a safer approach
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed — replaced |
||||||||||
| } | ||||||||||
|
|
||||||||||
| // Optional post-marker space | ||||||||||
| if p.at(MD_TEXTUAL_LITERAL) { | ||||||||||
| let text = p.cur_text(); | ||||||||||
| if text == " " || text == "\t" { | ||||||||||
| p.bump_remap(MD_QUOTE_POST_MARKER_SPACE); | ||||||||||
| } | ||||||||||
| } | ||||||||||
|
|
||||||||||
| prefix_m.complete(p, MD_QUOTE_PREFIX); | ||||||||||
| true | ||||||||||
| } | ||||||||||
|
|
||||||||||
| fn consume_code_newline(p: &mut MarkdownParser) -> bool { | ||||||||||
| if !p.at(NEWLINE) { | ||||||||||
| return false; | ||||||||||
| } | ||||||||||
|
|
||||||||||
| // Preserve newlines as code content and reset virtual line start. | ||||||||||
| let text_m = p.start(); | ||||||||||
| p.bump_remap(MD_TEXTUAL_LITERAL); | ||||||||||
| text_m.complete(p, MD_TEXTUAL); | ||||||||||
| p.set_virtual_line_start(); | ||||||||||
| true | ||||||||||
| } | ||||||||||
|
|
||||||||||
| fn consume_code_textual(p: &mut MarkdownParser) { | ||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There's a bit of misalignment among these new functions. Some return a boolean, some don't, but they all start with
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed — renamed |
||||||||||
| // Consume the token as code content (including NEWLINE tokens). | ||||||||||
| let text_m = p.start(); | ||||||||||
| p.bump_remap(MD_TEXTUAL_LITERAL); | ||||||||||
| text_m.complete(p, MD_TEXTUAL); | ||||||||||
| } | ||||||||||
|
|
||||||||||
| pub(crate) fn info_string_has_backtick(p: &mut MarkdownParser) -> bool { | ||||||||||
|
|
@@ -390,7 +452,9 @@ fn skip_fenced_content_indent(p: &mut MarkdownParser, indent: usize) { | |||||||||
| } | ||||||||||
|
|
||||||||||
| consumed += width; | ||||||||||
| p.parse_as_skipped_trivia_tokens(|p| p.bump(MD_TEXTUAL_LITERAL)); | ||||||||||
| let char_m = p.start(); | ||||||||||
| p.bump_remap(MD_INDENT_CHAR); | ||||||||||
| char_m.complete(p, MD_INDENT_TOKEN); | ||||||||||
| } | ||||||||||
| } | ||||||||||
|
|
||||||||||
|
|
@@ -399,9 +463,12 @@ fn line_has_closing_fence(p: &MarkdownParser, is_tilde_fence: bool, fence_len: u | |||||||||
| return false; | ||||||||||
| }; | ||||||||||
|
|
||||||||||
| let line_start = find_line_start(&source[..start]); | ||||||||||
| let line_start: usize = match p.state().virtual_line_start { | ||||||||||
| Some(virtual_start) => virtual_start.into(), | ||||||||||
| None => find_line_start(&source[..start]), | ||||||||||
| }; | ||||||||||
|
|
||||||||||
| if !is_whitespace_prefix(source, start, line_start) { | ||||||||||
| if line_start != start && !is_whitespace_prefix(source, start, line_start) { | ||||||||||
| return false; | ||||||||||
| } | ||||||||||
|
|
||||||||||
|
|
||||||||||
Uh oh!
There was an error while loading. Please reload this page.