chore: AnyEmbeddedSnippet terminology#7674
Conversation
|
WalkthroughThe PR renames and consolidates embedded-content types across the service: EmbeddedLanguageContent and EmbeddedSnippets are replaced by EmbeddedSnippet and AnyEmbeddedSnippet. HTML file handlers now return EmbeddedSnippet for script/style/JSON parsing. Workspace document structures and accessors are updated to store and expose AnyEmbeddedSnippet and EmbeddedSnippet, with expanded fields (element/content ranges, offset, file_source_index). Re-exports in workspace and mod are adjusted, and server paths now produce/consume AnyEmbeddedSnippet. ParseEmbedResult.nodes switches to Vec<(AnyEmbeddedSnippet, DocumentFileSource)>. No behavioural changes are introduced beyond type and field propagation. Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
crates/biome_service/src/workspace/server.rs (1)
500-506: Return type update looks good; drop an unnecessary clone in lint path
- get_language_snippets now returns Vec — good.
- In pull_diagnostics, parse() already returns an owned AnyParse; the trailing .clone() is redundant.
Apply:
- parse: embedded_node.parse().clone(), + parse: embedded_node.parse(),Also applies to: 1416-1421
crates/biome_service/src/file_handlers/html.rs (1)
469-502: Optional: factor shared snippet-building logicThe three parse_embedded_* functions share identical child extraction and EmbeddedSnippet::new patterns. Consider a small helper to reduce duplication and keep future fixes in one place.
Also applies to: 508-534
crates/biome_service/src/workspace/document.rs (1)
215-216: Remove redundant clone when preparing format nodesparse() already returns an owned AnyParse (cloned from the snippet). No need to clone again.
- .map(|node| FormatEmbedNode { - range: node.content_range(), - source: get_file_source(node.file_source_index()), - node: node.parse().clone(), - }) + .map(|node| FormatEmbedNode { + range: node.content_range(), + source: get_file_source(node.file_source_index()), + node: node.parse(), + })Also applies to: 223-231
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
crates/biome_service/src/file_handlers/html.rs(7 hunks)crates/biome_service/src/file_handlers/mod.rs(2 hunks)crates/biome_service/src/workspace.rs(1 hunks)crates/biome_service/src/workspace/document.rs(5 hunks)crates/biome_service/src/workspace/server.rs(3 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
crates/biome_*/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place core crates under /crates/biome_*/
Files:
crates/biome_service/src/file_handlers/html.rscrates/biome_service/src/workspace/server.rscrates/biome_service/src/workspace/document.rscrates/biome_service/src/file_handlers/mod.rscrates/biome_service/src/workspace.rs
**/*.{rs,toml}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Before committing, format Rust and TOML files (e.g., via
just f/just format)
Files:
crates/biome_service/src/file_handlers/html.rscrates/biome_service/src/workspace/server.rscrates/biome_service/src/workspace/document.rscrates/biome_service/src/file_handlers/mod.rscrates/biome_service/src/workspace.rs
**/*.rs
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Document rules, assists, and options via inline rustdoc in Rust source
Files:
crates/biome_service/src/file_handlers/html.rscrates/biome_service/src/workspace/server.rscrates/biome_service/src/workspace/document.rscrates/biome_service/src/file_handlers/mod.rscrates/biome_service/src/workspace.rs
crates/biome_service/src/workspace.rs
📄 CodeRabbit inference engine (crates/biome_service/CONTRIBUTING.md)
Workspacetrait implementation entry point is located at src/workspace.rs
Files:
crates/biome_service/src/workspace.rs
🧠 Learnings (1)
📚 Learning: 2025-10-02T13:00:18.232Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_service/CONTRIBUTING.md:0-0
Timestamp: 2025-10-02T13:00:18.232Z
Learning: Applies to crates/biome_service/src/workspace.rs : `Workspace` trait implementation entry point is located at src/workspace.rs
Applied to files:
crates/biome_service/src/workspace.rs
🧬 Code graph analysis (2)
crates/biome_service/src/file_handlers/html.rs (2)
packages/@biomejs/backend-jsonrpc/src/workspace.ts (1)
DocumentFileSource(9365-9373)crates/biome_service/src/workspace/document.rs (1)
new(150-164)
crates/biome_service/src/file_handlers/mod.rs (1)
packages/@biomejs/backend-jsonrpc/src/workspace.ts (1)
DocumentFileSource(9365-9373)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: Lint project (depot-ubuntu-24.04-arm-16)
- GitHub Check: Documentation
- GitHub Check: Check Dependencies
- GitHub Check: Test (depot-windows-2022-16)
- GitHub Check: Test (depot-ubuntu-24.04-arm-16)
- GitHub Check: Lint project (depot-windows-2022)
- GitHub Check: autofix
🔇 Additional comments (11)
crates/biome_service/src/workspace/server.rs (2)
18-18: Type rename wired inImporting AnyEmbeddedSnippet from document is correct and aligns with the new API.
516-536: parse_embedded_language_snippets now returns AnyEmbeddedSnippetSignature and body match the new ParseEmbedResult element type. Setting file_source_index before pushing is spot‑on.
crates/biome_service/src/workspace.rs (1)
58-58: Public re‑exports updated correctlyAnyEmbeddedSnippet and EmbeddedSnippet are the right surface types now.
crates/biome_service/src/file_handlers/mod.rs (2)
14-16: API import aligns with workspace changesBringing AnyEmbeddedSnippet into scope is correct.
465-466: ParseEmbedResult uses AnyEmbeddedSnippetType switch is consistent with producers and consumers.
crates/biome_service/src/file_handlers/html.rs (3)
7-7: Switch to EmbeddedSnippet importMatches the new embedded content type. All good.
422-463: parse_embedded_script: type and construction look right
- Return type updated to EmbeddedSnippet.
- element/content ranges and offset are correctly passed to EmbeddedSnippet::new.
491-496: Consistent snippet construction for CSS/JSONConstructor usage mirrors the JS path with correct ranges and offsets.
Also applies to: 526-531
crates/biome_service/src/workspace/document.rs (3)
15-19: Enum and conversions updated cleanlyAnyEmbeddedSnippet variants and From<EmbeddedSnippet<...>> impls are tidy and match usage across the crate.
Also applies to: 21-37
39-119: as_ methods now return references*Good change: avoids needless cloning of embedded snippets. The remaining accessors (parse, ranges, offsets, file_source_index) are consistent.
126-146: EmbeddedSnippet structure and ctor are soundFields and constructor parameters fit the embedded use‑case (element/content ranges + offset). node() and into_serde_diagnostics() correctly apply the content offset.
Also applies to: 148-186
D'oh! I already did that in my other PR 😅 |
3 participants
Summary
Renames
EmbeddedSnippetstoAnyEmbeddedSnippetandEmbeddedLanguageContenttoEmbeddedSnippet.Also applied my own suggestion to return references from the
as_*_embedded_snippet()methods, since I touched those methods anyway, so this shouldn't cause additional merge conflicts.Test Plan
Greenness.
Docs
N/A