Skip to content

Commit

Permalink
fix(format/hml): keep at most 1 empty line between elements
Browse files Browse the repository at this point in the history
  • Loading branch information
dyc3 committed Sep 25, 2024
1 parent ecf1f16 commit 0941f0b
Show file tree
Hide file tree
Showing 13 changed files with 271 additions and 60 deletions.
58 changes: 3 additions & 55 deletions crates/biome_html_formatter/src/html/lists/element_list.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,7 @@ use crate::{
comments::HtmlComments,
prelude::*,
utils::children::{
html_split_children, is_meaningful_html_text, HtmlChild, HtmlChildrenIterator,
HtmlRawSpace, HtmlSpace,
html_split_children, is_meaningful_html_text, HtmlChild, HtmlChildrenIterator, HtmlSpace,
},
};
use biome_formatter::{best_fitting, prelude::*, CstFormatContext};
Expand Down Expand Up @@ -106,7 +105,6 @@ impl FormatHtmlElementList {

while let Some(child) = children_iter.next() {
let mut child_breaks = false;

match &child {
// A single word: Both `a` and `b` are a word in `a b` because they're separated by HTML Whitespace.
HtmlChild::Word(word) => {
Expand Down Expand Up @@ -148,34 +146,7 @@ impl FormatHtmlElementList {
// * Whitespace before an opening tag: `a <div>`
HtmlChild::Whitespace => {
flat.write(&HtmlSpace, f);

// ```javascript
// <div>a
// {' '}</div>
// ```
let is_after_line_break =
last.as_ref().map_or(false, |last| last.is_any_line());

// `<div>aaa </div>` or `<div> </div>`
let is_trailing_or_only_whitespace = children_iter.peek().is_none();

if is_trailing_or_only_whitespace || is_after_line_break {
multiline.write_separator(&HtmlRawSpace, f);
}
// Leading whitespace. Only possible if used together with a expression child
//
// ```
// <div>
//
// {' '}
// <b />
// </div>
// ```
else if last.is_none() {
multiline.write_with_separator(&HtmlRawSpace, &hard_line_break(), f);
} else {
multiline.write_separator(&HtmlSpace, f);
}
multiline.write_separator(&soft_line_break_or_space(), f);
}

// A new line between some JSX text and an element
Expand Down Expand Up @@ -245,30 +216,7 @@ impl FormatHtmlElementList {
// An empty line between some JSX text and an element
HtmlChild::EmptyLine => {
child_breaks = true;

// Additional empty lines are not preserved when any of
// the children are a meaningful text node.
//
// <>
// <div>First</div>
//
// <div>Second</div>
//
// Third
// </>
//
// Becomes:
//
// <>
// <div>First</div>
// <div>Second</div>
// Third
// </>
if children_meta.meaningful_text {
multiline.write_separator(&hard_line_break(), f);
} else {
multiline.write_separator(&empty_line(), f);
}
multiline.write_separator(&empty_line(), f);
}

// Any child that isn't text
Expand Down
48 changes: 43 additions & 5 deletions crates/biome_html_formatter/src/utils/children.rs
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,7 @@ pub(crate) enum HtmlChild {
}

impl HtmlChild {
#[expect(dead_code)]
pub(crate) const fn is_any_line(&self) -> bool {
matches!(self, HtmlChild::EmptyLine | HtmlChild::Newline)
}
Expand Down Expand Up @@ -170,6 +171,7 @@ where
{
let mut builder = HtmlSplitChildrenBuilder::new();

let mut prev_was_content = false;
for child in children {
match child {
AnyHtmlElement::HtmlContent(text) => {
Expand All @@ -181,17 +183,18 @@ where

// Text starting with a whitespace
if let Some((_, HtmlTextChunk::Whitespace(_whitespace))) = chunks.peek() {
match chunks.next() {
Some((_, HtmlTextChunk::Whitespace(whitespace))) => {
if whitespace.contains('\n') {
// SAFETY: We just checked this above.
match chunks.next().unwrap() {
(_, HtmlTextChunk::Whitespace(whitespace)) => {
if whitespace.contains('\n') && !prev_was_content {
if chunks.peek().is_none() {
// A text only consisting of whitespace that also contains a new line isn't considered meaningful text.
// It can be entirely removed from the content without changing the semantics.
let newlines =
whitespace.chars().filter(|c| *c == '\n').count();

// Keep up to one blank line between tags/expressions and text.
// ```javascript
// Keep up to one blank line between tags.
// ```html
// <div>
//
// <MyElement />
Expand Down Expand Up @@ -236,9 +239,44 @@ where
}
}
}
prev_was_content = true;
}
child => {
let text = child.to_string();
let mut chunks = HtmlSplitChunksIterator::new(&text).peekable();

// Text starting with a whitespace
if let Some((_, HtmlTextChunk::Whitespace(_whitespace))) = chunks.peek() {
// SAFETY: We just checked this above.
match chunks.next().unwrap() {
(_, HtmlTextChunk::Whitespace(whitespace)) => {
if whitespace.contains('\n') {
// A text only consisting of whitespace that also contains a new line isn't considered meaningful text.
// It can be entirely removed from the content without changing the semantics.
let newlines = whitespace.chars().filter(|c| *c == '\n').count();

// Keep up to one blank line between tags.
// ```html
// <div>
//
// <MyElement />
// </div>
// ```
if newlines > 1 {
builder.entry(HtmlChild::EmptyLine);
} else {
builder.entry(HtmlChild::Newline);
}
} else {
builder.entry(HtmlChild::Whitespace)
}
}
_ => unreachable!(),
}
}

builder.entry(HtmlChild::NonText(child));
prev_was_content = false;
}
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
text




<div>foo</div>
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
---
source: crates/biome_formatter_test/src/snapshot_builder.rs
info: elements/spacing/case0.html
---
# Input

```html
text
<div>foo</div>
```


=============================

# Outputs

## Output 1

-----
Indent style: Tab
Indent width: 2
Line ending: LF
Line width: 80
Attribute Position: Auto
-----

```html
text
<div>foo</div>
```
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
<div>
<div>foo</div>

text

<div>foo</div>
</div>

Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
---
source: crates/biome_formatter_test/src/snapshot_builder.rs
info: elements/spacing/case1.html
---
# Input

```html
<div>
<div>foo</div>
text
<div>foo</div>
</div>
```


=============================

# Outputs

## Output 1

-----
Indent style: Tab
Indent width: 2
Line ending: LF
Line width: 80
Attribute Position: Auto
-----

```html
<div>
<div>foo</div>
text
<div>foo</div>
</div>
```
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
text




<div>foo</div>
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
---
source: crates/biome_formatter_test/src/snapshot_builder.rs
info: elements/spacing/case2.html
---
# Input

```html
text
<div>foo</div>
```


=============================

# Outputs

## Output 1

-----
Indent style: Tab
Indent width: 2
Line ending: LF
Line width: 80
Attribute Position: Auto
-----

```html
text
<div>foo</div>
```
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
<!-- foo -->
<!-- foo -->



test
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
---
source: crates/biome_formatter_test/src/snapshot_builder.rs
info: elements/spacing/case3.html
---
# Input

```html
<!-- foo -->
<!-- foo -->
test
```


=============================

# Outputs

## Output 1

-----
Indent style: Tab
Indent width: 2
Line ending: LF
Line width: 80
Attribute Position: Auto
-----

```html
<!-- foo -->
<!-- foo -->
test
```



## Unimplemented nodes/tokens

"<!-- foo -->" => 0..12
"<!-- foo -->\n" => 13..26
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
foo

bar
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
---
source: crates/biome_formatter_test/src/snapshot_builder.rs
info: elements/spacing/case4.html
---
# Input

```html
foo
bar
```


=============================

# Outputs

## Output 1

-----
Indent style: Tab
Indent width: 2
Line ending: LF
Line width: 80
Attribute Position: Auto
-----

```html
foo bar
```
Loading

0 comments on commit 0941f0b

Please sign in to comment.