Skip to content

Commit 9700f3a

Browse files
committed
feat(format/html): implement suppression comments
1 parent 8a832f2 commit 9700f3a

39 files changed

+377
-825
lines changed

crates/biome_html_factory/src/generated/node_factory.rs

-14
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

crates/biome_html_factory/src/generated/syntax_factory.rs

-33
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

crates/biome_html_formatter/src/generated.rs

-38
Original file line numberDiff line numberDiff line change
@@ -189,44 +189,6 @@ impl IntoFormat<HtmlFormatContext> for biome_html_syntax::HtmlClosingElement {
189189
)
190190
}
191191
}
192-
impl FormatRule<biome_html_syntax::HtmlComment>
193-
for crate::html::auxiliary::comment::FormatHtmlComment
194-
{
195-
type Context = HtmlFormatContext;
196-
#[inline(always)]
197-
fn fmt(
198-
&self,
199-
node: &biome_html_syntax::HtmlComment,
200-
f: &mut HtmlFormatter,
201-
) -> FormatResult<()> {
202-
FormatNodeRule::<biome_html_syntax::HtmlComment>::fmt(self, node, f)
203-
}
204-
}
205-
impl AsFormat<HtmlFormatContext> for biome_html_syntax::HtmlComment {
206-
type Format<'a> = FormatRefWithRule<
207-
'a,
208-
biome_html_syntax::HtmlComment,
209-
crate::html::auxiliary::comment::FormatHtmlComment,
210-
>;
211-
fn format(&self) -> Self::Format<'_> {
212-
FormatRefWithRule::new(
213-
self,
214-
crate::html::auxiliary::comment::FormatHtmlComment::default(),
215-
)
216-
}
217-
}
218-
impl IntoFormat<HtmlFormatContext> for biome_html_syntax::HtmlComment {
219-
type Format = FormatOwnedWithRule<
220-
biome_html_syntax::HtmlComment,
221-
crate::html::auxiliary::comment::FormatHtmlComment,
222-
>;
223-
fn into_format(self) -> Self::Format {
224-
FormatOwnedWithRule::new(
225-
self,
226-
crate::html::auxiliary::comment::FormatHtmlComment::default(),
227-
)
228-
}
229-
}
230192
impl FormatRule<biome_html_syntax::HtmlContent>
231193
for crate::html::auxiliary::content::FormatHtmlContent
232194
{

crates/biome_html_formatter/src/html/any/element.rs

-1
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ impl FormatRule<AnyHtmlElement> for FormatAnyHtmlElement {
1010
match node {
1111
AnyHtmlElement::HtmlBogusElement(node) => node.format().fmt(f),
1212
AnyHtmlElement::HtmlCdataSection(node) => node.format().fmt(f),
13-
AnyHtmlElement::HtmlComment(node) => node.format().fmt(f),
1413
AnyHtmlElement::HtmlContent(node) => node.format().fmt(f),
1514
AnyHtmlElement::HtmlElement(node) => node.format().fmt(f),
1615
AnyHtmlElement::HtmlSelfClosingElement(node) => node.format().fmt(f),

crates/biome_html_formatter/src/html/auxiliary/comment.rs

-10
This file was deleted.

crates/biome_html_formatter/src/html/auxiliary/mod.rs

-1
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ pub(crate) mod attribute_initializer_clause;
55
pub(crate) mod attribute_name;
66
pub(crate) mod cdata_section;
77
pub(crate) mod closing_element;
8-
pub(crate) mod comment;
98
pub(crate) mod content;
109
pub(crate) mod directive;
1110
pub(crate) mod element;

crates/biome_html_formatter/src/html/lists/element_list.rs

+25-1
Original file line numberDiff line numberDiff line change
@@ -181,6 +181,11 @@ impl FormatHtmlElementList {
181181
Some(WordSeparator::BetweenWords)
182182
}
183183

184+
Some(HtmlChild::Comment(_)) => {
185+
// FIXME: probably not correct behavior here
186+
Some(WordSeparator::Lines(0))
187+
}
188+
184189
// Last word or last word before an element without any whitespace in between
185190
Some(HtmlChild::NonText(next_child)) => Some(WordSeparator::EndOfText {
186191
is_soft_line_break: !matches!(
@@ -210,6 +215,12 @@ impl FormatHtmlElementList {
210215
}
211216
}
212217

218+
HtmlChild::Comment(comment) => {
219+
// FIXME: definitely wrong behavior
220+
flat.write(&format_args![comment], f);
221+
multiline.write_content(comment, f);
222+
}
223+
213224
// * Whitespace after the opening tag and before a meaningful text: `<div> a`
214225
// * Whitespace before the closing tag: `a </div>`
215226
// * Whitespace before an opening tag: `a <div>`
@@ -315,6 +326,11 @@ impl FormatHtmlElementList {
315326
}
316327
}
317328

329+
Some(HtmlChild::Comment(_)) => {
330+
// FIXME: definitely wrong behavior here.
331+
Some(LineMode::Hard)
332+
}
333+
318334
// Add a hard line break if what comes after the element is not a text or is all whitespace
319335
Some(HtmlChild::NonText(next_non_text)) => {
320336
// In the case of the formatter using the multiline layout, we want to treat inline elements like we do words.
@@ -426,7 +442,7 @@ impl FormatHtmlElementList {
426442
/// Tracks the tokens of [HtmlContent] nodes to be formatted and
427443
/// asserts that the suppression comments are checked (they get ignored).
428444
///
429-
/// This is necessary because the formatting of [HtmlContentList] bypasses the node formatting for
445+
/// This is necessary because the formatting of [HtmlElementList] bypasses the node formatting for
430446
/// [HtmlContent] and instead, formats the nodes itself.
431447
#[cfg(debug_assertions)]
432448
fn disarm_debug_assertions(&self, node: &HtmlElementList, f: &mut HtmlFormatter) {
@@ -524,6 +540,9 @@ enum WordSeparator {
524540
/// `a b`
525541
BetweenWords,
526542

543+
/// Seperator between 2 lines. Creates hard line breaks.
544+
Lines(usize),
545+
527546
/// A separator of a word at the end of a [HtmlText] element. Either because it is the last
528547
/// child in its parent OR it is right before the start of another child (element, expression, ...).
529548
///
@@ -571,6 +590,11 @@ impl Format<HtmlFormatContext> for WordSeparator {
571590
fn fmt(&self, f: &mut Formatter<HtmlFormatContext>) -> FormatResult<()> {
572591
match self {
573592
WordSeparator::BetweenWords => soft_line_break_or_space().fmt(f),
593+
WordSeparator::Lines(count) => match count {
594+
0 => Ok(()),
595+
1 => hard_line_break().fmt(f),
596+
_ => empty_line().fmt(f),
597+
},
574598
WordSeparator::EndOfText {
575599
is_soft_line_break,
576600
is_next_element_whitespace_sensitive,

crates/biome_html_formatter/src/utils/children.rs

+87-10
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ use std::{
66
use biome_formatter::{
77
Buffer, Format, FormatElement, FormatResult, format_args, prelude::*, write,
88
};
9-
use biome_html_syntax::AnyHtmlElement;
9+
use biome_html_syntax::{AnyHtmlElement, HtmlSyntaxNode};
1010
use biome_rowan::{SyntaxResult, TextLen, TextRange, TextSize, TokenText};
1111

1212
use crate::{HtmlFormatter, comments::HtmlComments, context::HtmlFormatContext};
@@ -75,6 +75,11 @@ pub(crate) enum HtmlChild {
7575
/// A Single word in a HTML text. For example, the words for `a b\nc` are `[a, b, c]`
7676
Word(HtmlWord),
7777

78+
/// A comment in a HTML text.
79+
///
80+
/// This is considered a seperate kind of "word" here because we must preserve whitespace between text and comments.
81+
Comment(HtmlWord),
82+
7883
/// A ` ` whitespace
7984
///
8085
/// ```html
@@ -164,7 +169,7 @@ impl Format<HtmlFormatContext> for HtmlRawSpace {
164169

165170
pub(crate) fn html_split_children<I>(
166171
children: I,
167-
_comments: &HtmlComments,
172+
comments: &HtmlComments,
168173
) -> SyntaxResult<Vec<HtmlChild>>
169174
where
170175
I: IntoIterator<Item = AnyHtmlElement>,
@@ -179,6 +184,11 @@ where
179184
// Keep track if there's any leading/trailing empty line, new line or whitespace
180185

181186
let value_token = text.value_token()?;
187+
let node = HtmlSyntaxNode::from(text);
188+
for comment in comments.leading_dangling_trailing_comments(&node) {
189+
// Manually mark these comments as formatted because they are. Because we override the formatting of text content in here, the formatter does not seem to recognize them as formatted.
190+
comment.mark_formatted();
191+
}
182192
let mut chunks = HtmlSplitChunksIterator::new(value_token.text()).peekable();
183193

184194
// Text starting with a whitespace
@@ -216,15 +226,41 @@ where
216226
}
217227
}
218228

229+
let mut prev_was_comment = false;
219230
while let Some(chunk) = chunks.next() {
220231
match chunk {
221232
(_, HtmlTextChunk::Whitespace(whitespace)) => {
222233
// Only handle trailing whitespace. Words must always be joined by new lines
223-
if chunks.peek().is_none() {
224-
if whitespace.contains('\n') {
225-
builder.entry(HtmlChild::Newline);
226-
} else {
227-
builder.entry(HtmlChild::Whitespace)
234+
let newlines = whitespace.chars().filter(|b| *b == '\n').count();
235+
match chunks.peek() {
236+
Some(&(_, HtmlTextChunk::Comment(_))) => {
237+
// if the next chunk is a comment, preserve the whitespace
238+
if newlines >= 2 {
239+
builder.entry(HtmlChild::EmptyLine)
240+
} else if newlines == 1 {
241+
builder.entry(HtmlChild::Newline)
242+
} else {
243+
builder.entry(HtmlChild::Whitespace)
244+
}
245+
}
246+
None => {
247+
if newlines >= 1 {
248+
builder.entry(HtmlChild::Newline)
249+
} else {
250+
builder.entry(HtmlChild::Whitespace)
251+
}
252+
}
253+
_ => {
254+
// if the previous chunk was a comment, we need to preserve the whitespace before the next chunk (which will never be whitespace).
255+
if prev_was_comment {
256+
if newlines >= 2 {
257+
builder.entry(HtmlChild::EmptyLine)
258+
} else if newlines == 1 {
259+
builder.entry(HtmlChild::Newline)
260+
} else {
261+
builder.entry(HtmlChild::Whitespace)
262+
}
263+
}
228264
}
229265
}
230266
}
@@ -237,7 +273,16 @@ where
237273

238274
builder.entry(HtmlChild::Word(HtmlWord::new(text, source_position)));
239275
}
276+
(relative_start, HtmlTextChunk::Comment(word)) => {
277+
let text = value_token
278+
.token_text()
279+
.slice(TextRange::at(relative_start, word.text_len()));
280+
let source_position = value_token.text_range().start() + relative_start;
281+
282+
builder.entry(HtmlChild::Comment(HtmlWord::new(text, source_position)));
283+
}
240284
}
285+
prev_was_comment = matches!(chunk, (_, HtmlTextChunk::Comment(_)));
241286
}
242287
prev_was_content = true;
243288
}
@@ -321,6 +366,7 @@ impl HtmlSplitChildrenBuilder {
321366
enum HtmlTextChunk<'a> {
322367
Whitespace(&'a str),
323368
Word(&'a str),
369+
Comment(&'a str),
324370
}
325371

326372
/// Splits a text into whitespace only and non-whitespace chunks.
@@ -352,23 +398,54 @@ impl<'a> Iterator for HtmlSplitChunksIterator<'a> {
352398
self.position += char.text_len();
353399

354400
let is_whitespace = matches!(char, ' ' | '\n' | '\t' | '\r');
401+
let mut maybe_comment = char == '<';
402+
let mut definitely_comment = false;
403+
let mut seen_end_comment_chars = 0;
355404

356405
while let Some(next) = self.chars.peek() {
357-
let next_is_whitespace = matches!(next, ' ' | '\n' | '\t' | '\r');
406+
if maybe_comment && !definitely_comment {
407+
match (self.position - start, next) {
408+
(idx, '!') if idx == 1.into() => {}
409+
(idx, '-') if idx == 2.into() || idx == 3.into() => {}
410+
(idx, _) if idx == 4.into() => {
411+
definitely_comment = true;
412+
}
413+
_ => {
414+
maybe_comment = false;
415+
}
416+
}
417+
}
358418

359-
if is_whitespace != next_is_whitespace {
360-
break;
419+
if definitely_comment {
420+
match (seen_end_comment_chars, next) {
421+
(0, '-') => seen_end_comment_chars += 1,
422+
(1, '-') => seen_end_comment_chars += 1,
423+
(2, '>') => seen_end_comment_chars += 1,
424+
_ => seen_end_comment_chars = 0,
425+
}
426+
} else {
427+
let next_is_whitespace = matches!(next, ' ' | '\n' | '\t' | '\r');
428+
429+
if is_whitespace != next_is_whitespace {
430+
break;
431+
}
361432
}
362433

363434
self.position += next.text_len();
364435
self.chars.next();
436+
437+
if seen_end_comment_chars == 3 {
438+
break;
439+
}
365440
}
366441

367442
let range = TextRange::new(start, self.position);
368443
let slice = &self.text[range];
369444

370445
let chunk = if is_whitespace {
371446
HtmlTextChunk::Whitespace(slice)
447+
} else if definitely_comment {
448+
HtmlTextChunk::Comment(slice)
372449
} else {
373450
HtmlTextChunk::Word(slice)
374451
};
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
<div>
2+
Foo
3+
4+
<!-- Bar -->
5+
</div>
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
Foo
2+
3+
<!-- Bar -->
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
<div>
2+
<!-- Foo -->
3+
4+
Bar
5+
</div>
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
<!-- Foo -->
2+
3+
Bar

0 commit comments

Comments
 (0)