Merged
Conversation
|
github-actions
bot
added
A-Parser
Contributor
Parser conformance results onjs/262
jsx/babel
markdown/commonmark
symbols/microsoft
ts/babel
ts/microsoft
|
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (21)
📒 Files selected for processing (43)
💤 Files with no reviewable changes (4)
✅ Files skipped from review due to trivial changes (14)
🚧 Files skipped from review as they are similar to previous changes (21)
WalkthroughThis PR renames the markdown formatter context from Possibly related PRs
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
crates/biome_markdown_formatter/src/markdown/auxiliary/paragraph.rs (1)
37-40: Refresh the option docs.Line 38 still describes the old boolean flag, but
trim_modenow carries the fullTextPrintModeenum. Tiny mismatch, easy future head-scratcher.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/biome_markdown_formatter/src/markdown/auxiliary/paragraph.rs` around lines 37 - 40, The doc comment for FormatMdParagraphOptions::trim_mode is outdated (mentions a boolean) — update the field doc to describe that trim_mode is a TextPrintMode enum controlling how paragraph start is trimmed (e.g., None/TrimStart/Full or whatever variants exist), clarifying expected behavior for each relevant TextPrintMode variant; ensure the comment sits directly above pub trim_mode: TextPrintMode so future readers of FormatMdParagraphOptions and users of trim_mode understand the enum semantics.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@crates/biome_markdown_formatter/src/markdown/auxiliary/hard_line.rs`:
- Around line 14-16: The code currently only returns early for pristine print
mode, but when print mode is Trim(TrimMode::All) we must also avoid emitting the
hard line; in the method that does the early-return (the branch that calls
format_verbatim_node(node.syntax()).fmt(f)), add a special-case check for
self.print_mode being Trim(TrimMode::All) (or equivalent enum variant) and
return before calling hard_line_break(); keep the existing pristine check and
ensure the new branch runs the same early return path so Trim(TrimMode::All)
drops leading/trailing MdHardLine without writing the newline (affecting the
same function that calls hard_line_break(), format_verbatim_node, and fmt(f)).
In `@crates/biome_markdown_formatter/src/markdown/auxiliary/textual.rs`:
- Around line 20-37: The clean-mode branch currently calls trim_matches on the
whole token, removing indentation on non-empty lines; instead, read the original
string from value_token.text(), split it into lines preserving newline
separators, and for each line only strip whitespace if the line is entirely
whitespace (i.e., line.trim().is_empty()), otherwise keep the line unchanged;
then rejoin lines (preserving original newlines) into cleaned and use
format_replaced(&value_token, &text(cleaned,
value_token.text_trimmed_range().start())) so print_mode.is_clean(),
value_token, format_replaced, text(), and value_token.text_trimmed_range() are
used as in the diff.
---
Nitpick comments:
In `@crates/biome_markdown_formatter/src/markdown/auxiliary/paragraph.rs`:
- Around line 37-40: The doc comment for FormatMdParagraphOptions::trim_mode is
outdated (mentions a boolean) — update the field doc to describe that trim_mode
is a TextPrintMode enum controlling how paragraph start is trimmed (e.g.,
None/TrimStart/Full or whatever variants exist), clarifying expected behavior
for each relevant TextPrintMode variant; ensure the comment sits directly above
pub trim_mode: TextPrintMode so future readers of FormatMdParagraphOptions and
users of trim_mode understand the enum semantics.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: a06cd915-e6dc-4f57-b273-8055415a6716
⛔ Files ignored due to path filters (21)
crates/biome_markdown_factory/src/generated/node_factory.rsis excluded by!**/generated/**,!**/generated/**and included by**crates/biome_markdown_factory/src/generated/syntax_factory.rsis excluded by!**/generated/**,!**/generated/**and included by**crates/biome_markdown_formatter/tests/specs/markdown/fenced_code_block.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/markdown/fenced_code_block_info_string.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/markdown/hard_line.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/markdown/inline_links.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/broken-plugins/missing-comments.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/code/backtick.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/multiparser-js/meta-in-code-block.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-104.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-106.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-110.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-111.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-113.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-476.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-91.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-94.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_syntax/src/generated/kind.rsis excluded by!**/generated/**,!**/generated/**and included by**crates/biome_markdown_syntax/src/generated/macros.rsis excluded by!**/generated/**,!**/generated/**and included by**crates/biome_markdown_syntax/src/generated/nodes.rsis excluded by!**/generated/**,!**/generated/**and included by**crates/biome_markdown_syntax/src/generated/nodes_mut.rsis excluded by!**/generated/**,!**/generated/**and included by**
📒 Files selected for processing (42)
crates/biome_markdown_formatter/src/comments.rscrates/biome_markdown_formatter/src/context.rscrates/biome_markdown_formatter/src/cst.rscrates/biome_markdown_formatter/src/generated.rscrates/biome_markdown_formatter/src/lib.rscrates/biome_markdown_formatter/src/markdown/any/block.rscrates/biome_markdown_formatter/src/markdown/any/bullet_list_member.rscrates/biome_markdown_formatter/src/markdown/any/code_block.rscrates/biome_markdown_formatter/src/markdown/any/container_block.rscrates/biome_markdown_formatter/src/markdown/any/inline.rscrates/biome_markdown_formatter/src/markdown/any/leaf_block.rscrates/biome_markdown_formatter/src/markdown/any/thematic_break_part.rscrates/biome_markdown_formatter/src/markdown/auxiliary/fenced_code_block.rscrates/biome_markdown_formatter/src/markdown/auxiliary/hard_line.rscrates/biome_markdown_formatter/src/markdown/auxiliary/header.rscrates/biome_markdown_formatter/src/markdown/auxiliary/inline_link.rscrates/biome_markdown_formatter/src/markdown/auxiliary/link_block.rscrates/biome_markdown_formatter/src/markdown/auxiliary/link_title.rscrates/biome_markdown_formatter/src/markdown/auxiliary/mod.rscrates/biome_markdown_formatter/src/markdown/auxiliary/paragraph.rscrates/biome_markdown_formatter/src/markdown/auxiliary/textual.rscrates/biome_markdown_formatter/src/markdown/lists/block_list.rscrates/biome_markdown_formatter/src/markdown/lists/bullet_list.rscrates/biome_markdown_formatter/src/markdown/lists/code_name_list.rscrates/biome_markdown_formatter/src/markdown/lists/hash_list.rscrates/biome_markdown_formatter/src/markdown/lists/indent_token_list.rscrates/biome_markdown_formatter/src/markdown/lists/inline_item_list.rscrates/biome_markdown_formatter/src/markdown/lists/quote_indent_list.rscrates/biome_markdown_formatter/src/markdown/lists/thematic_break_part_list.rscrates/biome_markdown_formatter/src/prelude.rscrates/biome_markdown_formatter/src/shared.rscrates/biome_markdown_formatter/src/trivia.rscrates/biome_markdown_formatter/src/verbatim.rscrates/biome_markdown_formatter/tests/language.rscrates/biome_markdown_formatter/tests/quick_test.rscrates/biome_markdown_formatter/tests/specs/markdown/fenced_code_block.mdcrates/biome_markdown_formatter/tests/specs/markdown/fenced_code_block_info_string.mdcrates/biome_markdown_formatter/tests/specs/markdown/hard_line.mdcrates/biome_markdown_formatter/tests/specs/markdown/inline_links.mdcrates/biome_markdown_syntax/src/text_ext.rsxtask/codegen/markdown.ungramxtask/codegen/src/markdown_kinds_src.rs
💤 Files with no reviewable changes (4)
- crates/biome_markdown_formatter/src/markdown/auxiliary/mod.rs
- xtask/codegen/src/markdown_kinds_src.rs
- crates/biome_markdown_formatter/src/markdown/auxiliary/link_block.rs
- xtask/codegen/markdown.ungram
Comment on lines
+20
to
+37
| } else if self.print_mode.is_clean() { | ||
| // Clean mode: strip spaces/tabs but preserve newlines. | ||
| // Used for code block content where trailing whitespace on empty | ||
| // lines should be removed but newlines must be kept. | ||
| let cleaned = value_token | ||
| .text() | ||
| .trim_matches(|c: char| c == ' ' || c == '\t'); | ||
| if cleaned == value_token.text() { | ||
| write!(f, [value_token.format()]) | ||
| } else { | ||
| write!( | ||
| f, | ||
| [format_replaced( | ||
| &value_token, | ||
| &text(cleaned, value_token.text_trimmed_range().start()) | ||
| )] | ||
| ) | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Documented Clean-mode contract:"
sed -n '7,23p' crates/biome_markdown_formatter/src/shared.rs
echo
echo "Current Clean-mode implementation:"
sed -n '20,37p' crates/biome_markdown_formatter/src/markdown/auxiliary/textual.rs
echo
python - <<'PY'
samples = [
" function f() {}",
"\t\tlet x = 1;",
" \n",
]
for sample in samples:
cleaned = sample.strip(" \t")
print(f"{sample!r} -> {cleaned!r}")
PYRepository: biomejs/biome
Length of output: 1516
Clean mode strips indentation from all lines, not just empty ones.
The current implementation uses trim_matches(|c| c == ' ' || c == '\t'), which removes spaces and tabs from both ends of the entire token. This violates the documented contract in crates/biome_markdown_formatter/src/shared.rs, which explicitly states that indentation should be preserved on non-empty code lines.
For example:
function f() {}becomesfunction f() {}(indentation lost)\t\tlet x = 1;becomeslet x = 1;(indentation lost)
Only whitespace-only lines should have their trailing/leading spaces removed. This requires a line-by-line approach rather than a token-wide trim.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/biome_markdown_formatter/src/markdown/auxiliary/textual.rs` around
lines 20 - 37, The clean-mode branch currently calls trim_matches on the whole
token, removing indentation on non-empty lines; instead, read the original
string from value_token.text(), split it into lines preserving newline
separators, and for each line only strip whitespace if the line is entirely
whitespace (i.e., line.trim().is_empty()), otherwise keep the line unchanged;
then rejoin lines (preserving original newlines) into cleaned and use
format_replaced(&value_token, &text(cleaned,
value_token.text_trimmed_range().start())) so print_mode.is_clean(),
value_token, format_replaced, text(), and value_token.text_trimmed_range() are
used as in the diff.
ematipico
force-pushed
the
feat/md-format-code-blocks
branch
from
a795ea8 to
ffb5a53
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
crates/biome_markdown_formatter/src/markdown/lists/inline_item_list.rs (1)
106-112: Inconsistent closure passing style.Line 106 passes the closure by reference (
&is_content), whilst line 112 passes it by value (is_content). Both work because the closure isCopy, but the inconsistency is a minor readability concern.✨ Suggested fix for consistency
- let first_content = items.iter().position(&is_content); + let first_content = items.iter().position(|item| is_content(item)); // Find the first non-empty item from the right. let last_content = items .iter() .rev() - .position(is_content) + .position(|item| is_content(item)) .map(|pos| items.len() - 1 - pos);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/biome_markdown_formatter/src/markdown/lists/inline_item_list.rs` around lines 106 - 112, The two calls to position are inconsistent: first_content uses items.iter().position(&is_content) while last_content uses items.iter().rev().position(is_content); make them consistent by passing the closure the same way in both places (e.g., change the first call to items.iter().position(is_content) or change the second to .position(&is_content)). Update the call site(s) in the inline_item_list logic so both uses of is_content use the same closure passing style.crates/biome_markdown_formatter/src/markdown/auxiliary/fenced_code_block.rs (1)
68-90:longest_fence_char_sequenceonly inspectsMdTextualitems.Per
MdInlineItemList's definition, content can include other inline variants (links, emphasis, etc.). However, fence characters inside structured inline elements wouldn't confuse the CommonMark parser at the block level, so this is likely intentional and correct.Worth a brief comment for future maintainers, but not blocking.
📝 Optional documentation improvement
/// Find the longest consecutive run of `fence_char` in the code block's content. +/// Only inspects raw textual nodes; structured inline elements (links, etc.) +/// don't affect block-level fence parsing. fn longest_fence_char_sequence(node: &MdFencedCodeBlock, fence_char: char) -> usize {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/biome_markdown_formatter/src/markdown/auxiliary/fenced_code_block.rs` around lines 68 - 90, The function longest_fence_char_sequence currently only iterates MdTextual items which is intentional because MdInlineItemList may contain structured inlines (links, emphasis, etc.) and fence characters inside those won't affect block-level fence parsing; add a concise comment above longest_fence_char_sequence referencing MdInlineItemList and MdTextual to explain why only textual tokens are checked and that structured inline elements cannot form valid fence sequences at the block level, so no further traversal is required.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@crates/biome_markdown_formatter/src/markdown/auxiliary/fenced_code_block.rs`:
- Around line 20-23: The code assumes l_fence.text() is non-empty and directly
indexes fence_text.as_bytes()[0], which can panic; update the fenced code block
handling to defensively check fence_text.is_empty() after obtaining fence_text
from l_fence (and after l_fence? unwrap), and if empty return an appropriate
error or early return (propagate Err) instead of indexing; then safely extract
the fence character (e.g., use fence_text.chars().next().unwrap() or similar
only after the emptiness check). Ensure you reference and update the logic
around l_fence, fence_text and fence_char in fenced_code_block.rs to avoid any
direct indexing on an empty slice.
- Around line 54-62: r_fence extraction can fail and currently no closing fence
is emitted, producing invalid Markdown; add an else branch for the if let
Ok(r_fence) = r_fence that writes a synthesized closing fence so output remains
well-formed. Specifically, after the existing Ok branch that calls write!(f,
[format_replaced(...)]), implement an else that calls write!(f, [...]) to emit a
closing fence derived from normalized_fence (use the same text/formatter helpers
— text and format_replaced or text(&normalized_fence, ...) — to produce the
closing fence string) so a closing fence is always written even when r_fence is
Err.
---
Nitpick comments:
In `@crates/biome_markdown_formatter/src/markdown/auxiliary/fenced_code_block.rs`:
- Around line 68-90: The function longest_fence_char_sequence currently only
iterates MdTextual items which is intentional because MdInlineItemList may
contain structured inlines (links, emphasis, etc.) and fence characters inside
those won't affect block-level fence parsing; add a concise comment above
longest_fence_char_sequence referencing MdInlineItemList and MdTextual to
explain why only textual tokens are checked and that structured inline elements
cannot form valid fence sequences at the block level, so no further traversal is
required.
In `@crates/biome_markdown_formatter/src/markdown/lists/inline_item_list.rs`:
- Around line 106-112: The two calls to position are inconsistent: first_content
uses items.iter().position(&is_content) while last_content uses
items.iter().rev().position(is_content); make them consistent by passing the
closure the same way in both places (e.g., change the first call to
items.iter().position(is_content) or change the second to
.position(&is_content)). Update the call site(s) in the inline_item_list logic
so both uses of is_content use the same closure passing style.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 9d52a6cb-36cd-4c23-acad-7f95789693c3
📒 Files selected for processing (3)
crates/biome_markdown_formatter/src/markdown/auxiliary/fenced_code_block.rscrates/biome_markdown_formatter/src/markdown/lists/inline_item_list.rscrates/biome_markdown_parser/src/to_html.rs
Comment on lines
+20
to
+23
| let l_fence = l_fence?; | ||
| let fence_text = l_fence.text(); | ||
| // SAFETY: fence_text has at least one character. | ||
| let fence_char = fence_text.as_bytes()[0] as char; |
Contributor
There was a problem hiding this comment.
Potential panic if fence token is empty.
The SAFETY comment asserts the fence has at least one character, but there's no runtime check. If the parser ever produces an empty l_fence token (malformed input, future parser changes), this will panic.
🛡️ Suggested defensive check
let l_fence = l_fence?;
let fence_text = l_fence.text();
- // SAFETY: fence_text has at least one character.
- let fence_char = fence_text.as_bytes()[0] as char;
+ // Fence must contain at least one ` or ~.
+ let fence_char = fence_text
+ .chars()
+ .next()
+ .unwrap_or('`');🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/biome_markdown_formatter/src/markdown/auxiliary/fenced_code_block.rs`
around lines 20 - 23, The code assumes l_fence.text() is non-empty and directly
indexes fence_text.as_bytes()[0], which can panic; update the fenced code block
handling to defensively check fence_text.is_empty() after obtaining fence_text
from l_fence (and after l_fence? unwrap), and if empty return an appropriate
error or early return (propagate Err) instead of indexing; then safely extract
the fence character (e.g., use fence_text.chars().next().unwrap() or similar
only after the emptiness check). Ensure you reference and update the logic
around l_fence, fence_text and fence_char in fenced_code_block.rs to avoid any
direct indexing on an empty slice.
Comment on lines
+54
to
+62
| if let Ok(r_fence) = r_fence { | ||
| write!( | ||
| f, | ||
| [format_replaced( | ||
| &r_fence, | ||
| &text(&normalized_fence, r_fence.text_trimmed_range().start()) | ||
| )] | ||
| )?; | ||
| } |
Contributor
There was a problem hiding this comment.
Missing closing fence when r_fence is Err.
If r_fence fails to extract, no closing fence is emitted. This could produce invalid Markdown output. Consider emitting a synthesised closing fence to maintain well-formedness.
🔧 Suggested fix to always emit a closing fence
- if let Ok(r_fence) = r_fence {
+ if let Ok(r_fence) = &r_fence {
write!(
f,
[format_replaced(
- &r_fence,
+ r_fence,
&text(&normalized_fence, r_fence.text_trimmed_range().start())
)]
)?;
+ } else {
+ // Synthesise a closing fence if the original is missing/malformed.
+ write!(f, [dynamic_text(&normalized_fence, TextSize::default())])?;
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/biome_markdown_formatter/src/markdown/auxiliary/fenced_code_block.rs`
around lines 54 - 62, r_fence extraction can fail and currently no closing fence
is emitted, producing invalid Markdown; add an else branch for the if let
Ok(r_fence) = r_fence that writes a synthesized closing fence so output remains
well-formed. Specifically, after the existing Ok branch that calls write!(f,
[format_replaced(...)]), implement an else that calls write!(f, [...]) to emit a
closing fence derived from normalized_fence (use the same text/formatter helpers
— text and format_replaced or text(&normalized_fence, ...) — to produce the
closing fence string) so a closing fence is always written even when r_fence is
Err.
Merging this PR will create unknown performance changes
Performance Changes
Comparing 
Footnotes
|
dyc3
approved these changes
Mar 29, 2026
This was referenced Mar 30, 2026
This was referenced Apr 11, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR:
MdFormatContexttoMarkdown*, because that's how our codegen is wiredTextPrintModewhich holds different semantics of how whitespaces should be handled. For example, for links we use different kinds of trimming. For code blocks, use cleanI designed the architecture around trimming and print mode, as well as the code blocks. Tests and implementations were tweaked and fixed using a coding agent.
Test Plan
Added new tests. Made sure we had small regressions around prettier snapshots.
Docs
N/A