feat(core): embedded formatting in html files#7467
Merged
Conversation
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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
Part of #6657
This PR establishes the infrastructure for embedded formatting and implements it for
.htmlfiles.The infrastructure brings my ideas of how we should do it, so feel free to discuss it.
Formatter infrastructure
The infrastructure was designed to avoid any sort of cyclic dependencies. This is an important part of how I designed it. Because of this, the formatting is done inside
biome_service, which is the crate that imports allbiome_*_formattercrates.The formatting of embedded code is in two passes. The first pass is the usual formatting that we always implement in all languages; the second pass is done via a new function called
format_embedded. This function accepts a closure that provides theTextRangeof where the embedded node is found (we will get to thatTextRangein a bit). The caller will use thatTextRangeto retrieve the CST root, and then use its own formatter (JS and CSS in case of the implementation). Eventually, the closure must return a formatterDocument.The function
format_embeddedis called insidebiome_formatter, which now tracks the embedded nodes during the first formatting pass. The node is tracked using the newTag:StartEmbedded(TextRange). TheTextRangeis precisely the range that I mentioned in the previous paragraph. This range is collected using a new function calledembedded_node_rangewhich is implemented in eachFormatNodeof the language. In this PR, we implement the function inside the elementHtmlEmbeddedContent. The range we save must match the range of CST we save in theWorkspace. That's how we track the embedded nodes.During the second pass, we replace
Tag:StartEmbedded(TextRange)with the format elements returned by the closure mentioned in the first paragraph. The elementTag::EndEmbeddedis currently replaced with a hard line. I left a note there, so in the future, we can use a better heuristic and choose a node that fits the situation.The replacement of the
FormatElements is done using a new visitor infrastructure. The code of the visitor was suggested using an AI agent. I suggested a solution that allows replacing a vector element with nested infrastructure.Parsing of embedded nodes
The parsing of embedded nodes is now done inside the
Workspacebecause we must retrieve the parsing options of the current project.ServiceLanguageandWorkspaceThe
ServiceLanguagehas been updated with a new capability calledformat_embedded. I chose to add a new capability because it needs to accept a list of embedded nodes saved inside the Workspace. This allows us to switch from one formatting to another depending on the presence of some embedded nodes.The
ServiceLanguagenow expose a new function calledparse_options. Up until now the resolution of these options was done only inside theparsefunction. Now the scope become larger, so I needed a common infrastructure to retrieve those options. When we parse embedded nodes, we use the parsing options defined in the configuration.Test Plan
I added some tests. For some reason, the CSS formatting is a bit funky. I will tackle that in the next PR.
Docs
I believe it doesn't require docs for now