feat(markdown): implement basic formatter features#9693
feat(markdown): implement basic formatter features#9693ematipico merged 13 commits intobiomejs:mainfrom
Conversation
|
f7083fd to
b1c73d8
Compare
|
🧠 Learnings used✅ Actions performedReview triggered.
|
Merging this PR will not alter performance
Comparing Footnotes
|
|
cc @jfmcdowell |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThis PR converts multiple Markdown auxiliary formatters from verbatim, syntax-preserving output to structured, field-based formatting. Changes include: normalising bullet markers with a thematic-break conflict fallback; formatting bullet list items and list-marker prefixes via extracted fields (adding a Possibly related PRs
Suggested reviewers
🚥 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)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
crates/biome_markdown_formatter/src/markdown/auxiliary/setext_header.rs (1)
80-95: YAML front matter detection heuristic may have false positives.This detects any
---thematic break preceding a setext header as YAML front matter, but a standalone thematic break before a setext header (not at document start) would also match. Consider additionally checking that the thematic break has no preceding non-newline siblings for stricter detection.🤖 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/setext_header.rs` around lines 80 - 95, The current is_yaml_front_matter(MdSetextHeader) returns true whenever a MD_THEMATIC_BREAK_BLOCK precedes the setext header even if that break is not at document start; update the heuristic to ensure the thematic break itself has no preceding non-newline siblings (i.e., walk backward from the found thematic break node and verify all previous siblings are MD_NEWLINE or absent) before returning true. Locate is_yaml_front_matter and modify the sibling-walking logic: when you detect MarkdownSyntaxKind::MD_THEMATIC_BREAK_BLOCK, perform an additional loop from that thematic break node checking prev_sibling() for any non-newline kinds and only consider it YAML front matter if none exist. Ensure you still skip over MD_NEWLINE nodes when searching in both walks.
🤖 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/bullet.rs`:
- Around line 22-25: The current early return in the function (checking
marker.text_trimmed() != "*" && marker.text_trimmed() != "+") causes `-` markers
to skip the alternation/normalization logic and go straight to
format_verbatim_node(node.syntax()).fmt(f); remove or modify that conditional so
alternation logic runs for `-` markers as well (i.e., include `-` in the set of
markers handled by the alternation/normalization path) and only fall back to
format_verbatim_node when genuinely necessary; update the branch that calls
format_verbatim_node and any downstream alternation code so it operates on
marker.text_trimmed() for "*", "+", and "-" consistently.
---
Nitpick comments:
In `@crates/biome_markdown_formatter/src/markdown/auxiliary/setext_header.rs`:
- Around line 80-95: The current is_yaml_front_matter(MdSetextHeader) returns
true whenever a MD_THEMATIC_BREAK_BLOCK precedes the setext header even if that
break is not at document start; update the heuristic to ensure the thematic
break itself has no preceding non-newline siblings (i.e., walk backward from the
found thematic break node and verify all previous siblings are MD_NEWLINE or
absent) before returning true. Locate is_yaml_front_matter and modify the
sibling-walking logic: when you detect
MarkdownSyntaxKind::MD_THEMATIC_BREAK_BLOCK, perform an additional loop from
that thematic break node checking prev_sibling() for any non-newline kinds and
only consider it YAML front matter if none exist. Ensure you still skip over
MD_NEWLINE nodes when searching in both walks.
🪄 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: 4d0aef8b-d49f-4d0e-9bd8-a2e48370d297
⛔ Files ignored due to path filters (103)
crates/biome_markdown_formatter/tests/specs/prettier/markdown/blockquote/notext-end.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/emphasis/asterisk.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/emphasis/complex.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/emphasis/special.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/front-matter/custom-parser.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/front-matter/empty.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/heading/setext.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/liquid/unbalanced-mismatched-braces.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/liquid/unbalanced-object-close-only.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/liquid/unbalanced-object-open-only.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/liquid/unbalanced-template-close-only.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/liquid/unbalanced-template-open-only.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/list/combined-lists.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/list/interrupt.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/list/nested-checkbox.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/list/nested-tab.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/list/tab.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/markdown/real-world-case.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/markdown/test-case.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/paragraph/special-prefix.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-108.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-117.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-120.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-132.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-133.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-140.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-141.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-152.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-244.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-260.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-27.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-272.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-277.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-28.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-282.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-288.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-325.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-35.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-356.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-36.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-363.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-364.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-367.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-368.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-369.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-373.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-376.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-377.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-378.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-379.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-380.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-384.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-385.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-386.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-387.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-388.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-395.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-396.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-397.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-399.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-401.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-402.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-403.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-404.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-408.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-413.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-414.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-418.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-42.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-423.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-424.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-427.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-429.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-433.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-436.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-438.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-439.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-440.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-442.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-444.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-450.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-482.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-485.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-489.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-49.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-499.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-50.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-51.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-52.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-54.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-57.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-58.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-59.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-602.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-603.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-611.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-63.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-64.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-70.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-71.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-83.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/strong/underscore.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/yaml/trailing-spaces.md.snapis excluded by!**/*.snapand included by**
📒 Files selected for processing (7)
crates/biome_markdown_formatter/src/markdown/auxiliary/bullet.rscrates/biome_markdown_formatter/src/markdown/auxiliary/bullet_list_item.rscrates/biome_markdown_formatter/src/markdown/auxiliary/inline_emphasis.rscrates/biome_markdown_formatter/src/markdown/auxiliary/inline_italic.rscrates/biome_markdown_formatter/src/markdown/auxiliary/list_marker_prefix.rscrates/biome_markdown_formatter/src/markdown/auxiliary/setext_header.rscrates/biome_markdown_formatter/tests/quick_test.rs
|
🧠 Learnings used✅ Actions performedReview triggered.
|
dyc3
left a comment
There was a problem hiding this comment.
Nice progress on the prettier snapshots, but there's some perf concerns. Also, would like to see some new tests outside of the prettier folder that are more isolated than the prettier tests.
| format_replaced(&l_fence?, &token("**")), | ||
| content.format(), | ||
| format_replaced(&r_fence?, &token("**")), |
There was a problem hiding this comment.
same here, reuse the existing tokens if they don't need changing
There was a problem hiding this comment.
We want to normalize the __ to **.
If we write it as r_fence.format() as @ematipico suggested, it will be preserved as-is. Is this where we want to diverge from prettier? Or should we keep prettier's behavior?
There was a problem hiding this comment.
IIUC, we'll need to make parser for the fences in MdInlineEmphasisFields more granular for it to format itself.
| .filter(|s| s.kind() == MarkdownSyntaxKind::MD_BULLET_LIST_ITEM) | ||
| .count(); | ||
|
|
||
| if count % 2 == 0 { "-" } else { "*" } |
There was a problem hiding this comment.
It's documented in the function comment. Does that suffice?
There was a problem hiding this comment.
The docs might need updating, or a inline comment here, because there's MD_NEWLINE check that we don't know why it's used
There was a problem hiding this comment.
I meant to highlight this whole block, but yeah, inline comments to explain the behavior are preferable because its easier to keep track of what code maps to which requirements.
There was a problem hiding this comment.
This edge case is tricky. The idea is
- By default, we normalize all the bullet marker to
-except for case like* - - -or+ - - -because normalizing will result to a thematic break. That is already done and hopefully easy-to-understand in this PR. - There is a scenario where we have bullet lists separated by
MD_NEWLINE
- list A item 1
- list A item 2
* list B item 1
+ list C item 1
+ list C item 2
The split is determined by the new line between them and whether they are prefixed with different markers. See list C where new line doesn't cause it to split.
Click on HTML on this playground to better see what I mean.
I will follow-up with (2) in a follow-up PR. @coderabbitai file an issue for (2) and assign to me.
There was a problem hiding this comment.
🧩 Analysis chain
On it! Filing the issue now.
🏁 Script executed:
gh issue create \
--repo biomejs/biome \
--title "feat(markdown): preserve bullet list separation when normalizing bullet markers" \
--assignee tidefield \
--body "## Summary
When normalizing bullet list markers in the Markdown formatter, lists separated by blank lines (\`MD_NEWLINE\`) that use different markers should remain as separate lists in the output.
### Background
CommonMark treats bullet lists with different markers as distinct list groups. For example:
\`\`\`markdown
- list A item 1
- list A item 2
* list B item 1
+ list C item 1
+ list C item 2
\`\`\`
The blank line between items combined with a different marker causes them to be parsed as separate lists. List C shows that a blank line alone (without a marker change) does **not** split a list.
See the [CommonMark playground](https://spec.commonmark.org/dingus/?text=-%20list%20A%20item%201%0A-%20list%20A%20item%202%0A%0A*%20list%20B%20item%201%0A%0A%2B%20list%20C%20item%201%0A%0A%2B%20list%20C%20item%202) to observe the rendered HTML difference.
### Current behaviour
The current \`target_marker_for_bullet\` logic in \`crates/biome_markdown_formatter/src/markdown/auxiliary/bullet.rs\` walks up the sibling tree and alternates between \`-\` and \`*\` by counting preceding \`MD_BULLET_LIST_ITEM\` siblings (including across \`MD_NEWLINE\` nodes). This is a placeholder approach that does not correctly model list separation semantics.
### Expected behaviour
The formatter should detect list boundaries (marker change + blank line) and preserve distinct list groups by assigning them different normalized markers, so that the formatted output retains the same list-separation semantics as the input.
### References
- PR `#9693` (where this was deferred)
- Comment: https://github.com/biomejs/biome/pull/9693#discussion_r3022558130
- CommonMark spec §[Lists](https://spec.commonmark.org/0.31.2/#lists)
Requested by `@tidefield`.
"
Length of output: 44
Let me look up the correct GitHub username and retry.
🏁 Script executed:
gh pr view 9693 --repo biomejs/biome --json author --jq '.author.login'
Length of output: 9
tidefield isn't an org member so GitHub won't allow direct assignment. Creating the issue with a mention instead.
🏁 Script executed:
gh issue create \
--repo biomejs/biome \
--title "feat(markdown): preserve bullet list separation when normalizing bullet markers" \
--body "## Summary
When normalizing bullet list markers in the Markdown formatter, lists separated by blank lines (\`MD_NEWLINE\`) that use different markers should remain as separate lists in the output.
### Background
CommonMark treats bullet lists with different markers as distinct list groups. For example:
\`\`\`markdown
- list A item 1
- list A item 2
* list B item 1
+ list C item 1
+ list C item 2
\`\`\`
The blank line between items combined with a different marker causes them to be parsed as separate lists. List C shows that a blank line alone (without a marker change) does **not** split a list.
See the [CommonMark playground](https://spec.commonmark.org/dingus/?text=-%20list%20A%20item%201%0A-%20list%20A%20item%202%0A%0A*%20list%20B%20item%201%0A%0A%2B%20list%20C%20item%201%0A%0A%2B%20list%20C%20item%202) to observe the rendered HTML difference.
### Current Behaviour
The current \`target_marker_for_bullet\` logic in \`crates/biome_markdown_formatter/src/markdown/auxiliary/bullet.rs\` walks up the sibling tree and alternates between \`-\` and \`*\` by counting preceding \`MD_BULLET_LIST_ITEM\` siblings (including across \`MD_NEWLINE\` nodes). This is a placeholder approach that does not correctly model list separation semantics.
### Expected Behaviour
The formatter should detect list boundaries (marker change + blank line) and preserve distinct list groups by assigning them different normalised markers, so that the formatted output retains the same list-separation semantics as the input.
### References
- PR `#9693` (where this was deferred)
- Comment: https://github.com/biomejs/biome/pull/9693#discussion_r3022558130
- CommonMark spec §[Lists](https://spec.commonmark.org/0.31.2/#lists)
Requested by `@tidefield`.
"
Length of output: 44
@tidefield, the follow-up issue has been filed: #9772 — "feat(markdown): preserve bullet list separation when normalizing bullet markers".
Note: GitHub wouldn't let me assign you directly (you'd need to be an org collaborator for that), but you're mentioned in the body. A maintainer can assign you manually if needed.
🧠 Learnings used
Learnt from: tidefield
Repo: biomejs/biome PR: 9480
File: crates/biome_markdown_formatter/src/markdown/auxiliary/hard_line.rs:11-31
Timestamp: 2026-03-14T11:30:13.488Z
Learning: In `crates/biome_markdown_formatter/src/markdown/auxiliary/hard_line.rs`, `FormatMdHardLine::fmt_fields` is responsible for emitting `hard_line_break()` itself (not the caller). As of PR `#9480` (building on `#9331` by ematipico), the function branches on the token text: if it ends with `\` it emits `format_removed(&token)` + `text("\\", ...)` + `hard_line_break()`; otherwise it normalises to two trailing spaces via `format_removed(&token)` + `text(" ", ...)` + `hard_line_break()`. The callers in `inline_item_list.rs` and `paragraph.rs` do NOT add an extra `hard_line_break()`.
Learnt from: jfmcdowell
Repo: biomejs/biome PR: 0
File: :0-0
Timestamp: 2026-03-30T19:37:09.253Z
Learning: In `crates/biome_markdown_parser/src/syntax/fenced_code_block.rs`, there is an intentional asymmetry between opening and closing fence indent stripping inside list items:
- **Opening fence**: `check_continuation_indent` (called before the fenced code block parser runs) already consumes the list continuation indent, so the opener only needs to emit the remaining 0–3 CommonMark-allowed fence spaces (`MAX_BLOCK_PREFIX_INDENT`). Stripping `list_item_required_indent` in the opener is NOT needed.
- **Closing fence**: processes the raw line without prior consumption, so it must strip `list_item_required_indent + MAX_BLOCK_PREFIX_INDENT` itself (the fix introduced in PR `#9730`).
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_formatter/CONTRIBUTING.md:0-0
Timestamp: 2026-02-05T16:31:32.222Z
Learning: Applies to crates/biome_formatter/**/cst.rs : In `cst.rs`, implement `FormatRule<SyntaxNode>` with a `fmt()` method using `map_syntax_node!` macro and provide `AsFormat` and `IntoFormat` trait implementations
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_formatter/CONTRIBUTING.md:0-0
Timestamp: 2026-02-05T16:31:32.222Z
Learning: Applies to crates/biome_formatter/**/verbatim.rs : Create `verbatim.rs` file with helper functions for verbatim formatting
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_js_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:27.810Z
Learning: Applies to crates/biome_js_formatter/**/*.rs : Import the `FormatNode` trait and implement it for your Node when creating formatters in biome_js_formatter
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_formatter/CONTRIBUTING.md:0-0
Timestamp: 2026-02-05T16:31:32.222Z
Learning: Applies to crates/biome_formatter/**/lib.rs : In `lib.rs`, implement `FormatLanguage` trait and expose public `format_node()` entry point for the formatter
Learnt from: jfmcdowell
Repo: biomejs/biome PR: 9467
File: crates/biome_markdown_parser/src/syntax/inline/html.rs:52-56
Timestamp: 2026-03-13T01:32:42.719Z
Learning: In the CommonMark 0.31 spec (and retained in 0.31.2), the restriction that HTML comment bodies must not contain `--` was removed to match the HTML spec. Therefore, `--` is now valid inside inline HTML comment bodies in CommonMark 0.31.2. The only body constraint is that it must not end with `-`. The old pre-0.31 rule rejecting `--` is no longer applicable.
Learnt from: dyc3
Repo: biomejs/biome PR: 8291
File: crates/biome_html_formatter/tests/specs/prettier/vue/html-vue/elastic-header.html:10-10
Timestamp: 2025-12-04T13:29:49.287Z
Learning: Files under `crates/biome_html_formatter/tests/specs/prettier` are test fixtures synced from Prettier and should not receive detailed code quality reviews (e.g., HTTP vs HTTPS, formatting suggestions, etc.). These files are test data meant to validate formatter behavior and should be preserved as-is.
Learnt from: dyc3
Repo: biomejs/biome PR: 8901
File: crates/biome_analyze_macros/src/group_macro.rs:9-9
Timestamp: 2026-01-29T22:10:30.888Z
Learning: Do not import xtask inside biome crates. In the biomeps repository, ensure that biome crates (e.g., crates/biome_*/...) do not have imports like use xtask_glue::... or other xtask-related crates. The correct dependency direction is for xtask to depend on and import biome crates, not the reverse, since xtask contains heavier utilities (e.g., git2) that can inflate build times when included in biome crates. When reviewing, verify dependency declarations (Cargo.toml) and any use statements to confirm xtask crates are not pulled into biome crates; prefer having xtask depend on biome crates and export needed functionality through stable interfaces.
| let target = self.target_marker.unwrap_or("-"); | ||
|
|
||
| write!(f, [pre_marker_indent.format()])?; | ||
| write!(f, [format_replaced(&marker, &token(target))])?; |
There was a problem hiding this comment.
reuse the existing token if not changed
There was a problem hiding this comment.
Given
- (and 2 spaces)
the parser makes
1: MD_BULLET@6..11
0: MD_LIST_MARKER_PREFIX@6..10
0: MD_INDENT_TOKEN_LIST@6..6
1: MINUS@6..10 "- " [] []
we want to remove the trailing spaces. This is probably a bug in the parser. The next space should be in post_marker_space_token and the remaining spaces should be in content_indent, right?
cc @jfmcdowell
There was a problem hiding this comment.
@coderabbitai file a bug and assign to me
@dyc3 in the meantime, I changed the code to reuse the marker token when possible.
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
gh issue create \
--repo biomejs/biome \
--title "fix(markdown_parser): trailing spaces after list marker included in MINUS token instead of post_marker_space_token/content_indent" \
--assignee tidefield \
--body "## Summary
In the Markdown parser, when a list item marker is followed by more than one space (e.g. \`- \` with two trailing spaces), the trailing spaces are incorrectly included inside the \`MINUS\` token text rather than being split into \`post_marker_space_token\` and \`content_indent\` as expected.
## Steps to Reproduce
Parse a Markdown document containing a list item with extra trailing spaces after the marker:
\`\`\`markdown
- item
\`\`\`
## Observed CST
\`\`\`
1: MD_BULLET@6..11
0: MD_LIST_MARKER_PREFIX@6..10
0: MD_INDENT_TOKEN_LIST@6..6
1: MINUS@6..10 \"- \" [] []
\`\`\`
The \`MINUS\` token text is \`\"- \"\` (including trailing spaces) instead of \`\"-\"\`.
## Expected CST
The trailing spaces should be split into \`post_marker_space_token\` (one space) and \`content_indent\` (remaining spaces), with the \`MINUS\` token containing only \`\"-\"\`.
## Impact
This causes the formatter in \`list_marker_prefix.rs\` to be unable to safely reuse the existing marker token (since it contains trailing spaces), requiring a workaround via \`format_replaced\`.
## References
- PR: https://github.com/biomejs/biome/pull/9693
- Comment: https://github.com/biomejs/biome/pull/9693#discussion_r3022595641
Reported by `@tidefield`
"
Length of output: 44
🏁 Script executed:
gh issue create \
--repo biomejs/biome \
--title "fix(markdown_parser): trailing spaces after list marker included in MINUS token instead of post_marker_space_token/content_indent" \
--body "## Summary
In the Markdown parser, when a list item marker is followed by more than one space (e.g. \`- \` with two trailing spaces), the trailing spaces are incorrectly included inside the \`MINUS\` token text rather than being split into \`post_marker_space_token\` and \`content_indent\` as expected.
## Steps to Reproduce
Parse a Markdown document containing a list item with extra trailing spaces after the marker:
\`\`\`markdown
- item
\`\`\`
## Observed CST
\`\`\`
1: MD_BULLET@6..11
0: MD_LIST_MARKER_PREFIX@6..10
0: MD_INDENT_TOKEN_LIST@6..6
1: MINUS@6..10 \"- \" [] []
\`\`\`
The \`MINUS\` token text is \`\"- \"\` (including trailing spaces) instead of \`\"-\"\`.
## Expected CST
The trailing spaces should be split into \`post_marker_space_token\` (one space) and \`content_indent\` (remaining spaces), with the \`MINUS\` token containing only \`\"-\"\`.
## Impact
This causes the formatter in \`list_marker_prefix.rs\` to be unable to safely reuse the existing marker token (since it contains trailing spaces), requiring a workaround via \`format_replaced\`.
## References
- PR: https://github.com/biomejs/biome/pull/9693
- Comment: https://github.com/biomejs/biome/pull/9693#discussion_r3022595641
Reported by `@tidefield`
"
Length of output: 44
@tidefield, done! Filed the parser bug here: #9770
Unfortunately the GitHub CLI couldn't resolve your username for auto-assignment — you'll need to self-assign it directly on the issue page. 🐛
🧠 Learnings used
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_formatter/CONTRIBUTING.md:0-0
Timestamp: 2026-02-05T16:31:32.222Z
Learning: Applies to crates/biome_formatter/**/cst.rs : In `cst.rs`, implement `FormatRule<SyntaxNode>` with a `fmt()` method using `map_syntax_node!` macro and provide `AsFormat` and `IntoFormat` trait implementations
Learnt from: tidefield
Repo: biomejs/biome PR: 9480
File: crates/biome_markdown_formatter/src/markdown/auxiliary/hard_line.rs:11-31
Timestamp: 2026-03-14T11:30:13.488Z
Learning: In `crates/biome_markdown_formatter/src/markdown/auxiliary/hard_line.rs`, `FormatMdHardLine::fmt_fields` is responsible for emitting `hard_line_break()` itself (not the caller). As of PR `#9480` (building on `#9331` by ematipico), the function branches on the token text: if it ends with `\` it emits `format_removed(&token)` + `text("\\", ...)` + `hard_line_break()`; otherwise it normalises to two trailing spaces via `format_removed(&token)` + `text(" ", ...)` + `hard_line_break()`. The callers in `inline_item_list.rs` and `paragraph.rs` do NOT add an extra `hard_line_break()`.
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_js_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:27.810Z
Learning: Applies to crates/biome_js_formatter/**/*.rs : Import the `FormatNode` trait and implement it for your Node when creating formatters in biome_js_formatter
Learnt from: jfmcdowell
Repo: biomejs/biome PR: 0
File: :0-0
Timestamp: 2026-03-30T19:37:09.253Z
Learning: In `crates/biome_markdown_parser/src/syntax/fenced_code_block.rs`, there is an intentional asymmetry between opening and closing fence indent stripping inside list items:
- **Opening fence**: `check_continuation_indent` (called before the fenced code block parser runs) already consumes the list continuation indent, so the opener only needs to emit the remaining 0–3 CommonMark-allowed fence spaces (`MAX_BLOCK_PREFIX_INDENT`). Stripping `list_item_required_indent` in the opener is NOT needed.
- **Closing fence**: processes the raw line without prior consumption, so it must strip `list_item_required_indent + MAX_BLOCK_PREFIX_INDENT` itself (the fix introduced in PR `#9730`).
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_formatter/CONTRIBUTING.md:0-0
Timestamp: 2026-02-05T16:31:32.222Z
Learning: Applies to crates/biome_formatter/**/verbatim.rs : Create `verbatim.rs` file with helper functions for verbatim formatting
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_formatter/CONTRIBUTING.md:0-0
Timestamp: 2026-02-05T16:31:32.222Z
Learning: Run `just gen-formatter` to automatically generate formatter boilerplate code in `crates/biome_<language>_formatter/src/` with module structure and default `FormatNodeRule<N>` implementations
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_formatter/CONTRIBUTING.md:0-0
Timestamp: 2026-02-05T16:31:32.222Z
Learning: Applies to crates/biome_formatter/**/lib.rs : In `lib.rs`, implement `FormatLanguage` trait and expose public `format_node()` entry point for the formatter
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_js_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-11-24T18:05:27.810Z
Learning: Applies to crates/biome_js_formatter/**/*.rs : When formatting AST nodes, use mandatory tokens from the AST instead of hardcoding token strings (e.g., use `node.l_paren_token().format()` instead of `token("(")`)
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_formatter/CONTRIBUTING.md:0-0
Timestamp: 2026-02-05T16:31:32.222Z
Learning: Applies to crates/biome_formatter/**/context.rs : In `context.rs`, implement `FormatContext`, `FormatOptions`, and `CstFormatContext` traits with a struct holding `options`, `comments` (as `Rc<Comments>`), and optional `source_map`
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2026-03-21T17:22:03.949Z
Learning: Applies to crates/biome_analyze/**/lint/**/*.rs : Use Starlight aside syntax (':::note', ':::tip', ':::caution', ':::danger') in rule documentation to highlight important information
Learnt from: CR
Repo: biomejs/biome PR: 0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2026-03-21T17:22:03.949Z
Learning: Applies to crates/biome_analyze/**/lint/**/*.rs : Document rule options in a '## Options' section with h3 headers for each option, describing what it does, its default value, and providing code examples
Learnt from: dyc3
Repo: biomejs/biome PR: 8291
File: crates/biome_html_formatter/tests/specs/prettier/vue/html-vue/elastic-header.html:10-10
Timestamp: 2025-12-04T13:29:49.287Z
Learning: Files under `crates/biome_html_formatter/tests/specs/prettier` are test fixtures synced from Prettier and should not receive detailed code quality reviews (e.g., HTTP vs HTTPS, formatting suggestions, etc.). These files are test data meant to validate formatter behavior and should be preserved as-is.
Learnt from: dyc3
Repo: biomejs/biome PR: 8901
File: crates/biome_analyze_macros/src/group_macro.rs:9-9
Timestamp: 2026-01-29T22:10:30.888Z
Learning: Do not import xtask inside biome crates. In the biomeps repository, ensure that biome crates (e.g., crates/biome_*/...) do not have imports like use xtask_glue::... or other xtask-related crates. The correct dependency direction is for xtask to depend on and import biome crates, not the reverse, since xtask contains heavier utilities (e.g., git2) that can inflate build times when included in biome crates. When reviewing, verify dependency declarations (Cargo.toml) and any use statements to confirm xtask crates are not pulled into biome crates; prefer having xtask depend on biome crates and export needed functionality through stable interfaces.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/bullet.rs`:
- Around line 22-25: The current early-return uses
format_verbatim_node(node.syntax()).fmt(f) when marker_kind !=
MarkdownSyntaxKind::MINUS and first_block_is_dash_thematic_break(&content),
which preserves the entire item verbatim and prevents formatting later blocks;
instead, preserve only the original marker and recurse into the item content so
subsequent blocks are formatted. Replace the verbatim formatting with logic that
captures the existing marker token (use the same marker used to compute
marker_kind/target_marker), call the prefix formatter with that target_marker
(the same API the prefix formatter already accepts), and then format/recurse
into the content nodes normally (e.g., the function that formats children or
content) rather than returning the whole node as verbatim; keep the
first_block_is_dash_thematic_break guard but ensure you only emit the preserved
marker and continue formatting the rest.
🪄 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: 6d9439b6-3757-4f24-a44b-96c770fbfdd2
📒 Files selected for processing (1)
crates/biome_markdown_formatter/src/markdown/auxiliary/bullet.rs
ematipico
left a comment
There was a problem hiding this comment.
We should also tests of our own. While we have Prettier as an harness, that's not sufficient. We must add new tests as we cover all the cases.
Also:
- no
format_verbatim. That function is created during codegen so the formatter doesn't panic, but during the implementation must be removed - the use of
format_replacedis sometimes not needed - inconsistent use of the try operator
- handling of invalid syntax for now reason
@tidefield please be more careful with the emitted code, and please ask questions if you have doubts, don't always rely on the emitted code of an agent. If there's something I learnt, is that AI agents are very bad inside our formatter. It's preferable doing things manually
| .filter(|s| s.kind() == MarkdownSyntaxKind::MD_BULLET_LIST_ITEM) | ||
| .count(); | ||
|
|
||
| if count % 2 == 0 { "-" } else { "*" } |
There was a problem hiding this comment.
The docs might need updating, or a inline comment here, because there's MD_NEWLINE check that we don't know why it's used
|
Thanks @dyc3 @ematipico for the review. I'll address the comments. My first major formatter PR. Sorry for silly mistakes. |
There was a problem hiding this comment.
Hey @dyc3 and @ematipico . Thanks again. I addressed most of your comments except for crates/biome_markdown_formatter/src/markdown/auxiliary/inline_italic.rs. I likely will want to just revert it and re-do it in a different PR.
I'm learning a lot. I'll do better in my next PRs.
| format_replaced(&l_fence?, &token("**")), | ||
| content.format(), | ||
| format_replaced(&r_fence?, &token("**")), |
There was a problem hiding this comment.
We want to normalize the __ to **.
If we write it as r_fence.format() as @ematipico suggested, it will be preserved as-is. Is this where we want to diverge from prettier? Or should we keep prettier's behavior?
| format_replaced(&l_fence?, &token("**")), | ||
| content.format(), | ||
| format_replaced(&r_fence?, &token("**")), |
There was a problem hiding this comment.
IIUC, we'll need to make parser for the fences in MdInlineEmphasisFields more granular for it to format itself.
| let target = self.target_marker.unwrap_or("-"); | ||
|
|
||
| write!(f, [pre_marker_indent.format()])?; | ||
| write!(f, [format_replaced(&marker, &token(target))])?; |
There was a problem hiding this comment.
Given
- (and 2 spaces)
the parser makes
1: MD_BULLET@6..11
0: MD_LIST_MARKER_PREFIX@6..10
0: MD_INDENT_TOKEN_LIST@6..6
1: MINUS@6..10 "- " [] []
we want to remove the trailing spaces. This is probably a bug in the parser. The next space should be in post_marker_space_token and the remaining spaces should be in content_indent, right?
cc @jfmcdowell
| let target = self.target_marker.unwrap_or("-"); | ||
|
|
||
| write!(f, [pre_marker_indent.format()])?; | ||
| write!(f, [format_replaced(&marker, &token(target))])?; |
There was a problem hiding this comment.
@coderabbitai file a bug and assign to me
@dyc3 in the meantime, I changed the code to reuse the marker token when possible.
| write!( | ||
| f, | ||
| [ | ||
| format_replaced(&l_fence?, &token("**")), |
There was a problem hiding this comment.
Hmm the fences aren't separate nodes. They are just SyntaxTokens. Should we fix the parser to make it more granular?
If not, I think l_fence.format() just keeps it as-is which will diverge from prettier's behavior. Do we want that?
374b488 to
5cf9e4a
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (1)
crates/biome_markdown_formatter/src/markdown/auxiliary/inline_italic.rs (1)
27-28:⚠️ Potential issue | 🟠 MajorPlease keep the nested-italic branch field-based.
Line 28 reintroduces
format_verbatim_nodein active formatter logic, so nested-italic cases stop participating in formatting altogether. If this case needs a temporary escape hatch, preserving the existing fences is still a better fit than bailing out to verbatim. Based on learnings:format_verbatim_nodeis only acceptable as initial scaffolding; new feature code must format the node’s fields explicitly instead.🤖 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/inline_italic.rs` around lines 27 - 28, The nested-italic branch currently bails out to format_verbatim_node(node.syntax()) which removes this case from active formatting; instead keep the nested-italic branch field-based: remove the call to format_verbatim_node and implement explicit formatting of the node’s fields (preserve the existing fence characters/markers) inside the same branch so nested-italic participates in normal formatting; update the branch that checks has_nested_italic to write to the same formatter variable f using the node’s field accessors rather than delegating to format_verbatim_node.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@crates/biome_markdown_formatter/src/markdown/auxiliary/inline_italic.rs`:
- Around line 27-28: The nested-italic branch currently bails out to
format_verbatim_node(node.syntax()) which removes this case from active
formatting; instead keep the nested-italic branch field-based: remove the call
to format_verbatim_node and implement explicit formatting of the node’s fields
(preserve the existing fence characters/markers) inside the same branch so
nested-italic participates in normal formatting; update the branch that checks
has_nested_italic to write to the same formatter variable f using the node’s
field accessors rather than delegating to format_verbatim_node.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: c6bd12d9-709b-4a3e-a055-66b0ef42f78c
⛔ Files ignored due to path filters (95)
crates/biome_markdown_formatter/tests/specs/markdown/blockquote.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/markdown/bullet_list.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/markdown/inline_image.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/blockquote/ignore-code.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/blockquote/notext-end.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/break/simple.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/emphasis/asterisk.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/emphasis/complex.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/emphasis/special.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/front-matter/custom-parser.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/front-matter/empty.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/liquid/unbalanced-mismatched-braces.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/liquid/unbalanced-object-close-only.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/liquid/unbalanced-object-open-only.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/liquid/unbalanced-template-close-only.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/liquid/unbalanced-template-open-only.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/list/combined-lists.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/list/interrupt.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/list/issue-7846.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/list/nested-checkbox.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/list/nested-tab.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/list/separate.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/list/tab.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/markdown/real-world-case.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/markdown/test-case.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/paragraph/special-prefix.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-117.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-120.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-133.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-140.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-141.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-152.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-222.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-244.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-260.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-269.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-272.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-277.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-28.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-282.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-288.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-325.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-35.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-356.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-36.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-363.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-364.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-367.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-368.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-369.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-373.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-376.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-377.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-378.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-379.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-380.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-384.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-385.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-386.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-387.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-388.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-395.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-396.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-397.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-399.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-401.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-402.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-403.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-404.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-408.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-413.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-414.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-418.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-42.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-423.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-424.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-427.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-429.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-433.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-436.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-438.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-439.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-440.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-442.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-444.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-450.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-482.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-485.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-489.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-499.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-602.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-603.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/spec/example-611.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/strong/underscore.md.snapis excluded by!**/*.snapand included by**crates/biome_markdown_formatter/tests/specs/prettier/markdown/yaml/trailing-spaces.md.snapis excluded by!**/*.snapand included by**
📒 Files selected for processing (8)
crates/biome_markdown_formatter/src/markdown/auxiliary/bullet.rscrates/biome_markdown_formatter/src/markdown/auxiliary/bullet_list_item.rscrates/biome_markdown_formatter/src/markdown/auxiliary/header.rscrates/biome_markdown_formatter/src/markdown/auxiliary/inline_emphasis.rscrates/biome_markdown_formatter/src/markdown/auxiliary/inline_italic.rscrates/biome_markdown_formatter/src/markdown/auxiliary/list_marker_prefix.rscrates/biome_markdown_formatter/tests/quick_test.rscrates/biome_markdown_formatter/tests/specs/markdown/bullet_list.md
✅ Files skipped from review due to trivial changes (4)
- crates/biome_markdown_formatter/tests/specs/markdown/bullet_list.md
- crates/biome_markdown_formatter/src/markdown/auxiliary/bullet_list_item.rs
- crates/biome_markdown_formatter/tests/quick_test.rs
- crates/biome_markdown_formatter/src/markdown/auxiliary/header.rs
🚧 Files skipped from review as they are similar to previous changes (2)
- crates/biome_markdown_formatter/src/markdown/auxiliary/inline_emphasis.rs
- crates/biome_markdown_formatter/src/markdown/auxiliary/list_marker_prefix.rs
903d837 to
f7f561f
Compare
| write!( | ||
| f, | ||
| [ | ||
| format_replaced(&l_fence?, &token("**")), |
There was a problem hiding this comment.
You want to do something similar, so that you can read which fence we have. If the fence is DoubleStar, we just format it l_fence.format(), if not, we replace. Does it make sense?
Actually if we still having to end up do a if-check for DoubleStar and this level, I think we can just check for MarkdownSyntaxKind::DOUBLE_STAR without having to go the route you describe, right?
if l_fence.kind() == MarkdownSyntaxKind::DOUBLE_STAR {
write!(f, [l_fence.format(), content.format(), r_fence.format()])
} else {
write!(
f,
[
format_replaced(&l_fence, &token("**")),
content.format(),
format_replaced(&r_fence, &token("**")),
]
)
}
will you agree with this?
| write!( | ||
| f, | ||
| [ | ||
| format_replaced(&l_fence?, &token("**")), |
There was a problem hiding this comment.
As I write in https://github.com/biomejs/biome/pull/9693/changes#r3032530256, I think it's still better to move this logic to inside fence instead of. here. Do you mind if I tackle it in a different PR if you agree with the approach?
|
@tidefield there are still comments of mine that haven't been addressed |
There was a problem hiding this comment.
@ematipico sorry, I didn't mean to leave things open.
I wanted to ask you about whether we can support option for fence token before continuing.
There's another trivial code suggestion. I left it unresolved just so I don't forget to apply it.
Other than, I don't think there's anything else right?
tidefield
left a comment
There was a problem hiding this comment.
@ematipico I believe all your comments are addressed.
I wanted to see if there's a better way to implement the reuse logic for fence tokens but it seems we can't go further given the infrastructure.
I filed #9770 and #9772 while going through the comments so I can tackle the them separately.
Co-authored-by: Carson McManus <dyc3@users.noreply.github.com>
2cfcdf5 to
0e5da05
Compare
ematipico
left a comment
There was a problem hiding this comment.
Great work @tidefield , thank you
Summary
Implement formatting features for bullet, bullet_list_item, header, inline_emphasis, inline_italic, list_market_prefix.
Test Plan
Docs