test(markdown): add minimal service plumbing for formatter integration#9070
test(markdown): add minimal service plumbing for formatter integration#9070jfmcdowell wants to merge 3 commits intobiomejs:nextfrom
Conversation
Add the minimum markdown service integration so markdown is recognized as a formattable language and formatter tests can run against the standard service path. - Add MarkdownFileSource detection for md/markdown extensions and markdown language id. - Add DocumentFileSource::Markdown routing and markdown handler capabilities in biome_service. - Add markdown ServiceLanguage settings/format option resolution and formatter entry points. - Add MarkdownFormatOptions Display + missing option builder methods required by service plumbing. - Add focused service tests for markdown file-source detection and formatter capability routing. - Add markdown exhaustive match handling in xtask rules_check. Scope intentionally excludes Prettier markdown comparison tests (tracked separately).
|
Merging this PR will not alter performance
Comparing Footnotes
|
WalkthroughAdds Markdown support across the workspace: new MarkdownFileSource type and module, re-exports in biome_markdown_syntax, workspace dependency additions, new Markdown file handler and ServiceLanguage implementation in biome_service (formatter/linter/assist settings and capability wiring), builder-style setters and a Display impl for MarkdownFormatOptions in biome_markdown_formatter, Cargo workspace dependency additions (notify) and a small rules_check update to recognise Markdown. Several tests and feature wiring for detection, parsing and formatting were added. No existing public struct signatures were removed. Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts (beta)
No actionable comments were generated in the recent review. 🎉 Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. 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
🤖 Fix all issues with AI agents
In `@crates/biome_service/Cargo.toml`:
- Line 74: The manifest uses an explicit version for the notify dependency
instead of the workspace reference; update the crate's Cargo.toml so the notify
dependency references the workspace-level declaration (use workspace = true) or
move notify into the root workspace [dependencies] and keep workspace = true
here; locate the notify entry (notify = { version = "8.2.0", features =
["crossbeam-channel"] }) and change it to a workspace reference consistent with
other dependencies or add notify to the workspace dependencies table and remove
the explicit version here.
🧹 Nitpick comments (2)
crates/biome_service/src/file_handlers/markdown.rs (1)
210-225: Parser metadata is discarded during AnyParse conversion.
parse_markdown_with_cachereturnslist_tightness,list_item_indents, andquote_indentsalongside the syntax tree (seecrates/biome_markdown_parser/src/lib.rs). These are lost when converting toAnyParseviaNodeParse. This is fine for the current minimal plumbing, but will need addressing when substantive formatting logic lands — list tightness in particular affects Markdown rendering semantics.crates/biome_service/src/file_handlers/mod.rs (1)
434-441:can_contain_embedsreturnsfalsefor Markdown — worth a TODO?Markdown files commonly contain fenced code blocks (e.g.,
```js ... ```), which are semantically embedded snippets. Returningfalseis fine for this minimal plumbing PR, but it might be worth leaving a short comment or TODO so this isn't forgotten when embedded-language support lands.Optional: leave a breadcrumb
Self::Html(_) | Self::Js(_) => true, Self::Css(_) | Self::Graphql(_) | Self::Json(_) | Self::Grit(_) + // TODO: Markdown fenced code blocks may contain embeds — revisit when embedded support lands. | Self::Markdown(_) | Self::Ignore | Self::Unknown => false,
| @@ -0,0 +1,267 @@ | |||
| use super::{ | |||
There was a problem hiding this comment.
Hi @jfmcdowell . Sorry I'm afraid this will conflict with #9067 soon. :(
I was having this implementation in my local so I can verify the snapshots are correctly captured in the tests. I'm fixing some errors before pushing it to my PR.
There was a problem hiding this comment.
FYI the reason I needed to implement what you had in this PR is because the tests need to have a TestFormatLanguage implementation which then requires ServiceLanguage implementation. So it overlaps with what you're doing in the PR.
ematipico
left a comment
There was a problem hiding this comment.
It looks good, however we need to gate the implementation under a cargo feature or Biome will start parsing the files
|
|
||
| pub fn try_from_extension(extension: &str) -> Result<Self, FileSourceError> { | ||
| match extension { | ||
| "md" | "markdown" => Ok(Self::markdown()), |
There was a problem hiding this comment.
This will enable handling markdown to our users, and we're not ready for this yet. To prevent this, do the same thing I did for SCSS in this PR: #9091
|
|
||
| pub fn try_from_language_id(language_id: &str) -> Result<Self, FileSourceError> { | ||
| match language_id { | ||
| "markdown" => Ok(Self::markdown()), |
There was a problem hiding this comment.
Same thing here, let's gate it under a cargo feature
| #[derive( | ||
| Debug, Clone, Default, Copy, Eq, PartialEq, Hash, serde::Serialize, serde::Deserialize, | ||
| )] | ||
| pub struct MarkdownFileSource { |
There was a problem hiding this comment.
nit: I would call it MdFileSource, just because it's shorter
Note
AI Assistance Disclosure: This PR was developed with assistance from Claude Code.
Summary
This PR adds the minimum Markdown formatter plumbing needed so Markdown is recognized as a formattable language in Biome.
PR #8962 introduced formatter boilerplate. This PR focuses on the required syntax/service/formatter-option integration to make Markdown format-capable from the service layer.
Test corpus and comparison/snapshot-heavy formatter tests are handled separately in #9067
Continues work on #3718
Why
Markdown must be recognized by
biome_serviceas a format-capable language before formatter infrastructure can be exercised end-to-end.Scope
1) Markdown file source + service routing
MarkdownFileSourcewith:md,markdownmarkdownDocumentFileSource::Markdownsupport in service routing/capability dispatch.biome_servicewith:2) Markdown formatter options plumbing
MarkdownFormatOptions(required by service language traitbounds).
with_indent_stylewith_indent_widthwith_line_endingwith_trailing_newline3) Service/settings integration
LanguageListSettings.biome_serviceCargo config.4) Exhaustive match maintenance
DocumentFileSource::Markdownarm inxtask/rules_checkfor exhaustive handling.5) Follow-up consistency cleanup in this PR
DocumentFileSource::to_markdown_file_source()accessor for parity with other file source helpers.Not Included
Those tests/corpus are being developed in #9067
Validation
just fjust l