-
-
Notifications
You must be signed in to change notification settings - Fork 762
fix(html_formatter): preserve whitespace around inline elements and text expressions #7987
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
fix(html_formatter): preserve whitespace around inline elements and text expressions #7987
Conversation
…ext expressions
Fixed an issue where whitespace was being incorrectly removed after closing
tags of inline elements and around text expressions (e.g., {name} in
Astro/Vue/Svelte). The formatter now:
- Preserves trailing whitespace after inline element closing tags when in
whitespace-sensitive mode
- Treats text expressions as whitespace-sensitive elements to maintain
proper spacing around them
This ensures proper rendering when formatting HTML with inline elements
followed by text, or when using framework-specific text expressions.
|
WalkthroughThis PR addresses whitespace handling in the HTML formatter, specifically fixing issues where spaces were incorrectly removed after inline closing tags and text expressions. The changes add conditional logic to preserve trailing whitespace after closing angle brackets when whitespace sensitivity is strict, and introduce a new Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro ⛔ Files ignored due to path filters (13)
📒 Files selected for processing (13)
🧰 Additional context used🧠 Learnings (22)📚 Learning: 2025-10-15T09:22:15.851ZApplied to files:
📚 Learning: 2025-10-15T09:22:15.851ZApplied to files:
📚 Learning: 2025-10-15T09:22:15.851ZApplied to files:
📚 Learning: 2025-10-15T09:22:46.002ZApplied to files:
📚 Learning: 2025-10-15T09:22:46.002ZApplied to files:
📚 Learning: 2025-10-15T09:22:15.851ZApplied to files:
📚 Learning: 2025-10-15T09:22:15.851ZApplied to files:
📚 Learning: 2025-10-15T09:22:46.002ZApplied to files:
📚 Learning: 2025-10-15T09:22:15.851ZApplied to files:
📚 Learning: 2025-10-15T09:22:15.851ZApplied to files:
📚 Learning: 2025-10-15T09:22:15.851ZApplied to files:
📚 Learning: 2025-10-15T09:22:15.851ZApplied to files:
📚 Learning: 2025-10-15T09:22:15.851ZApplied to files:
📚 Learning: 2025-08-05T14:43:29.581ZApplied to files:
📚 Learning: 2025-10-15T09:25:05.698ZApplied to files:
📚 Learning: 2025-10-15T09:25:05.698ZApplied to files:
📚 Learning: 2025-10-15T09:22:46.002ZApplied to files:
📚 Learning: 2025-10-15T09:22:15.851ZApplied to files:
📚 Learning: 2025-10-15T09:22:15.851ZApplied to files:
📚 Learning: 2025-10-15T09:22:15.851ZApplied to files:
📚 Learning: 2025-10-15T09:22:15.851ZApplied to files:
📚 Learning: 2025-10-15T09:24:31.042ZApplied to files:
🧬 Code graph analysis (1)crates/biome_html_formatter/src/html/auxiliary/closing_element.rs (1)
🔇 Additional comments (15)
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah this one's gonna be a little rough. if you're only using ai for this... good luck.
note that some of the playgrounds i posted are using svelte and not astro. there's something wrong with the playground and astro files.
also needs a changeset
| Start {expr1} | ||
| {expr2}middle end {expr3} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The space from the input {expr2} middle disappeared in the output.
It's a bit weird that it keeps the newline between {expr1} and {expr2}, but it matches prettier behavior, so keep it.
| ```vue | ||
| <template> | ||
| <p>{a} {b} {c}</p> | ||
| <div>{firstName}{middleInitial}.{lastName}</div> | ||
| <span> {padded} </span> | ||
| </template> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Vue uses {{ and }}
| ```vue | ||
| <template> | ||
| <p>User: {name} ({username})</p> | ||
| <div>{count} items in cart</div> | ||
| <span>{first} {last}</span> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same thing here
| <p>{name}({username})</p> | ||
| <p>Hello {firstName} {lastName}!</p> | ||
| <p>{a}+ {b}= {sum}</p> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- The space between
{name}and({username})disappeared. - The spaces before the
+and=disappeared.
| ```astro | ||
| <div> | ||
| <span>Name:</span> | ||
| {fullName} | ||
| <span>Age:</span> | ||
| {age} | ||
| </div> | ||
| <p> | ||
| <strong>Total:</strong> | ||
| {price} | ||
| <em>(tax included)</em> | ||
| </p> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is significantly different from prettier's output, and shows no difference from biome's current output.
| ----- | ||
|
|
||
| ```astro | ||
| <p>{name}({username})</p> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The space disappeared
|
It would be nice if the PR description would highlight what kind of strategy (business logic) was used to fix the bug. Without it, I wouldn't know what to review and how. |
| } | ||
|
|
||
| /// Whether the element is a text expression (like `{name}` in HTML-ish languages) | ||
| pub(crate) fn is_text_expression(element: &AnyHtmlElement) -> bool { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| pub(crate) fn is_text_expression(element: &AnyHtmlElement) -> bool { | |
| const pub(crate) fn is_text_expression(element: &AnyHtmlElement) -> bool { |
| && is_whitespace_sensitive | ||
| && r_angle_token.has_trailing_whitespace() | ||
| { | ||
| write!(f, [space()])?; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we add a whitespace?
Summary
Fixes #7875, fixes #7960 where whitespace was being incorrectly removed after closing tags of inline elements and around text expressions (e.g., {name} in Astro/Vue/Svelte).
AI assistance notice
I used Claude Code to write most of the code and comments in this pull request. As the human author, I take full responsibility for its contents.
Test Plan
I added snapshot tests for the straightforward and edge cases around preserving whitespace around text expressions and in tags in whitespace-sensitve mode.
Some snapshots that test other things like long tag children needed to be updated, but they appear to be unrelated to whitespace preservation, and can be safely changed without changing the meaning of the tests.
Note - unimplemented nodes/tokens are expected with snapshots of framework tests because of limited support for html-ish languages (compared to support for just html).