Skip to content
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

fix(biome_css_formatter): update css format style #4476

Draft
wants to merge 8 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 39 additions & 0 deletions crates/biome_css_formatter/src/css/properties/generic_property.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,45 @@ impl FormatNodeRule<CssGenericProperty> for FormatCssGenericProperty {
value,
} = node.as_fields();

let is_dash_identity = name.clone().unwrap().as_css_dashed_identifier().is_some();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't use unwrap(); you rarely need to use this method because it panics at runtime. In your case you can use CssDashedIdentifer::can_cast(name.syntax().kind());


let has_comma = value.iter().any(|v| v.text().eq(","));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Checking for a comma (,) like this isn't correct. You should rely on the CST/AST, so you don't need to transform a node into a test.

You can use the playground to surgically understand how to query it: https://biomejs.dev/playground/?lintRules=all&files.main.css=aAB0AG0AbAAgAHsACgAJAGYAbwBuAHQALQBmAGEAbQBpAGwAeQA6ACAAcwB5AHMAdABlAG0ALQB1AGkALAAgAC0AYQBwAHAAbABlAC0AcwB5AHMAdABlAG0AOwAKAH0A

(Click on the comma on the left side, and it should point to the relative node on the right, in the syntax tab)


if has_comma && !is_dash_identity {
write!(
f,
[
name.format(),
colon_token.format(),
indent(&format_with(|f| {
let last = value.last().unwrap();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No unwrap


for (idx, v) in value.iter().enumerate() {
if idx == 0 {
write!(f, [soft_line_break_or_space()])?;
}

if v.text().eq(",") {
write!(f, [v.format(), hard_line_break()])?;
} else if let Some(next) = value.iter().nth(idx + 1) {
if next.text().eq(",") || v == last {
write!(f, [v.format()])?;
} else {
write!(f, [v.format(), space()])?;
}
} else {
write!(f, [v.format()])?;
}
}
Comment on lines +28 to +44
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not really sure I understand what's happening here, so I am waiting for your to update the PR description and explain how the algorithm works.


Ok(())
})),
]
)?;

return Ok(());
}

write!(
f,
[name.format(), colon_token.format(), space(), value.format()]
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
---
source: crates/biome_formatter_test/src/snapshot_builder.rs
assertion_line: 212
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Update cargo-insta and revert these changes

info: css/atrule/font_face.css
---
# Input
Expand Down Expand Up @@ -114,33 +115,39 @@ Quote style: Double Quotes
```css
@font-face {
font-family: "Open Sans";
src: url("/fonts/OpenSans-Regular-webfont.woff2") format("woff2"),
src:
url("/fonts/OpenSans-Regular-webfont.woff2") format("woff2"),
url("/fonts/OpenSans-Regular-webfont.woff") format("woff");
}
@font-face {
font-family: "Open Sans";
src: url("/fonts/OpenSans-Regular-webfont.woff2") format("woff2"),
src:
url("/fonts/OpenSans-Regular-webfont.woff2") format("woff2"),
url("/fonts/OpenSans-Regular-webfont.woff") format("woff");
}
@font-face {
font-family: "Open Sans";
src: url("/fonts/OpenSans-Regular-webfont.woff2") format("woff2"),
src:
url("/fonts/OpenSans-Regular-webfont.woff2") format("woff2"),
url("/fonts/OpenSans-Regular-webfont.woff") format("woff");
}
@font-face {
font-family: "Open Sans";
src: url("/fonts/OpenSans-Regular-webfont.woff2") format("woff2"),
src:
url("/fonts/OpenSans-Regular-webfont.woff2") format("woff2"),
url("/fonts/OpenSans-Regular-webfont.woff") format("woff");
}
@font-face {
font-family: "Open Sans";
src: url("/fonts/OpenSans-Regular-webfont.woff2") format("woff2"),
src:
url("/fonts/OpenSans-Regular-webfont.woff2") format("woff2"),
url("/fonts/OpenSans-Regular-webfont.woff") format("woff");
}
@font-face {
font-family: "Open Sans";

src: url("/fonts/OpenSans-Regular-webfont.woff2") format("woff2"),
src:
url("/fonts/OpenSans-Regular-webfont.woff2") format("woff2"),
url("/fonts/OpenSans-Regular-webfont.woff") format("woff");
}
```
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
---
source: crates/biome_formatter_test/src/snapshot_builder.rs
assertion_line: 212
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you see this line,it means you're using an old version of cargo-insta. Run just upgrade-tools to update the tools we use internally.

info: css/declaration_list.css
---
# Input
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
---
source: crates/biome_formatter_test/src/snapshot_builder.rs
assertion_line: 212
info: css/properties/all.css
---
# Input
Expand Down Expand Up @@ -63,7 +64,9 @@ div {
all: revert-layer;

all: unknown-value;
all: a, value list;
all:
a,
value list;
}

:root {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
---
source: crates/biome_formatter_test/src/snapshot_builder.rs
assertion_line: 212
info: css/properties/border.css
---
# Input
Expand Down Expand Up @@ -69,7 +70,9 @@ div {
border: inherit;

border: zzz-unknown-value;
border: a, value list;
border:
a,
value list;

/* <line-style> */
border: SOLID;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
---
source: crates/biome_formatter_test/src/snapshot_builder.rs
assertion_line: 212
info: css/properties/custom.css
---
# Input
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
---
source: crates/biome_formatter_test/src/snapshot_builder.rs
assertion_line: 212
info: css/properties/generic.css
---
# Input
Expand Down Expand Up @@ -43,8 +44,13 @@ div {
/* Custom property, always generic */
unknown-property: one-value;
unknown-property: multiple values;
unknown-property: delimited, values;
unknown-property:
delimited,
values;
unknown-property: delimited / slash / values;
unknown-property: mixed, delimiters / can be, used;
unknown-property:
mixed,
delimiters / can be,
used;
}
```
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
html {
font-family: system-ui, -apple-system;
}

body {
font-family: sans-serif, serif, system-ui;
}

div {
font-family: system-ui;
}

div {
box-shadow: 10px 10px 10px 10px rgb(10 18 30 / 12%), 12px 10px 10px 10px rgb(10 18 30 / 16%);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
---
source: crates/biome_formatter_test/src/snapshot_builder.rs
assertion_line: 212
info: css/properties/long_values.css
---
# Input

```css
html {
font-family: system-ui, -apple-system;
}

body {
font-family: sans-serif, serif, system-ui;
}

div {
font-family: system-ui;
}

div {
box-shadow: 10px 10px 10px 10px rgb(10 18 30 / 12%), 12px 10px 10px 10px rgb(10 18 30 / 16%);
}

```


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

# Outputs

## Output 1

-----
Indent style: Tab
Indent width: 2
Line ending: LF
Line width: 80
Quote style: Double Quotes
-----

```css
html {
font-family:
system-ui,
-apple-system;
}

body {
font-family:
sans-serif,
serif,
system-ui;
}

div {
font-family: system-ui;
}

div {
box-shadow:
10px 10px 10px 10px rgb(10 18 30 / 12%),
12px 10px 10px 10px rgb(10 18 30 / 16%);
}
```
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
---
source: crates/biome_formatter_test/src/snapshot_builder.rs
assertion_line: 212
info: css/properties/unicode_range.css
---
# Input
Expand Down Expand Up @@ -84,16 +85,35 @@ Quote style: Double Quotes

```css
@font-face {
unicode-range: U+000-49F, U+2000-27FF, U+2900-2BFF, U+1D400-1D7FF, U+ff??;
unicode-range:
U+000-49F,
U+2000-27FF,
U+2900-2BFF,
U+1D400-1D7FF,
U+ff??;
}

@font-face {
unicode-range: U+0000-00FF, U+0131, U+0152-0153, U+02C6, U+02DA, U+02DC,
U+2000-206F, U+2074, U+20AC, U+2212, U+2215;
unicode-range:
U+0000-00FF,
U+0131,
U+0152-0153,
U+02C6,
U+02DA,
U+02DC,
U+2000-206F,
U+2074,
U+20AC,
U+2212,
U+2215;
}

@font-face {
unicode-range: U+0400-045F, U+0490-0491, U+04B0-04B1, U+2116;
unicode-range:
U+0400-045F,
U+0490-0491,
U+04B0-04B1,
U+2116;
}

@font-face {
Expand All @@ -104,8 +124,14 @@ Quote style: Double Quotes
unicode-range: U+0-7F;
unicode-range: U+0025-00FF; /* codepoint range */
unicode-range: U+4??; /* wildcard range */
unicode-range: U+0025-00FF, U+4??; /* multiple values */
unicode-range: U+A5, U+4E00-9FFF, U+30??, U+FF00-FF9F; /* multiple values */
unicode-range:
U+0025-00FF,
U+4??; /* multiple values */
unicode-range:
U+A5,
U+4E00-9FFF,
U+30??,
U+FF00-FF9F; /* multiple values */
unicode-range: U+????;
unicode-range: U+??????;
unicode-range: U+12;
Expand Down
Loading